| Message ID | 20240813055917.2320582-1-quic_periyasa@quicinc.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v2] wifi: cfg80211: check radio iface combination for multi radio per wiphy | expand |
Hi, So ... I don't think this is correct, for multiple reasons. One, you're completely removing *any* validation of the (global) interface combinations in case per-radio interface combinations are present. This is clearly not desirable. You're arguing the DFS vs. multi-channel check shouldn't be done, but that doesn't imply removing all checks entirely. Secondly, I'm not convinced that the DFS vs. multi-channel check should actually be removed, though I'll admit that this may be a bit questionable. My argument would be something like this: The global interface combinations exist to let existing software (that isn't aware of multi-radio yet) continue functioning as-is. Since it is not radio- aware, multi-channel can mean many different things to it, including the ability to use say two 2.4 GHz channels at the same time, by time- sharing. This is e.g. used to support concurrent P2P-GO and (BSS) client today. But DFS capability on this is broken, since you're not on the correct channel all the time, hence the check. Therefore, I think this patch is entirely wrong, and you need to advertise only a lowest common denominator on the global interface combinations (where the num_different_channels is actually possible to support on each individual radio, since no band separating is implied by it). johannes
Oh and with all that said - we still need a patch like this, but it should check all radios *and* the global combinations. johannes
On 8/28/2024 4:19 PM, Johannes Berg wrote: > Hi, > > So ... I don't think this is correct, for multiple reasons. > > One, you're completely removing *any* validation of the (global) > interface combinations in case per-radio interface combinations are > present. This is clearly not desirable. You're arguing the DFS vs. > multi-channel check shouldn't be done, but that doesn't imply removing > all checks entirely. > In case of per-radio interface combinations, two global interface combinations advertised. 1. NL80211_ATTR_WIPHY_INTERFACE_COMBINATIONS (new global - supported interface combinations for all radios combined) nl80211.h @NL80211_ATTR_WIPHY_RADIOS: Nested attribute describing physical radios belonging to this wiphy. See &enum nl80211_wiphy_radio_attrs. @NL80211_ATTR_WIPHY_INTERFACE_COMBINATIONS: Nested attribute listing the supported interface combinations for all radios combined. In each nested item, it contains attributes defined in &enum nl80211_if_combination_attrs. 2. NL80211_ATTR_INTERFACE_COMBINATIONS (exist global - supported interface combination of zeroth radio ) since new global is combined of all radios, so it fails most of the check in wiphy_verify_combinations() like DFS only works on one channel, Only a single P2P_DEVICE can be allowed. > Secondly, I'm not convinced that the DFS vs. multi-channel check should > actually be removed, though I'll admit that this may be a bit > questionable. My argument would be something like this: The global > interface combinations exist to let existing software (that isn't aware > of multi-radio yet) continue functioning as-is. Since it is not radio- > aware, multi-channel can mean many different things to it, including the > ability to use say two 2.4 GHz channels at the same time, by time- > sharing. This is e.g. used to support concurrent P2P-GO and (BSS) client > today. But DFS capability on this is broken, since you're not on the > correct channel all the time, hence the check. > Exist global interface combination is advertise the zeroth radio iface combination. So it is validated as part of per-radio interface combination check.
diff --git a/net/wireless/core.c b/net/wireless/core.c index 4d5d351bd0b5..de33bdde1e29 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -603,16 +603,19 @@ struct wiphy *wiphy_new_nm(const struct cfg80211_ops *ops, int sizeof_priv, } EXPORT_SYMBOL(wiphy_new_nm); -static int wiphy_verify_combinations(struct wiphy *wiphy) +static +int wiphy_verify_iface_combinations(struct wiphy *wiphy, + const struct ieee80211_iface_combination *iface_comb, + int n_iface_comb) { const struct ieee80211_iface_combination *c; int i, j; - for (i = 0; i < wiphy->n_iface_combinations; i++) { + for (i = 0; i < n_iface_comb; i++) { u32 cnt = 0; u16 all_iftypes = 0; - c = &wiphy->iface_combinations[i]; + c = &iface_comb[i]; /* * Combinations with just one interface aren't real, @@ -693,6 +696,29 @@ static int wiphy_verify_combinations(struct wiphy *wiphy) return 0; } +static int wiphy_verify_combinations(struct wiphy *wiphy) +{ + int i, ret; + + if (wiphy->n_radio) { + for (i = 0; i < wiphy->n_radio; i++) { + const struct wiphy_radio *radio = &wiphy->radio[i]; + + ret = wiphy_verify_iface_combinations(wiphy, + radio->iface_combinations, + radio->n_iface_combinations); + if (ret) + return ret; + } + } else { + ret = wiphy_verify_iface_combinations(wiphy, + wiphy->iface_combinations, + wiphy->n_iface_combinations); + } + + return ret; +} + int wiphy_register(struct wiphy *wiphy) { struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
Currently, wiphy_verify_combinations() fails for the multi-radio per wiphy due to the condition check on global interface combination that DFS only works on one channel. In a multi-radio scenario, global interface combination encompasses the capabilities of all radio combinations, so it supports more than one channel with DFS. For multi-radio per wiphy, interface combination verification needs to be performed for radio specific interface combinations. This is necessary as the global interface combination combines the capabilities of all radio combinations. Fixes: a01b1e9f9955 ("wifi: mac80211: add support for DFS with multiple radios") Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com> --- v2: - Rebased to ToT net/wireless/core.c | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) base-commit: cc32e9fb380d8afdbf3486d7063d5520bfb0f071