Date: Wed, 22 May 2013 05:43:15 +0000 (UTC) From: Adrian Chadd <adrian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: svn commit: r250904 - user/adrian/net80211_tx/sys/net80211 Message-ID: <201305220543.r4M5hFJo042176@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: adrian Date: Wed May 22 05:43:14 2013 New Revision: 250904 URL: http://svnweb.freebsd.org/changeset/base/250904 Log: First cut at deferred transmit for the VAP path. The aim here is to eventually do a hybrid direct dispatch if there's no transmitter, or a deferred dispatch if the transmitter is currently busy. But for now this will just have to do. It'll add a little more latency on top of things but it'll at least make things a bit more sane. * Create a new vap task for transmit * Add transmit and qflush for the vap interface; kill the if_start() version of the method. * The transmit/qflush routines still use the vap ifp if_snd queue, but they themselves do the packet setup, enqueue and dequeue/dispatch. Things are nicely separated enough to hopefully make it easier for me to figure out a better, flexible queue+dispatch framework. What's next: * Create a different taskqueue, one for each VAP, that handles transmit. Sticking it in the ic queue unfortunately serialises the VAP transmit with the ic transmit and stops multiple, overlapping VAPs from transmitting. This makes things much easier but it also hides a bunch of potential race conditions. So, let's break things out into a race-prone setup which somewhat matches the existing scenario in -HEAD - indepedent VAP and driver transmit threads. * .. it may be that instead of doing per-VAP methods, we do a per-IC transmit taskqueue but create a separate one just for transmit. That way if the driver wishes to serialise its own software queue mechanism with the net80211 transmit path - eg because it wishes to allocate its own sequence numbers w/ A-MPDU - then the driver code can just keep the global ic vap transmit lock held, rather than trying to keep some per-VAP transmit lock held. * .. and it's still not clear how to setup the TX, node (comlock) and driver locking in a way that isn't ugly and/or cause LORs. * Push frames from the raw transmit path into the VAP path directly, rather than doing a call to ic_raw_xmit() from here. Then the dispatch task can decide whether to call the vap transmit method or the device ic_raw_xmit method. Serialising both of these calls together will go a long way towards ensuring the driver state is kept sane. * Figure out how to shoehorn management traffic frame generation into this. Right now it grabs the TX path lock and then sets up the frame state independent of the VAP transmit path. It then calls ic_raw_xmit(). There's LOR problems doing it this way; it would just plain be nicer if we could setup these frames in the caller context and then assign 802.11 state (eg sequence number, tag with power save info, feed into FF/AMSDU/powersave/etc) state from inside the serialised VAP transmit path. Anyway, still lots to do. Modified: user/adrian/net80211_tx/sys/net80211/ieee80211.c user/adrian/net80211_tx/sys/net80211/ieee80211_freebsd.c user/adrian/net80211_tx/sys/net80211/ieee80211_output.c user/adrian/net80211_tx/sys/net80211/ieee80211_proto.c user/adrian/net80211_tx/sys/net80211/ieee80211_proto.h user/adrian/net80211_tx/sys/net80211/ieee80211_var.h Modified: user/adrian/net80211_tx/sys/net80211/ieee80211.c ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211.c Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211.c Wed May 22 05:43:14 2013 (r250904) @@ -433,7 +433,8 @@ ieee80211_vap_setup(struct ieee80211com if_initname(ifp, name, unit); ifp->if_softc = vap; /* back pointer */ ifp->if_flags = IFF_SIMPLEX | IFF_BROADCAST | IFF_MULTICAST; - ifp->if_start = ieee80211_start; + ifp->if_transmit = ieee80211_vap_if_transmit; + ifp->if_qflush = ieee80211_vap_if_qflush; ifp->if_ioctl = ieee80211_ioctl; ifp->if_init = ieee80211_init; /* NB: input+output filled in by ether_ifattach */ @@ -628,6 +629,7 @@ ieee80211_vap_detach(struct ieee80211vap */ ieee80211_draintask(ic, &vap->iv_nstate_task); ieee80211_draintask(ic, &vap->iv_swbmiss_task); + ieee80211_draintask(ic, &vap->iv_tx_task); /* XXX band-aid until ifnet handles this for us */ taskqueue_drain(taskqueue_swi, &ifp->if_linktask); Modified: user/adrian/net80211_tx/sys/net80211/ieee80211_freebsd.c ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211_freebsd.c Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211_freebsd.c Wed May 22 05:43:14 2013 (r250904) @@ -809,7 +809,7 @@ static void bpf_track(void *arg, struct ifnet *ifp, int dlt, int attach) { /* NB: identify vap's by if_start */ - if (dlt == DLT_IEEE802_11_RADIO && ifp->if_start == ieee80211_start) { + if (dlt == DLT_IEEE802_11_RADIO && ifp->if_transmit == ieee80211_vap_if_transmit) { struct ieee80211vap *vap = ifp->if_softc; /* * Track bpf radiotap listener state. We mark the vap Modified: user/adrian/net80211_tx/sys/net80211/ieee80211_output.c ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211_output.c Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211_output.c Wed May 22 05:43:14 2013 (r250904) @@ -120,7 +120,7 @@ doprint(struct ieee80211vap *vap, int su * do this first. */ static int -ieee80211_start_pkt(struct ieee80211vap *vap, struct mbuf *m) +ieee80211_vap_start_pkt(struct ieee80211vap *vap, struct mbuf *m) { #define IS_DWDS(vap) \ (vap->iv_opmode == IEEE80211_M_WDS && \ @@ -359,16 +359,90 @@ ieee80211_start_pkt(struct ieee80211vap } /* - * Start method for vap's. All packets from the stack come - * through here. We handle common processing of the packets - * before dispatching them to the underlying device. + * Entry point for transmission for all VAPs. + * + * This sanitises the mbuf flags and queues it into the transmit + * queue. */ -void -ieee80211_start(struct ifnet *ifp) +int +ieee80211_vap_if_transmit(struct ifnet *ifp, struct mbuf *m) { struct ieee80211vap *vap = ifp->if_softc; struct ieee80211com *ic = vap->iv_ic; struct ifnet *parent = ic->ic_ifp; + + /* NB: parent must be up and running */ + if (!IFNET_IS_UP_RUNNING(parent)) { + IEEE80211_DPRINTF(vap, IEEE80211_MSG_OUTPUT, + "%s: ignore queue, parent %s not up+running\n", + __func__, parent->if_xname); + /* XXX stat */ + m_free(m); + return (EINVAL);/* XXX errno? */ + } + + IF_LOCK(&ifp->if_snd); + + /* Enforce queue limits */ + if (_IF_QFULL(&ifp->if_snd)) { + IF_UNLOCK(&ifp->if_snd); + m_free(m); + return (ENOBUFS); /* XXX errno? */ + } + + /* + * Sanitize mbuf flags for net80211 use. We cannot + * clear M_PWR_SAV or M_MORE_DATA because these may + * be set for frames that are re-submitted from the + * power save queue. + * + * NB: This must be done before ieee80211_classify as + * it marks EAPOL in frames with M_EAPOL. + * + * XXX TODO: for VAP frames coming in from the stack + * itself, we should just inject them directly into + * the vap rather than via ieee80211_vap_transmit(). + * Yes, they still need to go into the ifnet queue + * and be dequeued, but we can skip this particular + * check as they're already "in" the net80211 layer. + */ + m->m_flags &= ~(M_80211_TX - M_PWR_SAV - M_MORE_DATA); + _IF_ENQUEUE(&ifp->if_snd, m); + IF_UNLOCK(&ifp->if_snd); + + /* Schedule the deferred TX task */ + ieee80211_runtask(ic, &vap->iv_tx_task); + + return (0); +} + +void +ieee80211_vap_if_qflush(struct ifnet *ifp) +{ + struct mbuf *m; + + IF_LOCK(&ifp->if_snd); + do { + _IF_DEQUEUE(&ifp->if_snd, m); + if (m != NULL) + m_free(m); + } while (m != NULL); + IF_UNLOCK(&ifp->if_snd); +} + +/* + * Do deferred VAP transmit. + * + * This walks the ifnet send queue and dispatches whichever frames + * require it. + */ +void +ieee80211_vap_tx_task(void *arg, int npending) +{ + struct ieee80211vap *vap = (struct ieee80211vap *) arg; + struct ifnet *ifp = vap->iv_ifp; + struct ieee80211com *ic = vap->iv_ic; + struct ifnet *parent = ic->ic_ifp; struct mbuf *m; /* NB: parent must be up and running */ @@ -409,24 +483,20 @@ ieee80211_start(struct ifnet *ifp) IEEE80211_UNLOCK(ic); } + /* + * Dispatch frames to the VAP packet processing routine. + * + * Since this is the only place (in theory!) which is actually + * dispatching frames for this VAP, there shouldn't be any races + * between dispatch threads. So it's OK to not hold a lock across + * the dequeue. If multiple places could dequeue+dispatch we'd + * need a lock to ensure they were dispatched in the correct order + */ for (;;) { IFQ_DEQUEUE(&ifp->if_snd, m); if (m == NULL) break; - /* - * Sanitize mbuf flags for net80211 use. We cannot - * clear M_PWR_SAV or M_MORE_DATA because these may - * be set for frames that are re-submitted from the - * power save queue. - * - * NB: This must be done before ieee80211_classify as - * it marks EAPOL in frames with M_EAPOL. - */ - m->m_flags &= ~(M_80211_TX - M_PWR_SAV - M_MORE_DATA); - /* - * Bump to the packet transmission path. - */ - (void) ieee80211_start_pkt(vap, m); + (void) ieee80211_vap_start_pkt(vap, m); /* mbuf is consumed here */ } } Modified: user/adrian/net80211_tx/sys/net80211/ieee80211_proto.c ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211_proto.c Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211_proto.c Wed May 22 05:43:14 2013 (r250904) @@ -199,6 +199,7 @@ ieee80211_proto_vattach(struct ieee80211 callout_init(&vap->iv_mgtsend, CALLOUT_MPSAFE); TASK_INIT(&vap->iv_nstate_task, 0, ieee80211_newstate_cb, vap); TASK_INIT(&vap->iv_swbmiss_task, 0, beacon_swmiss, vap); + TASK_INIT(&vap->iv_tx_task, 0, ieee80211_vap_tx_task, vap); /* * Install default tx rate handling: no fixed rate, lowest * supported rate for mgmt and multicast frames. Default @@ -1792,7 +1793,7 @@ ieee80211_newstate_cb(void *xvap, int np * XXX Kick-start a VAP queue - this should be a method, * not if_start()! */ - if_start(vap->iv_ifp); + ieee80211_runtask(ic, &vap->iv_tx_task); /* bring up any vaps waiting on us */ wakeupwaiting(vap); Modified: user/adrian/net80211_tx/sys/net80211/ieee80211_proto.h ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211_proto.h Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211_proto.h Wed May 22 05:43:14 2013 (r250904) @@ -108,7 +108,9 @@ int ieee80211_raw_output(struct ieee8021 void ieee80211_send_setup(struct ieee80211_node *, struct mbuf *, int, int, const uint8_t [IEEE80211_ADDR_LEN], const uint8_t [IEEE80211_ADDR_LEN], const uint8_t [IEEE80211_ADDR_LEN]); -void ieee80211_start(struct ifnet *ifp); +int ieee80211_vap_if_transmit(struct ifnet *ifp, struct mbuf *m); +void ieee80211_vap_if_qflush(struct ifnet *ifp); +void ieee80211_vap_tx_task(void *, int); int ieee80211_send_nulldata(struct ieee80211_node *); int ieee80211_classify(struct ieee80211_node *, struct mbuf *m); struct mbuf *ieee80211_mbuf_adjust(struct ieee80211vap *, int, Modified: user/adrian/net80211_tx/sys/net80211/ieee80211_var.h ============================================================================== --- user/adrian/net80211_tx/sys/net80211/ieee80211_var.h Wed May 22 05:21:19 2013 (r250903) +++ user/adrian/net80211_tx/sys/net80211/ieee80211_var.h Wed May 22 05:43:14 2013 (r250904) @@ -362,6 +362,7 @@ struct ieee80211vap { int iv_nstate_arg; /* pending state arg */ struct task iv_nstate_task; /* deferred state processing */ struct task iv_swbmiss_task;/* deferred iv_bmiss call */ + struct task iv_tx_task; /* VAP deferred TX task */ struct callout iv_mgtsend; /* mgmt frame response timer */ /* inactivity timer settings */ int iv_inact_init; /* setting for new station */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201305220543.r4M5hFJo042176>