2022-05-16 22:59:19

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v5 1/3] cfg80211: additional processing in NL80211_CMD_SET_BEACON

FILS discovery and unsolicited broadcast probe response transmissions
are configured as part of NL80211_CMD_START_AP, however both stop
after userspace uses the NL80211_CMD_SET_BEACON command as these
attributes are not processed in that command.

- Add the missing implementation.
- Modify the local variable in nl80211_set_beacon() and input parameter
to rdev_change_beacon() from type struct cfg80211_beacon_data to
type struct cfg80211_ap_settings to support the new processing.

Signed-off-by: Aloka Dixit <[email protected]>
---
include/net/cfg80211.h | 2 +-
net/wireless/nl80211.c | 28 ++++++++++++++++++----
net/wireless/rdev-ops.h | 2 +-
net/wireless/trace.h | 52 +++++++++++++++++++++++------------------
4 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 97a5804ccdcf..880ade4a0430 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4200,7 +4200,7 @@ struct cfg80211_ops {
int (*start_ap)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_ap_settings *settings);
int (*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
- struct cfg80211_beacon_data *info);
+ struct cfg80211_ap_settings *info);
int (*stop_ap)(struct wiphy *wiphy, struct net_device *dev);


diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 740b29481bc6..e738902a9b99 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5809,7 +5809,8 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct net_device *dev = info->user_ptr[1];
struct wireless_dev *wdev = dev->ieee80211_ptr;
- struct cfg80211_beacon_data params;
+ struct cfg80211_ap_settings *params;
+ struct nlattr *attrs;
int err;

if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
@@ -5822,16 +5823,35 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
if (!wdev->beacon_interval)
return -EINVAL;

- err = nl80211_parse_beacon(rdev, info->attrs, &params);
+ params = kzalloc(sizeof(*params), GFP_KERNEL);
+ if (!params)
+ return -ENOMEM;
+
+ err = nl80211_parse_beacon(rdev, info->attrs, &params->beacon);
if (err)
goto out;

+ attrs = info->attrs[NL80211_ATTR_FILS_DISCOVERY];
+ if (attrs) {
+ err = nl80211_parse_fils_discovery(rdev, attrs, params);
+ if (err)
+ goto out;
+ }
+
+ attrs = info->attrs[NL80211_ATTR_UNSOL_BCAST_PROBE_RESP];
+ if (attrs) {
+ err = nl80211_parse_unsol_bcast_probe_resp(rdev, attrs, params);
+ if (err)
+ goto out;
+ }
+
wdev_lock(wdev);
- err = rdev_change_beacon(rdev, dev, &params);
+ err = rdev_change_beacon(rdev, dev, params);
wdev_unlock(wdev);

out:
- kfree(params.mbssid_ies);
+ kfree(params->beacon.mbssid_ies);
+ kfree(params);
return err;
}

diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 439bcf52369c..131fbe9c3199 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -162,7 +162,7 @@ static inline int rdev_start_ap(struct cfg80211_registered_device *rdev,

static inline int rdev_change_beacon(struct cfg80211_registered_device *rdev,
struct net_device *dev,
- struct cfg80211_beacon_data *info)
+ struct cfg80211_ap_settings *info)
{
int ret;
trace_rdev_change_beacon(&rdev->wiphy, dev, info);
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 228079d7690a..97ca10cbbfee 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -597,44 +597,50 @@ TRACE_EVENT(rdev_start_ap,

TRACE_EVENT(rdev_change_beacon,
TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
- struct cfg80211_beacon_data *info),
+ struct cfg80211_ap_settings *info),
TP_ARGS(wiphy, netdev, info),
TP_STRUCT__entry(
WIPHY_ENTRY
NETDEV_ENTRY
- __dynamic_array(u8, head, info ? info->head_len : 0)
- __dynamic_array(u8, tail, info ? info->tail_len : 0)
- __dynamic_array(u8, beacon_ies, info ? info->beacon_ies_len : 0)
+ __dynamic_array(u8, head, info ? info->beacon.head_len : 0)
+ __dynamic_array(u8, tail, info ? info->beacon.tail_len : 0)
+ __dynamic_array(u8, beacon_ies,
+ info ? info->beacon.beacon_ies_len : 0)
__dynamic_array(u8, proberesp_ies,
- info ? info->proberesp_ies_len : 0)
+ info ? info->beacon.proberesp_ies_len : 0)
__dynamic_array(u8, assocresp_ies,
- info ? info->assocresp_ies_len : 0)
- __dynamic_array(u8, probe_resp, info ? info->probe_resp_len : 0)
+ info ? info->beacon.assocresp_ies_len : 0)
+ __dynamic_array(u8, probe_resp,
+ info ? info->beacon.probe_resp_len : 0)
),
TP_fast_assign(
WIPHY_ASSIGN;
NETDEV_ASSIGN;
if (info) {
- if (info->head)
- memcpy(__get_dynamic_array(head), info->head,
- info->head_len);
- if (info->tail)
- memcpy(__get_dynamic_array(tail), info->tail,
- info->tail_len);
- if (info->beacon_ies)
+ if (info->beacon.head)
+ memcpy(__get_dynamic_array(head),
+ info->beacon.head,
+ info->beacon.head_len);
+ if (info->beacon.tail)
+ memcpy(__get_dynamic_array(tail),
+ info->beacon.tail,
+ info->beacon.tail_len);
+ if (info->beacon.beacon_ies)
memcpy(__get_dynamic_array(beacon_ies),
- info->beacon_ies, info->beacon_ies_len);
- if (info->proberesp_ies)
+ info->beacon.beacon_ies,
+ info->beacon.beacon_ies_len);
+ if (info->beacon.proberesp_ies)
memcpy(__get_dynamic_array(proberesp_ies),
- info->proberesp_ies,
- info->proberesp_ies_len);
- if (info->assocresp_ies)
+ info->beacon.proberesp_ies,
+ info->beacon.proberesp_ies_len);
+ if (info->beacon.assocresp_ies)
memcpy(__get_dynamic_array(assocresp_ies),
- info->assocresp_ies,
- info->assocresp_ies_len);
- if (info->probe_resp)
+ info->beacon.assocresp_ies,
+ info->beacon.assocresp_ies_len);
+ if (info->beacon.probe_resp)
memcpy(__get_dynamic_array(probe_resp),
- info->probe_resp, info->probe_resp_len);
+ info->beacon.probe_resp,
+ info->beacon.probe_resp_len);
}
),
TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT, WIPHY_PR_ARG, NETDEV_PR_ARG)
--
2.31.1



2022-05-17 11:57:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] cfg80211: additional processing in NL80211_CMD_SET_BEACON

On Tue, 2022-05-17 at 07:45 +0300, Kalle Valo wrote:
>
> > +++ b/include/net/cfg80211.h
> > @@ -4200,7 +4200,7 @@ struct cfg80211_ops {
> > int (*start_ap)(struct wiphy *wiphy, struct net_device *dev,
> > struct cfg80211_ap_settings *settings);
> > int (*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
> > - struct cfg80211_beacon_data *info);
> > + struct cfg80211_ap_settings *info);
>
> Shouldn't patch 3 folded into patch 1? I don't see how patch 1 as is
> would compile without warnings.
>
Yes, and parts of patch 2 as well.

johannes

2022-05-17 14:21:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] cfg80211: additional processing in NL80211_CMD_SET_BEACON

Aloka Dixit <[email protected]> writes:

> FILS discovery and unsolicited broadcast probe response transmissions
> are configured as part of NL80211_CMD_START_AP, however both stop
> after userspace uses the NL80211_CMD_SET_BEACON command as these
> attributes are not processed in that command.
>
> - Add the missing implementation.
> - Modify the local variable in nl80211_set_beacon() and input parameter
> to rdev_change_beacon() from type struct cfg80211_beacon_data to
> type struct cfg80211_ap_settings to support the new processing.
>
> Signed-off-by: Aloka Dixit <[email protected]>
> ---
> include/net/cfg80211.h | 2 +-
> net/wireless/nl80211.c | 28 ++++++++++++++++++----
> net/wireless/rdev-ops.h | 2 +-
> net/wireless/trace.h | 52 +++++++++++++++++++++++------------------
> 4 files changed, 55 insertions(+), 29 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 97a5804ccdcf..880ade4a0430 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4200,7 +4200,7 @@ struct cfg80211_ops {
> int (*start_ap)(struct wiphy *wiphy, struct net_device *dev,
> struct cfg80211_ap_settings *settings);
> int (*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
> - struct cfg80211_beacon_data *info);
> + struct cfg80211_ap_settings *info);

Shouldn't patch 3 folded into patch 1? I don't see how patch 1 as is
would compile without warnings.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-05-17 23:54:17

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v5 1/3] cfg80211: additional processing in NL80211_CMD_SET_BEACON

On 5/17/2022 3:50 AM, Johannes Berg wrote:
> On Tue, 2022-05-17 at 07:45 +0300, Kalle Valo wrote:
>>
>>> +++ b/include/net/cfg80211.h
>>> @@ -4200,7 +4200,7 @@ struct cfg80211_ops {
>>> int (*start_ap)(struct wiphy *wiphy, struct net_device *dev,
>>> struct cfg80211_ap_settings *settings);
>>> int (*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
>>> - struct cfg80211_beacon_data *info);
>>> + struct cfg80211_ap_settings *info);
>>
>> Shouldn't patch 3 folded into patch 1? I don't see how patch 1 as is
>> would compile without warnings.
>>
> Yes, and parts of patch 2 as well.
>
> johannes


Little confused now between above comments and the following one:
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

Even if all driver changes from patch #3 are put with patch #1, it still
won't compile successfully without patch #2. Hence I had added all files
in a single patch for v4.

I'm now thinking that what you meant is split the actual FILS discovery
processing in a separate patch but keep the API changes (cfg80211,
mac80211 and drivers) in the first patch file.

Will do that in the next version.

Thanks