* [PATCH] mac80211: disable BSS color collision detection in case of no free colors @ 2021-12-03 4:41 Rameshkumar Sundaram 2021-12-03 8:08 ` Johannes Berg 2021-12-03 13:37 ` kernel test robot 0 siblings, 2 replies; 6+ messages in thread From: Rameshkumar Sundaram @ 2021-12-03 4:41 UTC (permalink / raw) To: johannes; +Cc: linux-wireless, Lavanya Suresh, Rameshkumar Sundaram From: Lavanya Suresh <lavaks@codeaurora.org> AP may run out of BSS color after color collision detection event from driver. Disable BSS color collision detection if no free colors are available based on bss color disabled bit of he_oper_params in beacon. It can be reenabled once new color is available. Signed-off-by: Lavanya Suresh <lavaks@codeaurora.org> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com> --- include/linux/ieee80211.h | 1 + net/mac80211/cfg.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index 11d7af2..cc629d7 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -1874,6 +1874,7 @@ struct ieee80211_he_mcs_nss_supp { __le16 tx_mcs_80p80; } __packed; +#define HE_OPERATION_BSS_COLOR_DISABLED ((u32)BIT(31)) /** * struct ieee80211_he_operation - HE capabilities element * diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index f36f249..eaa04b7 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -995,6 +995,8 @@ static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, struct beacon_data *new, *old; int new_head_len, new_tail_len; int size, err; + const u8 *cap; + struct ieee80211_he_operation *he_oper = NULL; u32 changed = BSS_CHANGED_BEACON; old = sdata_dereference(sdata->u.ap.beacon, sdata); @@ -1082,6 +1084,27 @@ static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, changed |= BSS_CHANGED_FTM_RESPONDER; } + if (sdata->vif.bss_conf.he_support) { + cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION, + params->tail, params->tail_len); + if (cap && cap[1] >= sizeof(*he_oper) + 1) + he_oper = (void *)(cap + 3); + + if (he_oper) { + if (he_oper->he_oper_params & HE_OPERATION_BSS_COLOR_DISABLED) { + if (sdata->vif.bss_conf.he_bss_color.enabled) { + sdata->vif.bss_conf.he_bss_color.enabled = false; + changed |= BSS_CHANGED_HE_BSS_COLOR; + } + } else { + if (!sdata->vif.bss_conf.he_bss_color.enabled) { + sdata->vif.bss_conf.he_bss_color.enabled = true; + changed |= BSS_CHANGED_HE_BSS_COLOR; + } + } + } + } + rcu_assign_pointer(sdata->u.ap.beacon, new); if (old) -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: disable BSS color collision detection in case of no free colors 2021-12-03 4:41 [PATCH] mac80211: disable BSS color collision detection in case of no free colors Rameshkumar Sundaram @ 2021-12-03 8:08 ` Johannes Berg 2021-12-03 11:42 ` Rameshkumar Sundaram (QUIC) 2021-12-03 13:37 ` kernel test robot 1 sibling, 1 reply; 6+ messages in thread From: Johannes Berg @ 2021-12-03 8:08 UTC (permalink / raw) To: Rameshkumar Sundaram; +Cc: linux-wireless, Lavanya Suresh On Fri, 2021-12-03 at 10:11 +0530, Rameshkumar Sundaram wrote: > > +++ b/net/mac80211/cfg.c > @@ -995,6 +995,8 @@ static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, > struct beacon_data *new, *old; > int new_head_len, new_tail_len; > int size, err; > + const u8 *cap; > + struct ieee80211_he_operation *he_oper = NULL; > u32 changed = BSS_CHANGED_BEACON; > > old = sdata_dereference(sdata->u.ap.beacon, sdata); > @@ -1082,6 +1084,27 @@ static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, > changed |= BSS_CHANGED_FTM_RESPONDER; > } > > + if (sdata->vif.bss_conf.he_support) { > + cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION, > + params->tail, params->tail_len); > + if (cap && cap[1] >= sizeof(*he_oper) + 1) > + he_oper = (void *)(cap + 3); > I'm not sure I like this mechanism - in ieee80211_start_ap() we explicitly take it from the parameters given via nl80211, so it feels the same should be true here. Why isn't it done that way? (And if we decide it should be this way then you should be using a new "const struct element *cap" instead of "const u8 *cap", with the better helpers functions etc.) johannes ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] mac80211: disable BSS color collision detection in case of no free colors 2021-12-03 8:08 ` Johannes Berg @ 2021-12-03 11:42 ` Rameshkumar Sundaram (QUIC) 2021-12-03 11:48 ` Johannes Berg 0 siblings, 1 reply; 6+ messages in thread From: Rameshkumar Sundaram (QUIC) @ 2021-12-03 11:42 UTC (permalink / raw) To: Johannes Berg, Rameshkumar Sundaram (QUIC); +Cc: linux-wireless, Lavanya Suresh > -----Original Message----- > From: Johannes Berg <johannes@sipsolutions.net> > Sent: Friday, December 3, 2021 1:39 PM > To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com> > Cc: linux-wireless@vger.kernel.org; Lavanya Suresh > <lavaks@codeaurora.org> > Subject: Re: [PATCH] mac80211: disable BSS color collision detection in case > of no free colors > > On Fri, 2021-12-03 at 10:11 +0530, Rameshkumar Sundaram wrote: > > > > +++ b/net/mac80211/cfg.c > > @@ -995,6 +995,8 @@ static int ieee80211_assign_beacon(struct > ieee80211_sub_if_data *sdata, > > struct beacon_data *new, *old; > > int new_head_len, new_tail_len; > > int size, err; > > + const u8 *cap; > > + struct ieee80211_he_operation *he_oper = NULL; > > u32 changed = BSS_CHANGED_BEACON; > > > > old = sdata_dereference(sdata->u.ap.beacon, sdata); @@ -1082,6 > > +1084,27 @@ static int ieee80211_assign_beacon(struct > ieee80211_sub_if_data *sdata, > > changed |= BSS_CHANGED_FTM_RESPONDER; > > } > > > > + if (sdata->vif.bss_conf.he_support) { > > + cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION, > > + params->tail, params->tail_len); > > + if (cap && cap[1] >= sizeof(*he_oper) + 1) > > + he_oper = (void *)(cap + 3); > > > > I'm not sure I like this mechanism - in ieee80211_start_ap() we explicitly take > it from the parameters given via nl80211, so it feels the same should be true > here. Why isn't it done that way? This is because in this case only beacon will change and in nl80211_set_beacon() we don’t parse NL80211_ATTR_HE_BSS_COLOR attribute or do nl80211_calculate_ap_params() > > (And if we decide it should be this way then you should be using a new > "const struct element *cap" instead of "const u8 *cap", with the better > helpers functions etc.) > Sure cfg80211_find_ext_ie() returns const u8 *, you want me to use cfg80211_find_ext_elem instead (i.e. like how nl80211_calculate_ap_params() does ? > johannes ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: disable BSS color collision detection in case of no free colors 2021-12-03 11:42 ` Rameshkumar Sundaram (QUIC) @ 2021-12-03 11:48 ` Johannes Berg 0 siblings, 0 replies; 6+ messages in thread From: Johannes Berg @ 2021-12-03 11:48 UTC (permalink / raw) To: Rameshkumar Sundaram (QUIC); +Cc: linux-wireless, Lavanya Suresh On Fri, 2021-12-03 at 11:42 +0000, Rameshkumar Sundaram (QUIC) wrote: > > > > > > > I'm not sure I like this mechanism - in ieee80211_start_ap() we > > explicitly take > > it from the parameters given via nl80211, so it feels the same > > should be true > > here. Why isn't it done that way? > > This is because in this case only beacon will change and in > nl80211_set_beacon() > we don’t parse NL80211_ATTR_HE_BSS_COLOR attribute or do > nl80211_calculate_ap_params() > Yeah but we could change that? Why not? johannes ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: disable BSS color collision detection in case of no free colors 2021-12-03 4:41 [PATCH] mac80211: disable BSS color collision detection in case of no free colors Rameshkumar Sundaram @ 2021-12-03 13:37 ` kernel test robot 2021-12-03 13:37 ` kernel test robot 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2021-12-03 13:37 UTC (permalink / raw) To: Rameshkumar Sundaram, johannes Cc: kbuild-all, linux-wireless, Lavanya Suresh, Rameshkumar Sundaram Hi Rameshkumar, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on jberg-mac80211-next/master] [also build test WARNING on jberg-mac80211/master v5.16-rc3 next-20211203] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Rameshkumar-Sundaram/mac80211-disable-BSS-color-collision-detection-in-case-of-no-free-colors/20211203-124303 base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master config: i386-randconfig-s001-20211203 (https://download.01.org/0day-ci/archive/20211203/202112032136.n3Acqu0R-lkp@intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/318efe87673a29286f893ea96b07869d9dce83bc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Rameshkumar-Sundaram/mac80211-disable-BSS-color-collision-detection-in-case-of-no-free-colors/20211203-124303 git checkout 318efe87673a29286f893ea96b07869d9dce83bc # save the config file to linux build tree mkdir build_dir make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash net/mac80211/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> net/mac80211/cfg.c:1096:36: sparse: sparse: restricted __le32 degrades to integer vim +1096 net/mac80211/cfg.c 991 992 static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, 993 struct cfg80211_beacon_data *params, 994 const struct ieee80211_csa_settings *csa, 995 const struct ieee80211_color_change_settings *cca) 996 { 997 struct beacon_data *new, *old; 998 int new_head_len, new_tail_len; 999 int size, err; 1000 const u8 *cap; 1001 struct ieee80211_he_operation *he_oper = NULL; 1002 u32 changed = BSS_CHANGED_BEACON; 1003 1004 old = sdata_dereference(sdata->u.ap.beacon, sdata); 1005 1006 1007 /* Need to have a beacon head if we don't have one yet */ 1008 if (!params->head && !old) 1009 return -EINVAL; 1010 1011 /* new or old head? */ 1012 if (params->head) 1013 new_head_len = params->head_len; 1014 else 1015 new_head_len = old->head_len; 1016 1017 /* new or old tail? */ 1018 if (params->tail || !old) 1019 /* params->tail_len will be zero for !params->tail */ 1020 new_tail_len = params->tail_len; 1021 else 1022 new_tail_len = old->tail_len; 1023 1024 size = sizeof(*new) + new_head_len + new_tail_len; 1025 1026 new = kzalloc(size, GFP_KERNEL); 1027 if (!new) 1028 return -ENOMEM; 1029 1030 /* start filling the new info now */ 1031 1032 /* 1033 * pointers go into the block we allocated, 1034 * memory is | beacon_data | head | tail | 1035 */ 1036 new->head = ((u8 *) new) + sizeof(*new); 1037 new->tail = new->head + new_head_len; 1038 new->head_len = new_head_len; 1039 new->tail_len = new_tail_len; 1040 1041 if (csa) { 1042 new->cntdwn_current_counter = csa->count; 1043 memcpy(new->cntdwn_counter_offsets, csa->counter_offsets_beacon, 1044 csa->n_counter_offsets_beacon * 1045 sizeof(new->cntdwn_counter_offsets[0])); 1046 } else if (cca) { 1047 new->cntdwn_current_counter = cca->count; 1048 new->cntdwn_counter_offsets[0] = cca->counter_offset_beacon; 1049 } 1050 1051 /* copy in head */ 1052 if (params->head) 1053 memcpy(new->head, params->head, new_head_len); 1054 else 1055 memcpy(new->head, old->head, new_head_len); 1056 1057 /* copy in optional tail */ 1058 if (params->tail) 1059 memcpy(new->tail, params->tail, new_tail_len); 1060 else 1061 if (old) 1062 memcpy(new->tail, old->tail, new_tail_len); 1063 1064 err = ieee80211_set_probe_resp(sdata, params->probe_resp, 1065 params->probe_resp_len, csa, cca); 1066 if (err < 0) { 1067 kfree(new); 1068 return err; 1069 } 1070 if (err == 0) 1071 changed |= BSS_CHANGED_AP_PROBE_RESP; 1072 1073 if (params->ftm_responder != -1) { 1074 sdata->vif.bss_conf.ftm_responder = params->ftm_responder; 1075 err = ieee80211_set_ftm_responder_params(sdata, 1076 params->lci, 1077 params->lci_len, 1078 params->civicloc, 1079 params->civicloc_len); 1080 1081 if (err < 0) { 1082 kfree(new); 1083 return err; 1084 } 1085 1086 changed |= BSS_CHANGED_FTM_RESPONDER; 1087 } 1088 1089 if (sdata->vif.bss_conf.he_support) { 1090 cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION, 1091 params->tail, params->tail_len); 1092 if (cap && cap[1] >= sizeof(*he_oper) + 1) 1093 he_oper = (void *)(cap + 3); 1094 1095 if (he_oper) { > 1096 if (he_oper->he_oper_params & HE_OPERATION_BSS_COLOR_DISABLED) { 1097 if (sdata->vif.bss_conf.he_bss_color.enabled) { 1098 sdata->vif.bss_conf.he_bss_color.enabled = false; 1099 changed |= BSS_CHANGED_HE_BSS_COLOR; 1100 } 1101 } else { 1102 if (!sdata->vif.bss_conf.he_bss_color.enabled) { 1103 sdata->vif.bss_conf.he_bss_color.enabled = true; 1104 changed |= BSS_CHANGED_HE_BSS_COLOR; 1105 } 1106 } 1107 } 1108 } 1109 1110 rcu_assign_pointer(sdata->u.ap.beacon, new); 1111 1112 if (old) 1113 kfree_rcu(old, rcu_head); 1114 1115 return changed; 1116 } 1117 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: disable BSS color collision detection in case of no free colors @ 2021-12-03 13:37 ` kernel test robot 0 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2021-12-03 13:37 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 6104 bytes --] Hi Rameshkumar, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on jberg-mac80211-next/master] [also build test WARNING on jberg-mac80211/master v5.16-rc3 next-20211203] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Rameshkumar-Sundaram/mac80211-disable-BSS-color-collision-detection-in-case-of-no-free-colors/20211203-124303 base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master config: i386-randconfig-s001-20211203 (https://download.01.org/0day-ci/archive/20211203/202112032136.n3Acqu0R-lkp(a)intel.com/config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/318efe87673a29286f893ea96b07869d9dce83bc git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Rameshkumar-Sundaram/mac80211-disable-BSS-color-collision-detection-in-case-of-no-free-colors/20211203-124303 git checkout 318efe87673a29286f893ea96b07869d9dce83bc # save the config file to linux build tree mkdir build_dir make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash net/mac80211/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> net/mac80211/cfg.c:1096:36: sparse: sparse: restricted __le32 degrades to integer vim +1096 net/mac80211/cfg.c 991 992 static int ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata, 993 struct cfg80211_beacon_data *params, 994 const struct ieee80211_csa_settings *csa, 995 const struct ieee80211_color_change_settings *cca) 996 { 997 struct beacon_data *new, *old; 998 int new_head_len, new_tail_len; 999 int size, err; 1000 const u8 *cap; 1001 struct ieee80211_he_operation *he_oper = NULL; 1002 u32 changed = BSS_CHANGED_BEACON; 1003 1004 old = sdata_dereference(sdata->u.ap.beacon, sdata); 1005 1006 1007 /* Need to have a beacon head if we don't have one yet */ 1008 if (!params->head && !old) 1009 return -EINVAL; 1010 1011 /* new or old head? */ 1012 if (params->head) 1013 new_head_len = params->head_len; 1014 else 1015 new_head_len = old->head_len; 1016 1017 /* new or old tail? */ 1018 if (params->tail || !old) 1019 /* params->tail_len will be zero for !params->tail */ 1020 new_tail_len = params->tail_len; 1021 else 1022 new_tail_len = old->tail_len; 1023 1024 size = sizeof(*new) + new_head_len + new_tail_len; 1025 1026 new = kzalloc(size, GFP_KERNEL); 1027 if (!new) 1028 return -ENOMEM; 1029 1030 /* start filling the new info now */ 1031 1032 /* 1033 * pointers go into the block we allocated, 1034 * memory is | beacon_data | head | tail | 1035 */ 1036 new->head = ((u8 *) new) + sizeof(*new); 1037 new->tail = new->head + new_head_len; 1038 new->head_len = new_head_len; 1039 new->tail_len = new_tail_len; 1040 1041 if (csa) { 1042 new->cntdwn_current_counter = csa->count; 1043 memcpy(new->cntdwn_counter_offsets, csa->counter_offsets_beacon, 1044 csa->n_counter_offsets_beacon * 1045 sizeof(new->cntdwn_counter_offsets[0])); 1046 } else if (cca) { 1047 new->cntdwn_current_counter = cca->count; 1048 new->cntdwn_counter_offsets[0] = cca->counter_offset_beacon; 1049 } 1050 1051 /* copy in head */ 1052 if (params->head) 1053 memcpy(new->head, params->head, new_head_len); 1054 else 1055 memcpy(new->head, old->head, new_head_len); 1056 1057 /* copy in optional tail */ 1058 if (params->tail) 1059 memcpy(new->tail, params->tail, new_tail_len); 1060 else 1061 if (old) 1062 memcpy(new->tail, old->tail, new_tail_len); 1063 1064 err = ieee80211_set_probe_resp(sdata, params->probe_resp, 1065 params->probe_resp_len, csa, cca); 1066 if (err < 0) { 1067 kfree(new); 1068 return err; 1069 } 1070 if (err == 0) 1071 changed |= BSS_CHANGED_AP_PROBE_RESP; 1072 1073 if (params->ftm_responder != -1) { 1074 sdata->vif.bss_conf.ftm_responder = params->ftm_responder; 1075 err = ieee80211_set_ftm_responder_params(sdata, 1076 params->lci, 1077 params->lci_len, 1078 params->civicloc, 1079 params->civicloc_len); 1080 1081 if (err < 0) { 1082 kfree(new); 1083 return err; 1084 } 1085 1086 changed |= BSS_CHANGED_FTM_RESPONDER; 1087 } 1088 1089 if (sdata->vif.bss_conf.he_support) { 1090 cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION, 1091 params->tail, params->tail_len); 1092 if (cap && cap[1] >= sizeof(*he_oper) + 1) 1093 he_oper = (void *)(cap + 3); 1094 1095 if (he_oper) { > 1096 if (he_oper->he_oper_params & HE_OPERATION_BSS_COLOR_DISABLED) { 1097 if (sdata->vif.bss_conf.he_bss_color.enabled) { 1098 sdata->vif.bss_conf.he_bss_color.enabled = false; 1099 changed |= BSS_CHANGED_HE_BSS_COLOR; 1100 } 1101 } else { 1102 if (!sdata->vif.bss_conf.he_bss_color.enabled) { 1103 sdata->vif.bss_conf.he_bss_color.enabled = true; 1104 changed |= BSS_CHANGED_HE_BSS_COLOR; 1105 } 1106 } 1107 } 1108 } 1109 1110 rcu_assign_pointer(sdata->u.ap.beacon, new); 1111 1112 if (old) 1113 kfree_rcu(old, rcu_head); 1114 1115 return changed; 1116 } 1117 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-12-03 13:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-03 4:41 [PATCH] mac80211: disable BSS color collision detection in case of no free colors Rameshkumar Sundaram 2021-12-03 8:08 ` Johannes Berg 2021-12-03 11:42 ` Rameshkumar Sundaram (QUIC) 2021-12-03 11:48 ` Johannes Berg 2021-12-03 13:37 ` kernel test robot 2021-12-03 13:37 ` kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.