On Tue, 2025-07-01 at 19:43 +0530, Amith A wrote:
> +++ b/net/wireless/nl80211.c
> @@ -9758,6 +9758,7 @@ static bool cfg80211_off_channel_oper_allowed(struct wireless_dev *wdev,
> {
> unsigned int link_id;
> bool all_ok = true;
> + int radio_idx;
>
> lockdep_assert_wiphy(wdev->wiphy);
>
> @@ -9765,10 +9766,16 @@ static bool cfg80211_off_channel_oper_allowed(struct wireless_dev *wdev,
> return false;
>
> if (!cfg80211_beaconing_iface_active(wdev))
> - return true;
> + return all_ok;
> +
> + if (wdev->valid_links) {
> + radio_idx = cfg80211_get_radio_idx_by_chan(wdev->wiphy, chan);
> + if (radio_idx < 0)
> + return !all_ok;
Why? Do you have a code style that says "don't return constants" or
something? I feel like I comment on this repeatedly - please don't use
variables for constant (return) values, it just makes it harder to read
and easier to modify the wrong way.
> @@ -9777,20 +9784,33 @@ static bool cfg80211_off_channel_oper_allowed(struct wireless_dev *wdev,
> /* we cannot leave radar channels */
> for_each_valid_link(wdev, link_id) {
> struct cfg80211_chan_def *chandef;
> + int link_radio_idx;
>
> chandef = wdev_chandef(wdev, link_id);
> if (!chandef || !chandef->chan)
> continue;
>
> + if (!(chandef->chan->flags & IEEE80211_CHAN_RADAR))
> + continue;
> +
> /*
> - * FIXME: don't require all_ok, but rather check only the
> - * correct HW resource/link onto which 'chan' falls,
> - * as only that link leaves the channel for doing
> - * the off-channel operation.
> + * chandef->chan is a radar channel. If the radio/link onto
> + * which this radar channel falls is the same radio/link onto
> + * which the input 'chan' falls, off-channel operation should
> + * not be allowed. Hence, set 'all_ok' to false.
> */
>
> - if (chandef->chan->flags & IEEE80211_CHAN_RADAR)
> + if (wdev->valid_links) {
> + link_radio_idx = cfg80211_get_radio_idx_by_chan(wdev->wiphy,
> + chandef->chan);
> + if (link_radio_idx < 0 ||
> + link_radio_idx == radio_idx) {
> + all_ok = false;
> + break;
> + }
> + } else {
> all_ok = false;
> + }
It seems you could simplify this a lot by just unconditionally assigning
the "radio_idx", possibly to -1, and then if it matches you refuse?
I don't see how link_radio_idx<0 could even happen, with a multi-radio
interface that's _already operating_ on that chandef?
johannes
