Date: Sun, 7 Oct 2012 01:25:49 -0600 From: PseudoCylon <moonlightakkiy@yahoo.ca> To: Adrian Chadd <adrian@freebsd.org> Cc: freebsd-wireless@freebsd.org Subject: Re: [RFC] How the TX path works; I'd like to fix this before 10.x Message-ID: <CAFZ_MY%2BVYQuxfJGZ70PWo%2BvN2H7y0VAG_APHb4u5guk%2BGQGQOA@mail.gmail.com> In-Reply-To: <CAJ-VmokesePjEOs92=GJ87knfcscx3yPF9TupRojf7dGjnRniQ@mail.gmail.com> References: <CAFZ_MYKampEVfyPOXyGd9Cshsr7xpP48%2BxOyV8uU8LuzMpTdRg@mail.gmail.com> <CAJ-VmokesePjEOs92=GJ87knfcscx3yPF9TupRojf7dGjnRniQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Oct 6, 2012 at 3:00 PM, Adrian Chadd <adrian@freebsd.org> wrote: > Well, i would like to implement if_transmit for net80211 tx, then have the > tx routine queue the frame and wakeup the tx taskqueue or tasklet. Im not > sure yet whether to put the tx processing in the existing ic taskqueue or > not, but since the current tx path runs separate to the ic taskqueue, the > hard work is already done for us. If we forget about hooking ieee80211_output() and leave ether_ifattach() default hooks http://fxr.watson.org/fxr/source/net80211/ieee80211.c#L548 if_transmit will be called with packets encaped in ethernet header. Then, in a will-be-added ieee80211_transmit(), call ieee80211_encap(), IFQ_HANDOFF(), and call (or enqueue) driver's new tx task. This can be done by just teaching new tx path aware drivers correct functions to hook. (So, we don't have to change all wifi drivers.) > > In parallel I would like to kill the ieee80211_output and raw xmit code, > instead doing the encap and such in the tx tasklet. If all packets, whether data or mgmt, are queued to the same buff (i.e. if_snd), probably raw_xmit won't be needed. And, somehow attach txparam to all packets. Then, driver can handle the packets in the same tx function by using info in txparam. (I think the driver code will be simpler.) But, I wonder if waiting for all previously queued packets being processed causes any trouble for shorter buffer-life packets, i.e. addba/probe response packets. AK > > We could later allow encap before serialisation but delay seqno and > encryption. Thats a later thing. > > In parallel we do need to fix driver tx. It all has to stay in order... Ie > the order that the tx tasklet does encap must reflect the driver setup. Or, > we push seqno allocation to each driver. Ew. > > Adrian > > On Oct 6, 2012 3:01 PM, "PseudoCylon" <moonlightakkiy@yahoo.ca> wrote: >> >> > ------------------------------ >> > >> > Message: 4 >> > Date: Fri, 5 Oct 2012 23:44:10 -0700 >> > From: Adrian Chadd <adrian@freebsd.org> >> > Subject: [RFC] How the TX path works; I'd like to fix this before 10.x >> > To: freebsd-wireless@freebsd.org >> > Message-ID: >> > >> > <CAJ-VmomS=7Udyixqqss_1261EXzEpeYBuAWqwiKEah=5V4rrWw@mail.gmail.com> >> > Content-Type: text/plain; charset=ISO-8859-1 >> > >> > Before I continue - if_transmit() is not the answer. if_transmit() >> > guarantess _no_ serialisation at all. So we'd stlil have to stuff >> > things into queues. And ensure only one thing is running at once. >> >> As far as I know, if_transmit/if_start are options for drivers, not >> for the serialisation. >> if_transmit to use device specific queue >> if_start to use if_snd queue >> i.e. >> http://fxr.watson.org/fxr/source/dev/e1000/if_em.c#L2941 >> >> > >> > I could wrap a big VAP TX lock around ieee80211_output() and >> > ieee80211_start(), ensuring they don't run over each other. But >> > long-held locks need to go away and die. yes, even the ones in the >> > ath(4) driver that I've introduced. They're there because of a lack of >> > synchronisation and queue design. A lot of the gige/10ge drivers use >> > long-held locks.. sigh. >> > >> > I could create a net80211 TX tasklet for each vap (or ic, _maybe_) >> > which serialises the TX path. ieee80211_start() would just schedule >> > the tasklet to run. That would serialise all of the parallel TX entry >> > points and solve a bunch of the subtle sequence number and other TX >> > state races that are occuring. That doesn't solve the ic_raw_xmit() or >> > ieee80211_output() problem, as both of those can also do TX 802.11 >> > encapsulation and kick off parts of the state machine. >> >> I prefer taskqueue over new lock. I have had enough of LORs already. >> >> We just need to decide where/how to funnel all tx entries. >> Currently, >> pseudo devices (i.e. if_bridge) \ >> IP stack --> ieee80211 stack --> >> driver >> ic_raw_xmit >> / >> so we need to ensure serialization (by lock or taskqueue) at 2 points, >> 1) between upper layer and ieee80211, >> 2) driver and ieee80211. >> If we solve the raw_xmit problem, there will be only 1 point to take care. >> >> >> An idea >> Guarantee only one thread is running at any moment. So, first queued >> first dequeued and tx'd without a lock. >> >> if_start() { >> if (sc_task == NOT_RUNNING) >> wakeup(sc_txtask); >> } >> >> if_txtask() { >> for (;sc_txtask_exit != DONE;) { >> IFQ_DRV_DEQUEUE(); >> if (m == NULL) { >> sc_txtask = NOT_RUNNING; >> tsleep(); >> sc_txtask = RUNNING; >> } else >> tx(); >> } >> } >> >> tx_callback() { >> ... >> if (sc_task == NOT_RUNNING) >> wakeup(sc_txtask); >> } >> >> iv_vap_create() { >> ... >> taskqueue_enqueue(sc_taskqueue); >> } >> >> iv_vap_delete() { >> ... >> sc_txtask_exit = DONE; >> if (sc_task == NOT_RUNNING) >> wakeup(sc_txtask); >> } >> >> > >> > It doesn't solve the same issues with the drivers .Yes, even if we >> > converted them to use if_transmit(). iwn(4) solves this by just having >> > a big IWN_LOCK() but it releases it when doing anything stack related. >> > I'm not sure if it holds it across the whole TX path through >> > iwn_start(). In any case, it's a big lock. ath(4) can and does have >> > multiple ath_start() instances running in multiple kernel threads, >> > fighting to dequeue frames from the ifnet. This still can cause issues >> > with non-aggregate sequence number and CCMP IV counter allocation. >> > Sigh. >> > >> > God even knows what to do about USB devices in all of this. >> > >> >> Similar to other drivers, i.e. iwn(4). The difference is USB drivers >> create per-USB-pipe queue in addition to if_snd queue. So that, tx >> path look like >> if_transmit -> queue -> if_start -> dequeue -> driver_tx -> usb_queue >> -> usb_stack >> It seems redundant. I'd like to change that to (for run(4)) >> if_transmit -> driver_tx -> usb_queue -> usb_stack >> >> >> AK >> _______________________________________________ >> freebsd-wireless@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-wireless >> To unsubscribe, send any mail to >> "freebsd-wireless-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAFZ_MY%2BVYQuxfJGZ70PWo%2BvN2H7y0VAG_APHb4u5guk%2BGQGQOA>