On Sat, Oct 19, 2019 at 1:14 AM Alexander Wetzel <alexander@xxxxxxxxxxxxxx> wrote: > > Am 16.10.19 um 12:34 schrieb Krishna Chaitanya: > > On Sun, Sep 1, 2019 at 8:50 PM Krishna Chaitanya > > <chaitanya.mgit@xxxxxxxxx> wrote: > >> > >> On Sun, Sep 1, 2019 at 4:15 PM Alexander Wetzel > >> <alexander@xxxxxxxxxxxxxx> wrote: > >>> > >>> > >>>>> Normally there are next no buffers in the tx path. When you hand over a > >>>>> frame to the kernel it's - according what I remember when investigating > >>>>> the issue - an atomic operation till the skb reaches mac80211 or > >>>>> whatever other driver we use. And the few "buffers" we have are fully > >>>>> driver controlled and we basically can just make sure those are flushed > >>>>> when we replace the key. The only potential issue I see are traffic > >>>>> shapers but then I think I eapol frames bypass them already. > >>> > >>>> the issue of race b/w set_key and send_m4 probably caused the delays in > >>>> socket_queues/QDISC for M4, can you please elaborate on how EAPoL > >>>> bypasses QDISC? > >>>> I don't see PACKET_QDISC_BYPASS being set for l2_packet_send. > >>> > >>> I had some hope that eapol packets would have a build in fast lane but > >>> you are right: They get added to the QDISC queue and processed like any > >>> other packets. > >>> > >>> But why do we not just set PACKET_QDISC_BYPASS for eapol frames? > >>> Just tried it and it seems to work as it should: fine. > >>> > >>> We could allow this setsockopt() call to fail, so all kernels < 3.14 > >>> would continue to push the eapol packets through the QDISC. But starting > >>> with 3.14 drivers flushing their queues prior to deleting a key can be > >>> sure the eapol frame has been send. > >>> > >>> The only potential down side I can find is the - required - lack of > >>> buffers. The driver could therefore refuse to accept the eapol packet > >>> which should cause a disconnect instead a queued and later send eapol frame. > >>> Using the control port for eapol would of course still better, but using > >>> PACKET_QDISC_BYPASS when this is not working looks like a good fallback > >>> and an improvement compared to as things are today. > >>> > >> I agree using that option to bypass QDISC is a definite improvment. > > > > To summarize: Patching hostap/wpa_s to use PACKET_QDISC_BYPASS is enough > > (along with driver stalling the set_key till EAPoL is flushed) and we > > don't need this patch > > anymore? Please confirm. > > > That is my understanding, yes. > > But the "simple" patch (https://patchwork.ozlabs.org/patch/1161750/) to > enable PACKET_QDISC_BYPASS in hostapd is little more than a POC and > doing it right will be quite invasive. > > And using CONTROL_PORT has some benefits we can't get otherwise. Here > the ones I see: > - The drivers may not have to flush the queues in all constellations > - EAPOL MPDUs can be send with the basic rate > - EAPOL MPDUs will normally be a separate TID Agree, I guess most of the drivers/FW do look for EAPoL and handle it specially, but good to have control_port. > Denis may have more, so far I only had passing contact with CONTROL_PORT. > > We discussed the the different solutions recently here on linux wireless: > https://marc.info/?l=linux-wireless&m=156873433609504&w=2 > > In short CONTROL_PORT was designed to handle this kind of races and > since the API is available should be the preferred solution. > > Regardless of using CONTROL_PORT or queue flushes the drivers/cards will > have to make sure they can rekey correctly. There are many pitfalls and > nearly all drivers have some oversights here. It does require multiple knobs to set the right to get initial and rekeying working. So, I guess we should try to get this patch along with bypassing QDISC patch in and set expectations to the drivers to make sure they flush EAPoL properly. _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
