| Message ID | 20230119221240.24441-1-quic_srirrama@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [v2] wifi: mac80211: support poll sta for ML clients | expand |
On Fri, 2023-01-20 at 03:42 +0530, Sriram R wrote: > Update the client probe handling which sends Null data > frames to check inactivity to support ML Stations as well. > > Replace the use of default bss conf with the link specific > conf and use the stations default link to send the probe > frame. Non ML Stations associated to the ML AP would use > its default link as well which is one of the active links. > > For Non ML AP, the default link id is 0 and it is taken care > as well. This seems very wrong. It seems like if we have a link here then userspace should pick it. But! The station could have disabled that link, for example, after having used it for association. The station always has the deflink used (unlike vif) but there's nothing that says that it must actually be active at any given time. However that obviously means userspace _cannot_ pick it. And then really it means we should address this frame to the MLD and let driver/firmware pick the right link, and do address translation on it, etc. After all, we want to know the MLD is still there, not a specific STA, right? johannes
>-----Original Message----- >From: Johannes Berg <johannes@sipsolutions.net> >Sent: Tuesday, February 14, 2023 6:18 PM >To: Sriram R (QUIC) <quic_srirrama@quicinc.com> >Cc: linux-wireless@vger.kernel.org >Subject: Re: [PATCH v2] wifi: mac80211: support poll sta for ML clients > >On Fri, 2023-01-20 at 03:42 +0530, Sriram R wrote: >> Update the client probe handling which sends Null data frames to check >> inactivity to support ML Stations as well. >> >> Replace the use of default bss conf with the link specific conf and >> use the stations default link to send the probe frame. Non ML Stations >> associated to the ML AP would use its default link as well which is >> one of the active links. >> >> For Non ML AP, the default link id is 0 and it is taken care as well. > >This seems very wrong. > >It seems like if we have a link here then userspace should pick it. > >But! The station could have disabled that link, for example, after having used >it for association. The station always has the deflink used (unlike vif) but >there's nothing that says that it must actually be active at any given time. > >However that obviously means userspace _cannot_ pick it. > >And then really it means we should address this frame to the MLD and let >driver/firmware pick the right link, and do address translation on it, etc. > >After all, we want to know the MLD is still there, not a specific STA, right? Exactly. It is enough to check if the MLD is active and not specific to any link. But, as you mentioned above its hard to know if polling on the default link is good enough to check MLD is alive.. So the suggestion is to use the link id as UNSPECIFIED and let driver decide to send the null data in any of the active links? Thanks, Sriram.R
On Tue, 2023-02-14 at 16:34 +0000, Sriram R (QUIC) wrote: > > > > After all, we want to know the MLD is still there, not a specific > > STA, right? > Exactly. It is enough to check if the MLD is active and not specific > to any link. > But, as you mentioned above its hard to know if polling on the default > link > is good enough to check MLD is alive.. So the suggestion is to use the > link id as UNSPECIFIED and let > driver decide to send the null data in any of the active links? Yes, put all the MLD addresses into the frame if we don't already do that anyway, and that should be good enough for the device to do the translation? johannes
>-----Original Message----- >From: Johannes Berg <johannes@sipsolutions.net> >Sent: Tuesday, February 14, 2023 10:06 PM >To: Sriram R (QUIC) <quic_srirrama@quicinc.com> >Cc: linux-wireless@vger.kernel.org >Subject: Re: [PATCH v2] wifi: mac80211: support poll sta for ML clients > >WARNING: This email originated from outside of Qualcomm. Please be wary >of any links or attachments, and do not enable macros. > >On Tue, 2023-02-14 at 16:34 +0000, Sriram R (QUIC) wrote: >> > >> > After all, we want to know the MLD is still there, not a specific >> > STA, right? >> Exactly. It is enough to check if the MLD is active and not specific >> to any link. >> But, as you mentioned above its hard to know if polling on the default >> link is good enough to check MLD is alive.. So the suggestion is to >> use the link id as UNSPECIFIED and let driver decide to send the null >> data in any of the active links? > >Yes, put all the MLD addresses into the frame if we don't already do that >anyway, and that should be good enough for the device to do the translation? Sure, then I think apart from filling the IEEE80211_TX_CTRL_MLO_LINK here other things are already In place. Thanks, Sriram.R
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index c885076fae89..72df8c708a2d 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -3980,17 +3980,13 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev, struct ieee80211_chanctx_conf *chanctx_conf; enum nl80211_band band; int ret; + u8 link_id; + struct ieee80211_bss_conf *conf; /* the lock is needed to assign the cookie later */ mutex_lock(&local->mtx); rcu_read_lock(); - chanctx_conf = rcu_dereference(sdata->vif.bss_conf.chanctx_conf); - if (WARN_ON(!chanctx_conf)) { - ret = -EINVAL; - goto unlock; - } - band = chanctx_conf->def.chan->band; sta = sta_info_get_bss(sdata, peer); if (sta) { qos = sta->sta.wme; @@ -3999,6 +3995,27 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev, goto unlock; } + /* In case of ML vif, we shall use the default sta link to + * send the probe frame. For non ML vif the link id 0 is + * the deflink + */ + link_id = sta->deflink.link_id; + + conf = rcu_dereference(sdata->vif.link_conf[link_id]); + + if (unlikely(!conf)) { + ret = -ENOLINK; + goto unlock; + } + + chanctx_conf = rcu_dereference(conf->chanctx_conf); + if (WARN_ON(!chanctx_conf)) { + ret = -EINVAL; + goto unlock; + } + + band = chanctx_conf->def.chan->band; + if (qos) { fc = cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_QOS_NULLFUNC | @@ -4024,8 +4041,8 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev, nullfunc->frame_control = fc; nullfunc->duration_id = 0; memcpy(nullfunc->addr1, sta->sta.addr, ETH_ALEN); - memcpy(nullfunc->addr2, sdata->vif.addr, ETH_ALEN); - memcpy(nullfunc->addr3, sdata->vif.addr, ETH_ALEN); + memcpy(nullfunc->addr2, conf->addr, ETH_ALEN); + memcpy(nullfunc->addr3, conf->addr, ETH_ALEN); nullfunc->seq_ctrl = 0; info = IEEE80211_SKB_CB(skb); @@ -4034,6 +4051,8 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev, IEEE80211_TX_INTFL_NL80211_FRAME_TX; info->band = band; + info->control.flags |= u32_encode_bits(link_id, IEEE80211_TX_CTRL_MLO_LINK); + skb_set_queue_mapping(skb, IEEE80211_AC_VO); skb->priority = 7; if (qos)
Update the client probe handling which sends Null data frames to check inactivity to support ML Stations as well. Replace the use of default bss conf with the link specific conf and use the stations default link to send the probe frame. Non ML Stations associated to the ML AP would use its default link as well which is one of the active links. For Non ML AP, the default link id is 0 and it is taken care as well. Signed-off-by: Sriram R <quic_srirrama@quicinc.com> --- v2: Updated wifi prefix in commit title net/mac80211/cfg.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-)