* [PATCH 0/4] brcmfmac: Add few features in AP mode
@ 2020-09-01 6:32 Wright Feng
2020-09-01 6:32 ` [PATCH 1/4] brcmfmac: add change_bss to support AP isolation Wright Feng
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Wright Feng @ 2020-09-01 6:32 UTC (permalink / raw)
To: linux-wireless
Cc: wright.feng, brcm80211-dev-list, brcm80211-dev-list,
Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo,
chi-hsien.lin
This patch series add support for AP isolation and forwarding mechanism
in AP mode.
Jia-Shyr Chuang (1):
brcmfmac: support the forwarding packet
Ting-Ying Li (2):
brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists
brcmfmac: add a variable for packet forwarding condition
Wright Feng (1):
brcmfmac: add change_bss to support AP isolation
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 64 ++++++++++-
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 1 +
.../broadcom/brcm80211/brcmfmac/core.c | 105 +++++++++++++++++-
.../broadcom/brcm80211/brcmfmac/core.h | 19 +++-
.../broadcom/brcm80211/brcmfmac/feature.c | 1 +
.../broadcom/brcm80211/brcmfmac/feature.h | 3 +-
.../broadcom/brcm80211/brcmfmac/msgbuf.c | 31 +++++-
7 files changed, 217 insertions(+), 7 deletions(-)
--
2.25.0
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-01 6:32 [PATCH 0/4] brcmfmac: Add few features in AP mode Wright Feng @ 2020-09-01 6:32 ` Wright Feng 2020-09-07 9:04 ` Kalle Valo 2020-09-01 6:32 ` [PATCH 2/4] brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists Wright Feng ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Wright Feng @ 2020-09-01 6:32 UTC (permalink / raw) To: linux-wireless Cc: wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin Hostap has a parameter "ap_isolate" which is used to prevent low-level bridging of frames between associated stations in the BSS. Regarding driver side, we add cfg80211 ops method change_bss to support setting AP isolation if firmware has ap_isolate feature. Signed-off-by: Wright Feng <wright.feng@cypress.com> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> --- .../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++++++++++++ .../broadcom/brcm80211/brcmfmac/feature.c | 1 + .../broadcom/brcm80211/brcmfmac/feature.h | 3 ++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 5d99771c3f64..7ef93cd66b2c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -5425,6 +5425,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev, return brcmf_set_pmk(ifp, NULL, 0); } +static int +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, + struct bss_parameters *params) +{ + struct brcmf_if *ifp; + int ret = 0; + u32 ap_isolate; + + brcmf_dbg(TRACE, "Enter\n"); + ifp = netdev_priv(dev); + if (params->ap_isolate >= 0) { + ap_isolate = (u32)params->ap_isolate; + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); + if (ret < 0) + brcmf_err("ap_isolate iovar failed: ret=%d\n", ret); + } + + return ret; +} + static struct cfg80211_ops brcmf_cfg80211_ops = { .add_virtual_intf = brcmf_cfg80211_add_iface, .del_virtual_intf = brcmf_cfg80211_del_iface, @@ -7492,6 +7512,9 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr, if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_WOWL_GTK)) ops->set_rekey_data = brcmf_cfg80211_set_rekey_data; #endif + if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_AP_ISOLATE)) + ops->change_bss = brcmf_cfg80211_change_bss; + err = wiphy_register(wiphy); if (err < 0) { bphy_err(drvr, "Could not register wiphy device (%d)\n", err); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c index 0dcefbd0c000..1ffa9742612d 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c @@ -278,6 +278,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr) brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_RSDB, "rsdb_mode"); brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_TDLS, "tdls_enable"); brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_MFP, "mfp"); + brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_AP_ISOLATE, "ap_isolate"); pfn_mac.version = BRCMF_PFN_MACADDR_CFG_VER; err = brcmf_fil_iovar_data_get(ifp, "pfn_macaddr", &pfn_mac, diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h index cda3fc1bab7f..243d9afddb8c 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h @@ -49,7 +49,8 @@ BRCMF_FEAT_DEF(MONITOR_FMT_RADIOTAP) \ BRCMF_FEAT_DEF(MONITOR_FMT_HW_RX_HDR) \ BRCMF_FEAT_DEF(DOT11H) \ - BRCMF_FEAT_DEF(SAE) + BRCMF_FEAT_DEF(SAE) \ + BRCMF_FEAT_DEF(AP_ISOLATE) /* * Quirks: -- 2.25.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-01 6:32 ` [PATCH 1/4] brcmfmac: add change_bss to support AP isolation Wright Feng @ 2020-09-07 9:04 ` Kalle Valo 2020-09-07 9:21 ` Arend Van Spriel 0 siblings, 1 reply; 17+ messages in thread From: Kalle Valo @ 2020-09-07 9:04 UTC (permalink / raw) To: Wright Feng Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin Wright Feng <wright.feng@cypress.com> writes: > Hostap has a parameter "ap_isolate" which is used to prevent low-level > bridging of frames between associated stations in the BSS. > Regarding driver side, we add cfg80211 ops method change_bss to support > setting AP isolation if firmware has ap_isolate feature. > > Signed-off-by: Wright Feng <wright.feng@cypress.com> > Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> > --- > .../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++++++++++++ > .../broadcom/brcm80211/brcmfmac/feature.c | 1 + > .../broadcom/brcm80211/brcmfmac/feature.h | 3 ++- > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > index 5d99771c3f64..7ef93cd66b2c 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > @@ -5425,6 +5425,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev, > return brcmf_set_pmk(ifp, NULL, 0); > } > > +static int > +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, > + struct bss_parameters *params) > +{ > + struct brcmf_if *ifp; > + int ret = 0; > + u32 ap_isolate; > + > + brcmf_dbg(TRACE, "Enter\n"); > + ifp = netdev_priv(dev); > + if (params->ap_isolate >= 0) { > + ap_isolate = (u32)params->ap_isolate; Is the cast to u32 really necessary? Please avoid casts as much as possible. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-07 9:04 ` Kalle Valo @ 2020-09-07 9:21 ` Arend Van Spriel 2020-09-07 9:49 ` Kalle Valo 0 siblings, 1 reply; 17+ messages in thread From: Arend Van Spriel @ 2020-09-07 9:21 UTC (permalink / raw) To: Kalle Valo, Wright Feng Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, chi-hsien.lin On 9/7/2020 11:04 AM, Kalle Valo wrote: > Wright Feng <wright.feng@cypress.com> writes: > >> Hostap has a parameter "ap_isolate" which is used to prevent low-level >> bridging of frames between associated stations in the BSS. >> Regarding driver side, we add cfg80211 ops method change_bss to support >> setting AP isolation if firmware has ap_isolate feature. >> >> Signed-off-by: Wright Feng <wright.feng@cypress.com> >> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> >> --- >> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++++++++++++ >> .../broadcom/brcm80211/brcmfmac/feature.c | 1 + >> .../broadcom/brcm80211/brcmfmac/feature.h | 3 ++- >> 3 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> index 5d99771c3f64..7ef93cd66b2c 100644 >> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> @@ -5425,6 +5425,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev, >> return brcmf_set_pmk(ifp, NULL, 0); >> } >> >> +static int >> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, >> + struct bss_parameters *params) >> +{ >> + struct brcmf_if *ifp; >> + int ret = 0; >> + u32 ap_isolate; >> + >> + brcmf_dbg(TRACE, "Enter\n"); >> + ifp = netdev_priv(dev); >> + if (params->ap_isolate >= 0) { >> + ap_isolate = (u32)params->ap_isolate; >> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); > > Is the cast to u32 really necessary? Please avoid casts as much as > possible. It seems to me. struct bss_parameters::ap_isolate is typed as int. It is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe function name is causing the confusion). Regards, Arend ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-07 9:21 ` Arend Van Spriel @ 2020-09-07 9:49 ` Kalle Valo 2020-09-07 10:09 ` Wright Feng 0 siblings, 1 reply; 17+ messages in thread From: Kalle Valo @ 2020-09-07 9:49 UTC (permalink / raw) To: Arend Van Spriel Cc: Wright Feng, linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, chi-hsien.lin Arend Van Spriel <arend.vanspriel@broadcom.com> writes: > On 9/7/2020 11:04 AM, Kalle Valo wrote: >> Wright Feng <wright.feng@cypress.com> writes: >> >>> Hostap has a parameter "ap_isolate" which is used to prevent low-level >>> bridging of frames between associated stations in the BSS. >>> Regarding driver side, we add cfg80211 ops method change_bss to support >>> setting AP isolation if firmware has ap_isolate feature. >>> >>> Signed-off-by: Wright Feng <wright.feng@cypress.com> >>> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> >>> --- >>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++++++++++++ >>> .../broadcom/brcm80211/brcmfmac/feature.c | 1 + >>> .../broadcom/brcm80211/brcmfmac/feature.h | 3 ++- >>> 3 files changed, 26 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>> index 5d99771c3f64..7ef93cd66b2c 100644 >>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>> @@ -5425,6 +5425,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev, >>> return brcmf_set_pmk(ifp, NULL, 0); >>> } >>> +static int >>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, >>> + struct bss_parameters *params) >>> +{ >>> + struct brcmf_if *ifp; >>> + int ret = 0; >>> + u32 ap_isolate; >>> + >>> + brcmf_dbg(TRACE, "Enter\n"); >>> + ifp = netdev_priv(dev); >>> + if (params->ap_isolate >= 0) { >>> + ap_isolate = (u32)params->ap_isolate; >>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); >> >> Is the cast to u32 really necessary? Please avoid casts as much as >> possible. > > It seems to me. struct bss_parameters::ap_isolate is typed as int. It > is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe > function name is causing the confusion). What extra value does this explicit type casting bring here? I don't see it. Implicit type casting would work the same, no? -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-07 9:49 ` Kalle Valo @ 2020-09-07 10:09 ` Wright Feng 2020-09-07 15:29 ` Kalle Valo [not found] ` <01010174692f7c3f-4b7369b2-0665-4324-b1c8-57bd22ac9ce7-000000@us-west-2.amazonses.com> 0 siblings, 2 replies; 17+ messages in thread From: Wright Feng @ 2020-09-07 10:09 UTC (permalink / raw) To: Kalle Valo, Arend Van Spriel Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Chi-Hsien Lin Kalle Valo 於 9/7/2020 5:49 PM 寫道: > Arend Van Spriel <arend.vanspriel@broadcom.com> writes: > >> On 9/7/2020 11:04 AM, Kalle Valo wrote: >>> Wright Feng <wright.feng@cypress.com> writes: >>> >>>> Hostap has a parameter "ap_isolate" which is used to prevent low-level >>>> bridging of frames between associated stations in the BSS. >>>> Regarding driver side, we add cfg80211 ops method change_bss to support >>>> setting AP isolation if firmware has ap_isolate feature. >>>> >>>> Signed-off-by: Wright Feng <wright.feng@cypress.com> >>>> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> >>>> --- >>>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 23 +++++++++++++++++++ >>>> .../broadcom/brcm80211/brcmfmac/feature.c | 1 + >>>> .../broadcom/brcm80211/brcmfmac/feature.h | 3 ++- >>>> 3 files changed, 26 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>>> index 5d99771c3f64..7ef93cd66b2c 100644 >>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>>> @@ -5425,6 +5425,26 @@ static int brcmf_cfg80211_del_pmk(struct wiphy *wiphy, struct net_device *dev, >>>> return brcmf_set_pmk(ifp, NULL, 0); >>>> } >>>> +static int >>>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, >>>> + struct bss_parameters *params) >>>> +{ >>>> + struct brcmf_if *ifp; >>>> + int ret = 0; >>>> + u32 ap_isolate; >>>> + >>>> + brcmf_dbg(TRACE, "Enter\n"); >>>> + ifp = netdev_priv(dev); >>>> + if (params->ap_isolate >= 0) { >>>> + ap_isolate = (u32)params->ap_isolate; >>>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); >>> >>> Is the cast to u32 really necessary? Please avoid casts as much as >>> possible. >> >> It seems to me. struct bss_parameters::ap_isolate is typed as int. It >> is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe >> function name is causing the confusion). > > What extra value does this explicit type casting bring here? I don't see > it. Implicit type casting would work the same, no? The value will be -1, 0 or 1. I will submit v2 patch that ignores doing iovar if getting params->ap_isolate -1 and removes explicit type casting. Thanks for the comment. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-07 10:09 ` Wright Feng @ 2020-09-07 15:29 ` Kalle Valo [not found] ` <01010174692f7c3f-4b7369b2-0665-4324-b1c8-57bd22ac9ce7-000000@us-west-2.amazonses.com> 1 sibling, 0 replies; 17+ messages in thread From: Kalle Valo @ 2020-09-07 15:29 UTC (permalink / raw) To: Wright Feng Cc: Arend Van Spriel, linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Chi-Hsien Lin Wright Feng <Wright.Feng@cypress.com> writes: >>>>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, >>>>> + struct bss_parameters *params) >>>>> +{ >>>>> + struct brcmf_if *ifp; >>>>> + int ret = 0; >>>>> + u32 ap_isolate; >>>>> + >>>>> + brcmf_dbg(TRACE, "Enter\n"); >>>>> + ifp = netdev_priv(dev); >>>>> + if (params->ap_isolate >= 0) { >>>>> + ap_isolate = (u32)params->ap_isolate; >>>>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); >>>> >>>> Is the cast to u32 really necessary? Please avoid casts as much as >>>> possible. >>> >>> It seems to me. struct bss_parameters::ap_isolate is typed as int. It >>> is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe >>> function name is causing the confusion). >> >> What extra value does this explicit type casting bring here? I don't see >> it. Implicit type casting would work the same, no? > > The value will be -1, 0 or 1. > I will submit v2 patch that ignores doing iovar if getting > params->ap_isolate -1 and removes explicit type casting. Thanks for the > comment. Oh, I didn't realise ap_isolate can be -1 as struct bss_parameters didn't document that. Can someone submit a patch to fix that? * @ap_isolate: do not forward packets between connected stations -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <01010174692f7c3f-4b7369b2-0665-4324-b1c8-57bd22ac9ce7-000000@us-west-2.amazonses.com>]
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation [not found] ` <01010174692f7c3f-4b7369b2-0665-4324-b1c8-57bd22ac9ce7-000000@us-west-2.amazonses.com> @ 2020-09-07 15:57 ` Arend Van Spriel 2020-09-08 2:13 ` Wright Feng 0 siblings, 1 reply; 17+ messages in thread From: Arend Van Spriel @ 2020-09-07 15:57 UTC (permalink / raw) To: Kalle Valo, Wright Feng Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Chi-Hsien Lin On September 7, 2020 5:29:14 PM Kalle Valo <kvalo@codeaurora.org> wrote: > Wright Feng <Wright.Feng@cypress.com> writes: > >>>>>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, >>>>>> + struct bss_parameters *params) >>>>>> +{ >>>>>> + struct brcmf_if *ifp; >>>>>> + int ret = 0; >>>>>> + u32 ap_isolate; >>>>>> + >>>>>> + brcmf_dbg(TRACE, "Enter\n"); >>>>>> + ifp = netdev_priv(dev); >>>>>> + if (params->ap_isolate >= 0) { >>>>>> + ap_isolate = (u32)params->ap_isolate; >>>>>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); >>>>> >>>>> Is the cast to u32 really necessary? Please avoid casts as much as >>>>> possible. >>>> >>>> It seems to me. struct bss_parameters::ap_isolate is typed as int. It >>>> is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe >>>> function name is causing the confusion). >>> >>> What extra value does this explicit type casting bring here? I don't see >>> it. Implicit type casting would work the same, no? >> >> The value will be -1, 0 or 1. >> I will submit v2 patch that ignores doing iovar if getting >> params->ap_isolate -1 and removes explicit type casting. Thanks for the >> comment. > > Oh, I didn't realise ap_isolate can be -1 as struct bss_parameters > didn't document that. Can someone submit a patch to fix that? > > * @ap_isolate: do not forward packets between connected stations Me too. I assumed it was a boolean reading that description. Regards, Arend ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-07 15:57 ` Arend Van Spriel @ 2020-09-08 2:13 ` Wright Feng 2020-09-08 4:29 ` Kalle Valo 0 siblings, 1 reply; 17+ messages in thread From: Wright Feng @ 2020-09-08 2:13 UTC (permalink / raw) To: Arend Van Spriel, Kalle Valo Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Chi-Hsien Lin Arend Van Spriel 於 9/7/2020 11:57 PM 寫道: > On September 7, 2020 5:29:14 PM Kalle Valo <kvalo@codeaurora.org> wrote: > >> Wright Feng <Wright.Feng@cypress.com> writes: >> >>>>>>> +brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device >>>>>>> *dev, >>>>>>> + struct bss_parameters *params) >>>>>>> +{ >>>>>>> + struct brcmf_if *ifp; >>>>>>> + int ret = 0; >>>>>>> + u32 ap_isolate; >>>>>>> + >>>>>>> + brcmf_dbg(TRACE, "Enter\n"); >>>>>>> + ifp = netdev_priv(dev); >>>>>>> + if (params->ap_isolate >= 0) { >>>>>>> + ap_isolate = (u32)params->ap_isolate; >>>>>>> + ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", ap_isolate); >>>>>> >>>>>> Is the cast to u32 really necessary? Please avoid casts as much as >>>>>> possible. >>>>> >>>>> It seems to me. struct bss_parameters::ap_isolate is typed as int. It >>>>> is passed to brcmf_fil_iovar_int_set() which requires a u32 (maybe >>>>> function name is causing the confusion). >>>> >>>> What extra value does this explicit type casting bring here? I don't >>>> see >>>> it. Implicit type casting would work the same, no? >>> >>> The value will be -1, 0 or 1. >>> I will submit v2 patch that ignores doing iovar if getting >>> params->ap_isolate -1 and removes explicit type casting. Thanks for the >>> comment. >> >> Oh, I didn't realise ap_isolate can be -1 as struct bss_parameters >> didn't document that. Can someone submit a patch to fix that? >> >> * @ap_isolate: do not forward packets between connected stations > > Me too. I assumed it was a boolean reading that description. > > Regards, > Arend > The ap_isolate -1 value in nl80211_set_bss means not to changed.I intend to add a check of "params->ap_isolate >= 0" like ath/wil6210/cfg80211.c does in brcmf_cfg80211_change_bss. And I will submit another patch to revise the comment in cfg80211.h as below. Let me know if you concern about it. --- diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index fc7e8807838d..f60281c866dc 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1764,6 +1764,7 @@ struct mpath_info { * (or NULL for no change) * @basic_rates_len: number of basic rates * @ap_isolate: do not forward packets between connected stations + * (0 = no, 1 = yes, -1 = do not change) * @ht_opmode: HT Operation mode * (u16 = opmode, -1 = do not change) * @p2p_ctwindow: P2P CT Window (-1 = no change) --- Regards, Wright ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] brcmfmac: add change_bss to support AP isolation 2020-09-08 2:13 ` Wright Feng @ 2020-09-08 4:29 ` Kalle Valo 0 siblings, 0 replies; 17+ messages in thread From: Kalle Valo @ 2020-09-08 4:29 UTC (permalink / raw) To: Wright Feng Cc: Arend Van Spriel, linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Chi-Hsien Lin Wright Feng <Wright.Feng@cypress.com> writes: >>> Oh, I didn't realise ap_isolate can be -1 as struct bss_parameters >>> didn't document that. Can someone submit a patch to fix that? >>> >>> * @ap_isolate: do not forward packets between connected stations >> >> Me too. I assumed it was a boolean reading that description. >> >> Regards, >> Arend > > The ap_isolate -1 value in nl80211_set_bss means not to changed.I > intend to add a check of "params->ap_isolate >= 0" like > ath/wil6210/cfg80211.c does in brcmf_cfg80211_change_bss. And I will > submit another patch to revise the comment in cfg80211.h as below. Let > me know if you concern about it. Great, thanks. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists 2020-09-01 6:32 [PATCH 0/4] brcmfmac: Add few features in AP mode Wright Feng 2020-09-01 6:32 ` [PATCH 1/4] brcmfmac: add change_bss to support AP isolation Wright Feng @ 2020-09-01 6:32 ` Wright Feng 2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng 2020-09-01 6:32 ` [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition Wright Feng 3 siblings, 0 replies; 17+ messages in thread From: Wright Feng @ 2020-09-01 6:32 UTC (permalink / raw) To: linux-wireless Cc: wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Ting-Ying Li From: Ting-Ying Li <tingying.li@cypress.com> Add a condition to determine whether arp/nd offload enabling request is allowed. If there is any interface acts as ap mode and is operating, then reject the request of arp oflload enabling from cfg80211. Signed-off-by: Ting-Ying Li <tingying.li@cypress.com> --- .../broadcom/brcm80211/brcmfmac/cfg80211.c | 17 ++++++++++++++++- .../broadcom/brcm80211/brcmfmac/cfg80211.h | 1 + .../wireless/broadcom/brcm80211/brcmfmac/core.c | 5 +++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 7ef93cd66b2c..d6972420d426 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -766,6 +766,21 @@ void brcmf_set_mpc(struct brcmf_if *ifp, int mpc) } } +bool brcmf_is_apmode_operating(struct wiphy *wiphy) +{ + struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy); + struct brcmf_cfg80211_vif *vif; + bool ret = false; + + list_for_each_entry(vif, &cfg->vif_list, list) { + if (brcmf_is_apmode(vif) && + test_bit(BRCMF_VIF_STATUS_AP_CREATED, &vif->sme_state)) + ret = true; + } + + return ret; +} + s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg, struct brcmf_if *ifp, bool aborted, bool fw_abort) @@ -4949,8 +4964,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev) bphy_err(drvr, "bss_enable config failed %d\n", err); } brcmf_set_mpc(ifp, 1); - brcmf_configure_arp_nd_offload(ifp, true); clear_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state); + brcmf_configure_arp_nd_offload(ifp, true); brcmf_net_setcarrier(ifp, false); return err; diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h index 333fdf394f95..8c14d00c6d0e 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h @@ -447,5 +447,6 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg, void brcmf_set_mpc(struct brcmf_if *ndev, int mpc); void brcmf_abort_scanning(struct brcmf_cfg80211_info *cfg); void brcmf_cfg80211_free_netdev(struct net_device *ndev); +bool brcmf_is_apmode_operating(struct wiphy *wiphy); #endif /* BRCMFMAC_CFG80211_H */ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index f89010a81ffb..20c510dca601 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -96,6 +96,11 @@ void brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable) s32 err; u32 mode; + if (enable && brcmf_is_apmode_operating(ifp->drvr->wiphy)) { + brcmf_dbg(TRACE, "Skip ARP/ND offload enable when soft AP is running\n"); + return; + } + if (enable) mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY; else -- 2.25.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/4] brcmfmac: support the forwarding packet 2020-09-01 6:32 [PATCH 0/4] brcmfmac: Add few features in AP mode Wright Feng 2020-09-01 6:32 ` [PATCH 1/4] brcmfmac: add change_bss to support AP isolation Wright Feng 2020-09-01 6:32 ` [PATCH 2/4] brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists Wright Feng @ 2020-09-01 6:32 ` Wright Feng 2020-09-01 9:39 ` Arend Van Spriel ` (2 more replies) 2020-09-01 6:32 ` [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition Wright Feng 3 siblings, 3 replies; 17+ messages in thread From: Wright Feng @ 2020-09-01 6:32 UTC (permalink / raw) To: linux-wireless Cc: wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Jia-Shyr Chuang, Ting-Ying Li From: Jia-Shyr Chuang <joseph.chuang@cypress.com> Support packet forwarding mechanism for some special usages on PCIE, and fix BE/VI priority issue when pumping iperf. Signed-off-by: Jia-Shyr Chuang <joseph.chuang@cypress.com> Signed-off-by: Ting-Ying Li <tingying.li@cypress.com> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> --- .../broadcom/brcm80211/brcmfmac/cfg80211.c | 13 ++- .../broadcom/brcm80211/brcmfmac/core.c | 100 +++++++++++++++++- .../broadcom/brcm80211/brcmfmac/core.h | 18 +++- .../broadcom/brcm80211/brcmfmac/msgbuf.c | 31 +++++- 4 files changed, 157 insertions(+), 5 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index d6972420d426..8c7941f85715 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -4799,7 +4799,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, err = -EINVAL; goto exit; } - + ifp->isap = false; /* Interface specific setup */ if (dev_role == NL80211_IFTYPE_AP) { if ((brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS)) && (!mbss)) @@ -4860,7 +4860,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, err); goto exit; } - + ifp->isap = true; brcmf_dbg(TRACE, "AP mode configuration complete\n"); } else if (dev_role == NL80211_IFTYPE_P2P_GO) { err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec); @@ -4892,6 +4892,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, goto exit; } + ifp->isap = true; brcmf_dbg(TRACE, "GO mode configuration complete\n"); } else { WARN_ON(1); @@ -6045,6 +6046,14 @@ brcmf_notify_connect_status(struct brcmf_if *ifp, } if (brcmf_is_apmode(ifp->vif)) { + if (e->event_code == BRCMF_E_ASSOC_IND || + e->event_code == BRCMF_E_REASSOC_IND) { + brcmf_findadd_sta(ifp, e->addr); + } else if ((e->event_code == BRCMF_E_DISASSOC_IND) || + (e->event_code == BRCMF_E_DEAUTH_IND) || + (e->event_code == BRCMF_E_DEAUTH)) { + brcmf_del_sta(ifp, e->addr); + } err = brcmf_notify_connect_status_ap(cfg, ndev, e, data); } else if (brcmf_is_linkup(ifp->vif, e)) { brcmf_dbg(CONN, "Linkup\n"); diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 20c510dca601..3257b784e019 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -62,6 +62,14 @@ struct wlc_d11rxhdr { s8 rxpwr[4]; } __packed; +#define BRCMF_IF_STA_LIST_LOCK_INIT(ifp) spin_lock_init(&(ifp)->sta_list_lock) +#define BRCMF_IF_STA_LIST_LOCK(ifp, flags) \ + spin_lock_irqsave(&(ifp)->sta_list_lock, (flags)) +#define BRCMF_IF_STA_LIST_UNLOCK(ifp, flags) \ + spin_unlock_irqrestore(&(ifp)->sta_list_lock, (flags)) + +#define BRCMF_STA_NULL ((struct brcmf_sta *)NULL) + char *brcmf_ifname(struct brcmf_if *ifp) { if (!ifp) @@ -899,7 +907,9 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx, init_waitqueue_head(&ifp->pend_8021x_wait); spin_lock_init(&ifp->netif_stop_lock); - + BRCMF_IF_STA_LIST_LOCK_INIT(ifp); + /* Initialize STA info list */ + INIT_LIST_HEAD(&ifp->sta_list); if (mac_addr != NULL) memcpy(ifp->mac_addr, mac_addr, ETH_ALEN); @@ -1557,3 +1567,91 @@ void __exit brcmf_core_exit(void) #endif } +/** Find STA with MAC address ea in an interface's STA list. */ +struct brcmf_sta * +brcmf_find_sta(struct brcmf_if *ifp, const u8 *ea) +{ + struct brcmf_sta *sta; + unsigned long flags; + + BRCMF_IF_STA_LIST_LOCK(ifp, flags); + list_for_each_entry(sta, &ifp->sta_list, list) { + if (!memcmp(sta->ea.octet, ea, ETH_ALEN)) { + brcmf_dbg(INFO, "Found STA: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x into sta list\n", + sta->ea.octet[0], sta->ea.octet[1], + sta->ea.octet[2], sta->ea.octet[3], + sta->ea.octet[4], sta->ea.octet[5]); + BRCMF_IF_STA_LIST_UNLOCK(ifp, flags); + return sta; + } + } + BRCMF_IF_STA_LIST_UNLOCK(ifp, flags); + + return BRCMF_STA_NULL; +} + +/** Add STA into the interface's STA list. */ +struct brcmf_sta * +brcmf_add_sta(struct brcmf_if *ifp, const u8 *ea) +{ + struct brcmf_sta *sta; + unsigned long flags; + + sta = kzalloc(sizeof(*sta), GFP_KERNEL); + if (sta == BRCMF_STA_NULL) { + brcmf_err("Alloc failed\n"); + return BRCMF_STA_NULL; + } + memcpy(sta->ea.octet, ea, ETH_ALEN); + brcmf_dbg(INFO, "Add STA: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x into sta list\n", + sta->ea.octet[0], sta->ea.octet[1], + sta->ea.octet[2], sta->ea.octet[3], + sta->ea.octet[4], sta->ea.octet[5]); + + /* link the sta and the dhd interface */ + sta->ifp = ifp; + INIT_LIST_HEAD(&sta->list); + + BRCMF_IF_STA_LIST_LOCK(ifp, flags); + + list_add_tail(&sta->list, &ifp->sta_list); + + BRCMF_IF_STA_LIST_UNLOCK(ifp, flags); + return sta; +} + +/** Delete STA from the interface's STA list. */ +void +brcmf_del_sta(struct brcmf_if *ifp, const u8 *ea) +{ + struct brcmf_sta *sta, *next; + unsigned long flags; + + BRCMF_IF_STA_LIST_LOCK(ifp, flags); + list_for_each_entry_safe(sta, next, &ifp->sta_list, list) { + if (!memcmp(sta->ea.octet, ea, ETH_ALEN)) { + brcmf_dbg(INFO, "del STA: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x from sta list\n", + ea[0], ea[1], ea[2], ea[3], + ea[4], ea[5]); + list_del(&sta->list); + kfree(sta); + } + } + + BRCMF_IF_STA_LIST_UNLOCK(ifp, flags); +} + +/** Add STA if it doesn't exist. Not reentrant. */ +struct brcmf_sta* +brcmf_findadd_sta(struct brcmf_if *ifp, const u8 *ea) +{ + struct brcmf_sta *sta = NULL; + + sta = brcmf_find_sta(ifp, ea); + + if (!sta) { + /* Add entry */ + sta = brcmf_add_sta(ifp, ea); + } + return sta; +} diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 33b2ab3b54b0..3cf0b8c8d7b1 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -185,6 +185,7 @@ struct brcmf_if { struct brcmf_fws_mac_descriptor *fws_desc; int ifidx; s32 bsscfgidx; + bool isap; u8 mac_addr[ETH_ALEN]; u8 netif_stop; spinlock_t netif_stop_lock; @@ -193,6 +194,19 @@ struct brcmf_if { struct in6_addr ipv6_addr_tbl[NDOL_MAX_ENTRIES]; u8 ipv6addr_idx; bool fwil_fwerr; + struct list_head sta_list; /* sll of associated stations */ + spinlock_t sta_list_lock; +}; + +struct ether_addr { + u8 octet[ETH_ALEN]; +}; + +/** Per STA params. A list of dhd_sta objects are managed in dhd_if */ +struct brcmf_sta { + void *ifp; /* associated brcm_if */ + struct ether_addr ea; /* stations ethernet mac address */ + struct list_head list; /* link into brcmf_if::sta_list */ }; int brcmf_netdev_wait_pend8021x(struct brcmf_if *ifp); @@ -215,5 +229,7 @@ int brcmf_net_mon_attach(struct brcmf_if *ifp); void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on); int __init brcmf_core_init(void); void __exit brcmf_core_exit(void); - +void brcmf_del_sta(struct brcmf_if *ifp, const u8 *ea); +struct brcmf_sta *brcmf_find_sta(struct brcmf_if *ifp, const u8 *ea); +struct brcmf_sta *brcmf_findadd_sta(struct brcmf_if *ifp, const u8 *ea); #endif /* BRCMFMAC_CORE_H */ diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index f1a20db8daab..c53c3cf96f92 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -1140,7 +1140,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) { struct brcmf_pub *drvr = msgbuf->drvr; struct msgbuf_rx_complete *rx_complete; - struct sk_buff *skb; + struct sk_buff *skb, *cpskb = NULL; + struct ethhdr *eh; u16 data_offset; u16 buflen; u16 flags; @@ -1189,6 +1190,34 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) return; } + eh = (struct ethhdr *)(skb->data); + if (ifp->isap) { + skb_set_network_header(skb, sizeof(struct ethhdr)); + skb->protocol = eh->h_proto; + skb->priority = cfg80211_classify8021d(skb, NULL); + if (is_unicast_ether_addr(eh->h_dest)) { + if (brcmf_find_sta(ifp, eh->h_dest)) { + /* determine the priority */ + if (skb->priority == 0 || skb->priority > 7) { + skb->priority = + cfg80211_classify8021d(skb, + NULL); + } + brcmf_proto_tx_queue_data(ifp->drvr, + ifp->ifidx, skb); + return; + } + } else { + cpskb = pskb_copy(skb, GFP_ATOMIC); + if (cpskb) { + brcmf_proto_tx_queue_data(ifp->drvr, + ifp->ifidx, + cpskb); + } else { + brcmf_err("Unable to do skb copy\n"); + } + } + } skb->protocol = eth_type_trans(skb, ifp->ndev); brcmf_netif_rx(ifp, skb); } -- 2.25.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] brcmfmac: support the forwarding packet 2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng @ 2020-09-01 9:39 ` Arend Van Spriel 2020-09-03 3:30 ` kernel test robot 2020-09-03 3:30 ` [RFC PATCH] brcmfmac: brcmf_add_sta can be static kernel test robot 2 siblings, 0 replies; 17+ messages in thread From: Arend Van Spriel @ 2020-09-01 9:39 UTC (permalink / raw) To: Wright Feng, linux-wireless Cc: brcm80211-dev-list, brcm80211-dev-list, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Jia-Shyr Chuang, Ting-Ying Li On 9/1/2020 8:32 AM, Wright Feng wrote: > From: Jia-Shyr Chuang <joseph.chuang@cypress.com> > > Support packet forwarding mechanism for some special usages on PCIE, > and fix BE/VI priority issue when pumping iperf. This is a bit to brief for what is being introduced here. Basically, your are introducing a shortcut if a packet from an associated STA is destined for another associated STA taking the linux network stack and its overhead out of the equation. Not sure if this is a desired feature for upstream driver as it likely is interfering with features like TSQ. So there should be something done to inform the network stack about this packet going into transmit path. I also don't understand what is PCIe specific about this and what BE/VI priority issue is being fixed. Please spend more words on the commit message. Some more specific comment below. Regards, Arend > Signed-off-by: Jia-Shyr Chuang <joseph.chuang@cypress.com> > Signed-off-by: Ting-Ying Li <tingying.li@cypress.com> > Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> > --- > .../broadcom/brcm80211/brcmfmac/cfg80211.c | 13 ++- > .../broadcom/brcm80211/brcmfmac/core.c | 100 +++++++++++++++++- > .../broadcom/brcm80211/brcmfmac/core.h | 18 +++- > .../broadcom/brcm80211/brcmfmac/msgbuf.c | 31 +++++- > 4 files changed, 157 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > index d6972420d426..8c7941f85715 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > @@ -4799,7 +4799,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, > err = -EINVAL; > goto exit; > } > - > + ifp->isap = false; > /* Interface specific setup */ > if (dev_role == NL80211_IFTYPE_AP) { > if ((brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS)) && (!mbss)) > @@ -4860,7 +4860,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, > err); > goto exit; > } > - > + ifp->isap = true; > brcmf_dbg(TRACE, "AP mode configuration complete\n"); > } else if (dev_role == NL80211_IFTYPE_P2P_GO) { > err = brcmf_fil_iovar_int_set(ifp, "chanspec", chanspec); > @@ -4892,6 +4892,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev, > goto exit; > } > > + ifp->isap = true; > brcmf_dbg(TRACE, "GO mode configuration complete\n"); > } else { > WARN_ON(1); > @@ -6045,6 +6046,14 @@ brcmf_notify_connect_status(struct brcmf_if *ifp, > } > > if (brcmf_is_apmode(ifp->vif)) { Given this function I don't think the new struct brcmf_if::isap flag is necessary. > + if (e->event_code == BRCMF_E_ASSOC_IND || > + e->event_code == BRCMF_E_REASSOC_IND) { > + brcmf_findadd_sta(ifp, e->addr); > + } else if ((e->event_code == BRCMF_E_DISASSOC_IND) || > + (e->event_code == BRCMF_E_DEAUTH_IND) || > + (e->event_code == BRCMF_E_DEAUTH)) { > + brcmf_del_sta(ifp, e->addr); > + } > err = brcmf_notify_connect_status_ap(cfg, ndev, e, data); > } else if (brcmf_is_linkup(ifp->vif, e)) { > brcmf_dbg(CONN, "Linkup\n"); > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > index 20c510dca601..3257b784e019 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > @@ -62,6 +62,14 @@ struct wlc_d11rxhdr { > s8 rxpwr[4]; > } __packed; > > +#define BRCMF_IF_STA_LIST_LOCK_INIT(ifp) spin_lock_init(&(ifp)->sta_list_lock) > +#define BRCMF_IF_STA_LIST_LOCK(ifp, flags) \ > + spin_lock_irqsave(&(ifp)->sta_list_lock, (flags)) > +#define BRCMF_IF_STA_LIST_UNLOCK(ifp, flags) \ > + spin_unlock_irqrestore(&(ifp)->sta_list_lock, (flags)) > + > +#define BRCMF_STA_NULL ((struct brcmf_sta *)NULL) Don't hide actual functions with macros like these. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/4] brcmfmac: support the forwarding packet 2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng 2020-09-01 9:39 ` Arend Van Spriel @ 2020-09-03 3:30 ` kernel test robot 2020-09-03 3:30 ` [RFC PATCH] brcmfmac: brcmf_add_sta can be static kernel test robot 2 siblings, 0 replies; 17+ messages in thread From: kernel test robot @ 2020-09-03 3:30 UTC (permalink / raw) To: Wright Feng, linux-wireless Cc: kbuild-all, wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Jia-Shyr Chuang [-- Attachment #1: Type: text/plain, Size: 1396 bytes --] Hi Wright, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on wireless-drivers-next/master] [also build test WARNING on wireless-drivers/master v5.9-rc3 next-20200902] [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/Wright-Feng/brcmfmac-Add-few-features-in-AP-mode/20200901-143506 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers-next.git master config: i386-randconfig-s002-20200902 (attached as .config) compiler: gcc-9 (Debian 9.3.0-15) 9.3.0 reproduce: # apt-get install sparse # sparse version: v0.6.2-191-g10164920-dirty # save the attached .config to linux build tree make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 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 >>) >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c:1594:18: sparse: sparse: symbol 'brcmf_add_sta' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 40269 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH] brcmfmac: brcmf_add_sta can be static 2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng 2020-09-01 9:39 ` Arend Van Spriel 2020-09-03 3:30 ` kernel test robot @ 2020-09-03 3:30 ` kernel test robot 2 siblings, 0 replies; 17+ messages in thread From: kernel test robot @ 2020-09-03 3:30 UTC (permalink / raw) To: Wright Feng, linux-wireless Cc: kbuild-all, wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Jia-Shyr Chuang Signed-off-by: kernel test robot <lkp@intel.com> --- core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c index 595795e8cf6ac..c71c07e1f245a 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c @@ -1591,7 +1591,7 @@ brcmf_find_sta(struct brcmf_if *ifp, const u8 *ea) } /** Add STA into the interface's STA list. */ -struct brcmf_sta * +static struct brcmf_sta * brcmf_add_sta(struct brcmf_if *ifp, const u8 *ea) { struct brcmf_sta *sta; ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition 2020-09-01 6:32 [PATCH 0/4] brcmfmac: Add few features in AP mode Wright Feng ` (2 preceding siblings ...) 2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng @ 2020-09-01 6:32 ` Wright Feng 2020-09-07 9:21 ` Kalle Valo 3 siblings, 1 reply; 17+ messages in thread From: Wright Feng @ 2020-09-01 6:32 UTC (permalink / raw) To: linux-wireless Cc: wright.feng, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, Kalle Valo, chi-hsien.lin, Ting-Ying Li From: Ting-Ying Li <tingying.li@cypress.com> When the "ap_isolate" function is not set by the host, host-based packet forwarding will be enabled if the packet forwarding mechanism is not offloaded to the lower layer. Signed-off-by: Ting-Ying Li <tingying.li@cypress.com> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> --- .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 13 ++++++++++++- .../net/wireless/broadcom/brcm80211/brcmfmac/core.h | 1 + .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 4 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index 8c7941f85715..ac0095989e43 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -5447,7 +5447,7 @@ brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, { struct brcmf_if *ifp; int ret = 0; - u32 ap_isolate; + u32 ap_isolate, val; brcmf_dbg(TRACE, "Enter\n"); ifp = netdev_priv(dev); @@ -5458,6 +5458,17 @@ brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev, brcmf_err("ap_isolate iovar failed: ret=%d\n", ret); } + /* Get ap_isolate value from firmware to detemine whether fmac */ + /* driver supports packet forwarding. */ + if (brcmf_fil_iovar_int_get(ifp, "ap_isolate", &val) == 0) { + ifp->fmac_pkt_fwd_en = + ((params->ap_isolate == 0) && (val == 1)) ? + true : false; + } else { + brcmf_err("get ap_isolate iovar failed: ret=%d\n", ret); + ifp->fmac_pkt_fwd_en = false; + } + return ret; } diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h index 3cf0b8c8d7b1..d5745f48e003 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h @@ -196,6 +196,7 @@ struct brcmf_if { bool fwil_fwerr; struct list_head sta_list; /* sll of associated stations */ spinlock_t sta_list_lock; + bool fmac_pkt_fwd_en; }; struct ether_addr { diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c index c53c3cf96f92..1c109257aefc 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c @@ -1190,8 +1190,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) return; } - eh = (struct ethhdr *)(skb->data); - if (ifp->isap) { + if (ifp->isap && ifp->fmac_pkt_fwd_en) { + eh = (struct ethhdr *)(skb->data); skb_set_network_header(skb, sizeof(struct ethhdr)); skb->protocol = eh->h_proto; skb->priority = cfg80211_classify8021d(skb, NULL); -- 2.25.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition 2020-09-01 6:32 ` [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition Wright Feng @ 2020-09-07 9:21 ` Kalle Valo 0 siblings, 0 replies; 17+ messages in thread From: Kalle Valo @ 2020-09-07 9:21 UTC (permalink / raw) To: Wright Feng Cc: linux-wireless, brcm80211-dev-list, brcm80211-dev-list, Arend van Spriel, Franky Lin, Hante Meuleman, chi-hsien.lin, Ting-Ying Li Wright Feng <wright.feng@cypress.com> writes: > From: Ting-Ying Li <tingying.li@cypress.com> > > When the "ap_isolate" function is not set by the host, > host-based packet forwarding will be enabled if the packet > forwarding mechanism is not offloaded to the lower layer. > > Signed-off-by: Ting-Ying Li <tingying.li@cypress.com> > Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com> [...] > + /* Get ap_isolate value from firmware to detemine whether fmac */ > + /* driver supports packet forwarding. */ > + if (brcmf_fil_iovar_int_get(ifp, "ap_isolate", &val) == 0) { > + ifp->fmac_pkt_fwd_en = > + ((params->ap_isolate == 0) && (val == 1)) ? > + true : false; > + } else { > + brcmf_err("get ap_isolate iovar failed: ret=%d\n", ret); > + ifp->fmac_pkt_fwd_en = false; > + } This is hard to read, you can simplify it to: if (brcmf_fil_iovar_int_get(ifp, "ap_isolate", &val) == 0 && params->ap_isolate == 0 && val == 1) { ifp->fmac_pkt_fwd_en = true; } else { brcmf_err("get ap_isolate iovar failed: ret=%d\n", ret); ifp->fmac_pkt_fwd_en = false; } -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-09-08 4:29 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-01 6:32 [PATCH 0/4] brcmfmac: Add few features in AP mode Wright Feng
2020-09-01 6:32 ` [PATCH 1/4] brcmfmac: add change_bss to support AP isolation Wright Feng
2020-09-07 9:04 ` Kalle Valo
2020-09-07 9:21 ` Arend Van Spriel
2020-09-07 9:49 ` Kalle Valo
2020-09-07 10:09 ` Wright Feng
2020-09-07 15:29 ` Kalle Valo
[not found] ` <01010174692f7c3f-4b7369b2-0665-4324-b1c8-57bd22ac9ce7-000000@us-west-2.amazonses.com>
2020-09-07 15:57 ` Arend Van Spriel
2020-09-08 2:13 ` Wright Feng
2020-09-08 4:29 ` Kalle Valo
2020-09-01 6:32 ` [PATCH 2/4] brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists Wright Feng
2020-09-01 6:32 ` [PATCH 3/4] brcmfmac: support the forwarding packet Wright Feng
2020-09-01 9:39 ` Arend Van Spriel
2020-09-03 3:30 ` kernel test robot
2020-09-03 3:30 ` [RFC PATCH] brcmfmac: brcmf_add_sta can be static kernel test robot
2020-09-01 6:32 ` [PATCH 4/4] brcmfmac: add a variable for packet forwarding condition Wright Feng
2020-09-07 9:21 ` Kalle Valo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).