On Tue, 2024-07-02 at 12:45 +0000, Youzwak, Jason A (PERATON LABS) wrote: > > We are implementing a Wi-Fi 7 feature called Emergency Preparedness Communication Services (EPCS) good luck :) > - Define new nl80211 commands: NL80211_CMD_EPCS_ENABLE and NL80211_CMD_EPCS_TEARDOWN seems reasonable so far > - Define new STA flag: WLAN_STA_EPCS_ENABLED That may be necessary at some level, but it might not be possible for drivers to handle it at the STA level - EDCA parameters are usually set per *link*, not per *station*. Also, a STA now represents a whole peer MLD, so affects potentially multiple links. > - In net/mac80211/mlme.c, in ieee80211_mgd_set_link_qos_params(), if the WLAN_STA_EPCS_ENABLED flag is set, ignore the EDCA parameters from the Beacon and apply Priority EDCA parameters. Except you never even call ieee80211_mgd_set_link_qos_params() so this cannot work. > 1. Sending EPCS Enable/Teardown commands to the kernel (mac80211) from userspace (hostapd) via netlink. The response was "Operation not supported on transport endpoint (-EOPNOTSUPP) (-95)". This is so basic I don't even know what it could be. There are tools to debug netlink, even "iw --debug", or you can print in the kernel what happens ... Or even run an ARCH=um kernel and attach gdb. > 2. Receiving, parsing and propagating the message via nl80211 and cfg80211. Specifically, we would like guidance on storing parameters received in the NL80211_CMD_EPCS_ENABLE message and retrieving them in the net/mac80211/mlme.c module. There's a whole 20kLOC file called net/wireless/nl80211.c that's full of such guidance, please read it. > 3. Applying the new EDCA parameters in mac80211 module. Now you're just asking us to do your work ... > - Is the kernel the right place to implement this or are there user-space solutions that we can leverage? On the AP side you can change EDCA parameters from userspace, but on the client side I think you do need such new commands, at least to override the automatic settings - looks like otherwise NL80211_ATTR_WIPHY_TXQ_PARAMS _could_ even be used. > - If kernel modifications are needed, does our proposed approach make sense, or are there better alternatives we should consider? > - Is there documented guidance on how to add such features to the kernel? > > Please see our initial work to provide basic support for EPCS messaging in Linux kernel 6.9.5 on Arch Linux in the patch below. Please check out https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches and more generally https://wireless.wiki.kernel.org/en/developers/documentation > +++ b/include/net/cfg80211.h > @@ -4855,6 +4855,10 @@ struct cfg80211_ops { > int (*del_tx_ts)(struct wiphy *wiphy, struct net_device *dev, > u8 tsid, const u8 *peer); this is for much later I suppose, but this patch is completely whitespace damaged ... > + /* JY: PLABS ADDED */ ??? > /* add new commands above here */ > + /* JY: PLABS ADDED */ ??? "above"? > +static int ieee80211_epcs_enable(struct wiphy *wiphy, struct wireless_dev *wdev) > +{ > + struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev); > + struct sta_info *sta; > + > + sta = sta_info_get_bss(sdata, sdata->deflink.u.mgd.bssid); > + set_sta_flag(sta, WLAN_STA_EPCS_ENABLED); sta could be NULL, then you crash. deflink is wrong for MLO and yet you say you want to implement a WiFi7 feature. Setting a STA flag has no immediate effect, so nothing will actually happen here. > +/* JY: PLABS ADDED */ > +static int nl80211_epcs_enable(struct sk_buff *skb, struct genl_info *info) > +{ > + struct cfg80211_registered_device *rdev = info->user_ptr[0]; > + struct net_device *dev = info->user_ptr[1]; > + struct wireless_dev *wdev = dev->ieee80211_ptr; > + > + rdev_epcs_enable(rdev, wdev); > + > + return 1; what's with all the whitespace? Also, "return 1"?? > + /* JY: PLABS ADDED */ > + { > + .cmd = NL80211_CMD_EPCS_ENABLE, > + // PLABS: If we encounter issues, remove validate > + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, Eh, no, always remove it. > + .doit = nl80211_epcs_enable, > + .flags = GENL_UNS_ADMIN_PERM, > + .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP | NL80211_FLAG_MLO_UNSUPPORTED), So ... a wifi7 feature that doesn't support MLO? > .small_ops = nl80211_small_ops, > .n_small_ops = ARRAY_SIZE(nl80211_small_ops), > - .resv_start_op = NL80211_CMD_REMOVE_LINK_STA + 1, > + // PLABS Changed > + //.resv_start_op = NL80211_CMD_REMOVE_LINK_STA + 1, > + .resv_start_op = NL80211_CMD_MAX + 1, No. I don't think that patch was helpful, but since I'm not going to implement it for you, you're going to have to do a lot more homework in understanding the code base, how this feature should work, how to submit changes, etc. Good luck! johannes
