On Mon, Dec 23, 2024 at 6:06 AM Jouni Malinen <j@xxxxx> wrote:
>
> On Fri, Nov 22, 2024 at 04:21:43PM +0800, Zong-Zhe Yang wrote:
> > When MLD, wpa_driver_nl80211_set_supp_port() gets
> > `nl80211: Failed to set STA flag: -22 (Invalid argument)`
> > due to imprecise netlink attributes.
>
> Would you be able to share a debug log showing that? I was unable to
> reproduce that in my testing.
>
It can be reproduced by connecting a MLD AP with an assoc link which
link_id != 0. (with mac80211 driver)
> > For MLD, NL80211_ATTR_MLO_LINK_ID is required. Otherwise,
> > sta_link_apply_parameters() cannot get the target link.
>
> That sounds wrong since Authorization status is at MLD level and mixing
> this with link ID would seem to imply this is being done at a different
> level, but it is possible that there are some inconvenient constraints
> in how NL80211_CMD_SET_STATION might need to be used.
>
NL80211_CMD_SET_STATION
nl80211_set_station
rdev_change_station
ieee80211_change_station <mac80211>
sta_apply_parameters
sta_link_apply_parameters
u32 link_id = params->link_id < 0 ? 0 : params->link_id;
[...]
Because @params->link_id is not filled, if assoc link id is not 0 either,
@link_id cannot map to any link in the following. Finally, an error returns.
This looked like an inconvenient constraint. And, I originally thought it's not
harmful to let wpa_supplicant pass the assoc link id at authorization status.
But, prehapes, there will be other suggestions for this problem.
> > Additionally, for MLD, NL80211_ATTR_MAC describes "link" address.
> > And, NL80211_ATTR_MLD_ADDR describes MLD address.
>
> Adding NL80211_ATTR_MLD_ADDR would seem reasonable, but that need for
> link specific information needs to be documented more clearly since this
> function is for an MLD level operation and specifying a link ID here is
> quite confusing.
>
For this problem, adding NL80211_ATTR_MLD_ADDR is not really required.
To make it less confusing, I can remove the filling of NL80211_ATTR_MLD_ADDR
in the next version.
> > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> > @@ -7730,6 +7732,16 @@ static int wpa_driver_nl80211_set_supp_port(void *priv, int authorized)
>
> > + if (!mld)
> > + goto send;
> > +
> > + if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID, mld->assoc_link_id) ||
> > + nla_put(msg, NL80211_ATTR_MLD_ADDR, ETH_ALEN, mld->ap_mld_addr)) {
> > + nlmsg_free(msg);
> > + return -ENOBUFS;
> > + }
> > +
> > +send:
> > ret = send_and_recv_cmd(drv, msg);
>
> That would look nicer if the unnecessary goto were removed:
>
> if (mld &&
> (nla_put_u8(...) ||
> nla_put(...)) {
> ...
> }
>
will do it like this.
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
