1c38c7f2 added the possibility of NL80211_CMD_FRAME_WAIT_CANCEL
being sent whenever the off-channel wait time associated with a
CMD_FRAME completes. Document this in the uapi/linux/nl80211.h file.
Signed-off-by: Denis Kenzior <[email protected]>
---
include/uapi/linux/nl80211.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 8fc3a43cac75..0d9aad98c983 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -657,7 +657,9 @@
* is used during CSA period.
* @NL80211_CMD_FRAME_WAIT_CANCEL: When an off-channel TX was requested, this
* command may be used with the corresponding cookie to cancel the wait
- * time if it is known that it is no longer necessary.
+ * time if it is known that it is no longer necessary. This command is
+ * also sent as an event whenever the driver has completed the off-channel
+ * wait time.
* @NL80211_CMD_ACTION: Alias for @NL80211_CMD_FRAME for backward compatibility.
* @NL80211_CMD_FRAME_TX_STATUS: Report TX status of a management frame
* transmitted with %NL80211_CMD_FRAME. %NL80211_ATTR_COOKIE identifies
--
2.21.0
Include wiphy address setup in wiphy dumps and new wiphy events. The
wiphy permanent address is exposed as ATTR_MAC. If addr_mask is setup,
then it is included as ATTR_MAC_MASK attribute. If multiple addresses
are available, then their are exposed in a nested ATTR_MAC_ADDRS array.
This information is already exposed via sysfs, but it makes sense to
include it in the wiphy dump as well.
Signed-off-by: Denis Kenzior <[email protected]>
---
net/wireless/nl80211.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 26bab9560c0f..65f3d47d9b63 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1852,6 +1852,31 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
switch (state->split_start) {
case 0:
+ if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
+ rdev->wiphy.perm_addr))
+ goto nla_put_failure;
+
+ if (!is_zero_ether_addr(rdev->wiphy.addr_mask) &&
+ nla_put(msg, NL80211_ATTR_MAC_MASK, ETH_ALEN,
+ rdev->wiphy.addr_mask))
+ goto nla_put_failure;
+
+ if (rdev->wiphy.n_addresses > 1) {
+ void *attr;
+
+ attr = nla_nest_start_noflag(msg,
+ NL80211_ATTR_MAC_ADDRS);
+ if (!attr)
+ goto nla_put_failure;
+
+ for (i = 0; i < rdev->wiphy.n_addresses; i++)
+ if (nla_put(msg, i + 1, ETH_ALEN,
+ rdev->wiphy.addresses[i].addr))
+ goto nla_put_failure;
+
+ nla_nest_end(msg, attr);
+ }
+
if (nla_put_u8(msg, NL80211_ATTR_WIPHY_RETRY_SHORT,
rdev->wiphy.retry_short) ||
nla_put_u8(msg, NL80211_ATTR_WIPHY_RETRY_LONG,
--
2.21.0
Didn't really review all of this yet, but
switch (state->split_start) {
> case 0:
> + if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
> + rdev->wiphy.perm_addr))
> + goto nla_put_failure;
We generally can't add anything to any of the cases before the split was
allowed, for compatibility with old userspace.
johannes
Denis Kenzior <[email protected]> writes:
> 1c38c7f2 added the possibility of NL80211_CMD_FRAME_WAIT_CANCEL
> being sent whenever the off-channel wait time associated with a
> CMD_FRAME completes. Document this in the uapi/linux/nl80211.h file.
The preferred format for referencing commits is:
Commit 1c38c7f22068 ("nl80211: send event when CMD_FRAME duration
expires") added...
--
Kalle Valo
Hi Johannes,
On 06/20/2019 01:58 AM, Johannes Berg wrote:
> Didn't really review all of this yet, but
>
> switch (state->split_start) {
>> case 0:
>> + if (nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN,
>> + rdev->wiphy.perm_addr))
>> + goto nla_put_failure;
>
> We generally can't add anything to any of the cases before the split was
> allowed, for compatibility with old userspace.
Can you educate me here? Is it because the non-split dump messages would
grow too large? But then non-dumps aren't split, so I still don't get
how anyone can be broken by this (that isn't already broken in the first
place).
Anyhow, What is the cut off point? It didn't seem worthwhile to send
yet-another-message for ~60 bytes of data, but if you want me to add it
as a separate message, no problem.
Regards,
-Denis
Hi Denis,
> > We generally can't add anything to any of the cases before the split was
> > allowed, for compatibility with old userspace.
>
> Can you educate me here? Is it because the non-split dump messages would
> grow too large?
No. Those messages aren't really relevant, userspace will need to do a
larger buffer for it.
The problem is that old userspace (like really old) didn't split even
dumps. Eventually, we had so much information here that the default dump
message size is exceeded, and we simply couldn't dump that particular
wiphy anymore.
We solved this by splitting the wiphy information into multiple
messages, but that needed new userspace, so when userspace doesn't
request split dumps, we fall through all the way to "case 8" and then
stop - old userspace cannot care about new information anyway.
The reason it was split into cases 0-8 that are combined in non-split
dumps is that it was safer that way - there were certain configurations
where even the original data would go above the message size limit.
> Anyhow, What is the cut off point? It didn't seem worthwhile to send
> yet-another-message for ~60 bytes of data, but if you want me to add it
> as a separate message, no problem.
It doesn't matter if you add it as a separate message or not, you can
add it to later messages, i.e. anything in or after "case 9" is fine.
johannes
Hi Johannes,
On 06/20/2019 02:17 PM, Johannes Berg wrote:
> Hi Denis,
>
>>> We generally can't add anything to any of the cases before the split was
>>> allowed, for compatibility with old userspace.
>>
>> Can you educate me here? Is it because the non-split dump messages would
>> grow too large?
>
> No. Those messages aren't really relevant, userspace will need to do a
> larger buffer for it.
>
> The problem is that old userspace (like really old) didn't split even
> dumps. Eventually, we had so much information here that the default dump
> message size is exceeded, and we simply couldn't dump that particular
> wiphy anymore.
>
> We solved this by splitting the wiphy information into multiple
> messages, but that needed new userspace, so when userspace doesn't
> request split dumps, we fall through all the way to "case 8" and then
> stop - old userspace cannot care about new information anyway.
>
> The reason it was split into cases 0-8 that are combined in non-split
> dumps is that it was safer that way - there were certain configurations
> where even the original data would go above the message size limit.
Ugh. So, if I understand this correctly, NEW_WIPHY events that are
generated when a new wiphy is plugged would only send the old 'legacy'
info and any info we add in cases 9+ would be 'lost' and the application
is forced into re-dumping the phy. This is pretty much counter to what
we want.
If you want to keep your sanity in userspace, you need proper 'object
appeared' / 'object disappeared' events from the kernel. And those
events should have all or nearly all info to not bother the kernel going
forward. It sounds like nl80211 API has run into the extend-ability
wall, no?
Any suggestions on how to resolve this? Should NEW_WIPHY events also do
the whole split_dump semantic and generate 15+ or whatever messages?
Regards,
-Denis
On Thu, 2019-06-20 at 15:05 -0500, Denis Kenzior wrote:
>
> Ugh. So, if I understand this correctly, NEW_WIPHY events that are
> generated when a new wiphy is plugged would only send the old 'legacy'
> info and any info we add in cases 9+ would be 'lost' and the application
> is forced into re-dumping the phy.
Yes.
> This is pretty much counter to what we want.
Well, you want the info, shouldn't matter how you get it?
> If you want to keep your sanity in userspace, you need proper 'object
> appeared' / 'object disappeared' events from the kernel.
Sure, but you don't really need to know *everything* about the events
right there ... you can already filter which ones you care about
(perhaps you know you never want to bind hwsim ones for example) and
then request data on those that you do need.
> And those
> events should have all or nearly all info to not bother the kernel going
> forward.
That's what you wish for, but ...
> It sounds like nl80211 API has run into the extend-ability
> wall, no?
I don't really see it that way.
> Any suggestions on how to resolve this? Should NEW_WIPHY events also do
> the whole split_dump semantic and generate 15+ or whatever messages?
No, that'd be awful, and anyway you'd have to send a new command because
otherwise old applications might be completely confused (not that I know
of any other than "iw event" that would event listen to this, but who
knows)
johannes
On Thu, 2019-06-20 at 22:09 +0200, Johannes Berg wrote:
>
> Sure, but you don't really need to know *everything* about the events
> right there ... you can already filter which ones you care about
> (perhaps you know you never want to bind hwsim ones for example) and
> then request data on those that you do need.
Btw, you can send a filter down when you do request the data, so you
only get the data for the new wiphy you actually just discovered.
So realistically, vs. your suggestion of sending all of the data in
multiple events, that just adds 2 messages (the request and the data you
already had), which isn't nearly as bad as you paint it.
johannes
Hi Johannes,
On 06/20/2019 03:09 PM, Johannes Berg wrote:
> On Thu, 2019-06-20 at 15:05 -0500, Denis Kenzior wrote:
>>
>> Ugh. So, if I understand this correctly, NEW_WIPHY events that are
>> generated when a new wiphy is plugged would only send the old 'legacy'
>> info and any info we add in cases 9+ would be 'lost' and the application
>> is forced into re-dumping the phy.
>
> Yes.
>
>> This is pretty much counter to what we want.
>
> Well, you want the info, shouldn't matter how you get it?
>
Well, it kind of does. You're asking userspace to introduce extra
complexity, extra round trips, extra stuff to go wrong just because the
kernel API has painted itself into a corner.
>> If you want to keep your sanity in userspace, you need proper 'object
>> appeared' / 'object disappeared' events from the kernel.
>
> Sure, but you don't really need to know *everything* about the events
> right there ... you can already filter which ones you care about
> (perhaps you know you never want to bind hwsim ones for example) and
> then request data on those that you do need.
Sure, but it would be nice to have all the info available if we do not
want to filter it...
>
>> And those
>> events should have all or nearly all info to not bother the kernel going
>> forward.
>
> That's what you wish for, but ...
Well, it is a pretty basic requirement for any event driven API, no?
>
>> It sounds like nl80211 API has run into the extend-ability
>> wall, no?
>
> I don't really see it that way.
>
>> Any suggestions on how to resolve this? Should NEW_WIPHY events also do
>> the whole split_dump semantic and generate 15+ or whatever messages?
>
> No, that'd be awful, and anyway you'd have to send a new command because
> otherwise old applications might be completely confused (not that I know
> of any other than "iw event" that would event listen to this, but who
> knows)
Well, given that we're the only ones that seem to care about this right
now, I don't see sending a new command as much of a big deal. I welcome
other ideas, but having the kernel send us an event, then us turning
around and requesting the *same* info is just silly.
Regards,
-Denis
Hi Johannes,
On 06/20/2019 03:21 PM, Johannes Berg wrote:
> On Thu, 2019-06-20 at 22:09 +0200, Johannes Berg wrote:
>>
>> Sure, but you don't really need to know *everything* about the events
>> right there ... you can already filter which ones you care about
>> (perhaps you know you never want to bind hwsim ones for example) and
>> then request data on those that you do need.
>
> Btw, you can send a filter down when you do request the data, so you
> only get the data for the new wiphy you actually just discovered.
Yes, I know that. I did help fix this ~3 years ago in commit
b7fb44dacae04. Nobody was using that prior, which really leads me to
wonder what other userspace tools are doing for hotplug and how broken
they are...
>
> So realistically, vs. your suggestion of sending all of the data in
> multiple events, that just adds 2 messages (the request and the data you
> already had), which isn't nearly as bad as you paint it.
I never 'painted' the message overhead as 'bad'. The performance
overhead of this ping-pong is probably irrelevant in the grand scheme of
things. But I find the approach inelegant.
But really I'm more worried about race conditions that userspace has to
deal with. We already have the weird case of ATTR_GENERATION (which
nobody actually uses btw). And then we also need to dump both the
wiphys and the interfaces separately, cross-reference them while dealing
with the possibility of a wiphy or interface going away or being added
at any point. Then there's the fact that some drivers always add a
default netdev, others that (possibly) don't and the possibility that
the system was left in a weird state.
So from that standpoint it is far better to have the kernel generate
atomic change events with all the info present than having userspace
re-poll it when stuff might have changed in the meantime.
Going back to your 2 message point. What about sending the 'NEW_WIPHY'
event with the info in labels 0-8. And then another event type with the
'rest' of the info. And perhaps another feature bit to tell userspace
to expect multiple events. That would still end up being 2 messages and
still be more efficient than the ping-pong you suggest.
Regards,
-Denis