Netdev List
 help / color / mirror / Atom feed
* [PATCH] wcn36xx: Disable 5GHz for wcn3610
From: Ramon Fried @ 2018-03-29  6:20 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: wcn36xx, linux-wireless, netdev, linux-kernel, Ramon Fried

wcn3610 can only operate on 2.4GHz band due to RF limitation.
If wcn36xx digital block is associated with an external IRIS
RF module, retrieve the id and disable 5GHz band in case of
wcn3610 id.

Signed-off-by: Ramon Fried <rfried@codeaurora.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index ab5be6d2c691..833531a68c95 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
 		BIT(NL80211_IFTYPE_MESH_POINT);
 
 	wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
-	if (wcn->rf_id != RF_IRIS_WCN3620)
+	if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
 		wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
 
 	wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
@@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
 	if (iris_node) {
 		if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
 			wcn->rf_id = RF_IRIS_WCN3620;
+		else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
+			wcn->rf_id = RF_IRIS_WCN3620;
 		of_node_put(iris_node);
 	}
 
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 81017e6703b4..bc4d1a10d90e 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -95,6 +95,7 @@ enum wcn36xx_ampdu_state {
 #define WCN36XX_MAX_POWER(__wcn) (__wcn->hw->conf.chandef.chan->max_power)
 
 #define RF_UNKNOWN	0x0000
+#define RF_IRIS_WCN3610	0x3610
 #define RF_IRIS_WCN3620	0x3620
 
 static inline void buff_to_be(u32 *buf, size_t len)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: Regression in 4.16-rc7 - ipsec vpn broken
From: Steffen Klassert @ 2018-03-29  6:51 UTC (permalink / raw)
  To: Derek Robson; +Cc: Herbert Xu, ben, linux-kernel, netdev
In-Reply-To: <CAMfSVWDZn28cYpseJCc=gO1uD1Cr_P5vKRW8FjaqczooyecHwg@mail.gmail.com>

Please always make sure to Cc netdev@vger.kernel.org
on networking problems.

On Wed, Mar 28, 2018 at 10:21:32PM +0000, Derek Robson wrote:
> The ipsec VPN is broken in 4.16-rc7 and seem to have been broken in all of
> 4.15
> 
> connecting from an iphone seems to give a timeout.
> 
> 
> A bisect brings me to this commit as the one that is the issue.
> 
> commit: acf568ee859f098279eadf551612f103afdacb4e  (xfrm: Reinject
> transport-mode packets through tasklet)

I have a fix queued for this commit in the ipsec tree.

Can you please try if the patch below fixes your problems?

Thanks!

Subject: [PATCH] xfrm: Fix transport mode skb control buffer usage.

A recent commit introduced a new struct xfrm_trans_cb
that is used with the sk_buff control buffer. Unfortunately
it placed the structure in front of the control buffer and
overlooked that the IPv4/IPv6 control buffer is still needed
for some layer 4 protocols. As a result the IPv4/IPv6 control
buffer is overwritten with this structure. Fix this by setting
a apropriate header in front of the structure.

Fixes acf568ee859f ("xfrm: Reinject transport-mode packets ...")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 1472c0857975..81788105c164 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -26,6 +26,12 @@ struct xfrm_trans_tasklet {
 };
 
 struct xfrm_trans_cb {
+	union {
+		struct inet_skb_parm	h4;
+#if IS_ENABLED(CONFIG_IPV6)
+		struct inet6_skb_parm	h6;
+#endif
+	} header;
 	int (*finish)(struct net *net, struct sock *sk, struct sk_buff *skb);
 };
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH v2] wcn36xx: Disable 5GHz for wcn3610
From: Ramon Fried @ 2018-03-29  6:58 UTC (permalink / raw)
  To: kvalo, loic.poulain
  Cc: wcn36xx, linux-wireless, netdev, linux-kernel, Ramon Fried

wcn3610 can only operate on 2.4GHz band due to RF limitation.
If wcn36xx digital block is associated with an external IRIS
RF module, retrieve the id and disable 5GHz band in case of
wcn3610 id.

Signed-off-by: Ramon Fried <rfried@codeaurora.org>
---
v2: fixed wrong assignment, which is logically introduces the 
	same behaviour, but for correctness.

 drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index ab5be6d2c691..833531a68c95 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
 		BIT(NL80211_IFTYPE_MESH_POINT);
 
 	wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
-	if (wcn->rf_id != RF_IRIS_WCN3620)
+	if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
 		wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
 
 	wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
@@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
 	if (iris_node) {
 		if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
 			wcn->rf_id = RF_IRIS_WCN3620;
+		else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
+			wcn->rf_id = RF_IRIS_WCN3610;
 		of_node_put(iris_node);
 	}
 
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index 81017e6703b4..bc4d1a10d90e 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -95,6 +95,7 @@ enum wcn36xx_ampdu_state {
 #define WCN36XX_MAX_POWER(__wcn) (__wcn->hw->conf.chandef.chan->max_power)
 
 #define RF_UNKNOWN	0x0000
+#define RF_IRIS_WCN3610	0x3610
 #define RF_IRIS_WCN3620	0x3620
 
 static inline void buff_to_be(u32 *buf, size_t len)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply related

* Re: [PATCH] wcn36xx: Disable 5GHz for wcn3610
From: Rafał Miłecki @ 2018-03-29  6:58 UTC (permalink / raw)
  To: Ramon Fried, kvalo, loic.poulain
  Cc: wcn36xx, linux-wireless, netdev, linux-kernel
In-Reply-To: <20180329062058.31340-1-rfried@codeaurora.org>

On 03/29/2018 08:20 AM, Ramon Fried wrote:
> wcn3610 can only operate on 2.4GHz band due to RF limitation.
> If wcn36xx digital block is associated with an external IRIS
> RF module, retrieve the id and disable 5GHz band in case of
> wcn3610 id.
>
> Signed-off-by: Ramon Fried <rfried@codeaurora.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
>  drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index ab5be6d2c691..833531a68c95 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>  		BIT(NL80211_IFTYPE_MESH_POINT);
>
>  	wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
> -	if (wcn->rf_id != RF_IRIS_WCN3620)
> +	if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
>  		wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
>
>  	wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
> @@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
>  	if (iris_node) {
>  		if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
>  			wcn->rf_id = RF_IRIS_WCN3620;
> +		else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
> +			wcn->rf_id = RF_IRIS_WCN3620;

RF_IRIS_WCN3610 ?

^ permalink raw reply

* Re: [PATCH] wcn36xx: Disable 5GHz for wcn3610
From: Ramon Fried @ 2018-03-29  7:00 UTC (permalink / raw)
  To: Rafał Miłecki, kvalo, loic.poulain
  Cc: wcn36xx, linux-wireless, linux-kernel, netdev
In-Reply-To: <93c900b5-4ddd-bbad-ebe2-4b9549ad8f1c@gmail.com>



On 3/29/2018 9:58 AM, Rafał Miłecki wrote:
> On 03/29/2018 08:20 AM, Ramon Fried wrote:
>> wcn3610 can only operate on 2.4GHz band due to RF limitation.
>> If wcn36xx digital block is associated with an external IRIS
>> RF module, retrieve the id and disable 5GHz band in case of
>> wcn3610 id.
>>
>> Signed-off-by: Ramon Fried <rfried@codeaurora.org>
>> ---
>>  drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
>>  drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index ab5be6d2c691..833531a68c95 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>>          BIT(NL80211_IFTYPE_MESH_POINT);
>>
>>      wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
>> -    if (wcn->rf_id != RF_IRIS_WCN3620)
>> +    if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
>>          wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
>>
>>      wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
>> @@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
>>      if (iris_node) {
>>          if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
>>              wcn->rf_id = RF_IRIS_WCN3620;
>> +        else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
>> +            wcn->rf_id = RF_IRIS_WCN3620;
>
> RF_IRIS_WCN3610 ?
You're correct. I also noticed just now.
Sent v2. Thanks.
>
> _______________________________________________
> wcn36xx mailing list
> wcn36xx@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/wcn36xx

^ permalink raw reply

* [PATCH 4/8] vti4: Don't override MTU passed on link creation via IFLA_MTU
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

Don't hardcode a MTU value on vti tunnel initialization,
ip_tunnel_newlink() is able to deal with this already. See also
commit ffc2b6ee4174 ("ip_gre: fix IFLA_MTU ignored on NEWLINK").

Fixes: 1181412c1a67 ("net/ipv4: VTI support new module for ip_vti.")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_vti.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 502e5222eaa9..3f091ccad9af 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -387,7 +387,6 @@ static int vti_tunnel_init(struct net_device *dev)
 	memcpy(dev->dev_addr, &iph->saddr, 4);
 	memcpy(dev->broadcast, &iph->daddr, 4);
 
-	dev->mtu		= ETH_DATA_LEN;
 	dev->flags		= IFF_NOARP;
 	dev->addr_len		= 4;
 	dev->features		|= NETIF_F_LLTX;
-- 
2.14.1

^ permalink raw reply related

* [PATCH 3/8] ip_tunnel: Clamp MTU to bounds on new link
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

Otherwise, it's possible to specify invalid MTU values directly
on creation of a link (via 'ip link add'). This is already
prevented on subsequent MTU changes by commit b96f9afee4eb
("ipv4/6: use core net MTU range checking").

Fixes: c54419321455 ("GRE: Refactor GRE tunneling code.")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_tunnel.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 6d21068f9b55..7c76dd17b6b9 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -1108,8 +1108,14 @@ int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
 		eth_hw_addr_random(dev);
 
 	mtu = ip_tunnel_bind_dev(dev);
-	if (!tb[IFLA_MTU])
+	if (tb[IFLA_MTU]) {
+		unsigned int max = 0xfff8 - dev->hard_header_len - nt->hlen;
+
+		dev->mtu = clamp(dev->mtu, (unsigned int)ETH_MIN_MTU,
+				 (unsigned int)(max - sizeof(struct iphdr)));
+	} else {
 		dev->mtu = mtu;
+	}
 
 	ip_tunnel_add(itn, nt);
 out:
-- 
2.14.1

^ permalink raw reply related

* [PATCH 2/8] vti4: Don't count header length twice on tunnel setup
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

This re-introduces the effect of commit a32452366b72 ("vti4:
Don't count header length twice.") which was accidentally
reverted by merge commit f895f0cfbb77 ("Merge branch 'master' of
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec").

The commit message from Steffen Klassert said:

    We currently count the size of LL_MAX_HEADER and struct iphdr
    twice for vti4 devices, this leads to a wrong device mtu.
    The size of LL_MAX_HEADER and struct iphdr is already counted in
    ip_tunnel_bind_dev(), so don't do it again in vti_tunnel_init().

And this is still the case now: ip_tunnel_bind_dev() already
accounts for the header length of the link layer (not
necessarily LL_MAX_HEADER, if the output device is found), plus
one IP header.

For example, with a vti device on top of veth, with MTU of 1500,
the existing implementation would set the initial vti MTU to
1332, accounting once for LL_MAX_HEADER (128, included in
hard_header_len by vti) and twice for the same IP header (once
from hard_header_len, once from ip_tunnel_bind_dev()).

It should instead be 1480, because ip_tunnel_bind_dev() is able
to figure out that the output device is veth, so no additional
link layer header is attached, and will properly count one
single IP header.

The existing issue had the side effect of avoiding PMTUD for
most xfrm policies, by arbitrarily lowering the initial MTU.
However, the only way to get a consistent PMTU value is to let
the xfrm PMTU discovery do its course, and commit d6af1a31cc72
("vti: Add pmtu handling to vti_xmit.") now takes care of local
delivery cases where the application ignores local socket
notifications.

Fixes: b9959fd3b0fa ("vti: switch to new ip tunnel code")
Fixes: f895f0cfbb77 ("Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/ip_vti.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 51b1669334fe..502e5222eaa9 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -387,7 +387,6 @@ static int vti_tunnel_init(struct net_device *dev)
 	memcpy(dev->dev_addr, &iph->saddr, 4);
 	memcpy(dev->broadcast, &iph->daddr, 4);
 
-	dev->hard_header_len	= LL_MAX_HEADER + sizeof(struct iphdr);
 	dev->mtu		= ETH_DATA_LEN;
 	dev->flags		= IFF_NOARP;
 	dev->addr_len		= 4;
-- 
2.14.1

^ permalink raw reply related

* [PATCH 1/8] xfrm: fix rcu_read_unlock usage in xfrm_local_error
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Taehee Yoo <ap420073@gmail.com>

In the xfrm_local_error, rcu_read_unlock should be called when afinfo
is not NULL. because xfrm_state_get_afinfo calls rcu_read_unlock
if afinfo is NULL.

Fixes: af5d27c4e12b ("xfrm: remove xfrm_state_put_afinfo")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_output.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 23468672a767..89b178a78dc7 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -285,8 +285,9 @@ void xfrm_local_error(struct sk_buff *skb, int mtu)
 		return;
 
 	afinfo = xfrm_state_get_afinfo(proto);
-	if (afinfo)
+	if (afinfo) {
 		afinfo->local_error(skb, mtu);
-	rcu_read_unlock();
+		rcu_read_unlock();
+	}
 }
 EXPORT_SYMBOL_GPL(xfrm_local_error);
-- 
2.14.1

^ permalink raw reply related

* pull request (net): ipsec 2018-03-29
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Fix a rcu_read_lock/rcu_read_unlock imbalance
   in the error path of xfrm_local_error().
   From Taehee Yoo.

2) Some VTI MTU fixes. From Stefano Brivio.

3) Fix a too early overwritten skb control buffer
   on xfrm transport mode.

Please not that this pull request has a merge conflict
in net/ipv4/ip_tunnel.c.

The conflict is between

commit f6cc9c054e77 ("ip_tunnel: Emit events for post-register MTU changes")

from the net tree and

commit 24fc79798b8d ("ip_tunnel: Clamp MTU to bounds on new link")

from the ipsec tree.

It can be solved as it is currently done in linux-next.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit 51d4740f88affd85d49c04e3c9cd129c0e33bcb9:

  net sched actions: return explicit error when tunnel_key mode is not specified (2018-03-15 14:43:41 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 9a3fb9fb84cc30577c1b012a6a3efda944684291:

  xfrm: Fix transport mode skb control buffer usage. (2018-03-23 07:56:04 +0100)

----------------------------------------------------------------
Stefano Brivio (6):
      vti4: Don't count header length twice on tunnel setup
      ip_tunnel: Clamp MTU to bounds on new link
      vti4: Don't override MTU passed on link creation via IFLA_MTU
      vti6: Properly adjust vti6 MTU from MTU of lower device
      vti6: Keep set MTU on link creation or change, validate it
      vti6: Fix dev->max_mtu setting

Steffen Klassert (1):
      xfrm: Fix transport mode skb control buffer usage.

Taehee Yoo (1):
      xfrm: fix rcu_read_unlock usage in xfrm_local_error

 net/ipv4/ip_tunnel.c   |  8 +++++++-
 net/ipv4/ip_vti.c      |  2 --
 net/ipv6/ip6_vti.c     | 36 +++++++++++++++++++++++-------------
 net/xfrm/xfrm_input.c  |  6 ++++++
 net/xfrm/xfrm_output.c |  5 +++--
 5 files changed, 39 insertions(+), 18 deletions(-)

^ permalink raw reply

* [PATCH 5/8] vti6: Properly adjust vti6 MTU from MTU of lower device
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

If a lower device is found, we don't need to subtract
LL_MAX_HEADER to calculate our MTU: just use its MTU, the link
layer headers are already taken into account by it.

If the lower device is not found, start from ETH_DATA_LEN
instead, and only in this case subtract a worst-case
LL_MAX_HEADER.

We then need to subtract our additional IPv6 header from the
calculation.

While at it, note that vti6 doesn't have a hardware header, so
it doesn't need to set dev->hard_header_len. And as
vti6_link_config() now always sets the MTU, there's no need to
set a default value in vti6_dev_setup().

This makes the behaviour consistent with IPv4 vti, after
commit a32452366b72 ("vti4: Don't count header length twice."),
which was accidentally reverted by merge commit f895f0cfbb77
("Merge branch 'master' of
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec").

While commit 53c81e95df17 ("ip6_vti: adjust vti mtu according to
mtu of lower device") improved on the original situation, this
was still not ideal. As reported in that commit message itself,
if we start from an underlying veth MTU of 9000, we end up with
an MTU of 8832, that is, 9000 - LL_MAX_HEADER - sizeof(ipv6hdr).
This should simply be 8880, or 9000 - sizeof(ipv6hdr) instead:
we found the lower device (veth) and we know we don't have any
additional link layer header, so there's no need to subtract an
hypothetical worst-case number.

Fixes: 53c81e95df17 ("ip6_vti: adjust vti mtu according to mtu of lower device")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/ip6_vti.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index fa3ae1cb50d3..2ceef41cc097 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -627,6 +627,7 @@ static void vti6_link_config(struct ip6_tnl *t)
 	struct net_device *dev = t->dev;
 	struct __ip6_tnl_parm *p = &t->parms;
 	struct net_device *tdev = NULL;
+	int mtu;
 
 	memcpy(dev->dev_addr, &p->laddr, sizeof(struct in6_addr));
 	memcpy(dev->broadcast, &p->raddr, sizeof(struct in6_addr));
@@ -656,8 +657,11 @@ static void vti6_link_config(struct ip6_tnl *t)
 		tdev = __dev_get_by_index(t->net, p->link);
 
 	if (tdev)
-		dev->mtu = max_t(int, tdev->mtu - dev->hard_header_len,
-				 IPV6_MIN_MTU);
+		mtu = tdev->mtu - sizeof(struct ipv6hdr);
+	else
+		mtu = ETH_DATA_LEN - LL_MAX_HEADER - sizeof(struct ipv6hdr);
+
+	dev->mtu = max_t(int, mtu, IPV6_MIN_MTU);
 }
 
 /**
@@ -866,8 +870,6 @@ static void vti6_dev_setup(struct net_device *dev)
 	dev->priv_destructor = vti6_dev_free;
 
 	dev->type = ARPHRD_TUNNEL6;
-	dev->hard_header_len = LL_MAX_HEADER + sizeof(struct ipv6hdr);
-	dev->mtu = ETH_DATA_LEN;
 	dev->min_mtu = IPV6_MIN_MTU;
 	dev->max_mtu = IP_MAX_MTU;
 	dev->flags |= IFF_NOARP;
-- 
2.14.1

^ permalink raw reply related

* [PATCH 8/8] xfrm: Fix transport mode skb control buffer usage.
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

A recent commit introduced a new struct xfrm_trans_cb
that is used with the sk_buff control buffer. Unfortunately
it placed the structure in front of the control buffer and
overlooked that the IPv4/IPv6 control buffer is still needed
for some layer 4 protocols. As a result the IPv4/IPv6 control
buffer is overwritten with this structure. Fix this by setting
a apropriate header in front of the structure.

Fixes acf568ee859f ("xfrm: Reinject transport-mode packets ...")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_input.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 1472c0857975..81788105c164 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -26,6 +26,12 @@ struct xfrm_trans_tasklet {
 };
 
 struct xfrm_trans_cb {
+	union {
+		struct inet_skb_parm	h4;
+#if IS_ENABLED(CONFIG_IPV6)
+		struct inet6_skb_parm	h6;
+#endif
+	} header;
 	int (*finish)(struct net *net, struct sock *sk, struct sk_buff *skb);
 };
 
-- 
2.14.1

^ permalink raw reply related

* [PATCH 7/8] vti6: Fix dev->max_mtu setting
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

We shouldn't allow a tunnel to have IP_MAX_MTU as MTU, because
another IPv6 header is going on top of our packets. Without this
patch, we might end up building packets bigger than IP_MAX_MTU.

Fixes: b96f9afee4eb ("ipv4/6: use core net MTU range checking")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/ip6_vti.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 971175142e14..ce18cd20389d 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -879,7 +879,7 @@ static void vti6_dev_setup(struct net_device *dev)
 
 	dev->type = ARPHRD_TUNNEL6;
 	dev->min_mtu = IPV6_MIN_MTU;
-	dev->max_mtu = IP_MAX_MTU;
+	dev->max_mtu = IP_MAX_MTU - sizeof(struct ipv6hdr);
 	dev->flags |= IFF_NOARP;
 	dev->addr_len = sizeof(struct in6_addr);
 	netif_keep_dst(dev);
-- 
2.14.1

^ permalink raw reply related

* [PATCH 6/8] vti6: Keep set MTU on link creation or change, validate it
From: Steffen Klassert @ 2018-03-29  7:11 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329071120.27353-1-steffen.klassert@secunet.com>

From: Stefano Brivio <sbrivio@redhat.com>

In vti6_link_config(), if MTU is already given on link creation
or change, validate and use it instead of recomputing it. To do
that, we need to propagate the knowledge that MTU was set by
userspace all the way down to vti6_link_config().

To keep this simple, vti6_dev_init() sets the new 'keep_mtu'
argument of vti6_link_config() to true: on initialization, we
don't have convenient access to netlink attributes there, but we
will anyway check whether dev->mtu is set in vti6_link_config().
If it's non-zero, it was set to the value of the IFLA_MTU
attribute during creation. Otherwise, determine a reasonable
value.

Fixes: ed1efb2aefbb ("ipv6: Add support for IPsec virtual tunnel interfaces")
Fixes: 53c81e95df17 ("ip6_vti: adjust vti mtu according to mtu of lower device")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Acked-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv6/ip6_vti.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 2ceef41cc097..971175142e14 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -622,7 +622,7 @@ static int vti6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	return 0;
 }
 
-static void vti6_link_config(struct ip6_tnl *t)
+static void vti6_link_config(struct ip6_tnl *t, bool keep_mtu)
 {
 	struct net_device *dev = t->dev;
 	struct __ip6_tnl_parm *p = &t->parms;
@@ -641,6 +641,11 @@ static void vti6_link_config(struct ip6_tnl *t)
 	else
 		dev->flags &= ~IFF_POINTOPOINT;
 
+	if (keep_mtu && dev->mtu) {
+		dev->mtu = clamp(dev->mtu, dev->min_mtu, dev->max_mtu);
+		return;
+	}
+
 	if (p->flags & IP6_TNL_F_CAP_XMIT) {
 		int strict = (ipv6_addr_type(&p->raddr) &
 			      (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL));
@@ -668,12 +673,14 @@ static void vti6_link_config(struct ip6_tnl *t)
  * vti6_tnl_change - update the tunnel parameters
  *   @t: tunnel to be changed
  *   @p: tunnel configuration parameters
+ *   @keep_mtu: MTU was set from userspace, don't re-compute it
  *
  * Description:
  *   vti6_tnl_change() updates the tunnel parameters
  **/
 static int
-vti6_tnl_change(struct ip6_tnl *t, const struct __ip6_tnl_parm *p)
+vti6_tnl_change(struct ip6_tnl *t, const struct __ip6_tnl_parm *p,
+		bool keep_mtu)
 {
 	t->parms.laddr = p->laddr;
 	t->parms.raddr = p->raddr;
@@ -683,11 +690,12 @@ vti6_tnl_change(struct ip6_tnl *t, const struct __ip6_tnl_parm *p)
 	t->parms.proto = p->proto;
 	t->parms.fwmark = p->fwmark;
 	dst_cache_reset(&t->dst_cache);
-	vti6_link_config(t);
+	vti6_link_config(t, keep_mtu);
 	return 0;
 }
 
-static int vti6_update(struct ip6_tnl *t, struct __ip6_tnl_parm *p)
+static int vti6_update(struct ip6_tnl *t, struct __ip6_tnl_parm *p,
+		       bool keep_mtu)
 {
 	struct net *net = dev_net(t->dev);
 	struct vti6_net *ip6n = net_generic(net, vti6_net_id);
@@ -695,7 +703,7 @@ static int vti6_update(struct ip6_tnl *t, struct __ip6_tnl_parm *p)
 
 	vti6_tnl_unlink(ip6n, t);
 	synchronize_net();
-	err = vti6_tnl_change(t, p);
+	err = vti6_tnl_change(t, p, keep_mtu);
 	vti6_tnl_link(ip6n, t);
 	netdev_state_change(t->dev);
 	return err;
@@ -808,7 +816,7 @@ vti6_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 			} else
 				t = netdev_priv(dev);
 
-			err = vti6_update(t, &p1);
+			err = vti6_update(t, &p1, false);
 		}
 		if (t) {
 			err = 0;
@@ -907,7 +915,7 @@ static int vti6_dev_init(struct net_device *dev)
 
 	if (err)
 		return err;
-	vti6_link_config(t);
+	vti6_link_config(t, true);
 	return 0;
 }
 
@@ -1012,7 +1020,7 @@ static int vti6_changelink(struct net_device *dev, struct nlattr *tb[],
 	} else
 		t = netdev_priv(dev);
 
-	return vti6_update(t, &p);
+	return vti6_update(t, &p, tb && tb[IFLA_MTU]);
 }
 
 static size_t vti6_get_size(const struct net_device *dev)
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH v2] wcn36xx: Disable 5GHz for wcn3610
From: Kalle Valo @ 2018-03-29  7:15 UTC (permalink / raw)
  To: Ramon Fried
  Cc: loic.poulain, wcn36xx, linux-wireless, netdev, linux-kernel,
	devicetree
In-Reply-To: <20180329065818.9338-1-rfried@codeaurora.org>

(adding devicetree list)

Ramon Fried <rfried@codeaurora.org> writes:

> wcn3610 can only operate on 2.4GHz band due to RF limitation.
> If wcn36xx digital block is associated with an external IRIS
> RF module, retrieve the id and disable 5GHz band in case of
> wcn3610 id.
>
> Signed-off-by: Ramon Fried <rfried@codeaurora.org>
> ---
> v2: fixed wrong assignment, which is logically introduces the 
> 	same behaviour, but for correctness.
>
>  drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
>  drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index ab5be6d2c691..833531a68c95 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>  		BIT(NL80211_IFTYPE_MESH_POINT);
>  
>  	wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
> -	if (wcn->rf_id != RF_IRIS_WCN3620)
> +	if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
>  		wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
>  
>  	wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
> @@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
>  	if (iris_node) {
>  		if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
>  			wcn->rf_id = RF_IRIS_WCN3620;
> +		else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
> +			wcn->rf_id = RF_IRIS_WCN3610;
>  		of_node_put(iris_node);
>  	}

Should we document qcom,wcn3610 just like wcn3620 is:

Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt:                    "qcom,wcn3620",

-- 
Kalle Valo

^ permalink raw reply

* INFO: task hung in stop_sync_thread (2)
From: syzbot @ 2018-03-29  7:25 UTC (permalink / raw)
  To: coreteam, davem, fw, horms, ja, kadlec, linux-kernel, lvs-devel,
	netdev, netfilter-devel, pablo, syzkaller-bugs, wensong

Hello,

syzbot hit the following crash on net-next commit
5d22d47b9ed96eddb35821dc2cc4f629f45827f7 (Tue Mar 27 17:33:21 2018 +0000)
Merge branch 'sfc-filter-locking'
syzbot dashboard link:  
https://syzkaller.appspot.com/bug?extid=5fe074c01b2032ce9618

Unfortunately, I don't have any reproducer for this crash yet.
Raw console output:  
https://syzkaller.appspot.com/x/log.txt?id=6119456711376896
Kernel config:  
https://syzkaller.appspot.com/x/.config?id=4372867303600475372
compiler: gcc (GCC) 7.1.1 20170620

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5fe074c01b2032ce9618@syzkaller.appspotmail.com
It will help syzbot understand when the bug is fixed. See footer for  
details.
If you forward the report, please keep this part and the footer.

IPVS: sync thread started: state = BACKUP, mcast_ifn = syz_tun, syncid = 4,  
id = 0
IPVS: stopping backup sync thread 25415 ...
INFO: task syz-executor7:25421 blocked for more than 120 seconds.
       Not tainted 4.16.0-rc6+ #284
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor7   D23688 25421   4408 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2862 [inline]
  __schedule+0x8fb/0x1ec0 kernel/sched/core.c:3440
  schedule+0xf5/0x430 kernel/sched/core.c:3499
  schedule_timeout+0x1a3/0x230 kernel/time/timer.c:1777
  do_wait_for_common kernel/sched/completion.c:86 [inline]
  __wait_for_common kernel/sched/completion.c:107 [inline]
  wait_for_common kernel/sched/completion.c:118 [inline]
  wait_for_completion+0x415/0x770 kernel/sched/completion.c:139
  kthread_stop+0x14a/0x7a0 kernel/kthread.c:530
  stop_sync_thread+0x3d9/0x740 net/netfilter/ipvs/ip_vs_sync.c:1996
  do_ip_vs_set_ctl+0x2b1/0x1cc0 net/netfilter/ipvs/ip_vs_ctl.c:2394
  nf_sockopt net/netfilter/nf_sockopt.c:106 [inline]
  nf_setsockopt+0x67/0xc0 net/netfilter/nf_sockopt.c:115
  ip_setsockopt+0x97/0xa0 net/ipv4/ip_sockglue.c:1253
  sctp_setsockopt+0x2ca/0x63e0 net/sctp/socket.c:4154
  sock_common_setsockopt+0x95/0xd0 net/core/sock.c:3039
  SYSC_setsockopt net/socket.c:1850 [inline]
  SyS_setsockopt+0x189/0x360 net/socket.c:1829
  do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x454889
RSP: 002b:00007fc927626c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
RAX: ffffffffffffffda RBX: 00007fc9276276d4 RCX: 0000000000454889
RDX: 000000000000048c RSI: 0000000000000000 RDI: 0000000000000017
RBP: 000000000072bf58 R08: 0000000000000018 R09: 0000000000000000
R10: 0000000020000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 000000000000051c R14: 00000000006f9b40 R15: 0000000000000001

Showing all locks held in the system:
2 locks held by khungtaskd/868:
  #0:  (rcu_read_lock){....}, at: [<00000000a1a8f002>]  
check_hung_uninterruptible_tasks kernel/hung_task.c:175 [inline]
  #0:  (rcu_read_lock){....}, at: [<00000000a1a8f002>] watchdog+0x1c5/0xd60  
kernel/hung_task.c:249
  #1:  (tasklist_lock){.+.+}, at: [<0000000037c2f8f9>]  
debug_show_all_locks+0xd3/0x3d0 kernel/locking/lockdep.c:4470
1 lock held by rsyslogd/4247:
  #0:  (&f->f_pos_lock){+.+.}, at: [<000000000d8d6983>]  
__fdget_pos+0x12b/0x190 fs/file.c:765
2 locks held by getty/4338:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4339:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4340:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4341:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4342:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4343:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
2 locks held by getty/4344:
  #0:  (&tty->ldisc_sem){++++}, at: [<00000000bee98654>]  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
  #1:  (&ldata->atomic_read_lock){+.+.}, at: [<00000000c1d180aa>]  
n_tty_read+0x2ef/0x1a40 drivers/tty/n_tty.c:2131
3 locks held by kworker/0:5/6494:
  #0:  ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
[<00000000a062b18e>] work_static include/linux/workqueue.h:198 [inline]
  #0:  ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
[<00000000a062b18e>] set_work_data kernel/workqueue.c:619 [inline]
  #0:  ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
[<00000000a062b18e>] set_work_pool_and_clear_pending kernel/workqueue.c:646  
[inline]
  #0:  ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
[<00000000a062b18e>] process_one_work+0xb12/0x1bb0 kernel/workqueue.c:2084
  #1:  ((addr_chk_work).work){+.+.}, at: [<00000000278427d5>]  
process_one_work+0xb89/0x1bb0 kernel/workqueue.c:2088
  #2:  (rtnl_mutex){+.+.}, at: [<00000000066e35ac>] rtnl_lock+0x17/0x20  
net/core/rtnetlink.c:74
1 lock held by syz-executor7/25421:
  #0:  (ipvs->sync_mutex){+.+.}, at: [<00000000d414a689>]  
do_ip_vs_set_ctl+0x277/0x1cc0 net/netfilter/ipvs/ip_vs_ctl.c:2393
2 locks held by syz-executor7/25427:
  #0:  (rtnl_mutex){+.+.}, at: [<00000000066e35ac>] rtnl_lock+0x17/0x20  
net/core/rtnetlink.c:74
  #1:  (ipvs->sync_mutex){+.+.}, at: [<00000000e6d48489>]  
do_ip_vs_set_ctl+0x10f8/0x1cc0 net/netfilter/ipvs/ip_vs_ctl.c:2388
1 lock held by syz-executor7/25435:
  #0:  (rtnl_mutex){+.+.}, at: [<00000000066e35ac>] rtnl_lock+0x17/0x20  
net/core/rtnetlink.c:74
1 lock held by ipvs-b:2:0/25415:
  #0:  (rtnl_mutex){+.+.}, at: [<00000000066e35ac>] rtnl_lock+0x17/0x20  
net/core/rtnetlink.c:74

=============================================

NMI backtrace for cpu 1
CPU: 1 PID: 868 Comm: khungtaskd Not tainted 4.16.0-rc6+ #284
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x194/0x24d lib/dump_stack.c:53
  nmi_cpu_backtrace+0x1d2/0x210 lib/nmi_backtrace.c:103
  nmi_trigger_cpumask_backtrace+0x123/0x180 lib/nmi_backtrace.c:62
  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
  trigger_all_cpu_backtrace include/linux/nmi.h:138 [inline]
  check_hung_task kernel/hung_task.c:132 [inline]
  check_hung_uninterruptible_tasks kernel/hung_task.c:190 [inline]
  watchdog+0x90c/0xd60 kernel/hung_task.c:249
  kthread+0x33c/0x400 kernel/kthread.c:238
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:406
Sending NMI from CPU 1 to CPUs 0:
NMI backtrace for cpu 0 skipped: idling at native_safe_halt+0x6/0x10  
arch/x86/include/asm/irqflags.h:54


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzkaller@googlegroups.com.

syzbot will keep track of this bug report.
If you forgot to add the Reported-by tag, once the fix for this bug is  
merged
into any tree, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug  
report.
Note: all commands must start from beginning of the line in the email body.

^ permalink raw reply

* [PATCH 1/4] esp4: remove redundant initialization of pointer esph
From: Steffen Klassert @ 2018-03-29  7:25 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329072549.2880-1-steffen.klassert@secunet.com>

From: Colin Ian King <colin.king@canonical.com>

Pointer esph is being assigned a value that is never read, esph is
re-assigned and only read inside an if statement, hence the
initialization is redundant and can be removed.

Cleans up clang warning:
net/ipv4/esp4.c:657:21: warning: Value stored to 'esph' during
its initialization is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 296d0b956bfe..97689012b357 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -654,7 +654,7 @@ static void esp_input_restore_header(struct sk_buff *skb)
 static void esp_input_set_header(struct sk_buff *skb, __be32 *seqhi)
 {
 	struct xfrm_state *x = xfrm_input_state(skb);
-	struct ip_esp_hdr *esph = (struct ip_esp_hdr *)skb->data;
+	struct ip_esp_hdr *esph;
 
 	/* For ESN we move the header forward by 4 bytes to
 	 * accomodate the high bits.  We will move it back after
-- 
2.14.1

^ permalink raw reply related

* [PATCH 4/4] xfrm_policy: use true and false for boolean values
From: Steffen Klassert @ 2018-03-29  7:25 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329072549.2880-1-steffen.klassert@secunet.com>

From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>

Assign true or false to boolean variables instead of an integer value.

This issue was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 12bd415d349e..2b7babb66175 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1740,7 +1740,7 @@ static void xfrm_pcpu_work_fn(struct work_struct *work)
 void xfrm_policy_cache_flush(void)
 {
 	struct xfrm_dst *old;
-	bool found = 0;
+	bool found = false;
 	int cpu;
 
 	might_sleep();
-- 
2.14.1

^ permalink raw reply related

* [PATCH 2/4] xfrm: mark kmem_caches as __ro_after_init
From: Steffen Klassert @ 2018-03-29  7:25 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329072549.2880-1-steffen.klassert@secunet.com>

From: Alexey Dobriyan <adobriyan@gmail.com>

Kmem caches aren't relocated once set up.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/xfrm/xfrm_input.c  | 3 ++-
 net/xfrm/xfrm_policy.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 1472c0857975..44fc54dc013c 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/bottom_half.h>
+#include <linux/cache.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -31,7 +32,7 @@ struct xfrm_trans_cb {
 
 #define XFRM_TRANS_SKB_CB(__skb) ((struct xfrm_trans_cb *)&((__skb)->cb[0]))
 
-static struct kmem_cache *secpath_cachep __read_mostly;
+static struct kmem_cache *secpath_cachep __ro_after_init;
 
 static DEFINE_SPINLOCK(xfrm_input_afinfo_lock);
 static struct xfrm_input_afinfo const __rcu *xfrm_input_afinfo[AF_INET6 + 1];
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 7a23078132cf..12bd415d349e 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -51,7 +51,7 @@ static DEFINE_SPINLOCK(xfrm_policy_afinfo_lock);
 static struct xfrm_policy_afinfo const __rcu *xfrm_policy_afinfo[AF_INET6 + 1]
 						__read_mostly;
 
-static struct kmem_cache *xfrm_dst_cache __read_mostly;
+static struct kmem_cache *xfrm_dst_cache __ro_after_init;
 static __read_mostly seqcount_t xfrm_policy_hash_generation;
 
 static void xfrm_init_pmtu(struct xfrm_dst **bundle, int nr);
-- 
2.14.1

^ permalink raw reply related

* pull request (net-next): ipsec-next 2018-03-29
From: Steffen Klassert @ 2018-03-29  7:25 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev

1) Remove a redundant pointer initialization esp_input_set_header().
   From Colin Ian King.

2) Mark the xfrm kmem_caches as __ro_after_init.
   From Alexey Dobriyan.

3) Do the checksum for an ipsec offlad packet in software
   if the device does not advertise NETIF_F_HW_ESP_TX_CSUM.
   From Shannon Nelson.

4) Use booleans for true and false instead of integers
   in xfrm_policy_cache_flush().
   From Gustavo A. R. Silva

Please pull or let me know if there are problems.

Thanks!

The following changes since commit cf19e5e2054f5172c07a152f9e04eb3bae3d86dd:

  Merge branch '40GbE' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue (2018-02-12 19:55:33 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master

for you to fetch changes up to 415a13296a1a49639cabf9d2fe92267810caa47a:

  xfrm_policy: use true and false for boolean values (2018-03-07 10:55:13 +0100)

----------------------------------------------------------------
Alexey Dobriyan (1):
      xfrm: mark kmem_caches as __ro_after_init

Colin Ian King (1):
      esp4: remove redundant initialization of pointer esph

Gustavo A. R. Silva (1):
      xfrm_policy: use true and false for boolean values

Shannon Nelson (1):
      esp: check the NETIF_F_HW_ESP_TX_CSUM bit before segmenting

 net/ipv4/esp4.c         | 2 +-
 net/ipv4/esp4_offload.c | 2 ++
 net/ipv6/esp6_offload.c | 2 ++
 net/xfrm/xfrm_input.c   | 3 ++-
 net/xfrm/xfrm_policy.c  | 4 ++--
 5 files changed, 9 insertions(+), 4 deletions(-)

^ permalink raw reply

* [PATCH 3/4] esp: check the NETIF_F_HW_ESP_TX_CSUM bit before segmenting
From: Steffen Klassert @ 2018-03-29  7:25 UTC (permalink / raw)
  To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <20180329072549.2880-1-steffen.klassert@secunet.com>

From: Shannon Nelson <shannon.nelson@oracle.com>

If I understand correctly, we should not be asking for a
checksum offload on an ipsec packet if the netdev isn't
advertising NETIF_F_HW_ESP_TX_CSUM.  In that case, we should
clear the NETIF_F_CSUM_MASK bits.

Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/esp4_offload.c | 2 ++
 net/ipv6/esp6_offload.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index da5635fc52c2..7cf755ef9efb 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -138,6 +138,8 @@ static struct sk_buff *esp4_gso_segment(struct sk_buff *skb,
 	if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
 	    (x->xso.dev != skb->dev))
 		esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
+	else if (!(features & NETIF_F_HW_ESP_TX_CSUM))
+		esp_features = features & ~NETIF_F_CSUM_MASK;
 
 	xo->flags |= XFRM_GSO_SEGMENT;
 
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 3fd1ec775dc2..27f59b61f70f 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -165,6 +165,8 @@ static struct sk_buff *esp6_gso_segment(struct sk_buff *skb,
 	if (!(features & NETIF_F_HW_ESP) || !x->xso.offload_handle ||
 	    (x->xso.dev != skb->dev))
 		esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
+	else if (!(features & NETIF_F_HW_ESP_TX_CSUM))
+		esp_features = features & ~NETIF_F_CSUM_MASK;
 
 	xo->flags |= XFRM_GSO_SEGMENT;
 
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH v2] wcn36xx: Disable 5GHz for wcn3610
From: Ramon Fried @ 2018-03-29  7:32 UTC (permalink / raw)
  To: Kalle Valo
  Cc: loic.poulain, wcn36xx, linux-wireless, netdev, linux-kernel,
	devicetree
In-Reply-To: <874lkzmjxu.fsf@kamboji.qca.qualcomm.com>

(adding Bjorn Andersson)


On 3/29/2018 10:15 AM, Kalle Valo wrote:
> (adding devicetree list)
>
> Ramon Fried <rfried@codeaurora.org> writes:
>
>> wcn3610 can only operate on 2.4GHz band due to RF limitation.
>> If wcn36xx digital block is associated with an external IRIS
>> RF module, retrieve the id and disable 5GHz band in case of
>> wcn3610 id.
>>
>> Signed-off-by: Ramon Fried <rfried@codeaurora.org>
>> ---
>> v2: fixed wrong assignment, which is logically introduces the 
>> 	same behaviour, but for correctness.
>>
>>  drivers/net/wireless/ath/wcn36xx/main.c    | 4 +++-
>>  drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 1 +
>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index ab5be6d2c691..833531a68c95 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -1146,7 +1146,7 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>>  		BIT(NL80211_IFTYPE_MESH_POINT);
>>  
>>  	wcn->hw->wiphy->bands[NL80211_BAND_2GHZ] = &wcn_band_2ghz;
>> -	if (wcn->rf_id != RF_IRIS_WCN3620)
>> +	if (wcn->rf_id != RF_IRIS_WCN3610 && wcn->rf_id != RF_IRIS_WCN3620)
>>  		wcn->hw->wiphy->bands[NL80211_BAND_5GHZ] = &wcn_band_5ghz;
>>  
>>  	wcn->hw->wiphy->max_scan_ssids = WCN36XX_MAX_SCAN_SSIDS;
>> @@ -1248,6 +1248,8 @@ static int wcn36xx_platform_get_resources(struct wcn36xx *wcn,
>>  	if (iris_node) {
>>  		if (of_device_is_compatible(iris_node, "qcom,wcn3620"))
>>  			wcn->rf_id = RF_IRIS_WCN3620;
>> +		else if (of_device_is_compatible(iris_node, "qcom,wcn3610"))
>> +			wcn->rf_id = RF_IRIS_WCN3610;
>>  		of_node_put(iris_node);
>>  	}
> Should we document qcom,wcn3610 just like wcn3620 is:
>
> Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt:                    "qcom,wcn3620",
IMHO the mentioned bindings is related to the PIL (peripheral image loaded) which is just the firmware part and has
nothing to do with wifi frontend(IRIS).

^ permalink raw reply

* Re: [PATCH v2] wcn36xx: Disable 5GHz for wcn3610
From: Loic Poulain @ 2018-03-29  7:40 UTC (permalink / raw)
  To: Ramon Fried
  Cc: Kalle Valo, wcn36xx, linux-wireless, netdev, linux-kernel,
	devicetree
In-Reply-To: <2d978343-2057-8424-a249-83a0394da5a7@codeaurora.org>

Hi Ramon,

On 29 March 2018 at 09:32, Ramon Fried <rfried@codeaurora.org> wrote:

>> Should we document qcom,wcn3610 just like wcn3620 is:
>>
>> Documentation/devicetree/bindings/remoteproc/qcom,wcnss-pil.txt:                    "qcom,wcn3620",
> IMHO the mentioned bindings is related to the PIL (peripheral image loaded) which is just the firmware part and has
> nothing to do with wifi frontend(IRIS).
>

It seems that the RF is a subnode of the PIL node:

"= SUBNODES
A required subnode of the WCNSS PIL is used to describe the attached rf module
and its resource dependencies. It is described by the following properties:"

So we should add qcom,wcn3610 to the exiting list of 'compatible' rf modules.

Regards,
Loic

^ permalink raw reply

* Re: [PATCH net-next v2 3/3] net: bcmgenet: Fix coalescing settings handling
From: Tal Gilboa @ 2018-03-29  7:42 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
	saeedm
In-Reply-To: <20180328221538.29290-4-f.fainelli@gmail.com>



On 3/29/2018 1:15 AM, Florian Fainelli wrote:
> There were a number of issues with setting the RX coalescing parameters:
> 
> - we would not be preserving values that would have been configured
>    across close/open calls, instead we would always reset to no timeout
>    and 1 interrupt per packet, this would also prevent DIM from setting its
>    default usec/pkts values
> 
> - when adaptive RX would be turned on, we woud not be fetching the
>    default parameters, we would stay with no timeout/1 packet per interrupt
>    until the estimator kicks in and changes that
> 
> - finally disabling adaptive RX coalescing while providing parameters
>    would not be honored, and we would stay with whatever DIM had previously
>    determined instead of the user requested parameters
> 
> Fixes: 9f4ca05827a2 ("net: bcmgenet: Add support for adaptive RX coalescing")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>   drivers/net/ethernet/broadcom/genet/bcmgenet.c | 92 +++++++++++++++++---------
>   drivers/net/ethernet/broadcom/genet/bcmgenet.h |  4 +-
>   2 files changed, 61 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 7db8edc643ec..f8af472f324f 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -625,26 +625,46 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
>   	return 0;
>   }
>   
> -static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring)
> +static void bcmgenet_set_rx_coalesce(struct bcmgenet_rx_ring *ring,
> +				     u32 usecs, u32 pkts)
>   {
>   	struct bcmgenet_priv *priv = ring->priv;
>   	unsigned int i = ring->index;
>   	u32 reg;
>   
> -	bcmgenet_rdma_ring_writel(priv, i, ring->dim.coal_pkts,
> -				  DMA_MBUF_DONE_THRESH);
> +	bcmgenet_rdma_ring_writel(priv, i, pkts, DMA_MBUF_DONE_THRESH);
>   
>   	reg = bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT + i);
>   	reg &= ~DMA_TIMEOUT_MASK;
> -	reg |= DIV_ROUND_UP(ring->dim.coal_usecs * 1000, 8192);
> +	reg |= DIV_ROUND_UP(usecs * 1000, 8192);
>   	bcmgenet_rdma_writel(priv, reg, DMA_RING0_TIMEOUT + i);
>   }
>   
> +static void bcmgenet_set_ring_rx_coalesce(struct bcmgenet_rx_ring *ring,
> +					  struct ethtool_coalesce *ec)
> +{
> +	struct net_dim_cq_moder moder;
> +	u32 usecs, pkts;
> +
> +	ring->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> +	ring->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> +	usecs = ring->rx_coalesce_usecs;
> +	pkts = ring->rx_max_coalesced_frames;
> +
> +	if (ec->use_adaptive_rx_coalesce && !ring->dim.use_dim) {
> +		moder = net_dim_get_def_profile(ring->dim.dim.mode);
> +		usecs = moder.usec;
> +		pkts = moder.pkts;
> +	}
> +
> +	ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> +	bcmgenet_set_rx_coalesce(ring, usecs, pkts);
> +}
> +
>   static int bcmgenet_set_coalesce(struct net_device *dev,
>   				 struct ethtool_coalesce *ec)
>   {
>   	struct bcmgenet_priv *priv = netdev_priv(dev);
> -	struct bcmgenet_rx_ring *ring;
>   	unsigned int i;
>   
>   	/* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -680,27 +700,9 @@ static int bcmgenet_set_coalesce(struct net_device *dev,
>   				  ec->tx_max_coalesced_frames,
>   				  DMA_MBUF_DONE_THRESH);
>   
> -	for (i = 0; i < priv->hw_params->rx_queues; i++) {
> -		ring = &priv->rx_rings[i];
> -		ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> -		ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> -		if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> -			ring->dim.coal_pkts = 1;
> -			ring->dim.coal_usecs = 0;
> -		}
> -		ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> -		bcmgenet_set_rx_coalesce(ring);
> -	}
> -
> -	ring = &priv->rx_rings[DESC_INDEX];
> -	ring->dim.coal_usecs = ec->rx_coalesce_usecs;
> -	ring->dim.coal_pkts = ec->rx_max_coalesced_frames;
> -	if (!ec->use_adaptive_rx_coalesce && ring->dim.use_dim) {
> -		ring->dim.coal_pkts = 1;
> -		ring->dim.coal_usecs = 0;
> -	}
> -	ring->dim.use_dim = ec->use_adaptive_rx_coalesce;
> -	bcmgenet_set_rx_coalesce(ring);
> +	for (i = 0; i < priv->hw_params->rx_queues; i++)
> +		bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[i], ec);
> +	bcmgenet_set_ring_rx_coalesce(&priv->rx_rings[DESC_INDEX], ec);
>   
>   	return 0;
>   }
> @@ -1924,10 +1926,7 @@ static void bcmgenet_dim_work(struct work_struct *work)
>   	struct net_dim_cq_moder cur_profile =
>   			net_dim_get_profile(dim->mode, dim->profile_ix);
>   
> -	ring->dim.coal_usecs = cur_profile.usec;
> -	ring->dim.coal_pkts = cur_profile.pkts;
> -
> -	bcmgenet_set_rx_coalesce(ring);
> +	bcmgenet_set_rx_coalesce(ring, cur_profile.usec, cur_profile.pkts);
>   	dim->state = NET_DIM_START_MEASURE;
>   }
>   
> @@ -2079,9 +2078,11 @@ static void init_umac(struct bcmgenet_priv *priv)
>   	dev_dbg(kdev, "done init umac\n");
>   }
>   
> -static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
> +static void bcmgenet_init_dim(struct bcmgenet_rx_ring *ring,
>   			      void (*cb)(struct work_struct *work))
>   {
> +	struct bcmgenet_net_dim *dim = &ring->dim;
> +
>   	INIT_WORK(&dim->dim.work, cb);
>   	dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>   	dim->event_ctr = 0;
> @@ -2089,6 +2090,25 @@ static void bcmgenet_init_dim(struct bcmgenet_net_dim *dim,
>   	dim->bytes = 0;
>   }
>   
> +static void bcmgenet_init_rx_coalesce(struct bcmgenet_rx_ring *ring)
> +{
> +	struct bcmgenet_net_dim *dim = &ring->dim;
> +	struct net_dim_cq_moder moder;
> +	u32 usecs, pkts;
> +
> +	usecs = ring->rx_coalesce_usecs;
> +	pkts = ring->rx_max_coalesced_frames;
> +
> +	/* If DIM was enabled, re-apply default parameters */
> +	if (dim->use_dim) {
> +		moder = net_dim_get_def_profile(dim->dim.mode);
> +		usecs = moder.usec;
> +		pkts = moder.pkts;
> +	}
> +
> +	bcmgenet_set_rx_coalesce(ring, usecs, pkts);
> +}
> +
>   /* Initialize a Tx ring along with corresponding hardware registers */
>   static void bcmgenet_init_tx_ring(struct bcmgenet_priv *priv,
>   				  unsigned int index, unsigned int size,
> @@ -2178,7 +2198,8 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
>   	if (ret)
>   		return ret;
>   
> -	bcmgenet_init_dim(&ring->dim, bcmgenet_dim_work);
> +	bcmgenet_init_dim(ring, bcmgenet_dim_work);
> +	bcmgenet_init_rx_coalesce(ring);
>   
>   	/* Initialize Rx NAPI */
>   	netif_napi_add(priv->dev, &ring->napi, bcmgenet_rx_poll,
> @@ -2186,7 +2207,6 @@ static int bcmgenet_init_rx_ring(struct bcmgenet_priv *priv,
>   
>   	bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_PROD_INDEX);
>   	bcmgenet_rdma_ring_writel(priv, index, 0, RDMA_CONS_INDEX);
> -	bcmgenet_rdma_ring_writel(priv, index, 1, DMA_MBUF_DONE_THRESH);
>   	bcmgenet_rdma_ring_writel(priv, index,
>   				  ((size << DMA_RING_SIZE_SHIFT) |
>   				   RX_BUF_LENGTH), DMA_RING_BUF_SIZE);
> @@ -3424,6 +3444,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
>   	struct net_device *dev;
>   	const void *macaddr;
>   	struct resource *r;
> +	unsigned int i;
>   	int err = -EIO;
>   	const char *phy_mode_str;
>   
> @@ -3552,6 +3573,11 @@ static int bcmgenet_probe(struct platform_device *pdev)
>   	netif_set_real_num_tx_queues(priv->dev, priv->hw_params->tx_queues + 1);
>   	netif_set_real_num_rx_queues(priv->dev, priv->hw_params->rx_queues + 1);
>   
> +	/* Set default coalescing parameters */
> +	for (i = 0; i < priv->hw_params->rx_queues; i++)
> +		priv->rx_rings[i].rx_max_coalesced_frames = 1;
> +	priv->rx_rings[DESC_INDEX].rx_max_coalesced_frames = 1;
> +

Do you still want to keep {usecs,frames}={0,1} as default static 
configuration? Is the latency so important for the static case? I'm 
assuming RT latency is at least 10us so you can somewhat increase the 
timer and counter without causing an increase in latency. This should be 
properly tested of course so maybe in a future patch.

>   	/* libphy will determine the link state */
>   	netif_carrier_off(dev);
>   
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 22c41e0430fb..b773bc07edf7 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -578,8 +578,6 @@ struct bcmgenet_net_dim {
>   	u16		event_ctr;
>   	unsigned long	packets;
>   	unsigned long	bytes;
> -	u32		coal_usecs;
> -	u32		coal_pkts;
>   	struct net_dim	dim;
>   };
>   
> @@ -598,6 +596,8 @@ struct bcmgenet_rx_ring {
>   	unsigned int	end_ptr;	/* Rx ring end CB ptr */
>   	unsigned int	old_discards;
>   	struct bcmgenet_net_dim dim;
> +	u32		rx_max_coalesced_frames;
> +	u32		rx_coalesce_usecs;
>   	void (*int_enable)(struct bcmgenet_rx_ring *);
>   	void (*int_disable)(struct bcmgenet_rx_ring *);
>   	struct bcmgenet_priv *priv;
> 
Reviewed-by: Tal Gilboa <talgi@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: systemport: Fix coalescing settings handling
From: Tal Gilboa @ 2018-03-29  7:42 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: davem, jaedon.shin, pgynther, opendmb, Michael Chan, gospo,
	saeedm
In-Reply-To: <20180328221538.29290-3-f.fainelli@gmail.com>

On 3/29/2018 1:15 AM, Florian Fainelli wrote:
> There were a number of issues with setting the RX coalescing parameters:
> 
> - we would not be preserving values that would have been configured
>    across close/open calls, instead we would always reset to no timeout
>    and 1 interrupt per packet, this would also prevent DIM from setting its
>    default usec/pkts values
> 
> - when adaptive RX would be turned on, we woud not be fetching the
>    default parameters, we would stay with no timeout/1 packet per
>    interrupt until the estimator kicks in and changes that
> 
> - finally disabling adaptive RX coalescing while providing parameters
>    would not be honored, and we would stay with whatever DIM had
>    previously determined instead of the user requested parameters
> 
> Fixes: b6e0e875421e ("net: systemport: Implement adaptive interrupt coalescing")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>   drivers/net/ethernet/broadcom/bcmsysport.c | 62 +++++++++++++++++++++---------
>   drivers/net/ethernet/broadcom/bcmsysport.h |  4 +-
>   2 files changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
> index 1e52bb7d822e..4a75b1de22e0 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -574,16 +574,16 @@ static int bcm_sysport_set_wol(struct net_device *dev,
>   	return 0;
>   }
>   
> -static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv)
> +static void bcm_sysport_set_rx_coalesce(struct bcm_sysport_priv *priv,
> +					u32 usecs, u32 pkts)
>   {
>   	u32 reg;
>   
>   	reg = rdma_readl(priv, RDMA_MBDONE_INTR);
>   	reg &= ~(RDMA_INTR_THRESH_MASK |
>   		 RDMA_TIMEOUT_MASK << RDMA_TIMEOUT_SHIFT);
> -	reg |= priv->dim.coal_pkts;
> -	reg |= DIV_ROUND_UP(priv->dim.coal_usecs * 1000, 8192) <<
> -			    RDMA_TIMEOUT_SHIFT;
> +	reg |= pkts;
> +	reg |= DIV_ROUND_UP(usecs * 1000, 8192) << RDMA_TIMEOUT_SHIFT;
>   	rdma_writel(priv, reg, RDMA_MBDONE_INTR);
>   }
>   
> @@ -626,6 +626,8 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
>   				    struct ethtool_coalesce *ec)
>   {
>   	struct bcm_sysport_priv *priv = netdev_priv(dev);
> +	struct net_dim_cq_moder moder;
> +	u32 usecs, pkts;
>   	unsigned int i;
>   
>   	/* Base system clock is 125Mhz, DMA timeout is this reference clock
> @@ -646,15 +648,21 @@ static int bcm_sysport_set_coalesce(struct net_device *dev,
>   	for (i = 0; i < dev->num_tx_queues; i++)
>   		bcm_sysport_set_tx_coalesce(&priv->tx_rings[i], ec);
>   
> -	priv->dim.coal_usecs = ec->rx_coalesce_usecs;
> -	priv->dim.coal_pkts = ec->rx_max_coalesced_frames;
> +	priv->rx_coalesce_usecs = ec->rx_coalesce_usecs;
> +	priv->rx_max_coalesced_frames = ec->rx_max_coalesced_frames;
> +	usecs = priv->rx_coalesce_usecs;
> +	pkts = priv->rx_max_coalesced_frames;
>   
> -	if (!ec->use_adaptive_rx_coalesce && priv->dim.use_dim) {
> -		priv->dim.coal_pkts = 1;
> -		priv->dim.coal_usecs = 0;
> +	if (ec->use_adaptive_rx_coalesce && !priv->dim.use_dim) {
> +		moder = net_dim_get_def_profile(priv->dim.dim.mode);
> +		usecs = moder.usec;
> +		pkts = moder.pkts;
>   	}
> +
>   	priv->dim.use_dim = ec->use_adaptive_rx_coalesce;
> -	bcm_sysport_set_rx_coalesce(priv);
> +
> +	/* Apply desired coalescing parameters */
> +	bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
>   
>   	return 0;
>   }
> @@ -1058,10 +1066,7 @@ static void bcm_sysport_dim_work(struct work_struct *work)
>   	struct net_dim_cq_moder cur_profile =
>   				net_dim_get_profile(dim->mode, dim->profile_ix);
>   
> -	priv->dim.coal_usecs = cur_profile.usec;
> -	priv->dim.coal_pkts = cur_profile.pkts;
> -
> -	bcm_sysport_set_rx_coalesce(priv);
> +	bcm_sysport_set_rx_coalesce(priv, cur_profile.usec, cur_profile.pkts);
>   	dim->state = NET_DIM_START_MEASURE;
>   }
>   
> @@ -1408,9 +1413,11 @@ static void bcm_sysport_adj_link(struct net_device *dev)
>   		phy_print_status(phydev);
>   }
>   
> -static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
> +static void bcm_sysport_init_dim(struct bcm_sysport_priv *priv,
>   				 void (*cb)(struct work_struct *work))
>   {
> +	struct bcm_sysport_net_dim *dim = &priv->dim;
> +
>   	INIT_WORK(&dim->dim.work, cb);
>   	dim->dim.mode = NET_DIM_CQ_PERIOD_MODE_START_FROM_EQE;
>   	dim->event_ctr = 0;
> @@ -1418,6 +1425,25 @@ static void bcm_sysport_init_dim(struct bcm_sysport_net_dim *dim,
>   	dim->bytes = 0;
>   }
>   
> +static void bcm_sysport_init_rx_coalesce(struct bcm_sysport_priv *priv)
> +{
> +	struct bcm_sysport_net_dim *dim = &priv->dim;
> +	struct net_dim_cq_moder moder;
> +	u32 usecs, pkts;
> +
> +	usecs = priv->rx_coalesce_usecs;
> +	pkts = priv->rx_max_coalesced_frames;
> +
> +	/* If DIM was enabled, re-apply default parameters */
> +	if (dim->use_dim) {
> +		moder = net_dim_get_def_profile(dim->dim.mode);
> +		usecs = moder.usec;
> +		pkts = moder.pkts;
> +	}
> +
> +	bcm_sysport_set_rx_coalesce(priv, usecs, pkts);
> +}
> +
>   static int bcm_sysport_init_tx_ring(struct bcm_sysport_priv *priv,
>   				    unsigned int index)
>   {
> @@ -1658,8 +1684,6 @@ static int bcm_sysport_init_rx_ring(struct bcm_sysport_priv *priv)
>   	rdma_writel(priv, 0, RDMA_END_ADDR_HI);
>   	rdma_writel(priv, priv->num_rx_desc_words - 1, RDMA_END_ADDR_LO);
>   
> -	rdma_writel(priv, 1, RDMA_MBDONE_INTR);
> -
>   	netif_dbg(priv, hw, priv->netdev,
>   		  "RDMA cfg, num_rx_bds=%d, rx_bds=%p\n",
>   		  priv->num_rx_bds, priv->rx_bds);
> @@ -1827,7 +1851,8 @@ static void bcm_sysport_netif_start(struct net_device *dev)
>   	struct bcm_sysport_priv *priv = netdev_priv(dev);
>   
>   	/* Enable NAPI */
> -	bcm_sysport_init_dim(&priv->dim, bcm_sysport_dim_work);
> +	bcm_sysport_init_dim(priv, bcm_sysport_dim_work);
> +	bcm_sysport_init_rx_coalesce(priv);
>   	napi_enable(&priv->napi);
>   
>   	/* Enable RX interrupt and TX ring full interrupt */
> @@ -2333,6 +2358,7 @@ static int bcm_sysport_probe(struct platform_device *pdev)
>   	/* libphy will adjust the link state accordingly */
>   	netif_carrier_off(dev);
>   
> +	priv->rx_max_coalesced_frames = 1;

Do you still want to keep {usecs,frames}={0,1} as default static 
configuration? Is the latency so important for the static case? I'm 
assuming RT latency is at least 10us so you can somewhat increase the 
timer and counter without causing an increase in latency. This should be 
properly tested of course so maybe in a future patch.

>   	u64_stats_init(&priv->syncp);
>   
>   	priv->dsa_notifier.notifier_call = bcm_sysport_dsa_notifier;
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
> index 57e18ef8f206..d6e5d0cbf3a3 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.h
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.h
> @@ -701,8 +701,6 @@ struct bcm_sysport_net_dim {
>   	u16			event_ctr;
>   	unsigned long		packets;
>   	unsigned long		bytes;
> -	u32			coal_usecs;
> -	u32			coal_pkts;
>   	struct net_dim		dim;
>   };
>   
> @@ -755,6 +753,8 @@ struct bcm_sysport_priv {
>   	unsigned int		rx_c_index;
>   
>   	struct bcm_sysport_net_dim	dim;
> +	u32			rx_max_coalesced_frames;
> +	u32			rx_coalesce_usecs;
>   
>   	/* PHY device */
>   	struct device_node	*phy_dn;
> 
Reviewed-by: Tal Gilboa <talgi@mellanox.com>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox