* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Felix Fietkau @ 2016-12-05 17:28 UTC (permalink / raw)
To: Ben Greear, Johannes Berg, linux-wireless
In-Reply-To: <5845813C.8070904@candelatech.com>
On 2016-12-05 16:01, Ben Greear wrote:
>
>
> On 12/05/2016 06:53 AM, Johannes Berg wrote:
>>
>>> Unless I screwed up, this patch also returns an average.
>>
>> Oops, sorry. I missed the whole mac_div() indirection thing.
>>
>> I'm not super convinced anyway though - all of this data already is
>> available in a much more reliable fashion, even trackable when stations
>> are removed (all data gets sent in the DEL_STATION notification), so
>> adding a crippled way to get the same data seems a bit strange?
>
> Ethtool stats are easy to program against, and I am already making the
> get-stats call to get other things, so it was a quick tweak to return
> these additional values. I agree the stats are of somewhat limited worth,
> but the cost to get them is also pretty small code wise :)
You're still adding bloat for everybody for the sake of a little
convenience on your side. I think that's a bad trade-off.
nl80211 is not *that* hard to program against...
I don't really see much point in duplicating an arbitrary subset of
nl80211 information in ethtool.
- Felix
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Adrian Chadd @ 2016-12-05 17:23 UTC (permalink / raw)
To: Ben Greear
Cc: Johannes Berg, Michal Kazior, linux-wireless,
ath10k@lists.infradead.org
In-Reply-To: <5845827B.2000000@candelatech.com>
fwiw, I'm facing the same kinds of cleanup problems with my port of
(oct 2015) ath10k to freebsd.
The oct 2015 ath10k tree doesn't have the firmware per-txq/tid/peer
feedback stuff in it, so this hasn't yet bitten me, but there rest of
the races have - mostly surrounding handling pending TX frames when a
VAP is deleted (vdev/interface in ath10k/mac80211 language) and if any
TX frames were stuck. Stuck TX frames happens more often than I'd like
because of how earlier firmware required peer entries to first appear
in the hardware.
Maybe we need some kind of lifecycle checkpoint for things like peer
addition/removal (for the txq issues ben had before) and the ability
to ask the firmware to stop/flush HTT TX and re-start it. That way we
can cleanly add/remove interfaces at any point without worrying about
any dangling frames in the transmit queue waiting for completion.
-adrian
^ permalink raw reply
* Re: [PATCH 1/7] rtlwifi: btcoexist: Update routines for RTL8192EE
From: Larry Finger @ 2016-12-05 17:17 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvalo, devel, Ping-Ke Shih, linux-wireless
In-Reply-To: <20161205113832.GD8176@mwanda>
On 12/05/2016 05:38 AM, Dan Carpenter wrote:
> On Sat, Dec 03, 2016 at 11:32:01AM -0600, Larry Finger wrote:
>> From: Ping-Ke Shih <pkshih@realtek.com>
>>
>> The btcoexist routines for the RTL8192EE have been extensively rewritten.
>>
>
> This patch does a million things and is totally unreviewable. The
> patch description doesn't say what patch does or why. It's 5k lines
> of diff. What the heck???
I am caught in a bind. The BT coexistence routines are written by a Realtek
contractor. As I cannot get the individual patches from Realtek, there is no
chance of getting them from a 3rd party.
Larry
^ permalink raw reply
* Re: [PATCH] mac80211: fix Tx BA session stuck issue during sw scanning
From: Chris Chiu @ 2016-12-05 16:56 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, linux
In-Reply-To: <1480949847.31788.33.camel@sipsolutions.net>
On Mon, Dec 5, 2016 at 10:57 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>> > The premise seems fairly reasonable, although I'm a little worried
>> > that if so much new traffic is coming in we never finish the scan
>> > suspend? Actually, the queues are still stopped, so it's only
>> > management frames that can come in, so that should be ok?
>> >
>>
>> Actually it will finish scan eventually and back to SCAN_DECISION
>> state but almost 20~30 seconds elapsed. The local->scanning should be
>> cleared after all channels been scanned. However, from the debug
>> messages I added in ieee80211_iface_work(), it still returns when
>> check local->scanning and the DELBA still has no chance to be
>> transferred then stuck again at the next scan state machine. Supposed
>> to be another scan request issued but I don't know who's the killer.
>> Except to find who issue the next scan request, BA session have no
>> chance to reset in this long scan period (>20s) still need to be
>> taken
>> care.
>
> No no, you misunderstood. My question is more like:
>
> Where is this traffic coming from, since netdev queues should be
> stopped?
>
> And then, if there's so much traffic coming in that we can take 20-30
> seconds to send it out, could we - with the change - get stuck forever?
>
My test scenario is just simply ping (not greedy ping). When it
stalls, you just see 1 ping packet has been retried on the air for 3
times w/o ack from AP. Then no more tx QoS data packet observed on the
air. From the log, it shows the tx ba session been expired and a DELBA
is queued but never sent out. So the STA wants to terminate current BA
session but can't make it. The air capture pcap file shows It can't
deal with the coming "Action" frame for "ADDBA" also. The Block Ack
state machine just stops until the whole sw scan done. In my case,
it's >300 seconds.
So this patch just tries to seek a chance to send out the DELBA and
process the BA related packets in ieee80211_iface_work(). It tries to
avoid the stuck forever thing. I tried 30000+ pings and verified no
stuck observed.
>> You're right. I just want to clear_bit and set_bit in this case,
>> sorry for that confusing. Or any better suggestion?
>
> We seem to be using set_bit/clear_bit so that seems reasonable, unless
> you can prove that all of those are under the lock and we can remove
> the atomics entirely ... Not that it matters hugely, we don't scan all
> the time after all!
>
I agree. Will modify this patch and then send again for approval.
> johannes
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Marcel Holtmann @ 2016-12-05 15:40 UTC (permalink / raw)
To: Johannes Berg; +Cc: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480950886.31788.44.camel@sipsolutions.net>
Hi Johannes,
>>> Well, no, that'd only work with an open connection :)
>
> Actually, it also works fairly easily for when firmware has 4-way-
> handshake offload, which will be coming to a kernel near you soon.
>
>> And even that is questionable in my mind for some of the more
>> advanced cases.
>
> Well, at least in that case you can have things running (for a while)
> if the manager crashes?
>
>> But I'm not sure what your point is, do you still have objections to
>> this approach?
>
> Well, first of all, you can keep things running, at least until you've
> figured out how to restart wpa_supplicant/whatever.
>
> There also aren't really any important resources to clear when
> userspace dies, at least nothing that userspace can't trivially clear
> later by disconnecting (even unconditionally and ignoring the
> result)...
>
> So basically I just don't see the advantage. It feels like trading a
> single line of userspace code to disconnect with some (not super
> complex, but still somewhat involved) kernel-side tracking. That
> doesn't really seem like a worthwhile tradeoff to me.
I do not get this argument. That is what WEXT forced you to do since the APIs were awful and clueless. I thought we have moved passed this messy userspace and racy guess work.
If I am a wireless management daemon, I want to decide what happens when I unexpectedly die or get killed for some reason where it is no longer in my control to do this. And there is no guarantee that the daemon even gets restarted. Now you leave dangling connections around. If userspace asks the kernel to clean up after it, then the kernel should be able to do so. That is the only sane option since everything else is racy. The daemon is not in control and has no idea how much time has passed or what happened while it was away. So the only clean thing to do, ask the kernel to disconnect in case something unexpected happens.
And frankly if the wireless daemon can not ask the kernel to clean up, how would a connection manager then even begin to handle this properly. It causes more races and more issues. It becomes a state tracking nightmare. And this is not some theoretical issue, we have seen all the issues with ConnMan. Remember that ConnMan runs in production consumer devices. So these things are real. These systems have to kill daemons for resource limits and other reasons that are out of the control of the daemon itself. And there is no recovery, and if there is, there is no knowing when that happens.
So userspace can clear later argument is not valid for me. If your userspace wants to do that, then fine. Our userspace does not want unsupervised or unmanaged connections. If it goes away, then it wants that these are cleared. And that option is clearly what this patch provides. I find this a clean and useful addition to nl80211. Why are you saying this is not worthwhile?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-05 15:32 UTC (permalink / raw)
To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480950886.31788.44.camel@sipsolutions.net>
Hi Johannes,
On 12/05/2016 09:14 AM, Johannes Berg wrote:
>>> Well, no, that'd only work with an open connection :)
>
> Actually, it also works fairly easily for when firmware has 4-way-
> handshake offload, which will be coming to a kernel near you soon.
>
Great, but still not applicable for all devices, and still questionable
how useful an unmanaged connection actually is. But okay.
>> And even that is questionable in my mind for some of the more
>> advanced cases.
>
> Well, at least in that case you can have things running (for a while)
> if the manager crashes?
>
To what purpose? That's like saying that maybe a socket should be kept
open in case an application crashes.
>> But I'm not sure what your point is, do you still have objections to
>> this approach?
>
> Well, first of all, you can keep things running, at least until you've
> figured out how to restart wpa_supplicant/whatever.
>
> There also aren't really any important resources to clear when
> userspace dies, at least nothing that userspace can't trivially clear
> later by disconnecting (even unconditionally and ignoring the
> result)...
Sure, but in the meantime you have a completely unmanaged connection,
with no connection manager to tell you what the state of it is. Maybe
okay for a command line system with a brain in front of it, but not much
use anywhere else.
But for that use case the SOCKET_OWNER attribute can be omitted...
>
> So basically I just don't see the advantage. It feels like trading a
> single line of userspace code to disconnect with some (not super
> complex, but still somewhat involved) kernel-side tracking. That
> doesn't really seem like a worthwhile tradeoff to me.
>
Well, we'd be trading quite a bit of race-prone user space code that
performs the clean up on start up for one line of attributes. I call
that a good trade.
Also, I thought it was good form / the UNIX way to clean up after
processes exit ungracefully. You seem to be arguing against that? I
would argue the kernel should be doing the opposite. It should always
clean up the connection unless user space has indicated it can be 'reused'.
Regards,
-Denis
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Ben Greear @ 2016-12-05 15:06 UTC (permalink / raw)
To: Johannes Berg, Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <1480950054.31788.36.camel@sipsolutions.net>
On 12/05/2016 07:00 AM, Johannes Berg wrote:
> On Mon, 2016-12-05 at 06:57 -0800, Ben Greear wrote:
>
>> I think clearing sdata-in-driver would fix the ath10k problem, at
>> least, but I was afraid it would break something else in mac80211 or
>> maybe in other thick firmware drivers.
>
> It's pretty much an internal thing - not sure what it'd break. OTOH,
> some drivers might actually assume that iterating finds them all, if
> they never clear the data even across a restart?
>
>> One way or another, we cannot be iterating over interfaces while
>> the interfaces are at the same time being (re)added.
>
> Well, we obviously *can* be, and we do in fact do that - it's just that
> ath10k specifically has issues with the data it's putting there, no?
It causes races that appear to be very difficult to resolve in the
driver alone. On normal bringup of an interface, the sdata-in-driver
flag is only set at the bottom of the add-interface. In case of re-config,
the flag is already set, and never cleared, so behaviour is different
w/regard to the iterate.
>
>> Maybe mac80211 should explicitly remove all interfaces from the
>> driver during crash recovery?
>
> I don't think that'll work. Removing them would interact with the
> firmware, which is dead, etc. That'd just cause trouble.
That issue already causes trouble and is dealt with in ath10k, I think,
but clearing the flag in mac80211 would probably be enough to fix the
iterate logic.
>> And the behaviour needs to be clearly documented somewhere
>> easy to find so that we can think about and program to the correct
>> API behaviour.
>
> We assume that the driver resets all its internal state - this whole
> interface iteration is a corner case we hadn't considered, I suppose.
Yeah, tricky beastie. I think the txq issue is also part of this since there
are references up in mac80211 and also down in ath10k. Part of my hack
to clean up that crash might be resolved by mac80211 doing better cleanup
API when firmware crashes.
Thanks,
Ben
>
> johannes
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-05 15:14 UTC (permalink / raw)
To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <5845824B.4090304@gmail.com>
> > Well, no, that'd only work with an open connection :)
Actually, it also works fairly easily for when firmware has 4-way-
handshake offload, which will be coming to a kernel near you soon.
> And even that is questionable in my mind for some of the more
> advanced cases.
Well, at least in that case you can have things running (for a while)
if the manager crashes?
> But I'm not sure what your point is, do you still have objections to
> this approach?
Well, first of all, you can keep things running, at least until you've
figured out how to restart wpa_supplicant/whatever.
There also aren't really any important resources to clear when
userspace dies, at least nothing that userspace can't trivially clear
later by disconnecting (even unconditionally and ignoring the
result)...
So basically I just don't see the advantage. It feels like trading a
single line of userspace code to disconnect with some (not super
complex, but still somewhat involved) kernel-side tracking. That
doesn't really seem like a worthwhile tradeoff to me.
johannes
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-05 15:05 UTC (permalink / raw)
To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480949899.31788.34.camel@sipsolutions.net>
Hi Johannes,
On 12/05/2016 08:58 AM, Johannes Berg wrote:
>
>> Detecting it is easy, sure. But I'm a bit lost on how you propose
>> to
>> 'use' it. The connection is active up until the next rekey
>> event. If
>> rekey offloading is supported, then this might never involve user
>> space.
>> But if it isn't (and I can't get rekey offload to work on any
>> recent
>> kernel I tried) then how do you propose userspace obtains the rekey
>> counter, or the various keys used in the connection?
>>
>> What about all the other state information? FT, Pre-Authentication,
>> etc?
>
> Well, no, that'd only work with an open connection :)
>
And even that is questionable in my mind for some of the more advanced
cases.
But I'm not sure what your point is, do you still have objections to
this approach?
Regards,
-Denis
^ permalink raw reply
* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Ben Greear @ 2016-12-05 15:01 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <1480949600.31788.29.camel@sipsolutions.net>
On 12/05/2016 06:53 AM, Johannes Berg wrote:
>
>> Unless I screwed up, this patch also returns an average.
>
> Oops, sorry. I missed the whole mac_div() indirection thing.
>
> I'm not super convinced anyway though - all of this data already is
> available in a much more reliable fashion, even trackable when stations
> are removed (all data gets sent in the DEL_STATION notification), so
> adding a crippled way to get the same data seems a bit strange?
Ethtool stats are easy to program against, and I am already making the
get-stats call to get other things, so it was a quick tweak to return
these additional values. I agree the stats are of somewhat limited worth,
but the cost to get them is also pretty small code wise :)
> In any case I'd want you to resend with the /* pr_..*/ stuff removed.
I can respin with the comment removed.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Johannes Berg @ 2016-12-05 15:00 UTC (permalink / raw)
To: Ben Greear, Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <58458058.5020008@candelatech.com>
On Mon, 2016-12-05 at 06:57 -0800, Ben Greear wrote:
> I think clearing sdata-in-driver would fix the ath10k problem, at
> least, but I was afraid it would break something else in mac80211 or
> maybe in other thick firmware drivers.
It's pretty much an internal thing - not sure what it'd break. OTOH,
some drivers might actually assume that iterating finds them all, if
they never clear the data even across a restart?
> One way or another, we cannot be iterating over interfaces while
> the interfaces are at the same time being (re)added.
Well, we obviously *can* be, and we do in fact do that - it's just that
ath10k specifically has issues with the data it's putting there, no?
> Maybe mac80211 should explicitly remove all interfaces from the
> driver during crash recovery?
I don't think that'll work. Removing them would interact with the
firmware, which is dead, etc. That'd just cause trouble.
> And the behaviour needs to be clearly documented somewhere
> easy to find so that we can think about and program to the correct
> API behaviour.
We assume that the driver resets all its internal state - this whole
interface iteration is a corner case we hadn't considered, I suppose.
johannes
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Johannes Berg @ 2016-12-05 14:58 UTC (permalink / raw)
To: Denis Kenzior, Andrew Zaborowski, linux-wireless
In-Reply-To: <58457FEA.4030305@gmail.com>
> Detecting it is easy, sure. But I'm a bit lost on how you propose
> to
> 'use' it. The connection is active up until the next rekey
> event. If
> rekey offloading is supported, then this might never involve user
> space.
> But if it isn't (and I can't get rekey offload to work on any
> recent
> kernel I tried) then how do you propose userspace obtains the rekey
> counter, or the various keys used in the connection?
>
> What about all the other state information? FT, Pre-Authentication,
> etc?
Well, no, that'd only work with an open connection :)
johannes
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.
From: Ben Greear @ 2016-12-05 14:58 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <1480949686.31788.31.camel@sipsolutions.net>
On 12/05/2016 06:54 AM, Johannes Berg wrote:
> On Mon, 2016-12-05 at 06:47 -0800, Ben Greear wrote:
>>
>> On 12/05/2016 05:59 AM, Johannes Berg wrote:
>>>
>>> +static ssize_t misc_read(struct file *file, char __user *user_buf,
>>>>
>>>> + size_t count, loff_t *ppos)
>>>> +{
>>>> + struct ieee80211_local *local = file->private_data;
>>>> + size_t bufsz = 1000;
>>>> + char *buf = kzalloc(bufsz, GFP_KERNEL);
>>>
>>> You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think
>>> you
>>> can put on the stack?
>>
>> I actually run with 64 queues in my tree,
>
> Heh, well, in that case the 1000 is actually potentially too small for
> you :) (it'll work because there never are many packets on the queues
> though)
>
>> and either way, I thought large-ish things on the stack were frowned
>> upon for systems that want to run smaller stacks?
>
> Yeah but the limit is more like 1KB :)
>
> Maybe allocate, but actually do 16*NUM_QUEUES? The 1000 is just
> arbitrarily magic in there.
Sounds good, I'll respin it.
Thanks,
Ben
>
> johannes
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Ben Greear @ 2016-12-05 14:57 UTC (permalink / raw)
To: Johannes Berg, Michal Kazior; +Cc: linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <1480945950.31788.4.camel@sipsolutions.net>
On 12/05/2016 05:52 AM, Johannes Berg wrote:
> On Mon, 2016-12-05 at 09:13 +0100, Michal Kazior wrote:
>> On 2 December 2016 at 03:29, <greearb@candelatech.com> wrote:
>>>
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> This appears to fix a problem where ath10k firmware would crash,
>>> mac80211 would start re-adding interfaces to the driver, but the
>>> iterate-active-interfaces logic would then try to use the half-
>>> built
>>> interfaces. With a bit of extra debug to catch the problem, the
>>> ath10k crash looks like this:
>>>
>>> ath10k_pci 0000:05:00.0: Initializing arvif: ffff8801ce97e320 on
>>> vif: ffff8801ce97e1d8
>>>
>>> [the print that happens after arvif->ar is assigned is not shown,
>>> so code did not make it that far before
>>> the tx-beacon-nowait method was called]
>>>
>>> tx-beacon-nowait: arvif: ffff8801ce97e320 ar: (null)
>> [...]
>>>
>>>
>>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>>> ---
>>> net/mac80211/util.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
>>> index 863f2c1..abe1f64 100644
>>> --- a/net/mac80211/util.c
>>> +++ b/net/mac80211/util.c
>>> @@ -705,7 +705,7 @@ static void __iterate_interfaces(struct
>>> ieee80211_local *local,
>>> break;
>>> }
>>> if (!(iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL)
>>> &&
>>> - active_only && !(sdata->flags &
>>> IEEE80211_SDATA_IN_DRIVER))
>>> + (active_only && (local->in_reconfig || !(sdata-
>>>> flags & IEEE80211_SDATA_IN_DRIVER))))
>>> continue;
>>
>> Doesn't this effectivelly prevent you from iterating over interfaces
>> completely during reconfig? As you bring up interfaces you might
>> need/want to iterate over others to re-adjust your own state.
>
> Agree, that doesn't really make sense.
>
>> I'd argue there should be another flag, IEEE80211_SDATA_RESUMING,
>> used with sdata->flags for resuming so that once it is re-added to
>> the driver it can be cleared (and therefore properly iterated over).
>
> That would make some sense, or perhaps the sdata_in_driver should be
> cleared (and remembered elsewhere) at some point during the restart.
I think clearing sdata-in-driver would fix the ath10k problem, at least,
but I was afraid it would break something else in mac80211 or maybe in
other thick firmware drivers.
One way or another, we cannot be iterating over interfaces while
the interfaces are at the same time being (re)added.
Maybe mac80211 should explicitly remove all interfaces from the driver
during crash recovery? And the behaviour needs to be clearly documented somewhere
easy to find so that we can think about and program to the correct API
behaviour.
Thanks,
Bne
>
> johannes
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] mac80211: fix Tx BA session stuck issue during sw scanning
From: Johannes Berg @ 2016-12-05 14:57 UTC (permalink / raw)
To: Chris Chiu; +Cc: linux-wireless, linux
In-Reply-To: <CAB4CAwfJeE15nhXuLi_V-FYeCj4wj=1ojij+pwZ8V7=gdER76Q@mail.gmail.com>
> > The premise seems fairly reasonable, although I'm a little worried
> > that if so much new traffic is coming in we never finish the scan
> > suspend? Actually, the queues are still stopped, so it's only
> > management frames that can come in, so that should be ok?
> >
>
> Actually it will finish scan eventually and back to SCAN_DECISION
> state but almost 20~30 seconds elapsed. The local->scanning should be
> cleared after all channels been scanned. However, from the debug
> messages I added in ieee80211_iface_work(), it still returns when
> check local->scanning and the DELBA still has no chance to be
> transferred then stuck again at the next scan state machine. Supposed
> to be another scan request issued but I don't know who's the killer.
> Except to find who issue the next scan request, BA session have no
> chance to reset in this long scan period (>20s) still need to be
> taken
> care.
No no, you misunderstood. My question is more like:
Where is this traffic coming from, since netdev queues should be
stopped?
And then, if there's so much traffic coming in that we can take 20-30
seconds to send it out, could we - with the change - get stuck forever?
> You're right. I just want to clear_bit and set_bit in this case,
> sorry for that confusing. Or any better suggestion?
We seem to be using set_bit/clear_bit so that seems reasonable, unless
you can prove that all of those are under the lock and we can remove
the atomics entirely ... Not that it matters hugely, we don't scan all
the time after all!
johannes
^ permalink raw reply
* Re: [PATCH][RFC] cfg80211: NL80211_ATTR_SOCKET_OWNER support for CMD_CONNECT
From: Denis Kenzior @ 2016-12-05 14:55 UTC (permalink / raw)
To: Johannes Berg, Andrew Zaborowski, linux-wireless
In-Reply-To: <1480945883.31788.3.camel@sipsolutions.net>
Hi Johannes,
On 12/05/2016 07:51 AM, Johannes Berg wrote:
> On Fri, 2016-12-02 at 21:56 +0100, Andrew Zaborowski wrote:
>> Disconnect or deauthenticate when the owning socket is closed if this
>> flag has been supplied to CMD_CONNECT, CMD_AUTHENTICATE or
>> CMD_ASSOCIATE.
>
> Huh. That I think needs a lot more commit log to justify this code -
> why do you think this would be useful? Connection managers dying don't
> really mean you need to disconnect immediately, after all, and if they
> restart it's pretty simple to detect the existing connection and
> perhaps even use it...
>
Detecting it is easy, sure. But I'm a bit lost on how you propose to
'use' it. The connection is active up until the next rekey event. If
rekey offloading is supported, then this might never involve user space.
But if it isn't (and I can't get rekey offload to work on any recent
kernel I tried) then how do you propose userspace obtains the rekey
counter, or the various keys used in the connection?
What about all the other state information? FT, Pre-Authentication, etc?
Can you point me to a connection manager that actually performs this
'reuse' and not just simply wipes the state clean when starting?
Regards,
-Denis
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.
From: Johannes Berg @ 2016-12-05 14:54 UTC (permalink / raw)
To: Ben Greear, linux-wireless
In-Reply-To: <58457DF8.5020808@candelatech.com>
On Mon, 2016-12-05 at 06:47 -0800, Ben Greear wrote:
>
> On 12/05/2016 05:59 AM, Johannes Berg wrote:
> >
> > +static ssize_t misc_read(struct file *file, char __user *user_buf,
> > >
> > > + size_t count, loff_t *ppos)
> > > +{
> > > + struct ieee80211_local *local = file->private_data;
> > > + size_t bufsz = 1000;
> > > + char *buf = kzalloc(bufsz, GFP_KERNEL);
> >
> > You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think
> > you
> > can put on the stack?
>
> I actually run with 64 queues in my tree,
Heh, well, in that case the 1000 is actually potentially too small for
you :) (it'll work because there never are many packets on the queues
though)
> and either way, I thought large-ish things on the stack were frowned
> upon for systems that want to run smaller stacks?
Yeah but the limit is more like 1KB :)
Maybe allocate, but actually do 16*NUM_QUEUES? The 1000 is just
arbitrarily magic in there.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Johannes Berg @ 2016-12-05 14:53 UTC (permalink / raw)
To: Ben Greear, linux-wireless
In-Reply-To: <58457D63.2010605@candelatech.com>
> Unless I screwed up, this patch also returns an average.
Oops, sorry. I missed the whole mac_div() indirection thing.
I'm not super convinced anyway though - all of this data already is
available in a much more reliable fashion, even trackable when stations
are removed (all data gets sent in the DEL_STATION notification), so
adding a crippled way to get the same data seems a bit strange?
In any case I'd want you to resend with the /* pr_..*/ stuff removed.
johannes
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.
From: Johannes Berg @ 2016-12-05 13:59 UTC (permalink / raw)
To: greearb, linux-wireless
In-Reply-To: <1480442753-6830-1-git-send-email-greearb@candelatech.com>
+static ssize_t misc_read(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ieee80211_local *local = file->private_data;
> + size_t bufsz = 1000;
> + char *buf = kzalloc(bufsz, GFP_KERNEL);
You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think you
can put on the stack?
johannes
^ permalink raw reply
* Re: [PATCH 2/4] cfg80211: Add new NL80211_CMD_SET_BTCOEX_PRIORITY to support BTCOEX
From: Johannes Berg @ 2016-12-05 14:49 UTC (permalink / raw)
To: c_traja, linux-wireless; +Cc: ath10k, tamizhchelvam
In-Reply-To: <1478610932-21954-3-git-send-email-c_traja@qti.qualcomm.com>
On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
>
> + * struct cfg80211_btcoex_priority - BTCOEX support frame type
> + *
> + * This structure defines the driver supporting frame types for
> BTCOEX
> + *
> + * @wlan_be_preferred: best effort frames preferred over bt traffic
> + * @wlan_bk_preferred: background frames preferred over bt traffic
> + * @wlan_vi_preferred: video frames preferred over bt traffic
> + * @wlan_vo_preferred: voice frames preferred over bt traffic
> + * @wlan_beacon_preferred: beacon preferred over bt traffic
> + * @wlan_mgmt_preferred: management frames preferred ovet bt traffic
typo: over
>
> /**
> + * wiphy_btcoex_support_flags
> + * This enum has the driver supported frame types for BTCOEX.
> + * @WIPHY_WLAN_BE_PREFERRED - Supports Best Effort frame for BTCOEX
> + * @WIPHY_WLAN_BK_PREFERRED - supports Background frame for BTCOEX
> + * @WIPHY_WLAN_VI_PREFERRED - supports Video frame for BTCOEX
> + * @WIPHY_WLAN_VO_PREFERRED - supports Voice frame for BTCOEX
> + * @WIPHY_WLAN_BEACON_PREFERRED - supports Beacon frame for BTCOEX
> + * @WIPHY_WLAN_MGMT_PREFERRED - supports Management frames for
> BTCOEX.
> + */
That's not making much sense to me?
> +/**
> + * enum wiphy_btcoex_priority - BTCOEX priority level
> + * This enum defines priority level for BTCOEX
> + * WIPHY_WLAN_PREFERRED_LOW - low priority frames over BT traffic
> + * WIPHY_WLAN_PREFERRED_HIGH - high priority frames over BT traffic
> + */
> +
> +enum wiphy_btcoex_priority {
> + WIPHY_WLAN_PREFERRED_LOW = false,
> + WIPHY_WLAN_PREFERRED_HIGH = true,
> +};
That false/true seems just strange.
> + * @btcoex_support_flags: This will have the driver supported
> + * frame types for BTCOEX. This value filled by using
> + * %enum wiphy_btcoex_support_flags while driver
> + * initialization.
The whole "will have" isn't really clear.
> + * @NL80211_ATTR_SET_BTCOEX_PRIORITY: nested attribute for driver
> supporting
> + * the BTCOEX. When used with
> %NL80211_CMD_SET_BTCOEX_PRIORITY it contains
> + * attributes according &enum nl80211_btcoex_priority to
> indicate
> + * which frame has high priority over BT.
There should be no "SET" in there.
> /**
> + * enum nl80211_btcoex_priority - BTCOEX parameter attributes
> + * This strcuture has enum values for driver supported wlan
> + * frame type for BTCOEX.
> + * @NL80211_WLAN_BE_PREFERRED - Best Effort frame
> + * @NL80211_WLAN_BK_PREFERRED - Background frame
> + * @NL80211_WLAN_VI_PREFERRED - Video frame
> + * @NL80211_WLAN_VO_PREFERRED - Voice frame
> + * @NL80211_WLAN_BEACON_PREFERRED - BEACON frame
> + * @NL80211_WLAN_MGMT_PREFERRED - MGMT frame
> + */
> +
> +enum nl80211_btcoex_priority {
> + __NL80211_WLAN_PREFERRED_INVALID,
> + NL80211_WLAN_BE_PREFERRED,
> + NL80211_WLAN_BK_PREFERRED,
> + NL80211_WLAN_VI_PREFERRED,
> + NL80211_WLAN_VO_PREFERRED,
> + NL80211_WLAN_BEACON_PREFERRED,
> + NL80211_WLAN_MGMT_PREFERRED,
> + __NL80211_WLAN_PREFERRED_LAST,
> + NL80211_WLAN_PREFERRED_MAX =
> + __NL80211_WLAN_PREFERRED_LAST - 1,
> +};
Wouldn't a bitmap be easier?
johannes
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: Show pending txqlen in debugfs.
From: Ben Greear @ 2016-12-05 14:47 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <1480946371.31788.7.camel@sipsolutions.net>
On 12/05/2016 05:59 AM, Johannes Berg wrote:
> +static ssize_t misc_read(struct file *file, char __user *user_buf,
>> + size_t count, loff_t *ppos)
>> +{
>> + struct ieee80211_local *local = file->private_data;
>> + size_t bufsz = 1000;
>> + char *buf = kzalloc(bufsz, GFP_KERNEL);
>
> You need at most IEEE80211_MAX_QUEUES * 16 (==256) which I think you
> can put on the stack?
I actually run with 64 queues in my tree, and either way, I thought large-ish
things on the stack were frowned upon for systems that want to run smaller stacks?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 1/4] cfg80211: Add support to enable or disable btcoex
From: Johannes Berg @ 2016-12-05 14:46 UTC (permalink / raw)
To: c_traja, linux-wireless; +Cc: ath10k, tamizhchelvam
In-Reply-To: <1478610932-21954-2-git-send-email-c_traja@qti.qualcomm.com>
On Tue, 2016-11-08 at 18:45 +0530, c_traja@qti.qualcomm.com wrote:
> From: Tamizh chelvam <c_traja@qti.qualcomm.com>
>
> This patch adds support to enable or disable btcoex by
> adding NL80211_ATTR_WIPHY_BTCOEX_ENABLE attribute in
> NL80211_CMD_SET_WIPHY command. By default BTCOEX disabled in driver.
I think overloading SET_WIPHY even more is a mistake - why not group
this with the new command you're introducing in the second patch?
Also, can have a flag attribute to enable/disable, or better even
disable when you're not setting any other parameters.
johannes
^ permalink raw reply
* Re: [PATCH] mac80211: Return avg sig, rx, tx values in ethtool stats.
From: Ben Greear @ 2016-12-05 14:44 UTC (permalink / raw)
To: Johannes Berg, linux-wireless
In-Reply-To: <1480946891.31788.10.camel@sipsolutions.net>
On 12/05/2016 06:08 AM, Johannes Berg wrote:
> On Tue, 2016-11-29 at 10:07 -0800, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> For non-station devices. This gives at least some useful
>> summary in some cases (especially when we know AP has only one
>> station attached, for instance).
>
> I never saw the point in having ethtool statistics for something you
> can get elsewhere, but I'm certainly not making it return a random
> station's data disregarding everything else. At least the existing ones
> are sums so that all stations are taken into account...
Unless I screwed up, this patch also returns an average.
But, easy enough to carry out of tree, and my use case for this
is pretty specific.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] RFC: Universal scan proposal
From: Johannes Berg @ 2016-12-05 14:28 UTC (permalink / raw)
To: dimitrysh, linux-wireless
In-Reply-To: <94eb2c110db85c2379054172dad0@google.com>
Hi Dmitry,
Sorry I didn't respond earlier.
> Currently we have sched scan with possibility of various
> intervals. We would like to extend it to support also
> different types of scan.
"Different types of scan" is a bit misleading though, isn't it? I mean,
mostly they differ in the reporting modes - the scanning itself still
happens at "various intervals"?
> In case of powerful wlan CPU, all this functionality
> can be offloaded.
> In general case FW processes additional scan requests
> and puts them into queue based on start time and interval.
> Once current request is fulfilled, FW adds it (if interval != 0)
> again to the queue with proper interval. If requests are
> overlapping, new request can be combined with either one before,
> or one after, assuming that requests are not mutually exclusive.
> Combining requests is done by combining scan channels, ssids,
> bssids and types of scan result. Once combined request was fulfilled
> it will be reinserted as two (or three) different requests based on
> their type and interval.
> Each request has attribute:
> Type: connectivity / location
I'm not super happy with this - after all, in theory nothing precludes
using the results for one thing or the other, it's just about when and
how they are gathered, no?
You do have a point wrt. an incomplete scan triggering something wrt.
roaming or so in the connection manager, but I think that if it finds a
better result there than the current connection it would make sense to
pick it - even without full information.
So ultimately, I think this might boil down to reporting the scan
finished more accurately/precisely, rather than saying the type of
scan?
> Report: none / batch / immediate
Not sure I see much point in "none"??
Can you define these more clearly? Do you think "batch" reporting
should be like the gscan buckets? Or actually have full information?
> Request may have priority and can be inserted into
> the head of the queue.
> Types of scans:
> - Normal scan
> - Scheduled scan
> - Hotlist (BSSID scan)
> - Roaming
> - AutoJoin
I think somebody else said this but I didn't find it now - I think this
would make more sense to define in terms of expected behaviour than use
cases for each type of scan.
johannes
^ permalink raw reply
* Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure
From: Johannes Berg @ 2016-12-05 13:52 UTC (permalink / raw)
To: Michal Kazior, Ben Greear; +Cc: linux-wireless, ath10k@lists.infradead.org
In-Reply-To: <CA+BoTQkEaC-ZSbeK=JB=q+ucF1PWcEQXkJEp6JYv4pfOf35cHg@mail.gmail.com>
On Mon, 2016-12-05 at 09:13 +0100, Michal Kazior wrote:
> On 2 December 2016 at 03:29, <greearb@candelatech.com> wrote:
> >
> > From: Ben Greear <greearb@candelatech.com>
> >
> > This appears to fix a problem where ath10k firmware would crash,
> > mac80211 would start re-adding interfaces to the driver, but the
> > iterate-active-interfaces logic would then try to use the half-
> > built
> > interfaces. With a bit of extra debug to catch the problem, the
> > ath10k crash looks like this:
> >
> > ath10k_pci 0000:05:00.0: Initializing arvif: ffff8801ce97e320 on
> > vif: ffff8801ce97e1d8
> >
> > [the print that happens after arvif->ar is assigned is not shown,
> > so code did not make it that far before
> > the tx-beacon-nowait method was called]
> >
> > tx-beacon-nowait: arvif: ffff8801ce97e320 ar: (null)
> [...]
> >
> >
> > Signed-off-by: Ben Greear <greearb@candelatech.com>
> > ---
> > net/mac80211/util.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> > index 863f2c1..abe1f64 100644
> > --- a/net/mac80211/util.c
> > +++ b/net/mac80211/util.c
> > @@ -705,7 +705,7 @@ static void __iterate_interfaces(struct
> > ieee80211_local *local,
> > break;
> > }
> > if (!(iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL)
> > &&
> > - active_only && !(sdata->flags &
> > IEEE80211_SDATA_IN_DRIVER))
> > + (active_only && (local->in_reconfig || !(sdata-
> > >flags & IEEE80211_SDATA_IN_DRIVER))))
> > continue;
>
> Doesn't this effectivelly prevent you from iterating over interfaces
> completely during reconfig? As you bring up interfaces you might
> need/want to iterate over others to re-adjust your own state.
Agree, that doesn't really make sense.
> I'd argue there should be another flag, IEEE80211_SDATA_RESUMING,
> used with sdata->flags for resuming so that once it is re-added to
> the driver it can be cleared (and therefore properly iterated over).
That would make some sense, or perhaps the sdata_in_driver should be
cleared (and remembered elsewhere) at some point during the restart.
johannes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox