Skip to contentSkip to conversation

brcmfmac: Turn off ARP offloading when configured for AP.

PATCH In review Arend van Spriel 5 participants 13 messages Jun 6, 2013 – Jul 4, 2013 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 brcmfmac: Turn off ARP offloading when configured for AP.

From: Hante Meuleman <[email protected]>

[backport of upstream commit b3657453f16a7b84eab9b93bb9a9a2901ffc70af]

ARP offloading should only be used in STA or P2P client mode. It is currently configured once at init. When being configured for AP ARP offloading should be turned off and when AP mode is left it can be turned back on.

---
 .../net/wireless/brcm80211/brcmfmac/dhd_common.c   |   18 ---------
 .../net/wireless/brcm80211/brcmfmac/fwil_types.h   |    6 +++
 .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c  |   40 +++++++++++++++++++-
 3 files changed, 45 insertions(+), 19 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c +0 -18
index 4544342..9480e19 100644
@@ -26,7 +26,6 @@
 #include "fwil.h"
 
 #define PKTFILTER_BUF_SIZE		128
-#define BRCMF_ARPOL_MODE		0xb	/* agent|snoop|peer_autoreply */
 #define BRCMF_DEFAULT_BCN_TIMEOUT	3
 #define BRCMF_DEFAULT_SCAN_CHANNEL_TIME	40
 #define BRCMF_DEFAULT_SCAN_UNASSOC_TIME	40
@@ -337,23 +336,6 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 		goto done;
 	}
 
-	/* Try to set and enable ARP offload feature, this may fail */
-	err = brcmf_fil_iovar_int_set(ifp, "arp_ol", BRCMF_ARPOL_MODE);
-	if (err) {
-		brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
-			  BRCMF_ARPOL_MODE, err);
-		err = 0;
-	} else {
-		err = brcmf_fil_iovar_int_set(ifp, "arpoe", 1);
-		if (err) {
-			brcmf_dbg(TRACE, "failed to enable ARP offload err = %d\n",
-				  err);
-			err = 0;
-		} else
-			brcmf_dbg(TRACE, "successfully enabled ARP offload to 0x%x\n",
-				  BRCMF_ARPOL_MODE);
-	}
-
 	/* Setup packet filter */
 	brcmf_c_pktfilter_offload_set(ifp, BRCMF_DEFAULT_PACKET_FILTER);
 	brcmf_c_pktfilter_offload_enable(ifp, BRCMF_DEFAULT_PACKET_FILTER,
drivers/net/wireless/brcm80211/brcmfmac/fwil_types.h +6 -0
index 0f2c83b..665ef69 100644
@@ -23,6 +23,12 @@
 
 #define BRCMF_FIL_ACTION_FRAME_SIZE	1800
 
+/* ARP Offload feature flags for arp_ol iovar */
+#define BRCMF_ARP_OL_AGENT		0x00000001
+#define BRCMF_ARP_OL_SNOOP		0x00000002
+#define BRCMF_ARP_OL_HOST_AUTO_REPLY	0x00000004
+#define BRCMF_ARP_OL_PEER_AUTO_REPLY	0x00000008
+
 
 enum brcmf_fil_p2p_if_types {
 	BRCMF_FIL_P2P_IF_CLIENT,
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c +39 -1
index 78da3ef..d5c4e24 100644
@@ -505,6 +505,38 @@ send_key_to_dongle(struct net_device *ndev, struct brcmf_wsec_key *key)
 	return err;
 }
 
+static s32
+brcmf_configure_arp_offload(struct brcmf_if *ifp, bool enable)
+{
+	s32 err;
+	u32 mode;
+
+	if (enable)
+		mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY;
+	else
+		mode = 0;
+
+	/* Try to set and enable ARP offload feature, this may fail, then it  */
+	/* is simply not supported and err 0 will be returned                 */
+	err = brcmf_fil_iovar_int_set(ifp, "arp_ol", mode);
+	if (err) {
+		brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
+			  mode, err);
+		err = 0;
+	} else {
+		err = brcmf_fil_iovar_int_set(ifp, "arpoe", enable);
+		if (err) {
+			brcmf_dbg(TRACE, "failed to configure (%d) ARP offload err = %d\n",
+				  enable, err);
+			err = 0;
+		} else
+			brcmf_dbg(TRACE, "successfully configured (%d) ARP offload to 0x%x\n",
+				  enable, mode);
+	}
+
+	return err;
+}
+
 static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
 						     const char *name,
 						     enum nl80211_iftype type,
@@ -3709,6 +3741,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	}
 
 	brcmf_set_mpc(ndev, 0);
+	brcmf_configure_arp_offload(ifp, false);
 
 	/* find the RSN_IE */
 	rsn_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
@@ -3815,8 +3848,10 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	set_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state);
 
 exit:
-	if (err)
+	if (err) {
 		brcmf_set_mpc(ndev, 1);
+		brcmf_configure_arp_offload(ifp, true);
+	}
 	return err;
 }
 
@@ -3857,6 +3892,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
 			brcmf_err("bss_enable config failed %d\n", err);
 	}
 	brcmf_set_mpc(ndev, 1);
+	brcmf_configure_arp_offload(ifp, true);
 	set_bit(BRCMF_VIF_STATUS_AP_CREATING, &ifp->vif->sme_state);
 	clear_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state);
 
@@ -4995,6 +5031,8 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
 	if (err)
 		goto default_conf_out;
 
+	brcmf_configure_arp_offload(ifp, true);
+
 	cfg->dongle_up = true;
 default_conf_out:
 
Signature · 1 line
1.7.10.4
#1 lore raw
Quoted 3 lines · Arend van Spriel

From: Hante Meuleman <[email protected]>

[backport of upstream commit b3657453f16a7b84eab9b93bb9a9a2901ffc70af]

[...]

You didn't say which version(s) this is for, but it certainly doesn't apply to 3.2.

Ben.

Signature · 2 lines
Ben Hutchings
friends: People who know you well, but like you anyway.
#2 lore raw
Quoted 11 lines · Ben Hutchings
Quoted 3 lines · Arend van Spriel

From: Hante Meuleman <[email protected]>

[backport of upstream commit b3657453f16a7b84eab9b93bb9a9a2901ffc70af]

[...]

You didn't say which version(s) this is for, but it certainly doesn't apply to 3.2.

Ben.

I actually did the backport because I got notified the upstream commit did not apply to 3.9-stable (see [1]).

Gr. AvS

[1] http://mid.gmane.org/[email protected]

#3 lore raw
Quoted 7 lines · Arend van Spriel
Quoted 6 lines · Ben Hutchings
Quoted 3 lines · Arend van Spriel

From: Hante Meuleman <[email protected]>

[backport of upstream commit b3657453f16a7b84eab9b93bb9a9a2901ffc70af]

[...]

Quoted 8 lines
Quoted 5 lines

You didn't say which version(s) this is for, but it certainly doesn't apply to 3.2.

Ben.

I actually did the backport because I got notified the upstream commit did not apply to 3.9-stable (see [1]).

In 3.2 brcmfmac did not support AP mode so that explains the fact that is does not apply ;-)

Quoted 3 lines

Gr. AvS

[1] http://mid.gmane.org/[email protected]

#4 lore raw
In reply to Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Regards,
Arend

Quoted 152 lines

[backport of upstream commit b3657453f16a7b84eab9b93bb9a9a2901ffc70af]

ARP offloading should only be used in STA or P2P client mode. It is currently configured once at init. When being configured for AP ARP offloading should be turned off and when AP mode is left it can be turned back on.

---
  .../net/wireless/brcm80211/brcmfmac/dhd_common.c   |   18 ---------
  .../net/wireless/brcm80211/brcmfmac/fwil_types.h   |    6 +++
  .../net/wireless/brcm80211/brcmfmac/wl_cfg80211.c  |   40 +++++++++++++++++++-
  3 files changed, 45 insertions(+), 19 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/dhd_common.c +0 -18
index 4544342..9480e19 100644
@@ -26,7 +26,6 @@
  #include "fwil.h"
 
  #define PKTFILTER_BUF_SIZE		128
-#define BRCMF_ARPOL_MODE		0xb	/* agent|snoop|peer_autoreply */
  #define BRCMF_DEFAULT_BCN_TIMEOUT	3
  #define BRCMF_DEFAULT_SCAN_CHANNEL_TIME	40
  #define BRCMF_DEFAULT_SCAN_UNASSOC_TIME	40
@@ -337,23 +336,6 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
  		goto done;
  	}
 
-	/* Try to set and enable ARP offload feature, this may fail */
-	err = brcmf_fil_iovar_int_set(ifp, "arp_ol", BRCMF_ARPOL_MODE);
-	if (err) {
-		brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
-			  BRCMF_ARPOL_MODE, err);
-		err = 0;
-	} else {
-		err = brcmf_fil_iovar_int_set(ifp, "arpoe", 1);
-		if (err) {
-			brcmf_dbg(TRACE, "failed to enable ARP offload err = %d\n",
-				  err);
-			err = 0;
-		} else
-			brcmf_dbg(TRACE, "successfully enabled ARP offload to 0x%x\n",
-				  BRCMF_ARPOL_MODE);
-	}
-
  	/* Setup packet filter */
  	brcmf_c_pktfilter_offload_set(ifp, BRCMF_DEFAULT_PACKET_FILTER);
  	brcmf_c_pktfilter_offload_enable(ifp, BRCMF_DEFAULT_PACKET_FILTER,
drivers/net/wireless/brcm80211/brcmfmac/fwil_types.h +6 -0
index 0f2c83b..665ef69 100644
@@ -23,6 +23,12 @@
 
  #define BRCMF_FIL_ACTION_FRAME_SIZE	1800
 
+/* ARP Offload feature flags for arp_ol iovar */
+#define BRCMF_ARP_OL_AGENT		0x00000001
+#define BRCMF_ARP_OL_SNOOP		0x00000002
+#define BRCMF_ARP_OL_HOST_AUTO_REPLY	0x00000004
+#define BRCMF_ARP_OL_PEER_AUTO_REPLY	0x00000008
+
 
  enum brcmf_fil_p2p_if_types {
  	BRCMF_FIL_P2P_IF_CLIENT,
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c +39 -1
index 78da3ef..d5c4e24 100644
@@ -505,6 +505,38 @@ send_key_to_dongle(struct net_device *ndev, struct brcmf_wsec_key *key)
  	return err;
  }
 
+static s32
+brcmf_configure_arp_offload(struct brcmf_if *ifp, bool enable)
+{
+	s32 err;
+	u32 mode;
+
+	if (enable)
+		mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY;
+	else
+		mode = 0;
+
+	/* Try to set and enable ARP offload feature, this may fail, then it  */
+	/* is simply not supported and err 0 will be returned                 */
+	err = brcmf_fil_iovar_int_set(ifp, "arp_ol", mode);
+	if (err) {
+		brcmf_dbg(TRACE, "failed to set ARP offload mode to 0x%x, err = %d\n",
+			  mode, err);
+		err = 0;
+	} else {
+		err = brcmf_fil_iovar_int_set(ifp, "arpoe", enable);
+		if (err) {
+			brcmf_dbg(TRACE, "failed to configure (%d) ARP offload err = %d\n",
+				  enable, err);
+			err = 0;
+		} else
+			brcmf_dbg(TRACE, "successfully configured (%d) ARP offload to 0x%x\n",
+				  enable, mode);
+	}
+
+	return err;
+}
+
  static struct wireless_dev *brcmf_cfg80211_add_iface(struct wiphy *wiphy,
  						     const char *name,
  						     enum nl80211_iftype type,
@@ -3709,6 +3741,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
  	}
 
  	brcmf_set_mpc(ndev, 0);
+	brcmf_configure_arp_offload(ifp, false);
 
  	/* find the RSN_IE */
  	rsn_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
@@ -3815,8 +3848,10 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
  	set_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state);
 
  exit:
-	if (err)
+	if (err) {
  		brcmf_set_mpc(ndev, 1);
+		brcmf_configure_arp_offload(ifp, true);
+	}
  	return err;
  }
 
@@ -3857,6 +3892,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev)
  			brcmf_err("bss_enable config failed %d\n", err);
  	}
  	brcmf_set_mpc(ndev, 1);
+	brcmf_configure_arp_offload(ifp, true);
  	set_bit(BRCMF_VIF_STATUS_AP_CREATING, &ifp->vif->sme_state);
  	clear_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state);
 
@@ -4995,6 +5031,8 @@ static s32 brcmf_config_dongle(struct brcmf_cfg80211_info *cfg)
  	if (err)
  		goto default_conf_out;
 
+	brcmf_configure_arp_offload(ifp, true);
+
  	cfg->dongle_up = true;
  default_conf_out:
 
#5 lore raw
Quoted 8 lines · Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

thanks,

greg k-h

#6 lore raw
Quoted 6 lines · Greg KH
Quoted 5 lines · Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

Quoted 7 lines
Quoted 3 lines

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

Thanks, Greg

I backported it for the 3.9 tree. It does not apply to 3.8 so I will have to create another backport if I need it there.

Regards,
Arend

#7 lore raw
Quoted 11 lines · Arend van Spriel
Quoted 10 lines · Greg KH
Quoted 8 lines · Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Quoted 8 lines
Quoted 3 lines

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

Thanks, Greg

I backported it for the 3.9 tree. It does not apply to 3.8 so I will have to create another backport if I need it there.

3.8 is long dead and not maintained by me anymore, but thanks.

Next time you submit stable patches, please let us know somewhere in the patch/subject what tree it applies to, that makes our lives much easier, otherwise I just guess and ususally get it wrong :)

thanks,

greg k-h

#8 lore raw
Quoted 16 lines · Greg KH
Quoted 15 lines · Arend van Spriel
Quoted 13 lines · Greg KH
Quoted 8 lines · Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

Quoted 6 lines
Quoted 4 lines

Thanks, Greg

I backported it for the 3.9 tree. It does not apply to 3.8 so I will have to create another backport if I need it there.

3.8 is long dead and not maintained by me anymore, but thanks.

Hi Greg,

Forgot to ask, but it seems like Canonical is maintaining additional stable kernel branches. I guess I should contact them for those kernel versions (if I care), right?

Regards,
Arend

Quoted 8 lines

Next time you submit stable patches, please let us know somewhere in the patch/subject what tree it applies to, that makes our lives much easier, otherwise I just guess and ususally get it wrong :)

thanks,

greg k-h

#9 lore raw

2013/7/2 Arend van Spriel <[email protected]>:

Quoted 31 lines · Greg KH
Quoted 30 lines · Arend van Spriel
Quoted 26 lines · Greg KH
Quoted 10 lines · Arend van Spriel
Quoted 8 lines · Arend van Spriel
Quoted 2 lines

From: Hante Meuleman <[email protected]>

Hi Greg,

Quoted 8 lines
Quoted 3 lines

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

Thanks, Greg

I backported it for the 3.9 tree. It does not apply to 3.8 so I will have to create another backport if I need it there.

Quoted 8 lines
Quoted 1 line

3.8 is long dead and not maintained by me anymore, but thanks.

Hi Greg,

Forgot to ask, but it seems like Canonical is maintaining additional stable kernel branches. I guess I should contact them for those kernel versions (if I care), right?

They track Greg's stables to pick up the patches. Know from my own experience, I didn't have to ping anyone :)

Signature · 1 line
Rafał
#10 lore raw
Quoted 10 lines · Rafał Miłecki

2013/7/2 Arend van Spriel <[email protected]>:

Quoted 5 lines

Hi Greg,

Forgot to ask, but it seems like Canonical is maintaining additional stable kernel branches. I guess I should contact them for those kernel versions (if I care), right?

They track Greg's stables to pick up the patches. Know from my own experience, I didn't have to ping anyone :)

Yes, we're picking up cc: stable patches automatically when they apply to the 3.8.y.z and 3.5.y.z "extended stable" branches[0], but we certainly appreciate specific backports for those versions when they don't apply cleanly.

-Kamal

[0] https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

#11 lore raw
Quoted 8 lines · Kamal Mostafa
Quoted 7 lines · Rafał Miłecki

2013/7/2 Arend van Spriel <[email protected]>:

Quoted 5 lines

Hi Greg,

Forgot to ask, but it seems like Canonical is maintaining additional stable kernel branches. I guess I should contact them for those kernel versions (if I care), right?

Quoted 8 lines
Quoted 3 lines

They track Greg's stables to pick up the patches. Know from my own experience, I didn't have to ping anyone :)

Yes, we're picking up cc: stable patches automatically when they apply to the 3.8.y.z and 3.5.y.z "extended stable" branches[0], but we certainly appreciate specific backports for those versions when they don't apply cleanly.

Thanks, Kamal

I send a backport for 3.9 and tried it on 3.8, but it does not apply so I would have to backport it. I will have a look and find out where to send them. Probably that info is on the referred wiki.

Regards,
Arend

Quoted 5 lines

-Kamal

[0] https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable

#12 lore raw
In reply to Greg KH
Quoted 6 lines · Greg KH
Quoted 5 lines · Arend van Spriel
Quoted 1 line · Arend van Spriel

From: Hante Meuleman <[email protected]>

Hi Greg,

Quoted 7 lines
Quoted 3 lines

I noticed your review announcement for v3.9.7 and did not see the change below. I sent it to stable because the original upstream commit did not apply. Did I miss some step in the process?

Ah, somehow I missed your patch, sorry about that, I do have it in my mbox. What kernel tree(s) do you want it to be applied to? I'll pick it up in my next round of releases.

Now applied, sorry for the delay.

greg k-h