2023-07-19 19:46:31

by Krishna Chaitanya

[permalink] [raw]
Subject: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

When WNM is enabled, a station can send its preferred BSS maximum idle
period in the association request, add a new netlink attribute to get
this value from the supplicant and add BSS maximum idle IE in the
association request.

Tested using mac80211_hwsim with the corresponding WPA supplicant patch.

Signed-off-by: Chaitanya Tata <[email protected]>
---
include/net/cfg80211.h | 2 ++
include/uapi/linux/nl80211.h | 2 ++
net/mac80211/ieee80211_i.h | 4 ++++
net/mac80211/mlme.c | 4 ++++
net/mac80211/util.c | 11 +++++++++++
net/wireless/nl80211.c | 11 +++++++++++
net/wireless/sme.c | 1 +
7 files changed, 35 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9e04f69712b1..fe8a5149a1d5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2961,6 +2961,7 @@ struct cfg80211_assoc_request {
struct cfg80211_assoc_link links[IEEE80211_MLD_MAX_NUM_LINKS];
const u8 *ap_mld_addr;
s8 link_id;
+ u16 bss_max_idle_period;
};

/**
@@ -3173,6 +3174,7 @@ struct cfg80211_connect_params {
size_t fils_erp_rrk_len;
bool want_1x;
struct ieee80211_edmg edmg;
+ u16 bss_max_idle_period;
};

/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c59fec406da5..b1608df96b83 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3341,6 +3341,8 @@ enum nl80211_attrs {

NL80211_ATTR_EMA_RNR_ELEMS,

+ NL80211_ATTR_BSS_MAX_IDLE_PERIOD,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a0a7839cb961..5d83e9bd30ea 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -436,6 +436,8 @@ struct ieee80211_mgd_assoc_data {
u8 fils_kek[FILS_MAX_KEK_LEN];
size_t fils_kek_len;

+ u16 bss_max_idle_period;
+
size_t ie_len;
u8 *ie_pos; /* used to fill ie[] with link[].elems */
u8 ie[];
@@ -2614,4 +2616,6 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata,
const struct ieee80211_eht_cap_elem *eht_cap_ie_elem,
u8 eht_cap_len,
struct link_sta_info *link_sta);
+
+u8 *ieee80211_add_bss_max_idle_ie(u8 *buf, u16 max_idle_period);
#endif /* IEEE80211_I_H */
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e13a0354c397..2d955b237014 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1203,6 +1203,8 @@ static size_t ieee80211_assoc_link_elems(struct ieee80211_sub_if_data *sdata,
ieee80211_add_s1g_capab_ie(sdata, &sband->s1g_cap, skb);
}

+ ieee80211_add_bss_max_idle_ie(skb_put(skb, 5), assoc_data->bss_max_idle_period);
+
if (iftd && iftd->vendor_elems.data && iftd->vendor_elems.len)
skb_put_data(skb, iftd->vendor_elems.data, iftd->vendor_elems.len);

@@ -7378,6 +7380,8 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
req->crypto.control_port_over_nl80211;
sdata->control_port_no_preauth = req->crypto.control_port_no_preauth;

+ assoc_data->bss_max_idle_period = req->bss_max_idle_period;
+
/* kick off associate process */
ifmgd->assoc_data = assoc_data;

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 1527d6aafc14..d0217fd50d50 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -5076,3 +5076,14 @@ void ieee80211_fragment_element(struct sk_buff *skb, u8 *len_pos)

*len_pos = elem_len;
}
+
+u8 *ieee80211_add_bss_max_idle_ie(u8 *buf, u16 max_idle_period)
+{
+ *buf++ = WLAN_EID_BSS_MAX_IDLE_PERIOD;
+ *buf++ = 3;
+ *(u16 *)buf++ = cpu_to_le16(max_idle_period);
+ /* Protected keep alive not applicable in association request */
+ *buf++ = 0;
+
+ return buf;
+}
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d95f8053020d..1e6c49096407 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -816,6 +816,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MAX_HW_TIMESTAMP_PEERS] = { .type = NLA_U16 },
[NL80211_ATTR_HW_TIMESTAMP_ENABLED] = { .type = NLA_FLAG },
[NL80211_ATTR_EMA_RNR_ELEMS] = { .type = NLA_NESTED },
+ [NL80211_ATTR_BSS_MAX_IDLE_PERIOD] = { .type = NLA_U16 },
};

/* policy for the key attributes */
@@ -11136,6 +11137,11 @@ static int nl80211_associate(struct sk_buff *skb, struct genl_info *info)
ap_addr = req.bss->bssid;
}

+ if (info->attrs[NL80211_ATTR_BSS_MAX_IDLE_PERIOD]) {
+ req.bss_max_idle_period =
+ nla_get_u16(info->attrs[NL80211_ATTR_BSS_MAX_IDLE_PERIOD]);
+ }
+
err = nl80211_crypto_settings(rdev, info, &req.crypto, 1);
if (!err) {
wdev_lock(dev->ieee80211_ptr);
@@ -11971,6 +11977,11 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
if (nla_get_flag(info->attrs[NL80211_ATTR_MLO_SUPPORT]))
connect.flags |= CONNECT_REQ_MLO_SUPPORT;

+ if (info->attrs[NL80211_ATTR_BSS_MAX_IDLE_PERIOD]) {
+ connect.bss_max_idle_period =
+ nla_get_u16(info->attrs[NL80211_ATTR_BSS_MAX_IDLE_PERIOD]);
+ }
+
wdev_lock(dev->ieee80211_ptr);

err = cfg80211_connect(rdev, dev, &connect, connkeys,
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 7bdeb8eea92d..3ab9adb3f1a4 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -200,6 +200,7 @@ static int cfg80211_conn_do_work(struct wireless_dev *wdev,
req.vht_capa = params->vht_capa;
req.vht_capa_mask = params->vht_capa_mask;
req.link_id = -1;
+ req.bss_max_idle_period = params->bss_max_idle_period;

req.bss = cfg80211_get_bss(&rdev->wiphy, params->channel,
params->bssid,
--
2.34.1



2023-08-08 19:22:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Thu, 2023-07-20 at 01:10 +0530, Chaitanya Tata wrote:
> When WNM is enabled, a station can send its preferred BSS maximum idle
> period in the association request, add a new netlink attribute to get
> this value from the supplicant and add BSS maximum idle IE in the
> association request.
>

I don't see anything here that even requires all this code, rather than
wpa_s just including the element itself in the association request extra
IEs?

johannes

2023-08-10 18:39:19

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Tue, Aug 8, 2023 at 4:43 PM Johannes Berg <[email protected]> wrote:
>
> On Thu, 2023-07-20 at 01:10 +0530, Chaitanya Tata wrote:
> > When WNM is enabled, a station can send its preferred BSS maximum idle
> > period in the association request, add a new netlink attribute to get
> > this value from the supplicant and add BSS maximum idle IE in the
> > association request.
> >
>
> I don't see anything here that even requires all this code, rather than
> wpa_s just including the element itself in the association request extra
> IEs?
Yeah, WPA supplicant can prepare this, please ignore, thanks.

2023-08-10 19:07:58

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Fri, Aug 11, 2023 at 12:05 AM Krishna Chaitanya
<[email protected]> wrote:
>
> On Tue, Aug 8, 2023 at 4:43 PM Johannes Berg <[email protected]> wrote:
> >
> > On Thu, 2023-07-20 at 01:10 +0530, Chaitanya Tata wrote:
> > > When WNM is enabled, a station can send its preferred BSS maximum idle
> > > period in the association request, add a new netlink attribute to get
> > > this value from the supplicant and add BSS maximum idle IE in the
> > > association request.
> > >
> >
> > I don't see anything here that even requires all this code, rather than
> > wpa_s just including the element itself in the association request extra
> > IEs?
> Yeah, WPA supplicant can prepare this, please ignore, thanks.
Just remembered why I had to implement this in kernel, the associate/connect
data structure in wpa_s `wpa_driver_associate_params` doesn't have any
ies/extra_ies, it only gives wpa_ie and rest all are parameters for mac80211
to use. So, had just extended this as well, do you think we should add
this ies/extra_ies
to the association like we do for scan? Or as MLME is in mac80211,
just use this patch as is?

2023-08-11 09:57:19

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Fri, 2023-08-11 at 00:17 +0530, Krishna Chaitanya wrote:
> On Fri, Aug 11, 2023 at 12:05 AM Krishna Chaitanya
> <[email protected]> wrote:
> >
> > On Tue, Aug 8, 2023 at 4:43 PM Johannes Berg <[email protected]> wrote:
> > >
> > > On Thu, 2023-07-20 at 01:10 +0530, Chaitanya Tata wrote:
> > > > When WNM is enabled, a station can send its preferred BSS maximum idle
> > > > period in the association request, add a new netlink attribute to get
> > > > this value from the supplicant and add BSS maximum idle IE in the
> > > > association request.
> > > >
> > >
> > > I don't see anything here that even requires all this code, rather than
> > > wpa_s just including the element itself in the association request extra
> > > IEs?
> > Yeah, WPA supplicant can prepare this, please ignore, thanks.

> Just remembered why I had to implement this in kernel, the associate/connect
> data structure in wpa_s `wpa_driver_associate_params` doesn't have any
> ies/extra_ies, it only gives wpa_ie and rest all are parameters for mac80211
> to use. So, had just extended this as well, do you think we should add
> this ies/extra_ies
> to the association like we do for scan? Or as MLME is in mac80211,
> just use this patch as is?

Not sure I follow all that reasoning - is that something internal to
wpa_supplicant? Fundamentally with my cfg/mac hat on I'm not sure I care
so much about wpa_s internal data structures?

Things like extended capabilities are also added to the "extra IEs" by
wpa_s, so surely this would work too?

johannes

2023-08-13 20:44:42

by Krishna Chaitanya

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Fri, Aug 11, 2023 at 3:11 PM Johannes Berg <[email protected]> wrote:
>
> On Fri, 2023-08-11 at 00:17 +0530, Krishna Chaitanya wrote:
> > On Fri, Aug 11, 2023 at 12:05 AM Krishna Chaitanya
> > <[email protected]> wrote:
> > >
> > > On Tue, Aug 8, 2023 at 4:43 PM Johannes Berg <[email protected]> wrote:
> > > >
> > > > On Thu, 2023-07-20 at 01:10 +0530, Chaitanya Tata wrote:
> > > > > When WNM is enabled, a station can send its preferred BSS maximum idle
> > > > > period in the association request, add a new netlink attribute to get
> > > > > this value from the supplicant and add BSS maximum idle IE in the
> > > > > association request.
> > > > >
> > > >
> > > > I don't see anything here that even requires all this code, rather than
> > > > wpa_s just including the element itself in the association request extra
> > > > IEs?
> > > Yeah, WPA supplicant can prepare this, please ignore, thanks.
>
> > Just remembered why I had to implement this in kernel, the associate/connect
> > data structure in wpa_s `wpa_driver_associate_params` doesn't have any
> > ies/extra_ies, it only gives wpa_ie and rest all are parameters for mac80211
> > to use. So, had just extended this as well, do you think we should add
> > this ies/extra_ies
> > to the association like we do for scan? Or as MLME is in mac80211,
> > just use this patch as is?
>
> Not sure I follow all that reasoning - is that something internal to
> wpa_supplicant? Fundamentally with my cfg/mac hat on I'm not sure I care
> so much about wpa_s internal data structures?
>
> Things like extended capabilities are also added to the "extra IEs" by
> wpa_s, so surely this would work too?

I was only pointing out that AFAIK there is no mechanism to pass "ies" in
the associate command from userspace, except for WPA IE.

2023-08-14 10:07:15

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] wifi: Add support for sending BSSMaxIdle in association request

On Mon, 2023-08-14 at 01:56 +0530, Krishna Chaitanya wrote:
> >
> > > Just remembered why I had to implement this in kernel, the associate/connect
> > > data structure in wpa_s `wpa_driver_associate_params` doesn't have any
> > > ies/extra_ies, it only gives wpa_ie and rest all are parameters for mac80211
> > > to use. So, had just extended this as well, do you think we should add
> > > this ies/extra_ies
> > > to the association like we do for scan? Or as MLME is in mac80211,
> > > just use this patch as is?
> >
> > Not sure I follow all that reasoning - is that something internal to
> > wpa_supplicant? Fundamentally with my cfg/mac hat on I'm not sure I care
> > so much about wpa_s internal data structures?
> >
> > Things like extended capabilities are also added to the "extra IEs" by
> > wpa_s, so surely this would work too?
>
> I was only pointing out that AFAIK there is no mechanism to pass "ies" in
> the associate command from userspace, except for WPA IE.

Oh you're talking about the associate command? You never mentioned that,
and in fact most of your patch is concerned with mac80211 ...

Would it kill the implementation you have to add "extra elements" rather
than all these individual settings? Does this thing affect the local
firmware implementation? I guess in a sense it must?

I don't know ... and I have no way of ever finding out! So again this is
one of those things where we're never going to see an upstream driver
using it, right? I'm getting really close to just giving up on that.
Since Jouni is happy to add vendor commands for settings left and right
in wpa_supplicant, I pretty much think this stuff has failed. We're
littering the nl80211 API with things that don't really get used
upstream, or like here, do get used but in a way that's
(a) pretty useless since it doesn't do anything but add an element
wpa_s could have added itself, and
(b) looks like just a fig-leaf for this exact reason? Shouldn't
something be configured here to the NIC too? NIC should support
it, etc.? What's even the purpose of this in mac80211?

johannes