From owner-freebsd-wireless@FreeBSD.ORG Sun Oct 7 17:57:17 2012 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 0A28E1065672 for ; Sun, 7 Oct 2012 17:57:17 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-da0-f54.google.com (mail-da0-f54.google.com [209.85.210.54]) by mx1.freebsd.org (Postfix) with ESMTP id BE71A8FC1D for ; Sun, 7 Oct 2012 17:57:16 +0000 (UTC) Received: by mail-da0-f54.google.com with SMTP id z9so1279046dad.13 for ; Sun, 07 Oct 2012 10:57:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=IMh6QL1HFyApdwvKzUysYxWOmk/ERIzhpAADERaQrYU=; b=IhfeUJhSrgiBTmzk71WQhvEaiiLCFCCjqukSAa9gWfmnVPP7bLEf4TMuluWF2aRFvu 1wEdAXqF2RrdmXOr0P8BJowSjmk+JPJmiXW9tlpoWaZkI2j9Jh8IxklmoXGWtShoGdGo HK/O61+Eik4krk851dGUCsXgj03FcLVjUxcgv3N2WSBDYThJ/cXofukARA/Qx9Mw0GvD vGbSeDArvrXKFgKKrWyqLnz5NXPxhiEZmeUwpb4Qw83hYvKEghd8LjiM3FKFavReDUze 3kitCKArykzlQvKFRFBEP3NQZeQSOZkCORUq4hPusgFR0q8XMx/Mwlmf3mCYqHWW6suD t12w== MIME-Version: 1.0 Received: by 10.66.72.132 with SMTP id d4mr37128287pav.61.1349632636014; Sun, 07 Oct 2012 10:57:16 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.68.223.136 with HTTP; Sun, 7 Oct 2012 10:57:15 -0700 (PDT) Received: by 10.68.223.136 with HTTP; Sun, 7 Oct 2012 10:57:15 -0700 (PDT) In-Reply-To: References: Date: Sun, 7 Oct 2012 10:57:15 -0700 X-Google-Sender-Auth: 4uUBMqIf0YBpO3XQXs2G_Ju_w2o Message-ID: From: Adrian Chadd To: PseudoCylon Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: freebsd-wireless@freebsd.org Subject: Re: [RFC] How the TX path works; I'd like to fix this before 10.x X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Oct 2012 17:57:17 -0000 Yes, the mgmt packet priority is worrying, but anything encrypted and/or with a sequence number needs to be serialised anyway. Maybe we can create per AC queues between the stack and driver. But that will need an API change. Hm! Adrian On Oct 7, 2012 3:25 AM, "PseudoCylon" wrote: > On Sat, Oct 6, 2012 at 3:00 PM, Adrian Chadd 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" wrote: > >> > >> > ------------------------------ > >> > > >> > Message: 4 > >> > Date: Fri, 5 Oct 2012 23:44:10 -0700 > >> > From: Adrian Chadd > >> > Subject: [RFC] How the TX path works; I'd like to fix this before 10.x > >> > To: freebsd-wireless@freebsd.org > >> > Message-ID: > >> > > >> > > >> > 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" >