From: Alexander Aring <alex.aring@gmail.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>,
linux-wpan - ML <linux-wpan@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
"open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
Michael Hennerich <michael.hennerich@analog.com>,
Harry Morris <h.morris@cascoda.com>,
Varka Bhadram <varkabhadram@gmail.com>,
Xue Liu <liuxuenetmail@gmail.com>, Alan Ott <alan@signal11.us>,
David Girault <david.girault@qorvo.com>,
Romuald Despres <romuald.despres@qorvo.com>,
Frederic Blain <frederic.blain@qorvo.com>,
Nicolas Schodet <nico@ni.fr.eu.org>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
"linux-wireless@vger.kernel.org Wireless"
<linux-wireless@vger.kernel.org>
Subject: Re: [wpan-next v2 23/27] net: mac802154: Add support for active scans
Date: Thu, 13 Jan 2022 19:39:16 -0500 [thread overview]
Message-ID: <CAB_54W4OGFabfXoTWU-3D_F3fSRNQn-bzthXoBDJ-ZBWUfos0Q@mail.gmail.com> (raw)
In-Reply-To: <CAB_54W7GzyQr05X3TUcPbAFPsvetAjX=vd3G9y9wQi+8msYGHQ@mail.gmail.com>
Hi,
On Thu, 13 Jan 2022 at 19:30, Alexander Aring <alex.aring@gmail.com> wrote:
>
> Hi,
>
> On Thu, 13 Jan 2022 at 12:14, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alexander,
> >
> > alex.aring@gmail.com wrote on Wed, 12 Jan 2022 18:16:11 -0500:
> >
> > > Hi,
> > >
> > > On Wed, 12 Jan 2022 at 12:34, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > ...
> > > > +static int mac802154_scan_send_beacon_req_locked(struct ieee802154_local *local)
> > > > +{
> > > > + struct sk_buff *skb;
> > > > + int ret;
> > > > +
> > > > + lockdep_assert_held(&local->scan_lock);
> > > > +
> > > > + skb = alloc_skb(IEEE802154_BEACON_REQ_SKB_SZ, GFP_KERNEL);
> > > > + if (!skb)
> > > > + return -ENOBUFS;
> > > > +
> > > > + ret = ieee802154_beacon_req_push(skb, &local->beacon_req);
> > > > + if (ret) {
> > > > + kfree_skb(skb);
> > > > + return ret;
> > > > + }
> > > > +
> > > > + return drv_xmit_async(local, skb);
> > >
> > > I think you need to implement a sync transmit handling here.
> >
> > True.
> >
> > > And what
> > > I mean is not using dryv_xmit_sync() (It is a long story and should
> > > not be used, it's just that the driver is allowed to call bus api
> > > functions which can sleep).
> >
> > Understood.
> >
>
> I think we should care about drivers which use drv_xmit_sync() or we
> disable scan operations for them... so the actual transmit function
> should prefer async but use sync if it's not implemented. I am not a
> fan of this inside the core, if some driver really want to workaround
> their bus system because it's simpler to use or whatever they should
> do that inside the driver and not let the core queue it for them in
> the right context. There are reasons why xmit_do is in softirq
> context.
>
> > > We don't have such a function yet (but I
> > > think it can be implemented), you should wait until the transmission
> > > is done. If we don't wait we fill framebuffers on the hardware while
> > > the hardware is transmitting the framebuffer which is... bad.
> >
> > Do you already have something in mind?
> >
> > If I focus on the scan operation, it could be that we consider the
> > queue empty, then we put this transfer, wait for completion and
> > continue. But this only work for places where we know we have full
> > control over what is transmitted (eg. during a scan) and not for all
> > transfers. Would this fit your idea?
> >
> > Or do you want something more generic with some kind of an
> > internal queue where we have the knowledge of what has been queued and
> > a token to link with every xmit_done call that is made?
> >
> > I'm open to suggestions.
> >
>
> That we currently allow only one skb at one time (because ?all?
> supported hardware doesn't have multiple tx framebuffers) this makes
> everything for now pretty simple.
>
> Don't let the queue run empty, the queue here is controlled by the
> qdsic (I suppose and I hope we are talking about the same queue) we
> already stop the queue which stops further skb to transmit to the
> hardware but there can be one ongoing which we need to wait for. I
> said in a previous mail a wait_for_completion()/complete() works here,
> but I think now that could be problematic because scan-op ->
> wait_for_completion() and complete() can run parallel in different
> contexts. I think we need to do that over a waitqueue and a
> wait_event(). Maybe you can track somehow with an atomic counter how
> many transmissions are currently ongoing (should be never higher than
> one currently). However the atomic counter will be future proofed when
> we support filling up more than one framebuffer. So the condition for
> wait_event() would be atomic_test(phy->ongoing_txs) - or something
> like that. Increment in transmit path and decrement in xmit_done path,
> if it hits zero wake_up() the queue so the condition will be checked
> again.
>
I am sorry, the wait_event() will fix the issue after calling
stop_queue() to be sure there are no ongoing transmissions. Now we
need to have some idea about how to implement the synchronous transmit
function. We have the function (drv_xmit_async, or something higher
level to take care of sync as well) to transmit a skb and the complete
handler is xmit_complete(). As I mentioned we allow only one skb for
one time... Somehow we need to have a wait here and a per phy
wait_for_completion()/complete() will in this case work. Even if there
is hardware outside which has more transmit buffers, we probably would
send one skb at one for slowpath (having full control of
transmissions) anyway.
- Alex
next prev parent reply other threads:[~2022-01-14 0:39 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-12 17:32 [wpan-next v2 00/27] IEEE 802.15.4 scan support Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 01/27] net: mac802154: Split the set channel hook implementation Miquel Raynal
2022-01-12 22:30 ` Alexander Aring
2022-01-12 22:53 ` Alexander Aring
2022-01-13 11:12 ` Miquel Raynal
2022-01-13 9:32 ` Miquel Raynal
2022-01-13 23:27 ` Alexander Aring
2022-01-12 17:32 ` [wpan-next v2 02/27] net: mac802154: Ensure proper channel selection at probe time Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 03/27] net: ieee802154: Improve the way supported channels are declared Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 04/27] net: ieee802154: Give more details to the core about the channel configurations Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 05/27] net: mac802154: Convert the symbol duration into nanoseconds Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 06/27] net: mac802154: Set the symbol duration automatically Miquel Raynal
2022-01-12 22:25 ` Alexander Aring
2022-01-13 9:52 ` Miquel Raynal
2022-01-13 23:36 ` Alexander Aring
2022-01-14 10:18 ` Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 07/27] net: ieee802154: hwsim: Ensure frame checksum are valid Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 08/27] net: ieee802154: Drop symbol duration settings when the core does it already Miquel Raynal
2022-01-12 22:26 ` Alexander Aring
2022-01-13 11:16 ` Miquel Raynal
2022-01-13 23:34 ` Alexander Aring
2022-01-14 10:21 ` Miquel Raynal
2022-01-16 23:21 ` Alexander Aring
2022-01-17 9:12 ` Miquel Raynal
2022-01-17 23:38 ` Alexander Aring
2022-01-18 10:38 ` Miquel Raynal
2022-01-18 22:43 ` Alexander Aring
2022-01-19 22:26 ` Miquel Raynal
2022-01-19 23:26 ` Alexander Aring
2022-01-12 17:32 ` [wpan-next v2 09/27] net: ieee802154: Move IEEE 802.15.4 Kconfig main entry Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 10/27] net: mac802154: Include the softMAC stack inside the IEEE 802.15.4 menu Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 11/27] net: ieee802154: Move the address structure earlier Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 12/27] net: ieee802154: Add a kernel doc header to the ieee802154_addr structure Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 13/27] net: ieee802154: Return meaningful error codes from the netlink helpers Miquel Raynal
2022-01-12 17:32 ` [wpan-next v2 14/27] net: ieee802154: Add support for internal PAN management Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 15/27] net: ieee802154: Define a beacon frame header Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 16/27] net: ieee802154: Define frame types Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 17/27] net: ieee802154: Add support for scanning requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 18/27] net: mac802154: Handle scan requests Miquel Raynal
2022-01-12 22:44 ` Alexander Aring
2022-01-13 17:07 ` Miquel Raynal
2022-01-14 0:01 ` Alexander Aring
2022-01-14 18:44 ` Miquel Raynal
2022-01-16 22:44 ` Alexander Aring
2022-01-16 22:50 ` Alexander Aring
2022-01-17 9:00 ` Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 19/27] net: ieee802154: Full PAN management Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 20/27] net: ieee802154: Add support for beacon requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 21/27] net: mac802154: Handle beacons requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 22/27] net: ieee802154: Trace the registration of new PANs Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 23/27] net: mac802154: Add support for active scans Miquel Raynal
2022-01-12 23:16 ` Alexander Aring
2022-01-13 17:14 ` Miquel Raynal
2022-01-14 0:30 ` Alexander Aring
2022-01-14 0:39 ` Alexander Aring [this message]
2022-01-12 17:33 ` [wpan-next v2 24/27] net: mac802154: Add support for processing beacon requests Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 25/27] net: mac802154: Inform device drivers about scans Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 26/27] net: mac802154: Inform device drivers about beacon operations Miquel Raynal
2022-01-12 17:33 ` [wpan-next v2 27/27] net: ieee802154: ca8210: Refuse most of the scan operations Miquel Raynal
2022-01-12 22:48 ` Alexander Aring
2022-01-13 9:29 ` Miquel Raynal
2022-01-14 1:00 ` Alexander Aring
2022-01-14 18:37 ` Miquel Raynal
2022-01-16 22:48 ` Alexander Aring
2022-01-17 9:04 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAB_54W4OGFabfXoTWU-3D_F3fSRNQn-bzthXoBDJ-ZBWUfos0Q@mail.gmail.com \
--to=alex.aring@gmail.com \
--cc=alan@signal11.us \
--cc=davem@davemloft.net \
--cc=david.girault@qorvo.com \
--cc=frederic.blain@qorvo.com \
--cc=h.morris@cascoda.com \
--cc=kuba@kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linux-wpan@vger.kernel.org \
--cc=liuxuenetmail@gmail.com \
--cc=michael.hennerich@analog.com \
--cc=miquel.raynal@bootlin.com \
--cc=netdev@vger.kernel.org \
--cc=nico@ni.fr.eu.org \
--cc=romuald.despres@qorvo.com \
--cc=stefan@datenfreihafen.org \
--cc=thomas.petazzoni@bootlin.com \
--cc=varkabhadram@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).