Linux-Wireless Archive on lore.kernel.org
 help / color / Atom feed
From: Arend Van Spriel <arend.vanspriel@broadcom.com>
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
Subject: Re: [PATCH 04/11] wil6210: fix PTK re-key race
Date: Fri, 13 Sep 2019 10:04:14 +0200
Message-ID: <74c0e328-320c-0999-836e-1bfb0fa224ff@broadcom.com> (raw)
In-Reply-To: <5716f475-856e-7fd2-637b-67927f4f78bc@wetzel-home.de>



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

  reply index

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-08  8:32 [PATCH 00/11] wil6210 patches Maya Erez
2019-09-08  8:32 ` [PATCH 01/11] wil6210: add wil_netif_rx() helper function Maya Erez
2019-09-12 15:08   ` Kalle Valo
2019-09-08  8:32 ` [PATCH 02/11] wil6210: add support for pci linkdown recovery Maya Erez
2019-09-12 15:22   ` Kalle Valo
2019-09-08  8:32 ` [PATCH 03/11] wil6210: add debugfs to show PMC ring content Maya Erez
2019-09-08  8:32 ` [PATCH 04/11] wil6210: fix PTK re-key race Maya Erez
2019-09-10 13:23   ` Kalle Valo
2019-09-11  7:50     ` Arend Van Spriel
2019-09-11 18:32     ` Alexander Wetzel
2019-09-12 17:39       ` Denis Kenzior
2019-09-12 21:04         ` Alexander Wetzel
2019-09-13  8:04           ` Arend Van Spriel [this message]
2019-09-13 14:33             ` Denis Kenzior
2019-09-13 20:48               ` Alexander Wetzel
2019-09-17 15:32                 ` Denis Kenzior
2019-09-13 18:43             ` Alexander Wetzel
2019-09-08  8:32 ` [PATCH 05/11] wil6210: make sure DR bit is read before rest of the status message Maya Erez
2019-09-08  8:32 ` [PATCH 06/11] wil6210: verify cid value is valid Maya Erez
2019-09-08  8:32 ` [PATCH 07/11] wil6210: properly initialize discovery_expired_work Maya Erez
2019-09-08  8:32 ` [PATCH 08/11] wil6210: report boottime_ns in scan results Maya Erez
2019-09-08  8:32 ` [PATCH 09/11] wil6210: use writel_relaxed in wil_debugfs_iomem_x32_set Maya Erez
2019-09-08  8:32 ` [PATCH 10/11] wil6210: fix RX short frame check Maya Erez
2019-09-08  8:32 ` [PATCH 11/11] wil6210: ignore reset errors for FW during probe Maya Erez

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74c0e328-320c-0999-836e-1bfb0fa224ff@broadcom.com \
    --to=arend.vanspriel@broadcom.com \
    --cc=alexander@wetzel-home.de \
    --cc=amasri@codeaurora.org \
    --cc=denkenz@gmail.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=merez@codeaurora.org \
    --cc=wil6210@qti.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org
	public-inbox-index linux-wireless

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git