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
