diff mbox series

[v2,1/2] wifi: nl80211: add link id of transmitted profile for MLO MBSSID

Message ID 20241025013857.2793346-2-quic_msinada@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show
Series MLO MBSSID Support | expand

Commit Message

Muna Sinada Oct. 25, 2024, 1:38 a.m. UTC 
From: Rameshkumar Sundaram <quic_ramess@quicinc.com>

Currently non-transmitted profile provides the interface index of the
transmitted profile. The index matches one of the interface indices
advertised by the kernel.

For MLO MBSSID, if the transmitted profile is part of an MLD, then the
transmitted profile is a specific link of that MLD. Utilizing only the
interface index of transmitted profile is no longer sufficient to
identify transmitted profile for MLO.

Add a new attribute to specify link id of the transmitted profile of
MBSSID group if the profile is part of an MLD. It is required to map
the nontransmitted link to corresponding transmitted link.

Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Co-developed-by: Aloka Dixit <quic_alokad@quicinc.com>
Signed-off-by: Aloka Dixit <quic_alokad@quicinc.com>
Signed-off-by: Muna Sinada <quic_msinada@quicinc.com>
---
v2: addressed review comments
 - cleaned up commit message
 - added link id value for non-MLO to documentation
 - rewrote documentation for link id NL attibute
 - cleaned up error path
 - removed usage of tx_netdev

---
 include/net/cfg80211.h       |  3 +++
 include/uapi/linux/nl80211.h |  7 ++++++
 net/wireless/nl80211.c       | 43 +++++++++++++++++++++++++++---------
 3 files changed, 42 insertions(+), 11 deletions(-)

Comments

Johannes Berg Oct. 25, 2024, 8:40 a.m. UTC | #1 
On Thu, 2024-10-24 at 18:38 -0700, Muna Sinada wrote:
> 
> + * @NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID: Parameter for a non-transmitted
> + *	profile which provides the link ID (u8) of the transmitted profile when
> + *	the latter is part of an MLD. This is a mandatory parameter for a
> + *	non-transmitted profile. If transmitted profile is not part of an MLD,
> + *	link_id will be set to -1.

The part about it being mandatory/-1 doesn't seem true, according to the
code it needs to be not present? Which sounds like something I'd ask
for, but now I don't really remember :) Please adjust the description.

>  		if (tx_ifindex != dev->ifindex) {
> -			struct net_device *tx_netdev =
> -				dev_get_by_index(wiphy_net(wiphy), tx_ifindex);
> -
> -			if (!tx_netdev || !tx_netdev->ieee80211_ptr ||
> -			    tx_netdev->ieee80211_ptr->wiphy != wiphy ||
> -			    tx_netdev->ieee80211_ptr->iftype !=
> -							NL80211_IFTYPE_AP) {
> -				dev_put(tx_netdev);
> -				return -EINVAL;
> +			config->tx_wdev =
> +			 dev_get_by_index(wiphy_net(wiphy), tx_ifindex)->ieee80211_ptr;
> +			if (!config->tx_wdev ||
> +			    config->tx_wdev->wiphy != wiphy ||
> +			    config->tx_wdev->iftype != NL80211_IFTYPE_AP) {
> +				err = -EINVAL;
> +				goto out;
>  			}
> -
> -			config->tx_wdev = tx_netdev->ieee80211_ptr;

Not sure why you change this so much? I'd argue the local variable was
used to make the code not indent so badly, it never would've been needed
for the dev_put() here either.

> +	config->tx_link_id = 0;
> +	if (config->tx_wdev->valid_links) {
> +		if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
> +			goto out;
> +
> +		config->tx_link_id =
> +			  nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]);
> +		if (!(config->tx_wdev->valid_links & BIT(config->tx_link_id))) {
> +			err = -ENOLINK;
> +			goto out;
> +		}

Is it valid if the dev==tx_wdev->netdev, but tx_link_id different? I'd
think not? Otherwise need to catch that? Or actually, move this into the
"tx_ifindex != dev->ifindex" part, no?

johannes
kernel test robot Oct. 26, 2024, 1:55 a.m. UTC | #2 
Hi Muna,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main ath/ath-next linus/master v6.12-rc4 next-20241025]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Muna-Sinada/wifi-nl80211-add-link-id-of-transmitted-profile-for-MLO-MBSSID/20241025-094154
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20241025013857.2793346-2-quic_msinada%40quicinc.com
patch subject: [PATCH v2 1/2] wifi: nl80211: add link id of transmitted profile for MLO MBSSID
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20241026/202410260903.OviN4GZO-lkp@intel.com/config)
compiler: clang version 19.1.2 (https://github.com/llvm/llvm-project 7ba7d8e2f7b6445b60679da826210cdde29eaf8b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241026/202410260903.OviN4GZO-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410260903.OviN4GZO-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from net/wireless/nl80211.c:16:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/x86/include/asm/cacheflush.h:5:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> net/wireless/nl80211.c:5588:13: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    5588 |         } else if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]) {
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/wireless/nl80211.c:5599:9: note: uninitialized use occurs here
    5599 |         return err;
         |                ^~~
   net/wireless/nl80211.c:5588:9: note: remove the 'if' if its condition is always false
    5588 |         } else if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]) {
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    5589 |                 goto out;
         |                 ~~~~~~~~~
    5590 |         }
         |         ~
   net/wireless/nl80211.c:5579:7: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
    5579 |                 if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/wireless/nl80211.c:5599:9: note: uninitialized use occurs here
    5599 |         return err;
         |                ^~~
   net/wireless/nl80211.c:5579:3: note: remove the 'if' if its condition is always false
    5579 |                 if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    5580 |                         goto out;
         |                         ~~~~~~~~
   net/wireless/nl80211.c:5527:9: note: initialize the variable 'err' to silence this warning
    5527 |         int err;
         |                ^
         |                 = 0
   6 warnings generated.


vim +5588 net/wireless/nl80211.c

  5519	
  5520	static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
  5521					       struct net_device *dev,
  5522					       struct nlattr *attrs,
  5523					       struct cfg80211_mbssid_config *config,
  5524					       u8 num_elems)
  5525	{
  5526		struct nlattr *tb[NL80211_MBSSID_CONFIG_ATTR_MAX + 1];
  5527		int err;
  5528	
  5529		if (!wiphy->mbssid_max_interfaces)
  5530			return -EOPNOTSUPP;
  5531	
  5532		if (nla_parse_nested(tb, NL80211_MBSSID_CONFIG_ATTR_MAX, attrs, NULL,
  5533				     NULL) ||
  5534		    !tb[NL80211_MBSSID_CONFIG_ATTR_INDEX])
  5535			return -EINVAL;
  5536	
  5537		config->ema = nla_get_flag(tb[NL80211_MBSSID_CONFIG_ATTR_EMA]);
  5538		if (config->ema) {
  5539			if (!wiphy->ema_max_profile_periodicity)
  5540				return -EOPNOTSUPP;
  5541	
  5542			if (num_elems > wiphy->ema_max_profile_periodicity)
  5543				return -EINVAL;
  5544		}
  5545	
  5546		config->index = nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_INDEX]);
  5547		if (config->index >= wiphy->mbssid_max_interfaces ||
  5548		    (!config->index && !num_elems))
  5549			return -EINVAL;
  5550	
  5551		if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX]) {
  5552			u32 tx_ifindex =
  5553				nla_get_u32(tb[NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX]);
  5554	
  5555			if ((!config->index && tx_ifindex != dev->ifindex) ||
  5556			    (config->index && tx_ifindex == dev->ifindex))
  5557				return -EINVAL;
  5558	
  5559			if (tx_ifindex != dev->ifindex) {
  5560				config->tx_wdev =
  5561				 dev_get_by_index(wiphy_net(wiphy), tx_ifindex)->ieee80211_ptr;
  5562				if (!config->tx_wdev ||
  5563				    config->tx_wdev->wiphy != wiphy ||
  5564				    config->tx_wdev->iftype != NL80211_IFTYPE_AP) {
  5565					err = -EINVAL;
  5566					goto out;
  5567				}
  5568			} else {
  5569				config->tx_wdev = dev->ieee80211_ptr;
  5570			}
  5571		} else if (!config->index) {
  5572			config->tx_wdev = dev->ieee80211_ptr;
  5573		} else {
  5574			return -EINVAL;
  5575		}
  5576	
  5577		config->tx_link_id = 0;
  5578		if (config->tx_wdev->valid_links) {
  5579			if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
  5580				goto out;
  5581	
  5582			config->tx_link_id =
  5583				  nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]);
  5584			if (!(config->tx_wdev->valid_links & BIT(config->tx_link_id))) {
  5585				err = -ENOLINK;
  5586				goto out;
  5587			}
> 5588		} else if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]) {
  5589			goto out;
  5590		}
  5591	
  5592		return 0;
  5593	
  5594	out:
  5595		if (config->tx_wdev && config->tx_wdev->netdev &&
  5596		    config->tx_wdev->netdev != dev)
  5597			dev_put(config->tx_wdev->netdev);
  5598	
  5599		return err;
  5600	}
  5601
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 192d72c8b465..cd313f59dc68 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1286,11 +1286,14 @@  struct cfg80211_crypto_settings {
  * struct cfg80211_mbssid_config - AP settings for multi bssid
  *
  * @tx_wdev: pointer to the transmitted interface in the MBSSID set
+ * @tx_link_id: link ID of the transmitted interface if it is part of an MLD.
+ *	If transmitted interface is not part of an MLD, link ID is set to -1.
  * @index: index of this AP in the multi bssid group.
  * @ema: set to true if the beacons should be sent out in EMA mode.
  */
 struct cfg80211_mbssid_config {
 	struct wireless_dev *tx_wdev;
+	int tx_link_id;
 	u8 index;
 	bool ema;
 };
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f97f5adc8d51..ebf48c11dc0b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -7987,6 +7987,12 @@  enum nl80211_sar_specs_attrs {
  *	Setting this flag is permitted only if the driver advertises EMA support
  *	by setting wiphy->ema_max_profile_periodicity to non-zero.
  *
+ * @NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID: Parameter for a non-transmitted
+ *	profile which provides the link ID (u8) of the transmitted profile when
+ *	the latter is part of an MLD. This is a mandatory parameter for a
+ *	non-transmitted profile. If transmitted profile is not part of an MLD,
+ *	link_id will be set to -1.
+ *
  * @__NL80211_MBSSID_CONFIG_ATTR_LAST: Internal
  * @NL80211_MBSSID_CONFIG_ATTR_MAX: highest attribute
  */
@@ -7998,6 +8004,7 @@  enum nl80211_mbssid_config_attributes {
 	NL80211_MBSSID_CONFIG_ATTR_INDEX,
 	NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX,
 	NL80211_MBSSID_CONFIG_ATTR_EMA,
+	NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID,
 
 	/* keep last */
 	__NL80211_MBSSID_CONFIG_ATTR_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7397a372c78e..32adca100f23 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -454,6 +454,8 @@  nl80211_mbssid_config_policy[NL80211_MBSSID_CONFIG_ATTR_MAX + 1] = {
 	[NL80211_MBSSID_CONFIG_ATTR_INDEX] = { .type = NLA_U8 },
 	[NL80211_MBSSID_CONFIG_ATTR_TX_IFINDEX] = { .type = NLA_U32 },
 	[NL80211_MBSSID_CONFIG_ATTR_EMA] = { .type = NLA_FLAG },
+	[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID] =
+		NLA_POLICY_MAX(NLA_U8, IEEE80211_MLD_MAX_NUM_LINKS),
 };
 
 static const struct nla_policy
@@ -5477,6 +5479,7 @@  static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
 				       u8 num_elems)
 {
 	struct nlattr *tb[NL80211_MBSSID_CONFIG_ATTR_MAX + 1];
+	int err;
 
 	if (!wiphy->mbssid_max_interfaces)
 		return -EOPNOTSUPP;
@@ -5509,18 +5512,14 @@  static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
 			return -EINVAL;
 
 		if (tx_ifindex != dev->ifindex) {
-			struct net_device *tx_netdev =
-				dev_get_by_index(wiphy_net(wiphy), tx_ifindex);
-
-			if (!tx_netdev || !tx_netdev->ieee80211_ptr ||
-			    tx_netdev->ieee80211_ptr->wiphy != wiphy ||
-			    tx_netdev->ieee80211_ptr->iftype !=
-							NL80211_IFTYPE_AP) {
-				dev_put(tx_netdev);
-				return -EINVAL;
+			config->tx_wdev =
+			 dev_get_by_index(wiphy_net(wiphy), tx_ifindex)->ieee80211_ptr;
+			if (!config->tx_wdev ||
+			    config->tx_wdev->wiphy != wiphy ||
+			    config->tx_wdev->iftype != NL80211_IFTYPE_AP) {
+				err = -EINVAL;
+				goto out;
 			}
-
-			config->tx_wdev = tx_netdev->ieee80211_ptr;
 		} else {
 			config->tx_wdev = dev->ieee80211_ptr;
 		}
@@ -5530,7 +5529,29 @@  static int nl80211_parse_mbssid_config(struct wiphy *wiphy,
 		return -EINVAL;
 	}
 
+	config->tx_link_id = 0;
+	if (config->tx_wdev->valid_links) {
+		if (!tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID])
+			goto out;
+
+		config->tx_link_id =
+			  nla_get_u8(tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]);
+		if (!(config->tx_wdev->valid_links & BIT(config->tx_link_id))) {
+			err = -ENOLINK;
+			goto out;
+		}
+	} else if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]) {
+		goto out;
+	}
+
 	return 0;
+
+out:
+	if (config->tx_wdev && config->tx_wdev->netdev &&
+	    config->tx_wdev->netdev != dev)
+		dev_put(config->tx_wdev->netdev);
+
+	return err;
 }
 
 static struct cfg80211_mbssid_elems *