Netdev List
 help / color / mirror / Atom feed
* Re: IPv6-UDP 0x0000 checksum
From: Eric Dumazet @ 2017-01-26 15:27 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless
In-Reply-To: <1485444276.5145.133.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>

On Thu, 2017-01-26 at 07:24 -0800, Eric Dumazet wrote:
> On Thu, 2017-01-26 at 15:49 +0100, Johannes Berg wrote:
> 
> > Unfortunately, I haven't been able to actually test this yet. I also
> > didn't find the code that would drop frames with CSUM 0 either, so I'm
> > thinking - for now - that if all the csum handling is skipped, dropping
> > 0 csum frames would also be, and then we'd accept a frame we should
> > actually have dropped.
> > 
> > I'll go test this I guess :)
> > 
> > Any pointers to where 0 csum frames are dropped?
> 
> Probably in udp6_csum_init()

vi +804 net/ipv6/udp.c

                if (!uh->check && !udp_sk(sk)->no_check6_rx) {
                        udp6_csum_zero_error(skb);
                        goto csum_error;
                }

^ permalink raw reply

* Re: IPv6-UDP 0x0000 checksum
From: Eric Dumazet @ 2017-01-26 15:24 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless
In-Reply-To: <1485442164.14760.11.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

On Thu, 2017-01-26 at 15:49 +0100, Johannes Berg wrote:

> Unfortunately, I haven't been able to actually test this yet. I also
> didn't find the code that would drop frames with CSUM 0 either, so I'm
> thinking - for now - that if all the csum handling is skipped, dropping
> 0 csum frames would also be, and then we'd accept a frame we should
> actually have dropped.
> 
> I'll go test this I guess :)
> 
> Any pointers to where 0 csum frames are dropped?

Probably in udp6_csum_init()

^ permalink raw reply

* [PATCH net-next v2 0/2] gtp: simple gtp improvements
From: Andreas Schultz @ 2017-01-26 15:21 UTC (permalink / raw)
  To: Pablo Neira
  Cc: netdev, Lionel Gauthier, openbsc, Harald Welte, SF Markus Elfring

This is the part of the previous "simple gtp improvements" series
that Pablo indicated should go into net-next.

The rcu_lock removal is small correctness changes. Passing invalid
to user space allows for more standards compliant handling of invalid
tunnels.

^ permalink raw reply

* [PATCH net-next v2 1/2] gtp: remove unnecessary rcu_read_lock
From: Andreas Schultz @ 2017-01-26 15:21 UTC (permalink / raw)
  To: Pablo Neira
  Cc: netdev, Lionel Gauthier, openbsc, Harald Welte, SF Markus Elfring
In-Reply-To: <20170126152149.3284-1-aschultz@tpip.net>

The rcu read lock is hold by default in the ip input path. There
is no need to hold it twice in the socket recv decapsulate code path.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810b..5c63a37 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -184,7 +184,6 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 			      sizeof(struct gtp0_header);
 	struct gtp0_header *gtp0;
 	struct pdp_ctx *pctx;
-	int ret = 0;
 
 	if (!pskb_may_pull(skb, hdrlen))
 		return -1;
@@ -197,26 +196,19 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 	if (gtp0->type != GTP_TPDU)
 		return 1;
 
-	rcu_read_lock();
 	pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
 	if (!pctx) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
-	rcu_read_unlock();
 
 	/* Get rid of the GTP + UDP headers. */
 	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
-	rcu_read_unlock();
-	return ret;
 }
 
 static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
@@ -226,7 +218,6 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 			      sizeof(struct gtp1_header);
 	struct gtp1_header *gtp1;
 	struct pdp_ctx *pctx;
-	int ret = 0;
 
 	if (!pskb_may_pull(skb, hdrlen))
 		return -1;
@@ -254,26 +245,19 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 
 	gtp1 = (struct gtp1_header *)(skb->data + sizeof(struct udphdr));
 
-	rcu_read_lock();
 	pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
 	if (!pctx) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		ret = -1;
-		goto out_rcu;
+		return -1;
 	}
-	rcu_read_unlock();
 
 	/* Get rid of the GTP + UDP headers. */
 	return iptunnel_pull_header(skb, hdrlen, skb->protocol, xnet);
-out_rcu:
-	rcu_read_unlock();
-	return ret;
 }
 
 static void gtp_encap_disable(struct gtp_dev *gtp)
-- 
2.10.2

^ permalink raw reply related

* [PATCH net-next v2 2/2] gtp: let userspace handle packets for invalid tunnels
From: Andreas Schultz @ 2017-01-26 15:21 UTC (permalink / raw)
  To: Pablo Neira
  Cc: netdev, Lionel Gauthier, openbsc, Harald Welte, SF Markus Elfring
In-Reply-To: <20170126152149.3284-1-aschultz@tpip.net>

enable userspace to send error replies for invalid tunnels

Acked-by: Harald Welte <laforge@netfilter.org>
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5c63a37..5dd7de6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -199,12 +199,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 	pctx = gtp0_pdp_find(gtp, be64_to_cpu(gtp0->tid));
 	if (!pctx) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		return -1;
+		return 1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		return -1;
+		return 1;
 	}
 
 	/* Get rid of the GTP + UDP headers. */
@@ -248,12 +248,12 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb,
 	pctx = gtp1_pdp_find(gtp, ntohl(gtp1->tid));
 	if (!pctx) {
 		netdev_dbg(gtp->dev, "No PDP ctx to decap skb=%p\n", skb);
-		return -1;
+		return 1;
 	}
 
 	if (!gtp_check_src_ms(skb, pctx, hdrlen)) {
 		netdev_dbg(gtp->dev, "No PDP ctx for this MS\n");
-		return -1;
+		return 1;
 	}
 
 	/* Get rid of the GTP + UDP headers. */
-- 
2.10.2

^ permalink raw reply related

* [PATCH v2 net] net: free ip_vs_dest structs when refcnt=0
From: David Windsor @ 2017-01-24  3:24 UTC (permalink / raw)
  To: netdev, kernel-hardening, netfilter-devel
  Cc: lvs-devel, wensong, ja, horms, pablo, dwindsor, keescook,
	elena.reshetova, ishkamiel

Currently, the ip_vs_dest cache frees ip_vs_dest objects when their
reference count becomes < 0.  Aside from not being semantically sound,
this is problematic for the new type refcount_t, which will be introduced
shortly in a separate patch. refcount_t is the new kernel type for
holding reference counts, and provides overflow protection and a
constrained interface relative to atomic_t (the type currently being
used for kernel reference counts).

Per Julian Anastasov: "The problem is that dest_trash currently holds
deleted dests (unlinked from RCU lists) with refcnt=0."  Changing
dest_trash to hold dest with refcnt=1 will allow us to free ip_vs_dest
structs when their refcnt=0, in ip_vs_dest_put_and_free().

Signed-off-by: David Windsor <dwindsor@gmail.com>
---
 include/net/ip_vs.h            | 2 +-
 net/netfilter/ipvs/ip_vs_ctl.c | 8 +++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index cd6018a..a3e78ad 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1421,7 +1421,7 @@ static inline void ip_vs_dest_put(struct ip_vs_dest *dest)
 
 static inline void ip_vs_dest_put_and_free(struct ip_vs_dest *dest)
 {
-	if (atomic_dec_return(&dest->refcnt) < 0)
+	if (atomic_dec_and_test(&dest->refcnt))
 		kfree(dest);
 }
 
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 55e0169..5fc4836 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -711,7 +711,6 @@ ip_vs_trash_get_dest(struct ip_vs_service *svc, int dest_af,
 		      dest->vport == svc->port))) {
 			/* HIT */
 			list_del(&dest->t_list);
-			ip_vs_dest_hold(dest);
 			goto out;
 		}
 	}
@@ -741,7 +740,7 @@ static void ip_vs_dest_free(struct ip_vs_dest *dest)
  *  When the ip_vs_control_clearup is activated by ipvs module exit,
  *  the service tables must have been flushed and all the connections
  *  are expired, and the refcnt of each destination in the trash must
- *  be 0, so we simply release them here.
+ *  be 1, so we simply release them here.
  */
 static void ip_vs_trash_cleanup(struct netns_ipvs *ipvs)
 {
@@ -1080,11 +1079,10 @@ static void __ip_vs_del_dest(struct netns_ipvs *ipvs, struct ip_vs_dest *dest,
 	if (list_empty(&ipvs->dest_trash) && !cleanup)
 		mod_timer(&ipvs->dest_trash_timer,
 			  jiffies + (IP_VS_DEST_TRASH_PERIOD >> 1));
-	/* dest lives in trash without reference */
+	/* dest lives in trash with reference */
 	list_add(&dest->t_list, &ipvs->dest_trash);
 	dest->idle_start = 0;
 	spin_unlock_bh(&ipvs->dest_trash_lock);
-	ip_vs_dest_put(dest);
 }
 
 
@@ -1160,7 +1158,7 @@ static void ip_vs_dest_trash_expire(unsigned long data)
 
 	spin_lock(&ipvs->dest_trash_lock);
 	list_for_each_entry_safe(dest, next, &ipvs->dest_trash, t_list) {
-		if (atomic_read(&dest->refcnt) > 0)
+		if (atomic_read(&dest->refcnt) > 1)
 			continue;
 		if (dest->idle_start) {
 			if (time_before(now, dest->idle_start +
-- 
2.7.4

^ permalink raw reply related

* RE: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM
From: Rafal Ozieblo @ 2017-01-26 15:15 UTC (permalink / raw)
  To: Andrei Pistirica, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, davem@davemloft.net,
	nicolas.ferre@atmel.com, harinikatakamlinux@gmail.com,
	harini.katakam@xilinx.com
  Cc: punnaia@xilinx.com, michals@xilinx.com, anirudh@xilinx.com,
	boris.brezillon@free-electrons.com,
	alexandre.belloni@free-electrons.com, tbultel@pixelsurmer.com,
	richardcochran@gmail.com
In-Reply-To: <1484841375-11420-1-git-send-email-andrei.pistirica@microchip.com>

> -----Original Message-----
> From: Andrei Pistirica [mailto:andrei.pistirica@microchip.com] 
> Sent: 19 stycznia 2017 16:56
> Subject: [PATCH net-next v2] macb: Common code to enable ptp support for MACB/GEM
> 
>  
> +static inline bool gem_has_ptp(struct macb *bp)
> +{
> +	return !!(bp->caps & MACB_CAPS_GEM_HAS_PTP);
> +}
Why don't you use hardware capabilities here? Would it be better to read it from hardware instead adding it to many configuration?

^ permalink raw reply

* [PATCH net v2 0/3] various gtp fixes
From: Andreas Schultz @ 2017-01-26 15:11 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte

This is the part of the previous "simple gtp improvements" series
that Pablo indicated should go into net.

The addition of the module alias fixes genl family autoloading,
clearing the DF bit fixes a protocol violation in regard to the
specification and the netns comparison fixes a corner case of
cross netns recv.

I'm not sure if Pablos comments on the previous version qualify
as ACK, so I left that out.

^ permalink raw reply

* [PATCH net v2 1/3] gtp: add genl family modules alias
From: Andreas Schultz @ 2017-01-26 15:11 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170126151134.2892-1-aschultz@tpip.net>

Auto-load the module when userspace asks for the gtp netlink
family.

Acked-by: Harald Welte <laforge@netfilter.org>
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8b6810b..7580ccc 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1376,3 +1376,4 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Harald Welte <hwelte@sysmocom.de>");
 MODULE_DESCRIPTION("Interface driver for GTP encapsulated traffic");
 MODULE_ALIAS_RTNL_LINK("gtp");
+MODULE_ALIAS_GENL_FAMILY("gtp");
-- 
2.10.2

^ permalink raw reply related

* [PATCH net v2 2/3] gtp: clear DF bit on GTP packet tx
From: Andreas Schultz @ 2017-01-26 15:11 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170126151134.2892-1-aschultz@tpip.net>

3GPP TS 29.281 and 3GPP TS 29.060 imply that GTP-U packets should be
sent with the DF bit cleared. For example 3GPP TS 29.060, Release 8,
Section 13.2.2:

> Backbone router: Any router in the backbone may fragment the GTP
> packet if needed, according to IPv4.

Acked-by: Harald Welte <laforge@netfilter.org>
Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 7580ccc..1df54d6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -612,7 +612,7 @@ static netdev_tx_t gtp_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 				    pktinfo.fl4.saddr, pktinfo.fl4.daddr,
 				    pktinfo.iph->tos,
 				    ip4_dst_hoplimit(&pktinfo.rt->dst),
-				    htons(IP_DF),
+				    0,
 				    pktinfo.gtph_port, pktinfo.gtph_port,
 				    true, false);
 		break;
-- 
2.10.2

^ permalink raw reply related

* [PATCH net v2 3/3] gtp: fix cross netns recv on gtp socket
From: Andreas Schultz @ 2017-01-26 15:11 UTC (permalink / raw)
  To: Pablo Neira; +Cc: netdev, Lionel Gauthier, openbsc, Harald Welte
In-Reply-To: <20170126151134.2892-1-aschultz@tpip.net>

The use of the passed through netlink src_net to check for a
cross netns operation was wrong. Using the GTP socket and the
GTP netdevice is always correct (even if the netdev has been
moved to new netns after link creation).

Remove the now obsolete net field from gtp_dev.

Signed-off-by: Andreas Schultz <aschultz@tpip.net>
---
 drivers/net/gtp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 1df54d6..72dd1ba 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -69,7 +69,6 @@ struct gtp_dev {
 	struct socket		*sock0;
 	struct socket		*sock1u;
 
-	struct net		*net;
 	struct net_device	*dev;
 
 	unsigned int		hash_size;
@@ -316,7 +315,7 @@ static int gtp_encap_recv(struct sock *sk, struct sk_buff *skb)
 
 	netdev_dbg(gtp->dev, "encap_recv sk=%p\n", sk);
 
-	xnet = !net_eq(gtp->net, dev_net(gtp->dev));
+	xnet = !net_eq(sock_net(sk), dev_net(gtp->dev));
 
 	switch (udp_sk(sk)->encap_type) {
 	case UDP_ENCAP_GTP0:
@@ -658,7 +657,7 @@ static void gtp_link_setup(struct net_device *dev)
 static int gtp_hashtable_new(struct gtp_dev *gtp, int hsize);
 static void gtp_hashtable_free(struct gtp_dev *gtp);
 static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
-			    int fd_gtp0, int fd_gtp1, struct net *src_net);
+			    int fd_gtp0, int fd_gtp1);
 
 static int gtp_newlink(struct net *src_net, struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[])
@@ -858,7 +857,6 @@ static int gtp_encap_enable(struct net_device *dev, struct gtp_dev *gtp,
 
 	gtp->sock0 = sock0;
 	gtp->sock1u = sock1u;
-	gtp->net = src_net;
 
 	tuncfg.sk_user_data = gtp;
 	tuncfg.encap_rcv = gtp_encap_recv;
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH RFC ipsec-next 2/2] xfrm: Add a device independent napi instance
From: Eric Dumazet @ 2017-01-26 15:10 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: netdev, David Miller, Sowmini Varadhan, Ilan Tayari
In-Reply-To: <1485440767.5145.126.camel@edumazet-glaptop3.roam.corp.google.com>

On Thu, 2017-01-26 at 06:26 -0800, Eric Dumazet wrote:

> 
> Alternative would be to use a 
> 
> static struct net_device xfrm_napi_anchor_device;
> 
> and use gro_cell

Also take a look at init_dummy_netdev()

Thanks !

^ permalink raw reply

* Re: [PATCHv3 4/5] arm: mvebu: Add device tree for 98DX3236 SoCs
From: Gregory CLEMENT @ 2017-01-26 15:09 UTC (permalink / raw)
  To: Chris Packham
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Rob Herring,
	Mark Rutland, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170106041517.9589-5-chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org>

Hi Chris,
 
 On ven., janv. 06 2017, Chris Packham <chris.packham-6g8wRflRTwXFdCa3tKVlE6U/zSkkHjvu@public.gmane.org> wrote:

> The Marvell 98DX3236, 98DX3336, 98DX4521 and variants are switch ASICs
> with integrated CPUs. They are similar to the Armada XP SoCs but have
> different I/O interfaces.

Before sending a new version I have a few remarks:


> diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> new file mode 100644
> index 000000000000..4b7b2fe3b682
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
> @@ -0,0 +1,254 @@
> +/*
> + * Device Tree Include file for Marvell 98dx3236 family SoC
> + *
> + * Copyright (C) 2016 Allied Telesis Labs
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.

There are few characters missing in the licence text, have a look on:
http://git.infradead.org/linux-mvebu.git/commitdiff/24f0b6fe52d21b5c59c4e948daae2234a39a25b2?hp=7ce7d89f48834cefece7804d38fc5d85382edf77



> + *
> + * Contains definitions specific to the 98dx3236 SoC that are not
> + * common to all Armada XP SoCs.
> + */
> +
> +#include "armada-xp.dtsi"
> +
> +/ {
> +	model = "Marvell 98DX3236 SoC";
> +	compatible = "marvell,armadaxp-98dx3236", "marvell,armadaxp", "marvell,armada-370-xp";
> +
> +	aliases {
> +		gpio0 = &gpio0;
> +		gpio1 = &gpio1;
> +		gpio2 = &gpio2;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		enable-method = "marvell,98dx3236-smp";
> +
> +		cpu@0 {
> +			device_type = "cpu";
> +			compatible = "marvell,sheeva-v7";
> +			reg = <0>;
> +			clocks = <&cpuclk 0>;
> +			clock-latency = <1000000>;
> +		};
> +	};
> +
> +	soc {
> +		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000
> +			  MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000
> +			  MBUS_ID(0x01, 0x2f) 0 0 0xf0000000 0x1000000
> +			  MBUS_ID(0x03, 0x00) 0 0 0xa8000000 0x4000000
> +			  MBUS_ID(0x08, 0x00) 0 0 0xac000000 0x100000>;
> +
> +		/*
> +		 * 98DX3236 has 1 x1 PCIe unit Gen2.0: One unit can be
The comment had been cut

> +		 */
> +		pcie-controller {

Please use a node label here as we have done in:

http://git.infradead.org/linux-mvebu.git/commitdiff/11f7135bb9dbe7ae3bb9a125e6123d4096a7e69e?hp=e72996b80d53b9b7616c8b68304ce4c422b4ddd1

Also use an address:
http://git.infradead.org/linux-mvebu.git/commitdiff/007d05d898050ffc70fd2737896528c5069f7269



> +			compatible = "marvell,armada-xp-pcie";
> +			status = "disabled";
> +			device_type = "pci";
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +
> +			msi-parent = <&mpic>;
> +			bus-range = <0x00 0xff>;
> +
> +			ranges =
> +			       <0x82000000 0 0x40000 MBUS_ID(0xf0, 0x01) 0x40000 0 0x00002000   /* Port 0.0 registers */
> +				0x82000000 0x1 0       MBUS_ID(0x04, 0xe8) 0 1 0 /* Port 0.0 MEM */
> +				0x81000000 0x1 0       MBUS_ID(0x04, 0xe0) 0 1 0 /* Port 0.0 IO  */
> +				0x82000000 0x2 0       MBUS_ID(0x04, 0xd8) 0 1 0 /* Port 0.1 MEM */>;
> +
> +			pcie@1,0 {
node label

> +				device_type = "pci";
> +				assigned-addresses = <0x82000800 0 0x40000 0 0x2000>;
> +				reg = <0x0800 0 0 0 0>;
> +				#address-cells = <3>;
> +				#size-cells = <2>;
> +				#interrupt-cells = <1>;
> +				ranges = <0x82000000 0 0 0x82000000 0x1 0 1 0
> +					  0x81000000 0 0 0x81000000 0x1 0 1 0>;
> +				interrupt-map-mask = <0 0 0 0>;
> +				interrupt-map = <0 0 0 0 &mpic 58>;
> +				marvell,pcie-port = <0>;
> +				marvell,pcie-lane = <0>;
> +				clocks = <&gateclk 5>;
> +				status = "disabled";
> +			};
> +		};
> +
> +		internal-regs {
> +			coreclk: mvebu-sar@18230 {
> +				compatible = "marvell,mv98dx3236-core-clock";
> +			};
> +
> +			cpuclk: clock-complex@18700 {
> +				compatible = "marvell,mv98dx3236-cpu-clock";
> +			};
> +
> +			corediv-clock@18740 {
> +				status = "disabled";
> +			};
> +
> +			xor@60900 {
> +				status = "disabled";
> +			};
> +
> +			crypto@90000 {
> +				status = "disabled";
> +			};
> +
> +			xor@f0900 {
> +				status = "disabled";
> +			};
> +
> +			xor@f0800 {
> +				compatible = "marvell,orion-xor";
> +				reg = <0xf0800 0x100
> +				       0xf0a00 0x100>;
> +				clocks = <&gateclk 22>;
> +				status = "okay";
> +
> +				xor10 {
> +					interrupts = <51>;
> +					dmacap,memcpy;
> +					dmacap,xor;
> +				};
> +				xor11 {
> +					interrupts = <52>;
> +					dmacap,memcpy;
> +					dmacap,xor;
> +					dmacap,memset;
> +				};
> +			};
> +
> +			gpio0: gpio@18100 {
> +				compatible = "marvell,orion-gpio";
> +				reg = <0x18100 0x40>;
> +				ngpios = <32>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <82>, <83>, <84>, <85>;
> +			};
> +
> +			/* does not exist */
> +			gpio1: gpio@18140 {
> +				compatible = "marvell,orion-gpio";
> +				reg = <0x18140 0x40>;
> +				status = "disabled";
> +			};
> +
> +			gpio2: gpio@18180 { /* rework some properties */
> +				compatible = "marvell,orion-gpio";
> +				reg = <0x18180 0x40>;
> +				ngpios = <1>; /* only gpio #32 */
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				interrupt-controller;
> +				#interrupt-cells = <2>;
> +				interrupts = <87>;
> +			};
> +
> +			nand: nand@d0000 {
> +				clocks = <&dfx_coredivclk 0>;
> +			};
> +		};
> +
> +		dfx-registers {
node label

> +			compatible = "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 MBUS_ID(0x08, 0x00) 0 0x100000>;
> +
> +                        dfx_coredivclk: corediv-clock@f8268 {
> +                                compatible = "marvell,mv98dx3236-corediv-clock";
> +                                reg = <0xf8268 0xc>;
> +                                #clock-cells = <1>;
> +                                clocks = <&mainpll>;
> +                                clock-output-names = "nand";
> +                        };
> +
> +			dfx: dfx@0 {
> +				compatible = "marvell,dfx-server";
> +				reg = <0 0x100000>;
> +			};
> +		};
> +
> +		switch {
node label

> +			compatible = "simple-bus";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges = <0 MBUS_ID(0x03, 0x00) 0 0x100000>;
> +
> +			pp0: packet-processor@0 {
> +				compatible = "marvell,prestera-98dx3236";
> +				reg = <0 0x4000000>;
> +				interrupts = <33>, <34>, <35>;
> +				dfx = <&dfx>;
> +			};
> +		};
> +	};
> +};
> +
> +&pinctrl {
> +	compatible = "marvell,98dx3236-pinctrl";
> +
> +	spi0_pins: spi0-pins {
> +		marvell,pins = "mpp0", "mpp1",
> +			       "mpp2", "mpp3";
> +		marvell,function = "spi0";
> +	};
> +};
> +
> +&sdio {
> +	status = "disabled";
> +};
> +
> +&crypto_sram0 {
> +	status = "disabled";
> +};
> +
> +&crypto_sram1 {
> +	status = "disabled";
> +};

same comments for the following device tree, in general you can refer to
this series:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-November/468585.html


> diff --git a/arch/arm/boot/dts/armada-xp-98dx3336.dtsi b/arch/arm/boot/dts/armada-xp-98dx3336.dtsi
> new file mode 100644
> index 000000000000..a9b0f47f8df9
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-98dx3336.dtsi
> @@ -0,0 +1,76 @@
> +/*
> + * Device Tree Include file for Marvell 98dx3336 family SoC
> + *
> + * Copyright (C) 2016 Allied Telesis Labs
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Contains definitions specific to the 98dx3336 SoC that are not
> + * common to all Armada XP SoCs.
> + */
> +
> +#include "armada-xp-98dx3236.dtsi"
> +
> +/ {
> +	model = "Marvell 98DX3336 SoC";
> +	compatible = "marvell,armadaxp-98dx3336", "marvell,armadaxp-98dx3236", "marvell,armadaxp", "marvell,armada-370-xp";
> +
> +	cpus {
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "marvell,sheeva-v7";
> +			reg = <1>;
> +			clocks = <&cpuclk 1>;
> +			clock-latency = <1000000>;
> +		};
> +	};
> +
> +	soc {
> +		internal-regs {


Why the following node is not part of the dtsi?

Gregory

> +			resume@20980 {
> +				compatible = "marvell,98dx3336-resume-ctrl";
> +				reg = <0x20980 0x10>;
> +			};
> +		};
> +	};
> +};
> +
> +&pp0 {
> +	compatible = "marvell,prestera-98dx3336";
> +};
> diff --git a/arch/arm/boot/dts/armada-xp-98dx4251.dtsi b/arch/arm/boot/dts/armada-xp-98dx4251.dtsi
> new file mode 100644
> index 000000000000..446e6e65ec59
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-98dx4251.dtsi
> @@ -0,0 +1,90 @@
> +/*
> + * Device Tree Include file for Marvell 98dx4521 family SoC
> + *
> + * Copyright (C) 2016 Allied Telesis Labs
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + *     modify it under the terms of the GNU General Public License as
> + *     published by the Free Software Foundation; either version 2 of the
> + *     License, or (at your option) any later version.
> + *
> + *     This file is distributed in the hope that it will be useful
> + *     but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *     GNU General Public License for more details.
> + *
> + * Or, alternatively
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED , WITHOUT WARRANTY OF ANY KIND
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Contains definitions specific to the 98dx4521 SoC that are not
> + * common to all Armada XP SoCs.
> + */
> +
> +#include "armada-xp-98dx3236.dtsi"
> +
> +/ {
> +	model = "Marvell 98DX4251 SoC";
> +	compatible = "marvell,armadaxp-98dx4521", "marvell,armadaxp-98dx3236", "marvell,armadaxp", "marvell,armada-370-xp";
> +
> +	cpus {
> +		cpu@1 {
> +			device_type = "cpu";
> +			compatible = "marvell,sheeva-v7";
> +			reg = <1>;
> +			clocks = <&cpuclk 1>;
> +			clock-latency = <1000000>;
> +		};
> +	};
> +
> +	soc {
> +		internal-regs {
> +			resume@20980 {
> +				compatible = "marvell,98dx3336-resume-ctrl";
> +				reg = <0x20980 0x10>;
> +			};
> +		};
> +	};
> +};
> +
> +&sdio {
> +	status = "okay";
> +};
> +
> +&pinctrl {
> +	compatible = "marvell,98dx4251-pinctrl";
> +
> +	sdio_pins: sdio-pins {
> +		marvell,pins = "mpp5", "mpp6", "mpp7",
> +			       "mpp8", "mpp9", "mpp10";
> +		marvell,function = "sd0";
> +	};
> +};
> +
> +&pp0 {
> +	compatible = "marvell,prestera-98dx4251";
> +};
> -- 
> 2.11.0.24.ge6920cf
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] Fix 64 bit addressing support for GEM
From: Rafal Ozieblo @ 2017-01-26 15:00 UTC (permalink / raw)
  To: Nicolas Ferre, netdev, linux-kernel, harinikatakamlinux; +Cc: Rafal Ozieblo

This patch adds support for 32 bit GEM in
64 bit system. It checks capability at runtime
and uses appropriate buffer descriptor.

Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
---
 drivers/net/ethernet/cadence/macb.c | 183 +++++++++++++++++++++++++-----------
 drivers/net/ethernet/cadence/macb.h |  20 +++-
 2 files changed, 145 insertions(+), 58 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index c0fb80a..2a06931 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -43,13 +43,13 @@
 #define DEFAULT_RX_RING_SIZE	512 /* must be power of 2 */
 #define MIN_RX_RING_SIZE	64
 #define MAX_RX_RING_SIZE	8192
-#define RX_RING_BYTES(bp)	(sizeof(struct macb_dma_desc)	\
+#define RX_RING_BYTES(bp)	(macb_dma_desc_get_size(bp)	\
 				 * (bp)->rx_ring_size)
 
 #define DEFAULT_TX_RING_SIZE	512 /* must be power of 2 */
 #define MIN_TX_RING_SIZE	64
 #define MAX_TX_RING_SIZE	4096
-#define TX_RING_BYTES(bp)	(sizeof(struct macb_dma_desc)	\
+#define TX_RING_BYTES(bp)	(macb_dma_desc_get_size(bp)	\
 				 * (bp)->tx_ring_size)
 
 /* level of occupied TX descriptors under which we wake up TX process */
@@ -78,6 +78,37 @@
  */
 #define MACB_HALT_TIMEOUT	1230
 
+/* DMA buffer descriptor might be different size
+ * depends on hardware configuration.
+ */
+static unsigned int macb_dma_desc_get_size(struct macb *bp)
+{
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+		return sizeof(struct macb_dma_desc) + sizeof(struct macb_dma_desc_64);
+#endif
+	return sizeof(struct macb_dma_desc);
+}
+
+static unsigned int macb_adj_dma_desc_idx(struct macb *bp, unsigned int idx)
+{
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	/* Dma buffer descriptor is 4 words length (instead of 2 words)
+	 * for 64b GEM.
+	 */
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+		idx <<= 1;
+#endif
+	return idx;
+}
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+static struct macb_dma_desc_64 *macb_64b_desc(struct macb *bp, struct macb_dma_desc *desc)
+{
+	return (struct macb_dma_desc_64 *)((void *)desc + sizeof(struct macb_dma_desc));
+}
+#endif
+
 /* Ring buffer accessors */
 static unsigned int macb_tx_ring_wrap(struct macb *bp, unsigned int index)
 {
@@ -87,7 +118,9 @@ static unsigned int macb_tx_ring_wrap(struct macb *bp, unsigned int index)
 static struct macb_dma_desc *macb_tx_desc(struct macb_queue *queue,
 					  unsigned int index)
 {
-	return &queue->tx_ring[macb_tx_ring_wrap(queue->bp, index)];
+	index = macb_tx_ring_wrap(queue->bp, index);
+	index = macb_adj_dma_desc_idx(queue->bp, index);
+	return &queue->tx_ring[index];
 }
 
 static struct macb_tx_skb *macb_tx_skb(struct macb_queue *queue,
@@ -101,7 +134,7 @@ static dma_addr_t macb_tx_dma(struct macb_queue *queue, unsigned int index)
 	dma_addr_t offset;
 
 	offset = macb_tx_ring_wrap(queue->bp, index) *
-		 sizeof(struct macb_dma_desc);
+			macb_dma_desc_get_size(queue->bp);
 
 	return queue->tx_ring_dma + offset;
 }
@@ -113,7 +146,9 @@ static unsigned int macb_rx_ring_wrap(struct macb *bp, unsigned int index)
 
 static struct macb_dma_desc *macb_rx_desc(struct macb *bp, unsigned int index)
 {
-	return &bp->rx_ring[macb_rx_ring_wrap(bp, index)];
+	index = macb_rx_ring_wrap(bp, index);
+	index = macb_adj_dma_desc_idx(bp, index);
+	return &bp->rx_ring[index];
 }
 
 static void *macb_rx_buffer(struct macb *bp, unsigned int index)
@@ -560,12 +595,32 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_skb *tx_skb)
 	}
 }
 
-static inline void macb_set_addr(struct macb_dma_desc *desc, dma_addr_t addr)
+static void macb_set_addr(struct macb *bp, struct macb_dma_desc *desc, dma_addr_t addr)
 {
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	struct macb_dma_desc_64 *desc_64;
+
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B) {
+		desc_64 = macb_64b_desc(bp, desc);
+		desc_64->addrh = (u32)(addr >> 32);
+	}
+#endif
 	desc->addr = (u32)addr;
+}
+
+static dma_addr_t macb_get_addr(struct macb *bp, struct macb_dma_desc *desc)
+{
+	dma_addr_t addr = 0;
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	desc->addrh = (u32)(addr >> 32);
+	struct macb_dma_desc_64 *desc_64;
+
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B) {
+		desc_64 = macb_64b_desc(bp, desc);
+		addr = ((u64)(desc_64->addrh) << 32);
+	}
 #endif
+	addr |= MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
+	return addr;
 }
 
 static void macb_tx_error_task(struct work_struct *work)
@@ -574,7 +629,7 @@ static void macb_tx_error_task(struct work_struct *work)
 						      tx_error_task);
 	struct macb		*bp = queue->bp;
 	struct macb_tx_skb	*tx_skb;
-	struct macb_dma_desc	*desc;
+	struct macb_dma_desc *desc;
 	struct sk_buff		*skb;
 	unsigned int		tail;
 	unsigned long		flags;
@@ -649,7 +704,7 @@ static void macb_tx_error_task(struct work_struct *work)
 
 	/* Set end of TX queue */
 	desc = macb_tx_desc(queue, 0);
-	macb_set_addr(desc, 0);
+	macb_set_addr(bp, desc, 0);
 	desc->ctrl = MACB_BIT(TX_USED);
 
 	/* Make descriptor updates visible to hardware */
@@ -658,7 +713,8 @@ static void macb_tx_error_task(struct work_struct *work)
 	/* Reinitialize the TX desc queue */
 	queue_writel(queue, TBQP, (u32)(queue->tx_ring_dma));
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	queue_writel(queue, TBQPH, (u32)(queue->tx_ring_dma >> 32));
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+		queue_writel(queue, TBQPH, (u32)(queue->tx_ring_dma >> 32));
 #endif
 	/* Make TX ring reflect state of hardware */
 	queue->tx_head = 0;
@@ -750,6 +806,7 @@ static void gem_rx_refill(struct macb *bp)
 	unsigned int		entry;
 	struct sk_buff		*skb;
 	dma_addr_t		paddr;
+	struct macb_dma_desc *desc;
 
 	while (CIRC_SPACE(bp->rx_prepared_head, bp->rx_tail,
 			  bp->rx_ring_size) > 0) {
@@ -759,6 +816,7 @@ static void gem_rx_refill(struct macb *bp)
 		rmb();
 
 		bp->rx_prepared_head++;
+		desc = macb_rx_desc(bp, entry);
 
 		if (!bp->rx_skbuff[entry]) {
 			/* allocate sk_buff for this free entry in ring */
@@ -782,14 +840,14 @@ static void gem_rx_refill(struct macb *bp)
 
 			if (entry == bp->rx_ring_size - 1)
 				paddr |= MACB_BIT(RX_WRAP);
-			macb_set_addr(&(bp->rx_ring[entry]), paddr);
-			bp->rx_ring[entry].ctrl = 0;
+			macb_set_addr(bp, desc, paddr);
+			desc->ctrl = 0;
 
 			/* properly align Ethernet header */
 			skb_reserve(skb, NET_IP_ALIGN);
 		} else {
-			bp->rx_ring[entry].addr &= ~MACB_BIT(RX_USED);
-			bp->rx_ring[entry].ctrl = 0;
+			desc->addr &= ~MACB_BIT(RX_USED);
+			desc->ctrl = 0;
 		}
 	}
 
@@ -835,16 +893,13 @@ static int gem_rx(struct macb *bp, int budget)
 		bool rxused;
 
 		entry = macb_rx_ring_wrap(bp, bp->rx_tail);
-		desc = &bp->rx_ring[entry];
+		desc = macb_rx_desc(bp, entry);
 
 		/* Make hw descriptor updates visible to CPU */
 		rmb();
 
 		rxused = (desc->addr & MACB_BIT(RX_USED)) ? true : false;
-		addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		addr |= ((u64)(desc->addrh) << 32);
-#endif
+		addr = macb_get_addr(bp, desc);
 		ctrl = desc->ctrl;
 
 		if (!rxused)
@@ -987,15 +1042,17 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
 static inline void macb_init_rx_ring(struct macb *bp)
 {
 	dma_addr_t addr;
+	struct macb_dma_desc *desc = NULL;
 	int i;
 
 	addr = bp->rx_buffers_dma;
 	for (i = 0; i < bp->rx_ring_size; i++) {
-		bp->rx_ring[i].addr = addr;
-		bp->rx_ring[i].ctrl = 0;
+		desc = macb_rx_desc(bp, i);
+		macb_set_addr(bp, desc, addr);
+		desc->ctrl = 0;
 		addr += bp->rx_buffer_size;
 	}
-	bp->rx_ring[bp->rx_ring_size - 1].addr |= MACB_BIT(RX_WRAP);
+	desc->addr |= MACB_BIT(RX_WRAP);
 	bp->rx_tail = 0;
 }
 
@@ -1008,15 +1065,14 @@ static int macb_rx(struct macb *bp, int budget)
 
 	for (tail = bp->rx_tail; budget > 0; tail++) {
 		struct macb_dma_desc *desc = macb_rx_desc(bp, tail);
-		u32 addr, ctrl;
+		u32 ctrl;
 
 		/* Make hw descriptor updates visible to CPU */
 		rmb();
 
-		addr = desc->addr;
 		ctrl = desc->ctrl;
 
-		if (!(addr & MACB_BIT(RX_USED)))
+		if (!(desc->addr & MACB_BIT(RX_USED)))
 			break;
 
 		if (ctrl & MACB_BIT(RX_SOF)) {
@@ -1336,7 +1392,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 	i = tx_head;
 	entry = macb_tx_ring_wrap(bp, i);
 	ctrl = MACB_BIT(TX_USED);
-	desc = &queue->tx_ring[entry];
+	desc = macb_tx_desc(queue, entry);
 	desc->ctrl = ctrl;
 
 	if (lso_ctrl) {
@@ -1358,7 +1414,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 		i--;
 		entry = macb_tx_ring_wrap(bp, i);
 		tx_skb = &queue->tx_skb[entry];
-		desc = &queue->tx_ring[entry];
+		desc = macb_tx_desc(queue, entry);
 
 		ctrl = (u32)tx_skb->size;
 		if (eof) {
@@ -1379,7 +1435,7 @@ static unsigned int macb_tx_map(struct macb *bp,
 			ctrl |= MACB_BF(MSS_MFS, mss_mfs);
 
 		/* Set TX buffer descriptor */
-		macb_set_addr(desc, tx_skb->mapping);
+		macb_set_addr(bp, desc, tx_skb->mapping);
 		/* desc->addr must be visible to hardware before clearing
 		 * 'TX_USED' bit in desc->ctrl.
 		 */
@@ -1586,11 +1642,9 @@ static void gem_free_rx_buffers(struct macb *bp)
 		if (!skb)
 			continue;
 
-		desc = &bp->rx_ring[i];
-		addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, desc->addr));
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		addr |= ((u64)(desc->addrh) << 32);
-#endif
+		desc = macb_rx_desc(bp, i);
+		addr = macb_get_addr(bp, desc);
+
 		dma_unmap_single(&bp->pdev->dev, addr, bp->rx_buffer_size,
 				 DMA_FROM_DEVICE);
 		dev_kfree_skb_any(skb);
@@ -1711,15 +1765,17 @@ static int macb_alloc_consistent(struct macb *bp)
 static void gem_init_rings(struct macb *bp)
 {
 	struct macb_queue *queue;
+	struct macb_dma_desc *desc = NULL;
 	unsigned int q;
 	int i;
 
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		for (i = 0; i < bp->tx_ring_size; i++) {
-			queue->tx_ring[i].addr = 0;
-			queue->tx_ring[i].ctrl = MACB_BIT(TX_USED);
+			desc = macb_tx_desc(queue, i);
+			macb_set_addr(bp, desc, 0);
+			desc->ctrl = MACB_BIT(TX_USED);
 		}
-		queue->tx_ring[bp->tx_ring_size - 1].ctrl |= MACB_BIT(TX_WRAP);
+		desc->ctrl |= MACB_BIT(TX_WRAP);
 		queue->tx_head = 0;
 		queue->tx_tail = 0;
 	}
@@ -1733,16 +1789,18 @@ static void gem_init_rings(struct macb *bp)
 static void macb_init_rings(struct macb *bp)
 {
 	int i;
+	struct macb_dma_desc *desc = NULL;
 
 	macb_init_rx_ring(bp);
 
 	for (i = 0; i < bp->tx_ring_size; i++) {
-		bp->queues[0].tx_ring[i].addr = 0;
-		bp->queues[0].tx_ring[i].ctrl = MACB_BIT(TX_USED);
+		desc = macb_tx_desc(&bp->queues[0], i);
+		macb_set_addr(bp, desc, 0);
+		desc->ctrl = MACB_BIT(TX_USED);
 	}
 	bp->queues[0].tx_head = 0;
 	bp->queues[0].tx_tail = 0;
-	bp->queues[0].tx_ring[bp->tx_ring_size - 1].ctrl |= MACB_BIT(TX_WRAP);
+	desc->ctrl |= MACB_BIT(TX_WRAP);
 }
 
 static void macb_reset_hw(struct macb *bp)
@@ -1863,7 +1921,8 @@ static void macb_configure_dma(struct macb *bp)
 			dmacfg &= ~GEM_BIT(TXCOEN);
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		dmacfg |= GEM_BIT(ADDR64);
+		if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+			dmacfg |= GEM_BIT(ADDR64);
 #endif
 		netdev_dbg(bp->dev, "Cadence configure DMA with 0x%08x\n",
 			   dmacfg);
@@ -1912,12 +1971,14 @@ static void macb_init_hw(struct macb *bp)
 	/* Initialize TX and RX buffers */
 	macb_writel(bp, RBQP, (u32)(bp->rx_ring_dma));
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	macb_writel(bp, RBQPH, (u32)(bp->rx_ring_dma >> 32));
+	if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+		macb_writel(bp, RBQPH, (u32)(bp->rx_ring_dma >> 32));
 #endif
 	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
 		queue_writel(queue, TBQP, (u32)(queue->tx_ring_dma));
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		queue_writel(queue, TBQPH, (u32)(queue->tx_ring_dma >> 32));
+		if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+			queue_writel(queue, TBQPH, (u32)(queue->tx_ring_dma >> 32));
 #endif
 
 		/* Enable interrupts */
@@ -2627,7 +2688,8 @@ static int macb_init(struct platform_device *pdev)
 			queue->IMR  = GEM_IMR(hw_q - 1);
 			queue->TBQP = GEM_TBQP(hw_q - 1);
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-			queue->TBQPH = GEM_TBQPH(hw_q -1);
+			if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+				queue->TBQPH = GEM_TBQPH(hw_q - 1);
 #endif
 		} else {
 			/* queue0 uses legacy registers */
@@ -2637,7 +2699,8 @@ static int macb_init(struct platform_device *pdev)
 			queue->IMR  = MACB_IMR;
 			queue->TBQP = MACB_TBQP;
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-			queue->TBQPH = MACB_TBQPH;
+			if (bp->hw_dma_cap == HW_DMA_CAP_64B)
+				queue->TBQPH = MACB_TBQPH;
 #endif
 		}
 
@@ -2730,13 +2793,14 @@ static int macb_init(struct platform_device *pdev)
 static int at91ether_start(struct net_device *dev)
 {
 	struct macb *lp = netdev_priv(dev);
+	struct macb_dma_desc *desc;
 	dma_addr_t addr;
 	u32 ctl;
 	int i;
 
 	lp->rx_ring = dma_alloc_coherent(&lp->pdev->dev,
 					 (AT91ETHER_MAX_RX_DESCR *
-					  sizeof(struct macb_dma_desc)),
+					  macb_dma_desc_get_size(lp)),
 					 &lp->rx_ring_dma, GFP_KERNEL);
 	if (!lp->rx_ring)
 		return -ENOMEM;
@@ -2748,7 +2812,7 @@ static int at91ether_start(struct net_device *dev)
 	if (!lp->rx_buffers) {
 		dma_free_coherent(&lp->pdev->dev,
 				  AT91ETHER_MAX_RX_DESCR *
-				  sizeof(struct macb_dma_desc),
+				  macb_dma_desc_get_size(lp),
 				  lp->rx_ring, lp->rx_ring_dma);
 		lp->rx_ring = NULL;
 		return -ENOMEM;
@@ -2756,13 +2820,14 @@ static int at91ether_start(struct net_device *dev)
 
 	addr = lp->rx_buffers_dma;
 	for (i = 0; i < AT91ETHER_MAX_RX_DESCR; i++) {
-		lp->rx_ring[i].addr = addr;
-		lp->rx_ring[i].ctrl = 0;
+		desc = macb_rx_desc(lp, i);
+		macb_set_addr(lp, desc, addr);
+		desc->ctrl = 0;
 		addr += AT91ETHER_MAX_RBUFF_SZ;
 	}
 
 	/* Set the Wrap bit on the last descriptor */
-	lp->rx_ring[AT91ETHER_MAX_RX_DESCR - 1].addr |= MACB_BIT(RX_WRAP);
+	desc->addr |= MACB_BIT(RX_WRAP);
 
 	/* Reset buffer index */
 	lp->rx_tail = 0;
@@ -2834,7 +2899,7 @@ static int at91ether_close(struct net_device *dev)
 
 	dma_free_coherent(&lp->pdev->dev,
 			  AT91ETHER_MAX_RX_DESCR *
-			  sizeof(struct macb_dma_desc),
+			  macb_dma_desc_get_size(lp),
 			  lp->rx_ring, lp->rx_ring_dma);
 	lp->rx_ring = NULL;
 
@@ -2885,13 +2950,15 @@ static int at91ether_start_xmit(struct sk_buff *skb, struct net_device *dev)
 static void at91ether_rx(struct net_device *dev)
 {
 	struct macb *lp = netdev_priv(dev);
+	struct macb_dma_desc *desc;
 	unsigned char *p_recv;
 	struct sk_buff *skb;
 	unsigned int pktlen;
 
-	while (lp->rx_ring[lp->rx_tail].addr & MACB_BIT(RX_USED)) {
+	desc = macb_rx_desc(lp, lp->rx_tail);
+	while (desc->addr & MACB_BIT(RX_USED)) {
 		p_recv = lp->rx_buffers + lp->rx_tail * AT91ETHER_MAX_RBUFF_SZ;
-		pktlen = MACB_BF(RX_FRMLEN, lp->rx_ring[lp->rx_tail].ctrl);
+		pktlen = MACB_BF(RX_FRMLEN, desc->ctrl);
 		skb = netdev_alloc_skb(dev, pktlen + 2);
 		if (skb) {
 			skb_reserve(skb, 2);
@@ -2905,17 +2972,19 @@ static void at91ether_rx(struct net_device *dev)
 			lp->stats.rx_dropped++;
 		}
 
-		if (lp->rx_ring[lp->rx_tail].ctrl & MACB_BIT(RX_MHASH_MATCH))
+		if (desc->ctrl & MACB_BIT(RX_MHASH_MATCH))
 			lp->stats.multicast++;
 
 		/* reset ownership bit */
-		lp->rx_ring[lp->rx_tail].addr &= ~MACB_BIT(RX_USED);
+		desc->addr &= ~MACB_BIT(RX_USED);
 
 		/* wrap after last buffer */
 		if (lp->rx_tail == AT91ETHER_MAX_RX_DESCR - 1)
 			lp->rx_tail = 0;
 		else
 			lp->rx_tail++;
+
+		desc = macb_rx_desc(lp, lp->rx_tail);
 	}
 }
 
@@ -3211,8 +3280,12 @@ static int macb_probe(struct platform_device *pdev)
 	device_init_wakeup(&pdev->dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET);
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	if (GEM_BFEXT(DBWDEF, gem_readl(bp, DCFG1)) > GEM_DBW32)
+	if (GEM_BFEXT(DAW64, gem_readl(bp, DCFG6))) {
+		pr_emerg("rozieblo: 64B!\n");
 		dma_set_mask(&pdev->dev, DMA_BIT_MASK(44));
+		bp->hw_dma_cap = HW_DMA_CAP_64B;
+	} else
+		bp->hw_dma_cap = HW_DMA_CAP_32B;
 #endif
 
 	spin_lock_init(&bp->lock);
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d67adad..fc8550a 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -385,6 +385,8 @@
 /* Bitfields in DCFG6. */
 #define GEM_PBUF_LSO_OFFSET			27
 #define GEM_PBUF_LSO_SIZE			1
+#define GEM_DAW64_OFFSET			23
+#define GEM_DAW64_SIZE				1
 
 /* Constants for CLK */
 #define MACB_CLK_DIV8				0
@@ -487,12 +489,20 @@
 struct macb_dma_desc {
 	u32	addr;
 	u32	ctrl;
+};
+
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-	u32     addrh;
-	u32     resvd;
-#endif
+enum macb_hw_dma_cap {
+	HW_DMA_CAP_32B,
+	HW_DMA_CAP_64B,
 };
 
+struct macb_dma_desc_64 {
+	u32 addrh;
+	u32 resvd;
+};
+#endif
+
 /* DMA descriptor bitfields */
 #define MACB_RX_USED_OFFSET			0
 #define MACB_RX_USED_SIZE			1
@@ -874,6 +884,10 @@ struct macb {
 	unsigned int		jumbo_max_len;
 
 	u32			wol;
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	enum macb_hw_dma_cap hw_dma_cap;
+#endif
 };
 
 static inline bool macb_is_gem(struct macb *bp)
-- 
2.4.5

^ permalink raw reply related

* Re: [PATCH] net, bpf: use kvzalloc helper
From: kbuild test robot @ 2017-01-26 14:58 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Daniel Borkmann, Alexei Starovoitov, Andrew Morton,
	Vlastimil Babka, Mel Gorman, Johannes Weiner, linux-mm, LKML,
	netdev@vger.kernel.org, marcelo.leitner
In-Reply-To: <20170126141349.GN6590@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 1613 bytes --]

Hi Michal,

[auto build test ERROR on next-20170125]
[cannot apply to linus/master linux/master nf-next/master v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.10-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/net-bpf-use-kvzalloc-helper/20170126-221904
config: x86_64-randconfig-x014-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   kernel/bpf/syscall.c: In function 'bpf_map_area_alloc':
>> kernel/bpf/syscall.c:61:9: error: implicit declaration of function 'kvzalloc' [-Werror=implicit-function-declaration]
     return kvzalloc(size, GFP_USER);
            ^~~~~~~~
>> kernel/bpf/syscall.c:61:9: warning: return makes pointer from integer without a cast [-Wint-conversion]
     return kvzalloc(size, GFP_USER);
            ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/kvzalloc +61 kernel/bpf/syscall.c

    55	{
    56		/*
    57		 * FIXME: we would really like to not trigger the OOM killer and rather
    58		 * fail instead. This is not supported right now. Please nag MM people
    59		 * if these OOM start bothering people.
    60		 */
  > 61		return kvzalloc(size, GFP_USER);
    62	}
    63	
    64	void bpf_map_area_free(void *area)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24846 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v2 0/2] net: ipv6: Improve user experience with multipath routes
From: Nicolas Dichtel @ 2017-01-26 14:58 UTC (permalink / raw)
  To: David Miller, dsa; +Cc: netdev
In-Reply-To: <20170124.110616.2098461870069191491.davem@davemloft.net>

Le 24/01/2017 à 17:06, David Miller a écrit :
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Fri, 20 Jan 2017 12:31:44 -0700
> 
>> BTW, I am in favor of not requiring a user API for this but just
>> doing it. I can't imagine anyone working with multipath routes not
>> wanting the efficiency of the RTA_MULTIPATH attribute. These patches
>> require an API only because of the rule not to break userspace. If
>> we conclude to just do it without an API, the multipath_add and
>> multipath_del need to be modified to only send a notification once
>> at the end of the actions.
> 
> After some consideration, I agree.
Sorry for my late reply but I fear that this will break some userland apps like
quagga.
I agree that it would be better but the API is here since v3.8, ie 4 years.


Regards,
Nicolas

^ permalink raw reply

* Re: IPv6-UDP 0x0000 checksum
From: Johannes Berg @ 2017-01-26 14:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless
In-Reply-To: <1485441942.5145.131.camel-XN9IlZ5yJG9HTL0Zs8A6p+yfmBU6pStAUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>

On Thu, 2017-01-26 at 06:45 -0800, Eric Dumazet wrote:
> On Thu, 2017-01-26 at 14:49 +0100, Johannes Berg wrote:
> 
> > Oops, sorry - receive. We can only indicate "CHECKSUM_UNNECESSARY",
> > nothing more advanced right now, but right now we'd indicate that
> > if
> > the packet had 0x0000 in the checksum field, but should've had
> > 0xffff.
> > 
> > On TX I believe we actually do in HW exactly what your patch just
> > did.
> 
> Can you describe the visible effects of this problem ?
> 
> Is that because of a conversion we might do later to
> CHECKSUM_COMPLETE ?

Unfortunately, I haven't been able to actually test this yet. I also
didn't find the code that would drop frames with CSUM 0 either, so I'm
thinking - for now - that if all the csum handling is skipped, dropping
0 csum frames would also be, and then we'd accept a frame we should
actually have dropped.

I'll go test this I guess :)

Any pointers to where 0 csum frames are dropped?

johannes

^ permalink raw reply

* Re: IPv6-UDP 0x0000 checksum
From: Eric Dumazet @ 2017-01-26 14:45 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-wireless
In-Reply-To: <1485438546.14760.7.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

On Thu, 2017-01-26 at 14:49 +0100, Johannes Berg wrote:

> Oops, sorry - receive. We can only indicate "CHECKSUM_UNNECESSARY",
> nothing more advanced right now, but right now we'd indicate that if
> the packet had 0x0000 in the checksum field, but should've had 0xffff.
> 
> On TX I believe we actually do in HW exactly what your patch just did.

Can you describe the visible effects of this problem ?

Is that because of a conversion we might do later to CHECKSUM_COMPLETE ?

^ permalink raw reply

* Re: [PATCH] net, bpf: use kvzalloc helper
From: kbuild test robot @ 2017-01-26 14:37 UTC (permalink / raw)
  To: Michal Hocko
  Cc: kbuild-all, Daniel Borkmann, Alexei Starovoitov, Andrew Morton,
	Vlastimil Babka, Mel Gorman, Johannes Weiner, linux-mm, LKML,
	netdev@vger.kernel.org, marcelo.leitner
In-Reply-To: <20170126141349.GN6590@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

Hi Michal,

[auto build test ERROR on next-20170125]
[cannot apply to linus/master linux/master nf-next/master v4.9-rc8 v4.9-rc7 v4.9-rc6 v4.10-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michal-Hocko/net-bpf-use-kvzalloc-helper/20170126-221904
config: x86_64-randconfig-x017-201704 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   net/netfilter/x_tables.c: In function 'xt_alloc_table_info':
>> net/netfilter/x_tables.c:1012:9: error: implicit declaration of function 'kvzalloc' [-Werror=implicit-function-declaration]
     info = kvzalloc(sz, GFP_KERNEL);
            ^~~~~~~~
>> net/netfilter/x_tables.c:1012:7: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     info = kvzalloc(sz, GFP_KERNEL);
          ^
   cc1: some warnings being treated as errors

vim +/kvzalloc +1012 net/netfilter/x_tables.c

  1006	
  1007		/*
  1008		 * FIXME: we would really like to not trigger the OOM killer and rather
  1009		 * fail instead. This is not supported right now. Please nag MM people
  1010		 * if these OOM start bothering people.
  1011		 */
> 1012		info = kvzalloc(sz, GFP_KERNEL);
  1013		info->size = size;
  1014		return info;
  1015	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34703 bytes --]

^ permalink raw reply

* Re: [PATCH RFC ipsec-next 2/2] xfrm: Add a device independent napi instance
From: Eric Dumazet @ 2017-01-26 14:26 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: netdev, David Miller, Sowmini Varadhan, Ilan Tayari
In-Reply-To: <1485413456-31197-3-git-send-email-steffen.klassert@secunet.com>

On Thu, 2017-01-26 at 07:50 +0100, Steffen Klassert wrote:
> This patch adds a napi instance for IPsec GRO.
> With this, we handle IPsec GRO with no impact
> on the generic networking code.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/xfrm/xfrm_input.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 6e3f025..c5fc12d 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -21,6 +21,90 @@
>  static DEFINE_SPINLOCK(xfrm_input_afinfo_lock);
>  static struct xfrm_input_afinfo __rcu *xfrm_input_afinfo[NPROTO];
>  
> +struct xfrm_napi {
> +	struct sk_buff_head	napi_skbs;
> +	struct napi_struct	napi;
> +};
> +
> +static struct xfrm_napi __percpu *napis;
> +
> +static void xfrm_gro_receive(struct sk_buff *skb)
> +{
> +	struct xfrm_napi *xnapi;
> +	struct net_device *dev = skb->dev;
> +
> +	if (!napis || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
> +		netif_rx(skb);
> +		return;
> +	}
> +
> +	xnapi = this_cpu_ptr(napis);
> +
> +	if (skb_queue_len(&xnapi->napi_skbs) > netdev_max_backlog) {
> +		atomic_long_inc(&dev->rx_dropped);
> +		kfree_skb(skb);
> +		return;
> +	}
> +
> +	__skb_queue_tail(&xnapi->napi_skbs, skb);
> +	if (skb_queue_len(&xnapi->napi_skbs) == 1)
> +		napi_schedule(&xnapi->napi);
> +}
> +
> +static int xfrm_gro_poll(struct napi_struct *napi, int budget)
> +{
> +	unsigned long flags;
> +	struct sk_buff *skb;
> +	int work_done = 0;
> +	struct xfrm_napi *xnapi = container_of(napi, struct xfrm_napi, napi);
> +
> +	while (work_done < budget) {
> +		skb = __skb_dequeue(&xnapi->napi_skbs);
> +		if (!skb)
> +			break;
> +		napi_gro_receive(napi, skb);
> +		work_done++;
> +	}
> +
> +	if (work_done < budget) {
> +		napi_gro_flush(napi, false);
> +
> +		if (likely(list_empty(&napi->poll_list))) {
> +			WARN_ON_ONCE(!test_and_clear_bit(NAPI_STATE_SCHED, &napi->state));
> +		} else {
> +			/* If n->poll_list is not empty, we need to mask irqs */
> +			local_irq_save(flags);
> +			__napi_complete(napi);
> +			local_irq_restore(flags);
> +		}
> +	}


The following should work and contain less copy/pasted code.

          if (work_done < budget)
               napi_complete(napi);


> +
> +	return work_done;
> +}
> +
> +static void xfrm_gro_init(void)
> +{
> +	int i;
> +
> +	/* Napi remains disabled if we can't alloc memory. */
> +	napis = alloc_percpu(struct xfrm_napi);

napis can be NULL here.

> +
> +	for_each_possible_cpu(i) {
> +		struct xfrm_napi *xnapi = per_cpu_ptr(napis, i);
> +
> +		__skb_queue_head_init(&xnapi->napi_skbs);
> +
> +		set_bit(NAPI_STATE_NO_BUSY_POLL, &xnapi->napi.state);
> +
> +		INIT_LIST_HEAD(&xnapi->napi.poll_list);
> +		xnapi->napi.poll = xfrm_gro_poll;
> +		xnapi->napi.weight = 64;
> +		xnapi->napi.gro_list = NULL;
> +		xnapi->napi.gro_count = 0;
> +		xnapi->napi.skb = NULL;
> +	}
> +}


Alternative would be to use a 

static struct net_device xfrm_napi_anchor_device;

and use gro_cell


I will present in next netdev conference a work based on NAPI,
and having less hand coded napi logic will help a lot.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: phy: Marvell: Add mv88e6390 internal PHY
From: Andrew Lunn @ 2017-01-26 14:16 UTC (permalink / raw)
  To: Gregory CLEMENT; +Cc: David Miller, netdev, Vivien Didelot
In-Reply-To: <87y3xyyqrh.fsf@free-electrons.com>

On Thu, Jan 26, 2017 at 01:01:06PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
>  
>  On mer., janv. 25 2017, Andrew Lunn <andrew@lunn.ch> wrote:
> 
> 
> > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> > index b5b73ff4329a..9debe1a4a6f6 100644
> > --- a/drivers/net/phy/marvell.c
> > +++ b/drivers/net/phy/marvell.c
> > @@ -2140,6 +2140,25 @@ static struct phy_driver marvell_drivers[] = {
> >  		.get_strings = marvell_get_strings,
> >  		.get_stats = marvell_get_stats,
> >  	},
> > +	{
> > +		.phy_id = MARVELL_PHY_ID_88E6390,
> > +		.phy_id_mask = MARVELL_PHY_ID_MASK,
> > +		.name = "Marvell 88E6390",
> > +		.features = PHY_GBIT_FEATURES,
> > +		.flags = PHY_HAS_INTERRUPT,
> > +		.probe = marvell_probe,
> 
> In order to get the temperature support I also had to modify the line
> above by using m88e1510_probe instead of the marvell_probe. Indeed the
> hwmon register was not done marvell_probe.

Yes, this is historic.

These patches come from one branch where i have all my mv88e6390
code. I had them in the opposite order to what we have now. I had this
PHY work around first, and then the temperature sensor move patches.

But to get the code into mainline, i pulled the patches apart. The
temperature code has hit mainline first, now follow the PHY ID
workaround. I need to fix this probe call as a result.

Although the temperature sensor does not work yet for the 6390, i'm
going to assume it will at some point work. So i think it is O.K. to
use the same ID, and have the 6390 PHYs return -25000mC until we
figure out what is wrong.

       Andrew

^ permalink raw reply

* Re: [PATCH 0/6 v3] kvmalloc
From: Michal Hocko @ 2017-01-26 14:13 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
	Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
	marcelo.leitner
In-Reply-To: <20170126134004.GM6590@dhcp22.suse.cz>

On Thu 26-01-17 14:40:04, Michal Hocko wrote:
> On Thu 26-01-17 14:10:06, Daniel Borkmann wrote:
> > On 01/26/2017 12:58 PM, Michal Hocko wrote:
> > > On Thu 26-01-17 12:33:55, Daniel Borkmann wrote:
> > > > On 01/26/2017 11:08 AM, Michal Hocko wrote:
> > > [...]
> > > > > If you disagree I can drop the bpf part of course...
> > > > 
> > > > If we could consolidate these spots with kvmalloc() eventually, I'm
> > > > all for it. But even if __GFP_NORETRY is not covered down to all
> > > > possible paths, it kind of does have an effect already of saying
> > > > 'don't try too hard', so would it be harmful to still keep that for
> > > > now? If it's not, I'd personally prefer to just leave it as is until
> > > > there's some form of support by kvmalloc() and friends.
> > > 
> > > Well, you can use kvmalloc(size, GFP_KERNEL|__GFP_NORETRY). It is not
> > > disallowed. It is not _supported_ which means that if it doesn't work as
> > > you expect you are on your own. Which is actually the situation right
> > > now as well. But I still think that this is just not right thing to do.
> > > Even though it might happen to work in some cases it gives a false
> > > impression of a solution. So I would rather go with
> > 
> > Hmm. 'On my own' means, we could potentially BUG somewhere down the
> > vmalloc implementation, etc, presumably? So it might in-fact be
> > harmful to pass that, right?
> 
> No it would mean that it might eventually hit the behavior which you are
> trying to avoid - in other words it may invoke OOM killer even though
> __GFP_NORETRY means giving up before any system wide disruptive actions
> a re taken.

I will separate both bpf and netfilter hunks into its own patch with the
clarification. Does the following look better?
---
>From ab6b2d724228e4abcc69c44f5ab1ce91009aa91d Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Thu, 26 Jan 2017 14:59:21 +0100
Subject: [PATCH] net, bpf: use kvzalloc helper

both bpf_map_area_alloc and xt_alloc_table_info try really hard to
play nicely with large memory requests which can be triggered from
the userspace (by an admin). See 5bad87348c70 ("netfilter: x_tables:
avoid warn and OOM killer on vmalloc call") resp. d407bd25a204 ("bpf:
don't trigger OOM killer under pressure with map alloc").

The current allocation pattern strongly resembles kvmalloc helper except
for one thing __GFP_NORETRY is not used for the vmalloc fallback. The
main reason why kvmalloc doesn't really support __GFP_NORETRY is
because vmalloc doesn't support this flag properly and it is far from
straightforward to make it understand it because there are some hard
coded GFP_KERNEL allocation deep in the call chains. This patch simply
replaces the open coded variants with kvmalloc and puts a note to
push on MM people to support __GFP_NORETRY in kvmalloc it this turns out
to be really needed along with OOM report pointing at vmalloc.

If there is an immediate need and no full support yet then
	kvmalloc(size, gfp | __GFP_NORETRY)
will work as good as __vmalloc(gfp | __GFP_NORETRY) - in other words it
might trigger the OOM in some cases.

Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 kernel/bpf/syscall.c     | 19 +++++--------------
 net/netfilter/x_tables.c | 16 ++++++----------
 2 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 19b6129eab23..a6dc4d596f14 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -53,21 +53,12 @@ void bpf_register_map_type(struct bpf_map_type_list *tl)
 
 void *bpf_map_area_alloc(size_t size)
 {
-	/* We definitely need __GFP_NORETRY, so OOM killer doesn't
-	 * trigger under memory pressure as we really just want to
-	 * fail instead.
+	/*
+	 * FIXME: we would really like to not trigger the OOM killer and rather
+	 * fail instead. This is not supported right now. Please nag MM people
+	 * if these OOM start bothering people.
 	 */
-	const gfp_t flags = __GFP_NOWARN | __GFP_NORETRY | __GFP_ZERO;
-	void *area;
-
-	if (size <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER)) {
-		area = kmalloc(size, GFP_USER | flags);
-		if (area != NULL)
-			return area;
-	}
-
-	return __vmalloc(size, GFP_KERNEL | __GFP_HIGHMEM | flags,
-			 PAGE_KERNEL);
+	return kvzalloc(size, GFP_USER);
 }
 
 void bpf_map_area_free(void *area)
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index d529989f5791..ba8ba633da72 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -995,16 +995,12 @@ struct xt_table_info *xt_alloc_table_info(unsigned int size)
 	if ((SMP_ALIGN(size) >> PAGE_SHIFT) + 2 > totalram_pages)
 		return NULL;
 
-	if (sz <= (PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER))
-		info = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
-	if (!info) {
-		info = __vmalloc(sz, GFP_KERNEL | __GFP_NOWARN |
-				     __GFP_NORETRY | __GFP_HIGHMEM,
-				 PAGE_KERNEL);
-		if (!info)
-			return NULL;
-	}
-	memset(info, 0, sizeof(*info));
+	/*
+	 * FIXME: we would really like to not trigger the OOM killer and rather
+	 * fail instead. This is not supported right now. Please nag MM people
+	 * if these OOM start bothering people.
+	 */
+	info = kvzalloc(sz, GFP_KERNEL);
 	info->size = size;
 	return info;
 }
-- 
2.11.0


-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: IPv6-UDP 0x0000 checksum
From: Johannes Berg @ 2017-01-26 13:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, linux-wireless
In-Reply-To: <1485438299.5145.117.camel@edumazet-glaptop3.roam.corp.google.com>

On Thu, 2017-01-26 at 05:44 -0800, Eric Dumazet wrote:
> On Thu, 2017-01-26 at 14:27 +0100, Johannes Berg wrote:
> > Hi,
> > 
> > It looks like right now we may have a hardware bug and accept
> > 0x0000 as
> > valid, when the outcome of the calculation is 0xffff.
> > 
> > What do you think we should do about this?
> > 
> > We could ignore the issue entirely, since 0 wasn't ever supposed to
> > be
> > sent anyway - but then we don't drop frames that we should drop. I
> > didn't manage to find the code in the IPv6/UDP stack that even does
> > that, but I assume it's there somewhere.
> > 
> > Alternatively, we could parse the packet to find the checksum
> > inside,
> > and if it's 0 then don't report CHECKSUM_UNNECESSARY, but that
> > seems
> > rather expensive/difficult due to the IPv6 variable header and all
> > that. If we wanted to go this route, are there any helper functions
> > for
> > this?
> > 
> > Unfortunately, in the current devices, we neither have a complete
> > indication that the packet was even UDP-IPv6, nor do we have the
> > raw
> > csum or anything like that. I think they're adding that to the next
> > hardware spin, but we probably need to address this issue now.

> Is this a xmit or rcv problem ?

Oops, sorry - receive. We can only indicate "CHECKSUM_UNNECESSARY",
nothing more advanced right now, but right now we'd indicate that if
the packet had 0x0000 in the checksum field, but should've had 0xffff.

On TX I believe we actually do in HW exactly what your patch just did.

johannes

^ permalink raw reply

* Re: IPv6-UDP 0x0000 checksum
From: Eric Dumazet @ 2017-01-26 13:44 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, linux-wireless
In-Reply-To: <1485437276.14760.3.camel@sipsolutions.net>

On Thu, 2017-01-26 at 14:27 +0100, Johannes Berg wrote:
> Hi,
> 
> It looks like right now we may have a hardware bug and accept 0x0000 as
> valid, when the outcome of the calculation is 0xffff.
> 
> What do you think we should do about this?
> 
> We could ignore the issue entirely, since 0 wasn't ever supposed to be
> sent anyway - but then we don't drop frames that we should drop. I
> didn't manage to find the code in the IPv6/UDP stack that even does
> that, but I assume it's there somewhere.
> 
> Alternatively, we could parse the packet to find the checksum inside,
> and if it's 0 then don't report CHECKSUM_UNNECESSARY, but that seems
> rather expensive/difficult due to the IPv6 variable header and all
> that. If we wanted to go this route, are there any helper functions for
> this?
> 
> Unfortunately, in the current devices, we neither have a complete
> indication that the packet was even UDP-IPv6, nor do we have the raw
> csum or anything like that. I think they're adding that to the next
> hardware spin, but we probably need to address this issue now.
> 
> johannes

Hi Johannes

I am afraid information is missing.

Is this a xmit or rcv problem ?

I recently fixed an issue, could this be this ?

commit 4f2e4ad56a65f3b7d64c258e373cb71e8d2499f4
Author: Eric Dumazet <edumazet@google.com>
Date:   Sat Oct 29 11:02:36 2016 -0700

    net: mangle zero checksum in skb_checksum_help()
    
    Sending zero checksum is ok for TCP, but not for UDP.
    
    UDPv6 receiver should by default drop a frame with a 0 checksum,
    and UDPv4 would not verify the checksum and might accept a corrupted
    packet.
    
    Simply replace such checksum by 0xffff, regardless of transport.
    
    This error was caught on SIT tunnels, but seems generic.
    
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Maciej Żenczykowski <maze@google.com>
    Cc: Willem de Bruijn <willemb@google.com>
    Acked-by: Maciej Żenczykowski <maze@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/core/dev.c b/net/core/dev.c
index 820bac239738eb021354ac95ca5bbdff1840cb8e..eaad4c28069ff523ac784bf2dffd0acff82341a0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2484,7 +2484,7 @@ int skb_checksum_help(struct sk_buff *skb)
                        goto out;
        }
 
-       *(__sum16 *)(skb->data + offset) = csum_fold(csum);
+       *(__sum16 *)(skb->data + offset) = csum_fold(csum) ?: CSUM_MANGLED_0;
 out_set_summed:
        skb->ip_summed = CHECKSUM_NONE;
 out:

^ permalink raw reply related

* Re: [PATCH 0/6 v3] kvmalloc
From: Michal Hocko @ 2017-01-26 13:40 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Andrew Morton, Vlastimil Babka, Mel Gorman,
	Johannes Weiner, linux-mm, LKML, netdev@vger.kernel.org,
	marcelo.leitner
In-Reply-To: <5889F52E.7030602@iogearbox.net>

On Thu 26-01-17 14:10:06, Daniel Borkmann wrote:
> On 01/26/2017 12:58 PM, Michal Hocko wrote:
> > On Thu 26-01-17 12:33:55, Daniel Borkmann wrote:
> > > On 01/26/2017 11:08 AM, Michal Hocko wrote:
> > [...]
> > > > If you disagree I can drop the bpf part of course...
> > > 
> > > If we could consolidate these spots with kvmalloc() eventually, I'm
> > > all for it. But even if __GFP_NORETRY is not covered down to all
> > > possible paths, it kind of does have an effect already of saying
> > > 'don't try too hard', so would it be harmful to still keep that for
> > > now? If it's not, I'd personally prefer to just leave it as is until
> > > there's some form of support by kvmalloc() and friends.
> > 
> > Well, you can use kvmalloc(size, GFP_KERNEL|__GFP_NORETRY). It is not
> > disallowed. It is not _supported_ which means that if it doesn't work as
> > you expect you are on your own. Which is actually the situation right
> > now as well. But I still think that this is just not right thing to do.
> > Even though it might happen to work in some cases it gives a false
> > impression of a solution. So I would rather go with
> 
> Hmm. 'On my own' means, we could potentially BUG somewhere down the
> vmalloc implementation, etc, presumably? So it might in-fact be
> harmful to pass that, right?

No it would mean that it might eventually hit the behavior which you are
trying to avoid - in other words it may invoke OOM killer even though
__GFP_NORETRY means giving up before any system wide disruptive actions
a re taken.

> 
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 8697f43cf93c..a6dc4d596f14 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -53,6 +53,11 @@ void bpf_register_map_type(struct bpf_map_type_list *tl)
> > 
> >   void *bpf_map_area_alloc(size_t size)
> >   {
> > +	/*
> > +	 * FIXME: we would really like to not trigger the OOM killer and rather
> > +	 * fail instead. This is not supported right now. Please nag MM people
> > +	 * if these OOM start bothering people.
> > +	 */
> 
> Ok, I know this is out of scope for this series, but since i) this
> is _not_ the _only_ spot right now which has such a construct and ii)
> I am already kind of nagging a bit ;), my question would be, what
> would it take to start supporting it?

propagate gfp mask all the way down from vmalloc to all places which
might allocate down the path and especially page table allocation
function are PITA because they are really deep. This is a lot of work...

But realistically, how big is this problem really? Is it really worth
it? You said this is an admin only interface and admin can kill the
machine by OOM and other means already.

Moreover and I should probably mention it explicitly, your d407bd25a204b
reduced the likelyhood of oom for other reason. kmalloc used GPF_USER
previously and with order > 0 && order <= PAGE_ALLOC_COSTLY_ORDER this
could indeed hit the OOM e.g. due to memory fragmentation. It would be
much harder to hit the OOM killer from vmalloc which doesn't issue
higher order allocation requests. Or have you ever seen the OOM killer
pointing to the vmalloc fallback path?
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ 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