2023-01-30 07:23:07

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v3 0/6] Puncturing support in AP mode

Add preamble puncturing support in AP mode.
This series is based on top of following:
https://patchwork.kernel.org/project/linux-wireless/patch/20230127123930.4fbc74582331.I3547481d49f958389f59dfeba3fcc75e72b0aa6e@changeid/

And it is next version for:
https://patchwork.kernel.org/project/linux-wireless/list/?series=625836&state=%2A&archive=both

Aloka Dixit (6):
wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP
wifi: mac80211: validate and configure puncturing bitmap in start_ap()
wifi: nl80211: configure puncturing in NL80211_CMD_CHANNEL_SWITCH
wifi: mac80211: configure puncturing bitmap in channel_switch()
wifi: cfg80211: add puncturing bitmap in channel switch notifications
wifi: nl80211: add puncturing bitmap in channel switch events

drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +-
drivers/net/wireless/marvell/mwifiex/11h.c | 2 +-
include/net/cfg80211.h | 16 +++++++-
include/net/mac80211.h | 5 ++-
include/uapi/linux/nl80211.h | 13 ++++++
net/mac80211/cfg.c | 28 +++++++++++--
net/mac80211/ieee80211_i.h | 2 +
net/mac80211/mlme.c | 14 +++----
net/wireless/nl80211.c | 48 ++++++++++++++++++----
net/wireless/trace.h | 24 +++++++----
10 files changed, 124 insertions(+), 30 deletions(-)


base-commit: 4ca69027691a0039279b64cfa0aa511d9c9fde59
prerequisite-patch-id: f3e39c8c03c23c977baa8946066285e597b93c7e
--
2.39.0



2023-01-30 07:23:07

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v3 2/6] wifi: mac80211: validate and configure puncturing bitmap in start_ap()

- Make puncturing bitmap 32 bit to match NL80211 interface.
- Export ieee80211_valid_disable_subchannel_bitmap() and use it to
validate the puncturing bitmap in AP mode.
- Modify 'change' in ieee80211_start_ap() from u32 to u64 to support
BSS_CHANGED_EHT_PUNCTURING.
- Configure the bitmap in link_conf and notify the driver.

Signed-off-by: Aloka Dixit <[email protected]>
Signed-off-by: Muna Sinada <[email protected]>
---
v3: This patch depends on following,
https://patchwork.kernel.org/project/linux-wireless/patch/20230127123930.4fbc74582331.I3547481d49f958389f59dfeba3fcc75e72b0aa6e@changeid/

include/net/mac80211.h | 2 +-
net/mac80211/cfg.c | 10 +++++++++-
net/mac80211/ieee80211_i.h | 2 ++
net/mac80211/mlme.c | 10 +++++-----
4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 54ffc0cc2918..b1c17c7ac044 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -738,7 +738,7 @@ struct ieee80211_bss_conf {
u8 tx_pwr_env_num;
u8 pwr_reduction;
bool eht_support;
- u16 eht_puncturing;
+ u32 eht_puncturing;

bool csa_active;
bool mu_mimo_owner;
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 24b8648cfafa..185e218e8668 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1220,7 +1220,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_local *local = sdata->local;
struct beacon_data *old;
struct ieee80211_sub_if_data *vlan;
- u32 changed = BSS_CHANGED_BEACON_INT |
+ u64 changed = BSS_CHANGED_BEACON_INT |
BSS_CHANGED_BEACON_ENABLED |
BSS_CHANGED_BEACON |
BSS_CHANGED_P2P_PS |
@@ -1296,6 +1296,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
IEEE80211_HE_PHY_CAP2_UL_MU_FULL_MU_MIMO;
}

+ if (params->eht_cap) {
+ if (!ieee80211_valid_disable_subchannel_bitmap(
+ &params->punct_bitmap,
+ params->chandef.width))
+ return -EINVAL;
+ link_conf->eht_puncturing = params->punct_bitmap;
+ changed |= BSS_CHANGED_EHT_PUNCTURING;
+ }
if (sdata->vif.type == NL80211_IFTYPE_AP &&
params->mbssid_config.tx_wdev) {
err = ieee80211_set_ap_mbssid_options(sdata,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e833d472ff72..6fd14a3930d6 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2566,4 +2566,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);
+bool ieee80211_valid_disable_subchannel_bitmap(u32 *bitmap,
+ enum nl80211_chan_width bw);
#endif /* IEEE80211_I_H */
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index a14a5ea2bffd..50efd5980387 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -119,8 +119,8 @@ static const struct ieee80211_per_bw_puncturing_values per_bw_puncturing[] = {
IEEE80211_PER_BW_VALID_PUNCTURING_VALUES(320)
};

-static bool ieee80211_valid_disable_subchannel_bitmap(u16 *bitmap,
- enum nl80211_chan_width bw)
+bool ieee80211_valid_disable_subchannel_bitmap(u32 *bitmap,
+ enum nl80211_chan_width bw)
{
u32 idx, i;

@@ -206,7 +206,7 @@ ieee80211_handle_puncturing_bitmap(struct ieee80211_link_data *link,
ieee80211_extract_dis_subch_bmap(eht_oper, chandef,
bitmap);

- if (ieee80211_valid_disable_subchannel_bitmap(&bitmap,
+ if (ieee80211_valid_disable_subchannel_bitmap((u32 *)&bitmap,
chandef->width))
break;
link->u.mgd.conn_flags |=
@@ -5638,7 +5638,7 @@ static bool ieee80211_config_puncturing(struct ieee80211_link_data *link,
extracted == link->conf->eht_puncturing)
return true;

- if (!ieee80211_valid_disable_subchannel_bitmap(&bitmap,
+ if (!ieee80211_valid_disable_subchannel_bitmap((u32 *)&bitmap,
link->conf->chandef.width)) {
link_info(link,
"Got an invalid disable subchannel bitmap from AP %pM: bitmap = 0x%x, bw = 0x%x. disconnect\n",
@@ -7132,7 +7132,7 @@ ieee80211_setup_assoc_link(struct ieee80211_sub_if_data *sdata,
u16 bitmap;

bitmap = get_unaligned_le16(disable_subchannel_bitmap);
- if (ieee80211_valid_disable_subchannel_bitmap(&bitmap,
+ if (ieee80211_valid_disable_subchannel_bitmap((u32 *)&bitmap,
link->conf->chandef.width))
ieee80211_handle_puncturing_bitmap(link,
eht_oper,
--
2.39.0


2023-01-30 07:23:09

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v3 6/6] wifi: nl80211: add puncturing bitmap in channel switch events

Include the puncturing bitmap in channel switch (started/finished)
notifications to userspace.

Signed-off-by: Aloka Dixit <[email protected]>
---
net/wireless/nl80211.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 55daddb0e6ad..4d415e04f5a3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -18984,7 +18984,7 @@ static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,
struct cfg80211_chan_def *chandef,
gfp_t gfp,
enum nl80211_commands notif,
- u8 count, bool quiet)
+ u8 count, bool quiet, u32 punct_bitmap)
{
struct wireless_dev *wdev = netdev->ieee80211_ptr;
struct sk_buff *msg;
@@ -19018,6 +19018,9 @@ static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,
goto nla_put_failure;
}

+ if (nla_put_u32(msg, NL80211_ATTR_PUNCT_BITMAP, punct_bitmap))
+ goto nla_put_failure;
+
genlmsg_end(msg, hdr);

genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
@@ -19067,7 +19070,8 @@ void cfg80211_ch_switch_notify(struct net_device *dev,
cfg80211_sched_dfs_chan_update(rdev);

nl80211_ch_switch_notify(rdev, dev, link_id, chandef, GFP_KERNEL,
- NL80211_CMD_CH_SWITCH_NOTIFY, 0, false);
+ NL80211_CMD_CH_SWITCH_NOTIFY, 0, false,
+ punct_bitmap);
}
EXPORT_SYMBOL(cfg80211_ch_switch_notify);

@@ -19089,7 +19093,7 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev,

nl80211_ch_switch_notify(rdev, dev, link_id, chandef, GFP_KERNEL,
NL80211_CMD_CH_SWITCH_STARTED_NOTIFY,
- count, quiet);
+ count, quiet, punct_bitmap);
}
EXPORT_SYMBOL(cfg80211_ch_switch_started_notify);

--
2.39.0


2023-01-30 07:23:09

by Aloka Dixit

[permalink] [raw]
Subject: [PATCH v3 5/6] wifi: cfg80211: add puncturing bitmap in channel switch notifications

Modify channel switch notify functions and corresponding trace
functions to include the new puncturing bitmap.

Signed-off-by: Aloka Dixit <[email protected]>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +-
drivers/net/wireless/marvell/mwifiex/11h.c | 2 +-
include/net/cfg80211.h | 6 ++++--
net/mac80211/cfg.c | 6 ++++--
net/mac80211/mlme.c | 4 ++--
net/wireless/nl80211.c | 10 +++++----
net/wireless/trace.h | 24 ++++++++++++++--------
7 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index a20e0aeae284..0c2b8b1a10d5 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1119,7 +1119,7 @@ void ath6kl_cfg80211_ch_switch_notify(struct ath6kl_vif *vif, int freq,
NL80211_CHAN_HT20 : NL80211_CHAN_NO_HT);

mutex_lock(&vif->wdev.mtx);
- cfg80211_ch_switch_notify(vif->ndev, &chandef, 0);
+ cfg80211_ch_switch_notify(vif->ndev, &chandef, 0, 0);
mutex_unlock(&vif->wdev.mtx);
}

diff --git a/drivers/net/wireless/marvell/mwifiex/11h.c b/drivers/net/wireless/marvell/mwifiex/11h.c
index 6a9d7bc1f41e..b0c40a776a2e 100644
--- a/drivers/net/wireless/marvell/mwifiex/11h.c
+++ b/drivers/net/wireless/marvell/mwifiex/11h.c
@@ -292,6 +292,6 @@ void mwifiex_dfs_chan_sw_work_queue(struct work_struct *work)
mwifiex_dbg(priv->adapter, MSG,
"indicating channel switch completion to kernel\n");
mutex_lock(&priv->wdev.mtx);
- cfg80211_ch_switch_notify(priv->netdev, &priv->dfs_chandef, 0);
+ cfg80211_ch_switch_notify(priv->netdev, &priv->dfs_chandef, 0, 0);
mutex_unlock(&priv->wdev.mtx);
}
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7714a44d312e..1ac5befc3cc8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8282,13 +8282,14 @@ bool cfg80211_reg_can_beacon_relax(struct wiphy *wiphy,
* @dev: the device which switched channels
* @chandef: the new channel definition
* @link_id: the link ID for MLO, must be 0 for non-MLO
+ * @punct_bitmap: the new puncturing bitmap
*
* Caller must acquire wdev_lock, therefore must only be called from sleepable
* driver context!
*/
void cfg80211_ch_switch_notify(struct net_device *dev,
struct cfg80211_chan_def *chandef,
- unsigned int link_id);
+ unsigned int link_id, u32 punct_bitmap);

/*
* cfg80211_ch_switch_started_notify - notify channel switch start
@@ -8297,6 +8298,7 @@ void cfg80211_ch_switch_notify(struct net_device *dev,
* @link_id: the link ID for MLO, must be 0 for non-MLO
* @count: the number of TBTTs until the channel switch happens
* @quiet: whether or not immediate quiet was requested by the AP
+ * @punct_bitmap: the future puncturing bitmap
*
* Inform the userspace about the channel switch that has just
* started, so that it can take appropriate actions (eg. starting
@@ -8305,7 +8307,7 @@ void cfg80211_ch_switch_notify(struct net_device *dev,
void cfg80211_ch_switch_started_notify(struct net_device *dev,
struct cfg80211_chan_def *chandef,
unsigned int link_id, u8 count,
- bool quiet);
+ bool quiet, u32 punct_bitmap);

/**
* ieee80211_operating_class_to_band - convert operating class to band
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8a1cb2f083f5..1c40586b3d98 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3600,7 +3600,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
if (err)
return err;

- cfg80211_ch_switch_notify(sdata->dev, &sdata->deflink.csa_chandef, 0);
+ cfg80211_ch_switch_notify(sdata->dev, &sdata->deflink.csa_chandef, 0,
+ sdata->vif.bss_conf.eht_puncturing);

return 0;
}
@@ -3879,7 +3880,8 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,

cfg80211_ch_switch_started_notify(sdata->dev,
&sdata->deflink.csa_chandef, 0,
- params->count, params->block_tx);
+ params->count, params->block_tx,
+ sdata->vif.bss_conf.csa_punct_bitmap);

if (changed) {
ieee80211_link_info_change_notify(sdata, &sdata->deflink,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 50efd5980387..e14799799dd4 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1839,7 +1839,7 @@ static void ieee80211_chswitch_post_beacon(struct ieee80211_link_data *link)
return;
}

- cfg80211_ch_switch_notify(sdata->dev, &link->reserved_chandef, 0);
+ cfg80211_ch_switch_notify(sdata->dev, &link->reserved_chandef, 0, 0);
}

void ieee80211_chswitch_done(struct ieee80211_vif *vif, bool success)
@@ -2049,7 +2049,7 @@ ieee80211_sta_process_chanswitch(struct ieee80211_link_data *link,
mutex_unlock(&local->mtx);

cfg80211_ch_switch_started_notify(sdata->dev, &csa_ie.chandef, 0,
- csa_ie.count, csa_ie.mode);
+ csa_ie.count, csa_ie.mode, 0);

if (local->ops->channel_switch) {
/* use driver's channel switch callback */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index efe841ba8865..55daddb0e6ad 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -19030,7 +19030,7 @@ static void nl80211_ch_switch_notify(struct cfg80211_registered_device *rdev,

void cfg80211_ch_switch_notify(struct net_device *dev,
struct cfg80211_chan_def *chandef,
- unsigned int link_id)
+ unsigned int link_id, u32 punct_bitmap)
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct wiphy *wiphy = wdev->wiphy;
@@ -19039,7 +19039,7 @@ void cfg80211_ch_switch_notify(struct net_device *dev,
ASSERT_WDEV_LOCK(wdev);
WARN_INVALID_LINK_ID(wdev, link_id);

- trace_cfg80211_ch_switch_notify(dev, chandef, link_id);
+ trace_cfg80211_ch_switch_notify(dev, chandef, link_id, punct_bitmap);

switch (wdev->iftype) {
case NL80211_IFTYPE_STATION:
@@ -19074,7 +19074,7 @@ EXPORT_SYMBOL(cfg80211_ch_switch_notify);
void cfg80211_ch_switch_started_notify(struct net_device *dev,
struct cfg80211_chan_def *chandef,
unsigned int link_id, u8 count,
- bool quiet)
+ bool quiet, u32 punct_bitmap)
{
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct wiphy *wiphy = wdev->wiphy;
@@ -19083,7 +19083,9 @@ void cfg80211_ch_switch_started_notify(struct net_device *dev,
ASSERT_WDEV_LOCK(wdev);
WARN_INVALID_LINK_ID(wdev, link_id);

- trace_cfg80211_ch_switch_started_notify(dev, chandef, link_id);
+ trace_cfg80211_ch_switch_started_notify(dev, chandef, link_id,
+ punct_bitmap);
+

nl80211_ch_switch_notify(rdev, dev, link_id, chandef, GFP_KERNEL,
NL80211_CMD_CH_SWITCH_STARTED_NOTIFY,
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index a405c3edbc47..e9db666e5a4e 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3245,39 +3245,47 @@ TRACE_EVENT(cfg80211_chandef_dfs_required,
TRACE_EVENT(cfg80211_ch_switch_notify,
TP_PROTO(struct net_device *netdev,
struct cfg80211_chan_def *chandef,
- unsigned int link_id),
- TP_ARGS(netdev, chandef, link_id),
+ unsigned int link_id,
+ u32 punct_bitmap),
+ TP_ARGS(netdev, chandef, link_id, punct_bitmap),
TP_STRUCT__entry(
NETDEV_ENTRY
CHAN_DEF_ENTRY
__field(unsigned int, link_id)
+ __field(u32, punct_bitmap)
),
TP_fast_assign(
NETDEV_ASSIGN;
CHAN_DEF_ASSIGN(chandef);
__entry->link_id = link_id;
+ __entry->punct_bitmap = punct_bitmap;
),
- TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT ", link:%d",
- NETDEV_PR_ARG, CHAN_DEF_PR_ARG, __entry->link_id)
+ TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT ", link:%d, punct_bitmap:%u",
+ NETDEV_PR_ARG, CHAN_DEF_PR_ARG, __entry->link_id,
+ __entry->punct_bitmap)
);

TRACE_EVENT(cfg80211_ch_switch_started_notify,
TP_PROTO(struct net_device *netdev,
struct cfg80211_chan_def *chandef,
- unsigned int link_id),
- TP_ARGS(netdev, chandef, link_id),
+ unsigned int link_id,
+ u32 punct_bitmap),
+ TP_ARGS(netdev, chandef, link_id, punct_bitmap),
TP_STRUCT__entry(
NETDEV_ENTRY
CHAN_DEF_ENTRY
__field(unsigned int, link_id)
+ __field(u32, punct_bitmap)
),
TP_fast_assign(
NETDEV_ASSIGN;
CHAN_DEF_ASSIGN(chandef);
__entry->link_id = link_id;
+ __entry->punct_bitmap = punct_bitmap;
),
- TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT ", link:%d",
- NETDEV_PR_ARG, CHAN_DEF_PR_ARG, __entry->link_id)
+ TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT ", link:%d, punct_bitmap:%u",
+ NETDEV_PR_ARG, CHAN_DEF_PR_ARG, __entry->link_id,
+ __entry->punct_bitmap)
);

TRACE_EVENT(cfg80211_radar_event,
--
2.39.0


2023-01-30 08:42:07

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] wifi: mac80211: validate and configure puncturing bitmap in start_ap()

On Sun, 2023-01-29 at 23:22 -0800, Aloka Dixit wrote:
> - Make puncturing bitmap 32 bit to match NL80211 interface.
> - Export ieee80211_valid_disable_subchannel_bitmap() and use it to
> validate the puncturing bitmap in AP mode.
> - Modify 'change' in ieee80211_start_ap() from u32 to u64 to support
> BSS_CHANGED_EHT_PUNCTURING.
> - Configure the bitmap in link_conf and notify the driver.
>
> Signed-off-by: Aloka Dixit <[email protected]>
> Signed-off-by: Muna Sinada <[email protected]>
> ---
> v3: This patch depends on following,
> https://patchwork.kernel.org/project/linux-wireless/patch/20230127123930.4fbc74582331.I3547481d49f958389f59dfeba3fcc75e72b0aa6e@changeid/
>
> include/net/mac80211.h | 2 +-
> net/mac80211/cfg.c | 10 +++++++++-
> net/mac80211/ieee80211_i.h | 2 ++
> net/mac80211/mlme.c | 10 +++++-----
> 4 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 54ffc0cc2918..b1c17c7ac044 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -738,7 +738,7 @@ struct ieee80211_bss_conf {
> u8 tx_pwr_env_num;
> u8 pwr_reduction;
> bool eht_support;
> - u16 eht_puncturing;
> + u32 eht_puncturing;

Don't see a reason to change this right now.

> @@ -1296,6 +1296,14 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> IEEE80211_HE_PHY_CAP2_UL_MU_FULL_MU_MIMO;
> }
>
> + if (params->eht_cap) {
> + if (!ieee80211_valid_disable_subchannel_bitmap(
> + &params->punct_bitmap,
> + params->chandef.width))
> + return -EINVAL;
> + link_conf->eht_puncturing = params->punct_bitmap;
> + changed |= BSS_CHANGED_EHT_PUNCTURING;
> + }
> if (sdata->vif.type == NL80211_IFTYPE_AP &&

An extra blank line wouldn't be bad :)

And some other bits of this would change by moving the validation to
cfg80211, of course.

Might even be worth doing that as a completely separate patch 1/7?

johannes

2023-01-30 08:45:35

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 5/6] wifi: cfg80211: add puncturing bitmap in channel switch notifications

On Sun, 2023-01-29 at 23:22 -0800, Aloka Dixit wrote:
>
> + * @punct_bitmap: the new puncturing bitmap
> *
> * Caller must acquire wdev_lock, therefore must only be called from sleepable
> * driver context!
> */
> void cfg80211_ch_switch_notify(struct net_device *dev,
> struct cfg80211_chan_def *chandef,
> - unsigned int link_id);
> + unsigned int link_id, u32 punct_bitmap);

Same here: u32 -> u16 since that's sufficient

Maybe we can keep it u32 in the trace, not sure that matters.

johannes

2023-01-30 08:49:16

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Puncturing support in AP mode

Thanks!

> Aloka Dixit (6):
> wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP
> wifi: mac80211: validate and configure puncturing bitmap in start_ap()
> wifi: nl80211: configure puncturing in NL80211_CMD_CHANNEL_SWITCH
> wifi: mac80211: configure puncturing bitmap in channel_switch()
> wifi: cfg80211: add puncturing bitmap in channel switch notifications
> wifi: nl80211: add puncturing bitmap in channel switch events
>

I feel like maybe you can/should squash 1 and 3, 2 and 4, and 5 and 6?

1/3 are cfg80211 for config, 2/4 are mac80211 for config, and 5/6 are
basically both cfg80211 anyway? Of course 5 updates the users.

And maybe I'd then reorder and put them in this order:

- new: validation move
- 1/3
- 5/6
- 2/4

so 5/6 just pass 0 from mac80211 to cfg80211_ch_switch_started_notify()
and cfg80211_ch_switch_notify(), and the mac80211 patch (2/4) fills in
the actual values. That way the cfg80211 patch is 'pure' API, no
functionality changes.

What do you think?

johannes

2023-01-30 19:45:12

by Aloka Dixit

[permalink] [raw]
Subject: Re: [PATCH v3 0/6] Puncturing support in AP mode

On 1/30/2023 12:48 AM, Johannes Berg wrote:
> Thanks!
>
>> Aloka Dixit (6):
>> wifi: nl80211: configure puncturing bitmap in NL80211_CMD_START_AP
>> wifi: mac80211: validate and configure puncturing bitmap in start_ap()
>> wifi: nl80211: configure puncturing in NL80211_CMD_CHANNEL_SWITCH
>> wifi: mac80211: configure puncturing bitmap in channel_switch()
>> wifi: cfg80211: add puncturing bitmap in channel switch notifications
>> wifi: nl80211: add puncturing bitmap in channel switch events
>>
>
> I feel like maybe you can/should squash 1 and 3, 2 and 4, and 5 and 6?
>
> 1/3 are cfg80211 for config, 2/4 are mac80211 for config, and 5/6 are
> basically both cfg80211 anyway? Of course 5 updates the users.
>
> And maybe I'd then reorder and put them in this order:
>
> - new: validation move
> - 1/3
> - 5/6
> - 2/4
>
> so 5/6 just pass 0 from mac80211 to cfg80211_ch_switch_started_notify()
> and cfg80211_ch_switch_notify(), and the mac80211 patch (2/4) fills in
> the actual values. That way the cfg80211 patch is 'pure' API, no
> functionality changes.
>
> What do you think?
>
> johannes

I added separate patches for channel switch because it kept the patches
small. No worries, will combine those.

Thanks.