On 10/16/2024 12:50 PM, Muna Sinada wrote:
> On 9/11/2024 2:36 AM, Johannes Berg wrote:
>> On Tue, 2024-09-10 at 13:45 -0700, Muna Sinada wrote:
>>> if (!wiphy->mbssid_max_interfaces)
>>> return -EOPNOTSUPP;
>>> @@ -5509,9 +5513,7 @@ 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);
>>> -
>>> + 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 !=
>>> @@ -5530,7 +5532,28 @@ 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 err;
>>> +
>>> + 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 err;
>>> + }
>>> + } else if (tb[NL80211_MBSSID_CONFIG_ATTR_TX_LINK_ID]) {
>>> + goto err;
>>> + }
>>> +
>>> return 0;
>>> +
>>> +err:
>>> + if (tx_netdev) {
>>> + config->tx_wdev = NULL;
>>> + dev_put(tx_netdev);
>>> + }
>>
>> Why not use config->tx_wdev and avoid changes around tx_netdev?
>
> tx_netdev instance is being utilized on exisiting code to perform checks and grab its
>ieee80211_ptr. W are not making changes to tx_netdev itself. In this specific patch we
> are making changes to config->tx_wdev. I was wondering if I could get clarification on
>this.
>
>>
>> There's also an existing error path that does dev_put(), so you should
>> unify that.
>>
>> johannes
>
> Thank you,
> Muna
I apologize for the lack of link breaks in my response. Will make sure to include them
in the future.
