2022-08-26 10:11:23

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

> In case of 4way handshake offload, transition disable policy
> updated by the AP during EAPOL 3/4 is not updated to the cfg80211
> layer. This results in mismatch between transition disable policy
> between the upper layer and the driver


I find this a bit confusingly worded - cfg80211 doesn't care, so it
doesn't need to be updated _in_ cfg80211. "To" cfg80211 sounds a bit
weird. Maybe just phrase that in terms of userspace?

> +++ b/include/net/cfg80211.h
> @@ -7669,7 +7669,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
> * indicate the 802.11 association.
> */
> void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
> - gfp_t gfp);
> + const u8 td_bitmap, gfp_t gfp);

Missing docs.

Also missing the corresponding driver update, even if it should just
pass 0 for now in this patch to not have a lot of new driver logic here?

Or maybe it should be valid to pass -1 (i.e. make it a s16) to avoid
passing it to userspace when the driver doesn't know?

Also no point in marking the parameter const.

>
> +++ b/include/uapi/linux/nl80211.h
> @@ -3268,6 +3268,7 @@ enum nl80211_attrs {
>
> NL80211_ATTR_TX_HW_TIMESTAMP,
> NL80211_ATTR_RX_HW_TIMESTAMP,
> + NL80211_ATTR_TD_BITMAP,

Also missing docs.

> void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
> - struct net_device *netdev, const u8 *bssid)
> + struct net_device *netdev, const u8 *bssid,
> + const u8 td_bitmap)


same here about const

> if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
> nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
> - nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
> + nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid) ||
> + nla_put_u8(msg, NL80211_ATTR_TD_BITMAP, td_bitmap))

and yeah maybe here check for <0 or something if it should sometimes
result in a missing attribute?

johannes