Message ID | BN0P110MB12102BF7BDD2D28E95373DD9D4D3A@BN0P110MB1210.NAMP110.PROD.OUTLOOK.COM (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Johannes Berg |
Headers | show |
Series | Adding EPCS Support | expand |
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
Johannes, Thank you for your feedback. Our intention is to implement the changes ourselves. I apologize if it appeared otherwise; we are not seeking someone else to implement the changes, but rather making sure that we are on the right track. Thanks for confirming that kernel module changes will likely be necessary to apply EDCA parameters from userspace. I intend to build a user-mode Linux kernel for further debugging and add checks for NULL, following your advice. Additionally, I have been utilizing the "nlmon" interface for debugging purposes (https://jvns.ca/blog/2017/09/03/debugging-netlink-requests/) . The code snippet I gave was just a primary attempt. When calling the NL80211_CMD_EPCS_ENABLE, we will need to include the link_id and EDCA settings as parameters. We plan to use NL80211_ATTR_WIPHY_TXQ_PARAMS per your suggestion. Thanks again, Jason
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 1e09329ac..33c123b1d 100644 --- a/include/net/cfg80211.h +++ 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); + /* JY: PLABS ADDED */ + int (*epcs_enable)(struct wiphy *wiphy, struct wireless_dev *wdev); + int (*epcs_teardown)(struct wiphy *wiphy, struct wireless_dev *wdev); + int (*tdls_channel_switch)(struct wiphy *wiphy, struct net_device *dev, const u8 *addr, u8 oper_class, diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index f23ecbdd8..69ddd2092 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -1592,6 +1592,9 @@ enum nl80211_commands { NL80211_CMD_SET_TID_TO_LINK_MAPPING, /* add new commands above here */ + /* JY: PLABS ADDED */ + NL80211_CMD_EPCS_ENABLE, + NL80211_CMD_EPCS_TEARDOWN, /* used to define NL80211_CMD_MAX below */ __NL80211_CMD_AFTER_LAST, diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 07abaf782..a3d979909 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -4347,6 +4347,32 @@ static int ieee80211_set_ap_chanwidth(struct wiphy *wiphy, return ret; } +/* JY: PLABS BEGIN */ + +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); + + return 0; +} + +static int ieee80211_epcs_teardown(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); + clear_sta_flag(sta, WLAN_STA_EPCS_ENABLED); + + return 0; +} + +/* JY: PLABS END */ + static int ieee80211_add_tx_ts(struct wiphy *wiphy, struct net_device *dev, u8 tsid, const u8 *peer, u8 up, u16 admitted_time) @@ -5165,4 +5191,7 @@ const struct cfg80211_ops mac80211_config_ops = { .del_link_station = ieee80211_del_link_station, .set_hw_timestamp = ieee80211_set_hw_timestamp, .set_ttlm = ieee80211_set_ttlm, + /* JY: PLABS ADDED */ + .epcs_enable = ieee80211_epcs_enable, + .epcs_teardown = ieee80211_epcs_teardown, }; diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c index 1e9389c49..100e26729 100644 --- a/net/mac80211/debugfs_sta.c +++ b/net/mac80211/debugfs_sta.c @@ -80,6 +80,8 @@ static const char * const sta_flag_names[] = { FLAG(PS_DELIVER), FLAG(USES_ENCRYPTION), FLAG(DECAP_OFFLOAD), + /* JY: PLABS ADDED */ + FLAG(EPCS_ENABLED), #undef FLAG }; diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 497677e3d..aba87d1d4 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -2720,7 +2720,30 @@ void ieee80211_mgd_set_link_qos_params(struct ieee80211_link_data *link) struct ieee80211_tx_queue_params *params = link->tx_conf; u8 ac; + /* PLABS JY: Changes BEGIN */ + bool epcs_enabled = false; + struct sta_info *sta = NULL; + + /* New flag lives in the Station Info */ + sta = sta_info_get(sdata, sdata->vif.cfg.ap_addr); + + /* Get result 0 or 1 */ + epcs_enabled = test_sta_flag(sta, WLAN_STA_EPCS_ENABLED); + + mlme_dbg(sdata, "JY: PLABS mac80211 module changes\n"); + mlme_dbg(sdata, "EPCS Enabled=%s\n", epcs_enabled ? "true" : "false"); + for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) { + if (epcs_enabled) { + mlme_dbg(sdata, "JY: PLABS We are entering priority mode!\n"); + // Setting EDCA parameter to Voice + params[ac].aifs = 3; + params[ac].cw_min = 4; + params[ac].cw_max = 5; + params[ac].txop = 65; + params[ac].acm = 0; + } + mlme_dbg(sdata, "WMM AC=%d acm=%d aifs=%d cWmin=%d cWmax=%d txop=%d uapsd=%d, downgraded=%d\n", ac, params[ac].acm, diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index a52fb7638..ac2a39d57 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -104,6 +104,8 @@ enum ieee80211_sta_info_flags { WLAN_STA_PS_DELIVER, WLAN_STA_USES_ENCRYPTION, WLAN_STA_DECAP_OFFLOAD, + /* JY: PLABS ADDED */ + WLAN_STA_EPCS_ENABLED, NUM_WLAN_STA_FLAGS, }; diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 65c416e8d..8be612b49 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2641,6 +2641,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, if (rdev->wiphy.features & NL80211_FEATURE_SUPPORTS_WMM_ADMISSION) CMD(add_tx_ts, ADD_TX_TS); + // PLABS Added + CMD(epcs_enable, EPCS_ENABLE); CMD(set_multicast_to_unicast, SET_MULTICAST_TO_UNICAST); CMD(update_connect_params, UPDATE_CONNECT_PARAMS); CMD(update_ft_ies, UPDATE_FT_IES); @@ -15299,6 +15301,30 @@ static int nl80211_set_qos_map(struct sk_buff *skb, return ret; } +/* 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; +} + +/* JY: PLABS ADDED */ +static int nl80211_epcs_teardown(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_teardown(rdev, wdev); + + return 1; +} + static int nl80211_add_tx_ts(struct sk_buff *skb, struct genl_info *info) { struct cfg80211_registered_device *rdev = info->user_ptr[0]; @@ -17488,6 +17514,24 @@ static const struct genl_small_ops nl80211_small_ops[] = { .flags = GENL_UNS_ADMIN_PERM, .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP), }, + /* JY: PLABS ADDED */ + { + .cmd = NL80211_CMD_EPCS_ENABLE, + // PLABS: If we encounter issues, remove validate + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, + .doit = nl80211_epcs_enable, + .flags = GENL_UNS_ADMIN_PERM, + .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP | NL80211_FLAG_MLO_UNSUPPORTED), + }, + /* JY: PLABS ADDED */ + { + .cmd = NL80211_CMD_EPCS_TEARDOWN, + // PLABS: If we encounter issues, remove validate + .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP, + .doit = nl80211_epcs_teardown, + .flags = GENL_UNS_ADMIN_PERM, + .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP | NL80211_FLAG_MLO_UNSUPPORTED), + }, }; static struct genl_family nl80211_fam __ro_after_init = { @@ -17504,7 +17548,9 @@ static struct genl_family nl80211_fam __ro_after_init = { .n_ops = ARRAY_SIZE(nl80211_ops), .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, .mcgrps = nl80211_mcgrps, .n_mcgrps = ARRAY_SIZE(nl80211_mcgrps), .parallel_ops = true, diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h index 43897a526..8aec66206 100644 --- a/net/wireless/rdev-ops.h +++ b/net/wireless/rdev-ops.h @@ -637,6 +637,17 @@ static inline void rdev_rfkill_poll(struct cfg80211_registered_device *rdev) trace_rdev_return_void(&rdev->wiphy); } +/* JY: PLABS ADDED */ +static inline void rdev_epcs_enable(struct cfg80211_registered_device *rdev, struct wireless_dev *wdev) +{ + rdev->ops->epcs_enable(&rdev->wiphy, wdev); +} + +/* JY: PLABS ADDED */ +static inline void rdev_epcs_teardown(struct cfg80211_registered_device *rdev, struct wireless_dev *wdev) +{ + rdev->ops->epcs_teardown(&rdev->wiphy, wdev); +} #ifdef CONFIG_NL80211_TESTMODE static inline int rdev_testmode_cmd(struct cfg80211_registered_device *rdev,