Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 04/11] wil6210: fix PTK re-key race
To:     Alexander Wetzel <alexander@wetzel-home.de>,
        Denis Kenzior <denkenz@gmail.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        Maya Erez <merez@codeaurora.org>
Cc:     Ahmad Masri <amasri@codeaurora.org>,
        linux-wireless@vger.kernel.org, wil6210@qti.qualcomm.com
References: <1567931575-27984-5-git-send-email-merez@codeaurora.org>
 <20190910132315.D7AC7602F2@smtp.codeaurora.org>
 <7b636313-fa4a-5ee4-935a-ba2ed5dde1e5@wetzel-home.de>
 <2c6bc637-62c2-020c-ab83-d2e1677d96b2@gmail.com>
 <5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de>
From:   Arend Van Spriel <arend.vanspriel@broadcom.com>
Message-ID: <74c0e328-320c-0999-836e-1bfb0fa224ff@broadcom.com>
Date:   Fri, 13 Sep 2019 10:04:14 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
In-Reply-To: <5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk


On 9/12/2019 11:04 PM, Alexander Wetzel wrote:
> Hi Denis,
> 
>>> I don't know anything about the driver here but in mac80211 the idea 
>>> to avoid the race is to simply flush the queues prior deleting the 
>>> outgoing key.
>>
>> Maybe a silly question, but what does flushing the queue mean in this 
>> context?  Is it waiting for all the packets to be sent or dropping 
>> them on the floor?
>>
> 
> It's stopping them to make sure nothing can be added and then sends out 
> all MPDUs in the queues.
> 
>>>
>>> Now wpa_supplicant is not yet bypassing qdisks, but adding the socket 
>>> parameter PACKET_QDISC_BYPASS is basically a one-liner in 
>>> wpa_supplicant and should allow a generic way for drivers to avoid 
>>> the race with a simple queue flush...
>>
>> Can you expand on this actually?  What would the sequence of events be?
>>
> 
> 1) wpa_supplicant hands over eapol #4 to the kernel.
>     When bypassing the QDISC the frame is directly added to a driver
>     queue or directly send out. When the send call returns the driver
>     has eapol 4 either in the queuem already send it or the send command
>     has failed.
> 
> 2) wpa_supplicant deletes the old key (NL80211_CMD_DEL_KEY)
> 
> 3) The driver stops all hw queues and sends out all MPDUs queued up to
>     that time
> 
> 4) Driver makes sure no traffic can be send with no/wrong key or PN to
>     STA
> 
> 5) the driver really removes the key from the HW/installs the new and
>     resumes normal operation
> 
> I've just posted my hostpad patch to use PACKET_QDISC_BYPASS for eapol 
> frames; It's probably too optimistic and need more code to retry a 
> transmit to compensate for the missing QDISC buffers.
> 
>> Also, how would this be made to work with CONTROL_PORT over NL80211 ?
>>
> 
> Control port is an optional feature drivers can provide. wpa_supplicant 
> should just use it when available or fall back to the "traditional" path 
> when not. Now the driver don't has to flush all queues when using 
> control port, as long as it makes sure the control port frame will be 
> send out prior to deleting the key.
> 
> But then the driver must know that eapol frames will really be handed 
> over via control port; So I guess flushing all queues is still the 
> simpler solution. So I guess it will change next to nothing...

Well, in the steps you describe (maybe its just how you describe it) it 
relies on how the driver is handling it all. I mean step 4) seems more 
the goal of the whole approach.

Basically, we now have two bypass methods dealing with the same/similar 
issue:

1) bypass the QDISC.
2) bypass network stack entirely with CONTROL_PORT.

How does option 1) work for drivers that skip the QDISC for all traffic 
and rely on mac80211 to schedule the packets? Guess mac80211 can control 
that, right?

Regards,
Arend