Netdev List
 help / color / mirror / Atom feed
* [PATCH v7 bpf-next 04/10] xdp: Helper function to clear kernel pointers in xdp_frame
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

xdp_frame has kernel pointers which should not be readable from bpf
programs. When we want to reuse xdp_frame region but it may be read by
bpf programs later, we can use this helper to clear kernel pointers.
This is more efficient than calling memset() for the entire struct.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/net/xdp.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/xdp.h b/include/net/xdp.h
index fcb033f..76b9525 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -84,6 +84,13 @@ struct xdp_frame {
 	struct net_device *dev_rx; /* used by cpumap */
 };
 
+/* Clear kernel pointers in xdp_frame */
+static inline void xdp_scrub_frame(struct xdp_frame *frame)
+{
+	frame->data = NULL;
+	frame->dev_rx = NULL;
+}
+
 /* Convert xdp_buff to xdp_frame */
 static inline
 struct xdp_frame *convert_to_xdp_frame(struct xdp_buff *xdp)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 05/10] veth: Handle xdp_frames in xdp napi ring
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

This is preparation for XDP TX and ndo_xdp_xmit.
This allows napi handler to handle xdp_frames through xdp ring as well
as sk_buff.

v7:
- Use xdp_scrub_frame() instead of memset().

v3:
- Revert v2 change around rings and use a flag to differentiate skb and
  xdp_frame, since bulk skb xmit makes little performance difference
  for now.

v2:
- Use another ring instead of using flag to differentiate skb and
  xdp_frame. This approach makes bulk skb transmit possible in
  veth_xmit later.
- Clear xdp_frame feilds in skb->head.
- Implement adjust_tail.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
 drivers/net/veth.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 82 insertions(+), 5 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 9edf104..9993878 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -22,12 +22,12 @@
 #include <linux/bpf.h>
 #include <linux/filter.h>
 #include <linux/ptr_ring.h>
-#include <linux/skb_array.h>
 #include <linux/bpf_trace.h>
 
 #define DRV_NAME	"veth"
 #define DRV_VERSION	"1.0"
 
+#define VETH_XDP_FLAG		BIT(0)
 #define VETH_RING_SIZE		256
 #define VETH_XDP_HEADROOM	(XDP_PACKET_HEADROOM + NET_IP_ALIGN)
 
@@ -115,6 +115,24 @@ static void veth_get_ethtool_stats(struct net_device *dev,
 
 /* general routines */
 
+static bool veth_is_xdp_frame(void *ptr)
+{
+	return (unsigned long)ptr & VETH_XDP_FLAG;
+}
+
+static void *veth_ptr_to_xdp(void *ptr)
+{
+	return (void *)((unsigned long)ptr & ~VETH_XDP_FLAG);
+}
+
+static void veth_ptr_free(void *ptr)
+{
+	if (veth_is_xdp_frame(ptr))
+		xdp_return_frame(veth_ptr_to_xdp(ptr));
+	else
+		kfree_skb(ptr);
+}
+
 static void __veth_xdp_flush(struct veth_priv *priv)
 {
 	/* Write ptr_ring before reading rx_notify_masked */
@@ -249,6 +267,61 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
 	return skb;
 }
 
+static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
+					struct xdp_frame *frame)
+{
+	int len = frame->len, delta = 0;
+	struct bpf_prog *xdp_prog;
+	unsigned int headroom;
+	struct sk_buff *skb;
+
+	rcu_read_lock();
+	xdp_prog = rcu_dereference(priv->xdp_prog);
+	if (likely(xdp_prog)) {
+		struct xdp_buff xdp;
+		u32 act;
+
+		xdp.data_hard_start = frame->data - frame->headroom;
+		xdp.data = frame->data;
+		xdp.data_end = frame->data + frame->len;
+		xdp.data_meta = frame->data - frame->metasize;
+		xdp.rxq = &priv->xdp_rxq;
+
+		act = bpf_prog_run_xdp(xdp_prog, &xdp);
+
+		switch (act) {
+		case XDP_PASS:
+			delta = frame->data - xdp.data;
+			len = xdp.data_end - xdp.data;
+			break;
+		default:
+			bpf_warn_invalid_xdp_action(act);
+		case XDP_ABORTED:
+			trace_xdp_exception(priv->dev, xdp_prog, act);
+		case XDP_DROP:
+			goto err_xdp;
+		}
+	}
+	rcu_read_unlock();
+
+	headroom = frame->data - delta - (void *)frame;
+	skb = veth_build_skb(frame, headroom, len, 0);
+	if (!skb) {
+		xdp_return_frame(frame);
+		goto err;
+	}
+
+	xdp_scrub_frame(frame);
+	skb->protocol = eth_type_trans(skb, priv->dev);
+err:
+	return skb;
+err_xdp:
+	rcu_read_unlock();
+	xdp_return_frame(frame);
+
+	return NULL;
+}
+
 static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 					struct sk_buff *skb)
 {
@@ -359,12 +432,16 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget)
 	int i, done = 0;
 
 	for (i = 0; i < budget; i++) {
-		struct sk_buff *skb = __ptr_ring_consume(&priv->xdp_ring);
+		void *ptr = __ptr_ring_consume(&priv->xdp_ring);
+		struct sk_buff *skb;
 
-		if (!skb)
+		if (!ptr)
 			break;
 
-		skb = veth_xdp_rcv_skb(priv, skb);
+		if (veth_is_xdp_frame(ptr))
+			skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr));
+		else
+			skb = veth_xdp_rcv_skb(priv, ptr);
 
 		if (skb)
 			napi_gro_receive(&priv->xdp_napi, skb);
@@ -417,7 +494,7 @@ static void veth_napi_del(struct net_device *dev)
 	napi_disable(&priv->xdp_napi);
 	netif_napi_del(&priv->xdp_napi);
 	priv->rx_notify_masked = false;
-	ptr_ring_cleanup(&priv->xdp_ring, __skb_array_destroy_skb);
+	ptr_ring_cleanup(&priv->xdp_ring, veth_ptr_free);
 }
 
 static int veth_enable_xdp(struct net_device *dev)
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 06/10] veth: Add ndo_xdp_xmit
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

This allows NIC's XDP to redirect packets to veth. The destination veth
device enqueues redirected packets to the napi ring of its peer, then
they are processed by XDP on its peer veth device.
This can be thought as calling another XDP program by XDP program using
REDIRECT, when the peer enables driver XDP.

Note that when the peer veth device does not set driver xdp, redirected
packets will be dropped because the peer is not ready for NAPI.

v4:
- Don't use xdp_ok_fwd_dev() because checking IFF_UP is not necessary.
  Add comments about it and check only MTU.

v2:
- Drop the part converting xdp_frame into skb when XDP is not enabled.
- Implement bulk interface of ndo_xdp_xmit.
- Implement XDP_XMIT_FLUSH bit and drop ndo_xdp_flush.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
 drivers/net/veth.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 9993878..3e1582a 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -17,6 +17,7 @@
 #include <net/rtnetlink.h>
 #include <net/dst.h>
 #include <net/xfrm.h>
+#include <net/xdp.h>
 #include <linux/veth.h>
 #include <linux/module.h>
 #include <linux/bpf.h>
@@ -125,6 +126,11 @@ static void *veth_ptr_to_xdp(void *ptr)
 	return (void *)((unsigned long)ptr & ~VETH_XDP_FLAG);
 }
 
+static void *veth_xdp_to_ptr(void *ptr)
+{
+	return (void *)((unsigned long)ptr | VETH_XDP_FLAG);
+}
+
 static void veth_ptr_free(void *ptr)
 {
 	if (veth_is_xdp_frame(ptr))
@@ -267,6 +273,50 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
 	return skb;
 }
 
+static int veth_xdp_xmit(struct net_device *dev, int n,
+			 struct xdp_frame **frames, u32 flags)
+{
+	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+	struct net_device *rcv;
+	unsigned int max_len;
+	int i, drops = 0;
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+		return -EINVAL;
+
+	rcv = rcu_dereference(priv->peer);
+	if (unlikely(!rcv))
+		return -ENXIO;
+
+	rcv_priv = netdev_priv(rcv);
+	/* Non-NULL xdp_prog ensures that xdp_ring is initialized on receive
+	 * side. This means an XDP program is loaded on the peer and the peer
+	 * device is up.
+	 */
+	if (!rcu_access_pointer(rcv_priv->xdp_prog))
+		return -ENXIO;
+
+	max_len = rcv->mtu + rcv->hard_header_len + VLAN_HLEN;
+
+	spin_lock(&rcv_priv->xdp_ring.producer_lock);
+	for (i = 0; i < n; i++) {
+		struct xdp_frame *frame = frames[i];
+		void *ptr = veth_xdp_to_ptr(frame);
+
+		if (unlikely(frame->len > max_len ||
+			     __ptr_ring_produce(&rcv_priv->xdp_ring, ptr))) {
+			xdp_return_frame_rx_napi(frame);
+			drops++;
+		}
+	}
+	spin_unlock(&rcv_priv->xdp_ring.producer_lock);
+
+	if (flags & XDP_XMIT_FLUSH)
+		__veth_xdp_flush(rcv_priv);
+
+	return n - drops;
+}
+
 static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 					struct xdp_frame *frame)
 {
@@ -767,6 +817,7 @@ static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
 	.ndo_features_check	= passthru_features_check,
 	.ndo_set_rx_headroom	= veth_set_rx_headroom,
 	.ndo_bpf		= veth_xdp,
+	.ndo_xdp_xmit		= veth_xdp_xmit,
 };
 
 #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 07/10] bpf: Make redirect_info accessible from modules
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

We are going to add kern_flags field in redirect_info for kernel
internal use.
In order to avoid function call to access the flags, make redirect_info
accessible from modules. Also as it is now non-static, add prefix bpf_
to redirect_info.

v6:
- Fix sparse warning around EXPORT_SYMBOL.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/linux/filter.h | 10 ++++++++++
 net/core/filter.c      | 29 +++++++++++------------------
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index c73dd73..4717af8 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -537,6 +537,16 @@ struct sk_msg_buff {
 	struct list_head list;
 };
 
+struct bpf_redirect_info {
+	u32 ifindex;
+	u32 flags;
+	struct bpf_map *map;
+	struct bpf_map *map_to_flush;
+	unsigned long   map_owner;
+};
+
+DECLARE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
+
 /* Compute the linear packet data range [data, data_end) which
  * will be accessed by various program types (cls_bpf, act_bpf,
  * lwt, ...). Subsystems allowing direct data access must (!)
diff --git a/net/core/filter.c b/net/core/filter.c
index 104d560..2766a55 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2080,19 +2080,12 @@ static int __bpf_redirect(struct sk_buff *skb, struct net_device *dev,
 	.arg3_type      = ARG_ANYTHING,
 };
 
-struct redirect_info {
-	u32 ifindex;
-	u32 flags;
-	struct bpf_map *map;
-	struct bpf_map *map_to_flush;
-	unsigned long   map_owner;
-};
-
-static DEFINE_PER_CPU(struct redirect_info, redirect_info);
+DEFINE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
+EXPORT_PER_CPU_SYMBOL_GPL(bpf_redirect_info);
 
 BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 
 	if (unlikely(flags & ~(BPF_F_INGRESS)))
 		return TC_ACT_SHOT;
@@ -2105,7 +2098,7 @@ struct redirect_info {
 
 int skb_do_redirect(struct sk_buff *skb)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	struct net_device *dev;
 
 	dev = dev_get_by_index_rcu(dev_net(skb->dev), ri->ifindex);
@@ -3198,7 +3191,7 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
 
 void xdp_do_flush_map(void)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	struct bpf_map *map = ri->map_to_flush;
 
 	ri->map_to_flush = NULL;
@@ -3243,7 +3236,7 @@ static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
 static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
 			       struct bpf_prog *xdp_prog)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	unsigned long map_owner = ri->map_owner;
 	struct bpf_map *map = ri->map;
 	u32 index = ri->ifindex;
@@ -3283,7 +3276,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
 int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 		    struct bpf_prog *xdp_prog)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	struct net_device *fwd;
 	u32 index = ri->ifindex;
 	int err;
@@ -3315,7 +3308,7 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
 				       struct xdp_buff *xdp,
 				       struct bpf_prog *xdp_prog)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	unsigned long map_owner = ri->map_owner;
 	struct bpf_map *map = ri->map;
 	u32 index = ri->ifindex;
@@ -3366,7 +3359,7 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
 int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 			    struct xdp_buff *xdp, struct bpf_prog *xdp_prog)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 	u32 index = ri->ifindex;
 	struct net_device *fwd;
 	int err = 0;
@@ -3397,7 +3390,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 
 BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 
 	if (unlikely(flags))
 		return XDP_ABORTED;
@@ -3421,7 +3414,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
 BPF_CALL_4(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex, u64, flags,
 	   unsigned long, map_owner)
 {
-	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 
 	if (unlikely(flags))
 		return XDP_ABORTED;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 08/10] xdp: Helpers for disabling napi_direct of xdp_return_frame
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

We need some mechanism to disable napi_direct on calling
xdp_return_frame_rx_napi() from some context.
When veth gets support of XDP_REDIRECT, it will redirects packets which
are redirected from other devices. On redirection veth will reuse
xdp_mem_info of the redirection source device to make return_frame work.
But in this case .ndo_xdp_xmit() called from veth redirection uses
xdp_mem_info which is not guarded by NAPI, because the .ndo_xdp_xmit()
is not called directly from the rxq which owns the xdp_mem_info.

This approach introduces a flag in bpf_redirect_info to indicate that
napi_direct should be disabled even when _rx_napi variant is used as
well as helper functions to use it.

A NAPI handler who wants to use this flag needs to call
xdp_set_return_frame_no_direct() before processing packets, and call
xdp_clear_return_frame_no_direct() after xdp_do_flush_map() before
exiting NAPI.

v4:
- Use bpf_redirect_info for storing the flag instead of xdp_mem_info to
  avoid per-frame copy cost.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 include/linux/filter.h | 25 +++++++++++++++++++++++++
 net/core/xdp.c         |  6 ++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 4717af8..2b072da 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -543,10 +543,14 @@ struct bpf_redirect_info {
 	struct bpf_map *map;
 	struct bpf_map *map_to_flush;
 	unsigned long   map_owner;
+	u32 kern_flags;
 };
 
 DECLARE_PER_CPU(struct bpf_redirect_info, bpf_redirect_info);
 
+/* flags for bpf_redirect_info kern_flags */
+#define BPF_RI_F_RF_NO_DIRECT	BIT(0)	/* no napi_direct on return_frame */
+
 /* Compute the linear packet data range [data, data_end) which
  * will be accessed by various program types (cls_bpf, act_bpf,
  * lwt, ...). Subsystems allowing direct data access must (!)
@@ -775,6 +779,27 @@ static inline bool bpf_dump_raw_ok(void)
 struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off,
 				       const struct bpf_insn *patch, u32 len);
 
+static inline bool xdp_return_frame_no_direct(void)
+{
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+	return ri->kern_flags & BPF_RI_F_RF_NO_DIRECT;
+}
+
+static inline void xdp_set_return_frame_no_direct(void)
+{
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+	ri->kern_flags |= BPF_RI_F_RF_NO_DIRECT;
+}
+
+static inline void xdp_clear_return_frame_no_direct(void)
+{
+	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
+
+	ri->kern_flags &= ~BPF_RI_F_RF_NO_DIRECT;
+}
+
 static inline int xdp_ok_fwd_dev(const struct net_device *fwd,
 				 unsigned int pktlen)
 {
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 5728538..3dd99e1 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -330,10 +330,12 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
 		/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
 		xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
 		page = virt_to_head_page(data);
-		if (xa)
+		if (xa) {
+			napi_direct &= !xdp_return_frame_no_direct();
 			page_pool_put_page(xa->page_pool, page, napi_direct);
-		else
+		} else {
 			put_page(page);
+		}
 		rcu_read_unlock();
 		break;
 	case MEM_TYPE_PAGE_SHARED:
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 09/10] veth: Add XDP TX and REDIRECT
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

This allows further redirection of xdp_frames like

 NIC   -> veth--veth -> veth--veth
 (XDP)          (XDP)         (XDP)

The intermediate XDP, redirecting packets from NIC to the other veth,
reuses xdp_mem_info from NIC so that page recycling of the NIC works on
the destination veth's XDP.
In this way return_frame is not fully guarded by NAPI, since another
NAPI handler on another cpu may use the same xdp_mem_info concurrently.
Thus disable napi_direct by xdp_set_return_frame_no_direct() during the
NAPI context.

v4:
- Use xdp_[set|clear]_return_frame_no_direct() instead of a flag in
  xdp_mem_info.

v3:
- Fix double free when veth_xdp_tx() returns a positive value.
- Convert xdp_xmit and xdp_redir variables into flags.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/veth.c | 119 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 110 insertions(+), 9 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 3e1582a..a2ba1c0 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -32,6 +32,10 @@
 #define VETH_RING_SIZE		256
 #define VETH_XDP_HEADROOM	(XDP_PACKET_HEADROOM + NET_IP_ALIGN)
 
+/* Separating two types of XDP xmit */
+#define VETH_XDP_TX		BIT(0)
+#define VETH_XDP_REDIR		BIT(1)
+
 struct pcpu_vstats {
 	u64			packets;
 	u64			bytes;
@@ -45,6 +49,7 @@ struct veth_priv {
 	struct bpf_prog		*_xdp_prog;
 	struct net_device __rcu	*peer;
 	atomic64_t		dropped;
+	struct xdp_mem_info	xdp_mem;
 	unsigned		requested_headroom;
 	bool			rx_notify_masked;
 	struct ptr_ring		xdp_ring;
@@ -317,10 +322,42 @@ static int veth_xdp_xmit(struct net_device *dev, int n,
 	return n - drops;
 }
 
+static void veth_xdp_flush(struct net_device *dev)
+{
+	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+	struct net_device *rcv;
+
+	rcu_read_lock();
+	rcv = rcu_dereference(priv->peer);
+	if (unlikely(!rcv))
+		goto out;
+
+	rcv_priv = netdev_priv(rcv);
+	/* xdp_ring is initialized on receive side? */
+	if (unlikely(!rcu_access_pointer(rcv_priv->xdp_prog)))
+		goto out;
+
+	__veth_xdp_flush(rcv_priv);
+out:
+	rcu_read_unlock();
+}
+
+static int veth_xdp_tx(struct net_device *dev, struct xdp_buff *xdp)
+{
+	struct xdp_frame *frame = convert_to_xdp_frame(xdp);
+
+	if (unlikely(!frame))
+		return -EOVERFLOW;
+
+	return veth_xdp_xmit(dev, 1, &frame, 0);
+}
+
 static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
-					struct xdp_frame *frame)
+					struct xdp_frame *frame,
+					unsigned int *xdp_xmit)
 {
 	int len = frame->len, delta = 0;
+	struct xdp_frame orig_frame;
 	struct bpf_prog *xdp_prog;
 	unsigned int headroom;
 	struct sk_buff *skb;
@@ -344,6 +381,29 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 			delta = frame->data - xdp.data;
 			len = xdp.data_end - xdp.data;
 			break;
+		case XDP_TX:
+			orig_frame = *frame;
+			xdp.data_hard_start = frame;
+			xdp.rxq->mem = frame->mem;
+			if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
+				trace_xdp_exception(priv->dev, xdp_prog, act);
+				frame = &orig_frame;
+				goto err_xdp;
+			}
+			*xdp_xmit |= VETH_XDP_TX;
+			rcu_read_unlock();
+			goto xdp_xmit;
+		case XDP_REDIRECT:
+			orig_frame = *frame;
+			xdp.data_hard_start = frame;
+			xdp.rxq->mem = frame->mem;
+			if (xdp_do_redirect(priv->dev, &xdp, xdp_prog)) {
+				frame = &orig_frame;
+				goto err_xdp;
+			}
+			*xdp_xmit |= VETH_XDP_REDIR;
+			rcu_read_unlock();
+			goto xdp_xmit;
 		default:
 			bpf_warn_invalid_xdp_action(act);
 		case XDP_ABORTED:
@@ -368,12 +428,13 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 err_xdp:
 	rcu_read_unlock();
 	xdp_return_frame(frame);
-
+xdp_xmit:
 	return NULL;
 }
 
 static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
-					struct sk_buff *skb)
+					struct sk_buff *skb,
+					unsigned int *xdp_xmit)
 {
 	u32 pktlen, headroom, act, metalen;
 	void *orig_data, *orig_data_end;
@@ -445,6 +506,26 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	switch (act) {
 	case XDP_PASS:
 		break;
+	case XDP_TX:
+		get_page(virt_to_page(xdp.data));
+		consume_skb(skb);
+		xdp.rxq->mem = priv->xdp_mem;
+		if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
+			trace_xdp_exception(priv->dev, xdp_prog, act);
+			goto err_xdp;
+		}
+		*xdp_xmit |= VETH_XDP_TX;
+		rcu_read_unlock();
+		goto xdp_xmit;
+	case XDP_REDIRECT:
+		get_page(virt_to_page(xdp.data));
+		consume_skb(skb);
+		xdp.rxq->mem = priv->xdp_mem;
+		if (xdp_do_redirect(priv->dev, &xdp, xdp_prog))
+			goto err_xdp;
+		*xdp_xmit |= VETH_XDP_REDIR;
+		rcu_read_unlock();
+		goto xdp_xmit;
 	default:
 		bpf_warn_invalid_xdp_action(act);
 	case XDP_ABORTED:
@@ -475,9 +556,15 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	rcu_read_unlock();
 	kfree_skb(skb);
 	return NULL;
+err_xdp:
+	rcu_read_unlock();
+	page_frag_free(xdp.data);
+xdp_xmit:
+	return NULL;
 }
 
-static int veth_xdp_rcv(struct veth_priv *priv, int budget)
+static int veth_xdp_rcv(struct veth_priv *priv, int budget,
+			unsigned int *xdp_xmit)
 {
 	int i, done = 0;
 
@@ -488,10 +575,12 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget)
 		if (!ptr)
 			break;
 
-		if (veth_is_xdp_frame(ptr))
-			skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr));
-		else
-			skb = veth_xdp_rcv_skb(priv, ptr);
+		if (veth_is_xdp_frame(ptr)) {
+			skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr),
+					       xdp_xmit);
+		} else {
+			skb = veth_xdp_rcv_skb(priv, ptr, xdp_xmit);
+		}
 
 		if (skb)
 			napi_gro_receive(&priv->xdp_napi, skb);
@@ -506,9 +595,11 @@ static int veth_poll(struct napi_struct *napi, int budget)
 {
 	struct veth_priv *priv =
 		container_of(napi, struct veth_priv, xdp_napi);
+	unsigned int xdp_xmit = 0;
 	int done;
 
-	done = veth_xdp_rcv(priv, budget);
+	xdp_set_return_frame_no_direct();
+	done = veth_xdp_rcv(priv, budget, &xdp_xmit);
 
 	if (done < budget && napi_complete_done(napi, done)) {
 		/* Write rx_notify_masked before reading ptr_ring */
@@ -519,6 +610,12 @@ static int veth_poll(struct napi_struct *napi, int budget)
 		}
 	}
 
+	if (xdp_xmit & VETH_XDP_TX)
+		veth_xdp_flush(priv->dev);
+	if (xdp_xmit & VETH_XDP_REDIR)
+		xdp_do_flush_map();
+	xdp_clear_return_frame_no_direct();
+
 	return done;
 }
 
@@ -565,6 +662,9 @@ static int veth_enable_xdp(struct net_device *dev)
 		err = veth_napi_add(dev);
 		if (err)
 			goto err;
+
+		/* Save original mem info as it can be overwritten */
+		priv->xdp_mem = priv->xdp_rxq.mem;
 	}
 
 	rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
@@ -582,6 +682,7 @@ static void veth_disable_xdp(struct net_device *dev)
 
 	rcu_assign_pointer(priv->xdp_prog, NULL);
 	veth_napi_del(dev);
+	priv->xdp_rxq.mem = priv->xdp_mem;
 	xdp_rxq_info_unreg(&priv->xdp_rxq);
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH v7 bpf-next 10/10] veth: Support per queue XDP ring
From: Toshiaki Makita @ 2018-08-02 10:55 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann
  Cc: Toshiaki Makita, netdev, Jesper Dangaard Brouer, Jakub Kicinski,
	John Fastabend
In-Reply-To: <1533207314-2572-1-git-send-email-makita.toshiaki@lab.ntt.co.jp>

Move XDP and napi related fields from veth_priv to newly created veth_rq
structure.

When xdp_frames are enqueued from ndo_xdp_xmit and XDP_TX, rxq is
selected by current cpu.

When skbs are enqueued from the peer device, rxq is one to one mapping
of its peer txq. This way we have a restriction that the number of rxqs
must not less than the number of peer txqs, but leave the possibility to
achieve bulk skb xmit in the future because txq lock would make it
possible to remove rxq ptr_ring lock.

v3:
- Add extack messages.
- Fix array overrun in veth_xmit.

Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
 drivers/net/veth.c | 278 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 188 insertions(+), 90 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a2ba1c0..0bb409b 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -42,20 +42,24 @@ struct pcpu_vstats {
 	struct u64_stats_sync	syncp;
 };
 
-struct veth_priv {
+struct veth_rq {
 	struct napi_struct	xdp_napi;
 	struct net_device	*dev;
 	struct bpf_prog __rcu	*xdp_prog;
-	struct bpf_prog		*_xdp_prog;
-	struct net_device __rcu	*peer;
-	atomic64_t		dropped;
 	struct xdp_mem_info	xdp_mem;
-	unsigned		requested_headroom;
 	bool			rx_notify_masked;
 	struct ptr_ring		xdp_ring;
 	struct xdp_rxq_info	xdp_rxq;
 };
 
+struct veth_priv {
+	struct net_device __rcu	*peer;
+	atomic64_t		dropped;
+	struct bpf_prog		*_xdp_prog;
+	struct veth_rq		*rq;
+	unsigned int		requested_headroom;
+};
+
 /*
  * ethtool interface
  */
@@ -144,19 +148,19 @@ static void veth_ptr_free(void *ptr)
 		kfree_skb(ptr);
 }
 
-static void __veth_xdp_flush(struct veth_priv *priv)
+static void __veth_xdp_flush(struct veth_rq *rq)
 {
 	/* Write ptr_ring before reading rx_notify_masked */
 	smp_mb();
-	if (!priv->rx_notify_masked) {
-		priv->rx_notify_masked = true;
-		napi_schedule(&priv->xdp_napi);
+	if (!rq->rx_notify_masked) {
+		rq->rx_notify_masked = true;
+		napi_schedule(&rq->xdp_napi);
 	}
 }
 
-static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
+static int veth_xdp_rx(struct veth_rq *rq, struct sk_buff *skb)
 {
-	if (unlikely(ptr_ring_produce(&priv->xdp_ring, skb))) {
+	if (unlikely(ptr_ring_produce(&rq->xdp_ring, skb))) {
 		dev_kfree_skb_any(skb);
 		return NET_RX_DROP;
 	}
@@ -164,21 +168,22 @@ static int veth_xdp_rx(struct veth_priv *priv, struct sk_buff *skb)
 	return NET_RX_SUCCESS;
 }
 
-static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb, bool xdp)
+static int veth_forward_skb(struct net_device *dev, struct sk_buff *skb,
+			    struct veth_rq *rq, bool xdp)
 {
-	struct veth_priv *priv = netdev_priv(dev);
-
 	return __dev_forward_skb(dev, skb) ?: xdp ?
-		veth_xdp_rx(priv, skb) :
+		veth_xdp_rx(rq, skb) :
 		netif_rx(skb);
 }
 
 static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
+	struct veth_rq *rq = NULL;
 	struct net_device *rcv;
 	int length = skb->len;
 	bool rcv_xdp = false;
+	int rxq;
 
 	rcu_read_lock();
 	rcv = rcu_dereference(priv->peer);
@@ -188,9 +193,15 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	rcv_priv = netdev_priv(rcv);
-	rcv_xdp = rcu_access_pointer(rcv_priv->xdp_prog);
+	rxq = skb_get_queue_mapping(skb);
+	if (rxq < rcv->real_num_rx_queues) {
+		rq = &rcv_priv->rq[rxq];
+		rcv_xdp = rcu_access_pointer(rq->xdp_prog);
+		if (rcv_xdp)
+			skb_record_rx_queue(skb, rxq);
+	}
 
-	if (likely(veth_forward_skb(rcv, skb, rcv_xdp) == NET_RX_SUCCESS)) {
+	if (likely(veth_forward_skb(rcv, skb, rq, rcv_xdp) == NET_RX_SUCCESS)) {
 		struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
 
 		u64_stats_update_begin(&stats->syncp);
@@ -203,7 +214,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	if (rcv_xdp)
-		__veth_xdp_flush(rcv_priv);
+		__veth_xdp_flush(rq);
 
 	rcu_read_unlock();
 
@@ -278,12 +289,18 @@ static struct sk_buff *veth_build_skb(void *head, int headroom, int len,
 	return skb;
 }
 
+static int veth_select_rxq(struct net_device *dev)
+{
+	return smp_processor_id() % dev->real_num_rx_queues;
+}
+
 static int veth_xdp_xmit(struct net_device *dev, int n,
 			 struct xdp_frame **frames, u32 flags)
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
 	struct net_device *rcv;
 	unsigned int max_len;
+	struct veth_rq *rq;
 	int i, drops = 0;
 
 	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
@@ -294,30 +311,31 @@ static int veth_xdp_xmit(struct net_device *dev, int n,
 		return -ENXIO;
 
 	rcv_priv = netdev_priv(rcv);
+	rq = &rcv_priv->rq[veth_select_rxq(rcv)];
 	/* Non-NULL xdp_prog ensures that xdp_ring is initialized on receive
 	 * side. This means an XDP program is loaded on the peer and the peer
 	 * device is up.
 	 */
-	if (!rcu_access_pointer(rcv_priv->xdp_prog))
+	if (!rcu_access_pointer(rq->xdp_prog))
 		return -ENXIO;
 
 	max_len = rcv->mtu + rcv->hard_header_len + VLAN_HLEN;
 
-	spin_lock(&rcv_priv->xdp_ring.producer_lock);
+	spin_lock(&rq->xdp_ring.producer_lock);
 	for (i = 0; i < n; i++) {
 		struct xdp_frame *frame = frames[i];
 		void *ptr = veth_xdp_to_ptr(frame);
 
 		if (unlikely(frame->len > max_len ||
-			     __ptr_ring_produce(&rcv_priv->xdp_ring, ptr))) {
+			     __ptr_ring_produce(&rq->xdp_ring, ptr))) {
 			xdp_return_frame_rx_napi(frame);
 			drops++;
 		}
 	}
-	spin_unlock(&rcv_priv->xdp_ring.producer_lock);
+	spin_unlock(&rq->xdp_ring.producer_lock);
 
 	if (flags & XDP_XMIT_FLUSH)
-		__veth_xdp_flush(rcv_priv);
+		__veth_xdp_flush(rq);
 
 	return n - drops;
 }
@@ -326,6 +344,7 @@ static void veth_xdp_flush(struct net_device *dev)
 {
 	struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
 	struct net_device *rcv;
+	struct veth_rq *rq;
 
 	rcu_read_lock();
 	rcv = rcu_dereference(priv->peer);
@@ -333,11 +352,12 @@ static void veth_xdp_flush(struct net_device *dev)
 		goto out;
 
 	rcv_priv = netdev_priv(rcv);
+	rq = &rcv_priv->rq[veth_select_rxq(rcv)];
 	/* xdp_ring is initialized on receive side? */
-	if (unlikely(!rcu_access_pointer(rcv_priv->xdp_prog)))
+	if (unlikely(!rcu_access_pointer(rq->xdp_prog)))
 		goto out;
 
-	__veth_xdp_flush(rcv_priv);
+	__veth_xdp_flush(rq);
 out:
 	rcu_read_unlock();
 }
@@ -352,7 +372,7 @@ static int veth_xdp_tx(struct net_device *dev, struct xdp_buff *xdp)
 	return veth_xdp_xmit(dev, 1, &frame, 0);
 }
 
-static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
+static struct sk_buff *veth_xdp_rcv_one(struct veth_rq *rq,
 					struct xdp_frame *frame,
 					unsigned int *xdp_xmit)
 {
@@ -363,7 +383,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 	struct sk_buff *skb;
 
 	rcu_read_lock();
-	xdp_prog = rcu_dereference(priv->xdp_prog);
+	xdp_prog = rcu_dereference(rq->xdp_prog);
 	if (likely(xdp_prog)) {
 		struct xdp_buff xdp;
 		u32 act;
@@ -372,7 +392,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 		xdp.data = frame->data;
 		xdp.data_end = frame->data + frame->len;
 		xdp.data_meta = frame->data - frame->metasize;
-		xdp.rxq = &priv->xdp_rxq;
+		xdp.rxq = &rq->xdp_rxq;
 
 		act = bpf_prog_run_xdp(xdp_prog, &xdp);
 
@@ -385,8 +405,8 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 			orig_frame = *frame;
 			xdp.data_hard_start = frame;
 			xdp.rxq->mem = frame->mem;
-			if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
-				trace_xdp_exception(priv->dev, xdp_prog, act);
+			if (unlikely(veth_xdp_tx(rq->dev, &xdp) < 0)) {
+				trace_xdp_exception(rq->dev, xdp_prog, act);
 				frame = &orig_frame;
 				goto err_xdp;
 			}
@@ -397,7 +417,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 			orig_frame = *frame;
 			xdp.data_hard_start = frame;
 			xdp.rxq->mem = frame->mem;
-			if (xdp_do_redirect(priv->dev, &xdp, xdp_prog)) {
+			if (xdp_do_redirect(rq->dev, &xdp, xdp_prog)) {
 				frame = &orig_frame;
 				goto err_xdp;
 			}
@@ -407,7 +427,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 		default:
 			bpf_warn_invalid_xdp_action(act);
 		case XDP_ABORTED:
-			trace_xdp_exception(priv->dev, xdp_prog, act);
+			trace_xdp_exception(rq->dev, xdp_prog, act);
 		case XDP_DROP:
 			goto err_xdp;
 		}
@@ -422,7 +442,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 	}
 
 	xdp_scrub_frame(frame);
-	skb->protocol = eth_type_trans(skb, priv->dev);
+	skb->protocol = eth_type_trans(skb, rq->dev);
 err:
 	return skb;
 err_xdp:
@@ -432,8 +452,7 @@ static struct sk_buff *veth_xdp_rcv_one(struct veth_priv *priv,
 	return NULL;
 }
 
-static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
-					struct sk_buff *skb,
+static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, struct sk_buff *skb,
 					unsigned int *xdp_xmit)
 {
 	u32 pktlen, headroom, act, metalen;
@@ -443,7 +462,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	struct xdp_buff xdp;
 
 	rcu_read_lock();
-	xdp_prog = rcu_dereference(priv->xdp_prog);
+	xdp_prog = rcu_dereference(rq->xdp_prog);
 	if (unlikely(!xdp_prog)) {
 		rcu_read_unlock();
 		goto out;
@@ -497,7 +516,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	xdp.data = skb_mac_header(skb);
 	xdp.data_end = xdp.data + pktlen;
 	xdp.data_meta = xdp.data;
-	xdp.rxq = &priv->xdp_rxq;
+	xdp.rxq = &rq->xdp_rxq;
 	orig_data = xdp.data;
 	orig_data_end = xdp.data_end;
 
@@ -509,9 +528,9 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	case XDP_TX:
 		get_page(virt_to_page(xdp.data));
 		consume_skb(skb);
-		xdp.rxq->mem = priv->xdp_mem;
-		if (unlikely(veth_xdp_tx(priv->dev, &xdp) < 0)) {
-			trace_xdp_exception(priv->dev, xdp_prog, act);
+		xdp.rxq->mem = rq->xdp_mem;
+		if (unlikely(veth_xdp_tx(rq->dev, &xdp) < 0)) {
+			trace_xdp_exception(rq->dev, xdp_prog, act);
 			goto err_xdp;
 		}
 		*xdp_xmit |= VETH_XDP_TX;
@@ -520,8 +539,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	case XDP_REDIRECT:
 		get_page(virt_to_page(xdp.data));
 		consume_skb(skb);
-		xdp.rxq->mem = priv->xdp_mem;
-		if (xdp_do_redirect(priv->dev, &xdp, xdp_prog))
+		xdp.rxq->mem = rq->xdp_mem;
+		if (xdp_do_redirect(rq->dev, &xdp, xdp_prog))
 			goto err_xdp;
 		*xdp_xmit |= VETH_XDP_REDIR;
 		rcu_read_unlock();
@@ -529,7 +548,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	default:
 		bpf_warn_invalid_xdp_action(act);
 	case XDP_ABORTED:
-		trace_xdp_exception(priv->dev, xdp_prog, act);
+		trace_xdp_exception(rq->dev, xdp_prog, act);
 	case XDP_DROP:
 		goto drop;
 	}
@@ -545,7 +564,7 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	off = xdp.data_end - orig_data_end;
 	if (off != 0)
 		__skb_put(skb, off);
-	skb->protocol = eth_type_trans(skb, priv->dev);
+	skb->protocol = eth_type_trans(skb, rq->dev);
 
 	metalen = xdp.data - xdp.data_meta;
 	if (metalen)
@@ -563,27 +582,26 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_priv *priv,
 	return NULL;
 }
 
-static int veth_xdp_rcv(struct veth_priv *priv, int budget,
-			unsigned int *xdp_xmit)
+static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
 {
 	int i, done = 0;
 
 	for (i = 0; i < budget; i++) {
-		void *ptr = __ptr_ring_consume(&priv->xdp_ring);
+		void *ptr = __ptr_ring_consume(&rq->xdp_ring);
 		struct sk_buff *skb;
 
 		if (!ptr)
 			break;
 
 		if (veth_is_xdp_frame(ptr)) {
-			skb = veth_xdp_rcv_one(priv, veth_ptr_to_xdp(ptr),
+			skb = veth_xdp_rcv_one(rq, veth_ptr_to_xdp(ptr),
 					       xdp_xmit);
 		} else {
-			skb = veth_xdp_rcv_skb(priv, ptr, xdp_xmit);
+			skb = veth_xdp_rcv_skb(rq, ptr, xdp_xmit);
 		}
 
 		if (skb)
-			napi_gro_receive(&priv->xdp_napi, skb);
+			napi_gro_receive(&rq->xdp_napi, skb);
 
 		done++;
 	}
@@ -593,25 +611,25 @@ static int veth_xdp_rcv(struct veth_priv *priv, int budget,
 
 static int veth_poll(struct napi_struct *napi, int budget)
 {
-	struct veth_priv *priv =
-		container_of(napi, struct veth_priv, xdp_napi);
+	struct veth_rq *rq =
+		container_of(napi, struct veth_rq, xdp_napi);
 	unsigned int xdp_xmit = 0;
 	int done;
 
 	xdp_set_return_frame_no_direct();
-	done = veth_xdp_rcv(priv, budget, &xdp_xmit);
+	done = veth_xdp_rcv(rq, budget, &xdp_xmit);
 
 	if (done < budget && napi_complete_done(napi, done)) {
 		/* Write rx_notify_masked before reading ptr_ring */
-		smp_store_mb(priv->rx_notify_masked, false);
-		if (unlikely(!__ptr_ring_empty(&priv->xdp_ring))) {
-			priv->rx_notify_masked = true;
-			napi_schedule(&priv->xdp_napi);
+		smp_store_mb(rq->rx_notify_masked, false);
+		if (unlikely(!__ptr_ring_empty(&rq->xdp_ring))) {
+			rq->rx_notify_masked = true;
+			napi_schedule(&rq->xdp_napi);
 		}
 	}
 
 	if (xdp_xmit & VETH_XDP_TX)
-		veth_xdp_flush(priv->dev);
+		veth_xdp_flush(rq->dev);
 	if (xdp_xmit & VETH_XDP_REDIR)
 		xdp_do_flush_map();
 	xdp_clear_return_frame_no_direct();
@@ -622,56 +640,90 @@ static int veth_poll(struct napi_struct *napi, int budget)
 static int veth_napi_add(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
-	int err;
+	int err, i;
 
-	err = ptr_ring_init(&priv->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
-	if (err)
-		return err;
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct veth_rq *rq = &priv->rq[i];
+
+		err = ptr_ring_init(&rq->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
+		if (err)
+			goto err_xdp_ring;
+	}
 
-	netif_napi_add(dev, &priv->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
-	napi_enable(&priv->xdp_napi);
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct veth_rq *rq = &priv->rq[i];
+
+		netif_napi_add(dev, &rq->xdp_napi, veth_poll, NAPI_POLL_WEIGHT);
+		napi_enable(&rq->xdp_napi);
+	}
 
 	return 0;
+err_xdp_ring:
+	for (i--; i >= 0; i--)
+		ptr_ring_cleanup(&priv->rq[i].xdp_ring, veth_ptr_free);
+
+	return err;
 }
 
 static void veth_napi_del(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	int i;
 
-	napi_disable(&priv->xdp_napi);
-	netif_napi_del(&priv->xdp_napi);
-	priv->rx_notify_masked = false;
-	ptr_ring_cleanup(&priv->xdp_ring, veth_ptr_free);
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct veth_rq *rq = &priv->rq[i];
+
+		napi_disable(&rq->xdp_napi);
+		napi_hash_del(&rq->xdp_napi);
+	}
+	synchronize_net();
+
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct veth_rq *rq = &priv->rq[i];
+
+		netif_napi_del(&rq->xdp_napi);
+		rq->rx_notify_masked = false;
+		ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
+	}
 }
 
 static int veth_enable_xdp(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
-	int err;
+	int err, i;
 
-	if (!xdp_rxq_info_is_reg(&priv->xdp_rxq)) {
-		err = xdp_rxq_info_reg(&priv->xdp_rxq, dev, 0);
-		if (err < 0)
-			return err;
+	if (!xdp_rxq_info_is_reg(&priv->rq[0].xdp_rxq)) {
+		for (i = 0; i < dev->real_num_rx_queues; i++) {
+			struct veth_rq *rq = &priv->rq[i];
 
-		err = xdp_rxq_info_reg_mem_model(&priv->xdp_rxq,
-						 MEM_TYPE_PAGE_SHARED, NULL);
-		if (err < 0)
-			goto err;
+			err = xdp_rxq_info_reg(&rq->xdp_rxq, dev, i);
+			if (err < 0)
+				goto err_rxq_reg;
+
+			err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
+							 MEM_TYPE_PAGE_SHARED,
+							 NULL);
+			if (err < 0)
+				goto err_reg_mem;
+
+			/* Save original mem info as it can be overwritten */
+			rq->xdp_mem = rq->xdp_rxq.mem;
+		}
 
 		err = veth_napi_add(dev);
 		if (err)
-			goto err;
-
-		/* Save original mem info as it can be overwritten */
-		priv->xdp_mem = priv->xdp_rxq.mem;
+			goto err_rxq_reg;
 	}
 
-	rcu_assign_pointer(priv->xdp_prog, priv->_xdp_prog);
+	for (i = 0; i < dev->real_num_rx_queues; i++)
+		rcu_assign_pointer(priv->rq[i].xdp_prog, priv->_xdp_prog);
 
 	return 0;
-err:
-	xdp_rxq_info_unreg(&priv->xdp_rxq);
+err_reg_mem:
+	xdp_rxq_info_unreg(&priv->rq[i].xdp_rxq);
+err_rxq_reg:
+	for (i--; i >= 0; i--)
+		xdp_rxq_info_unreg(&priv->rq[i].xdp_rxq);
 
 	return err;
 }
@@ -679,11 +731,17 @@ static int veth_enable_xdp(struct net_device *dev)
 static void veth_disable_xdp(struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	int i;
 
-	rcu_assign_pointer(priv->xdp_prog, NULL);
+	for (i = 0; i < dev->real_num_rx_queues; i++)
+		rcu_assign_pointer(priv->rq[i].xdp_prog, NULL);
 	veth_napi_del(dev);
-	priv->xdp_rxq.mem = priv->xdp_mem;
-	xdp_rxq_info_unreg(&priv->xdp_rxq);
+	for (i = 0; i < dev->real_num_rx_queues; i++) {
+		struct veth_rq *rq = &priv->rq[i];
+
+		rq->xdp_rxq.mem = rq->xdp_mem;
+		xdp_rxq_info_unreg(&rq->xdp_rxq);
+	}
 }
 
 static int veth_open(struct net_device *dev)
@@ -840,6 +898,12 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 			goto err;
 		}
 
+		if (dev->real_num_rx_queues < peer->real_num_tx_queues) {
+			NL_SET_ERR_MSG_MOD(extack, "XDP expects number of rx queues not less than peer tx queues");
+			err = -ENOSPC;
+			goto err;
+		}
+
 		if (dev->flags & IFF_UP) {
 			err = veth_enable_xdp(dev);
 			if (err) {
@@ -974,13 +1038,31 @@ static int veth_validate(struct nlattr *tb[], struct nlattr *data[],
 	return 0;
 }
 
+static int veth_alloc_queues(struct net_device *dev)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+
+	priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL);
+	if (!priv->rq)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void veth_free_queues(struct net_device *dev)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+
+	kfree(priv->rq);
+}
+
 static struct rtnl_link_ops veth_link_ops;
 
 static int veth_newlink(struct net *src_net, struct net_device *dev,
 			struct nlattr *tb[], struct nlattr *data[],
 			struct netlink_ext_ack *extack)
 {
-	int err;
+	int err, i;
 	struct net_device *peer;
 	struct veth_priv *priv;
 	char ifname[IFNAMSIZ];
@@ -1033,6 +1115,12 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 		return PTR_ERR(peer);
 	}
 
+	err = veth_alloc_queues(peer);
+	if (err) {
+		put_net(net);
+		goto err_peer_alloc_queues;
+	}
+
 	if (!ifmp || !tbp[IFLA_ADDRESS])
 		eth_hw_addr_random(peer);
 
@@ -1061,6 +1149,10 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	 * should be re-allocated
 	 */
 
+	err = veth_alloc_queues(dev);
+	if (err)
+		goto err_alloc_queues;
+
 	if (tb[IFLA_ADDRESS] == NULL)
 		eth_hw_addr_random(dev);
 
@@ -1080,22 +1172,28 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	 */
 
 	priv = netdev_priv(dev);
-	priv->dev = dev;
+	for (i = 0; i < dev->real_num_rx_queues; i++)
+		priv->rq[i].dev = dev;
 	rcu_assign_pointer(priv->peer, peer);
 
 	priv = netdev_priv(peer);
-	priv->dev = peer;
+	for (i = 0; i < peer->real_num_rx_queues; i++)
+		priv->rq[i].dev = peer;
 	rcu_assign_pointer(priv->peer, dev);
 
 	return 0;
 
 err_register_dev:
+	veth_free_queues(dev);
+err_alloc_queues:
 	/* nothing to do */
 err_configure_peer:
 	unregister_netdevice(peer);
 	return err;
 
 err_register_peer:
+	veth_free_queues(peer);
+err_peer_alloc_queues:
 	free_netdev(peer);
 	return err;
 }
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH bpf] xdp: add NULL pointer check in __xdp_return()
From: Björn Töpel @ 2018-08-02 10:59 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Jesper Dangaard Brouer, Taehee Yoo, kafai, ast,
	Björn Töpel, Netdev
In-Reply-To: <a6387196-c294-37be-c90d-58eecbf2608a@iogearbox.net>

Den ons 1 aug. 2018 kl 22:25 skrev Daniel Borkmann <daniel@iogearbox.net>:
>
> On 08/01/2018 04:43 PM, Björn Töpel wrote:
> > Den ons 1 aug. 2018 kl 16:14 skrev Jesper Dangaard Brouer <brouer@redhat.com>:
> >> On Mon, 23 Jul 2018 11:41:02 +0200
> >> Björn Töpel <bjorn.topel@gmail.com> wrote:
> >>
> >>>>>> diff --git a/net/core/xdp.c b/net/core/xdp.c
> >>>>>> index 9d1f220..1c12bc7 100644
> >>>>>> --- a/net/core/xdp.c
> >>>>>> +++ b/net/core/xdp.c
> >>>>>> @@ -345,7 +345,8 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> >>>>>>               rcu_read_lock();
> >>>>>>               /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> >>>>>>               xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> >>>>>> -             xa->zc_alloc->free(xa->zc_alloc, handle);
> >>>>>> +             if (xa)
> >>>>>> +                     xa->zc_alloc->free(xa->zc_alloc, handle);
> >>>>> hmm...It is not clear to me the "!xa" case don't have to be handled?
> >>>>
> >>>> Thank you for reviewing!
> >>>>
> >>>> Returning NULL pointer is bug case such as calling after use
> >>>> xdp_rxq_info_unreg().
> >>>> so that, I think it can't handle at that moment.
> >>>> we can make __xdp_return to add WARN_ON_ONCE() or
> >>>> add return error code to driver.
> >>>> But I'm not sure if these is useful information.
> >>>>
> >>>> I might have misunderstood scenario of MEM_TYPE_ZERO_COPY
> >>>> because there is no use case of MEM_TYPE_ZERO_COPY yet.
> >>>
> >>> Taehee, again, sorry for the slow response and thanks for patch!
> >>>
> >>> If xa is NULL, the driver has a buggy/broken implementation. What
> >>> would be a proper way of dealing with this? BUG?
> >>
> >> Hmm... I don't like these kind of changes to the hot-path code!
> >>
> >> You might not realize this, but adding BUG() and WARN_ON() to the code
> >> affect performance in ways you might not realize!  These macros gets
> >> compiled and uses an asm instruction called "ud2".  Seeing the "ud2"
> >> instruction causes the CPUs instruction cache prefetcher to stop.
> >> Thus, if some code ends up below this instruction, this will cause more
> >> i-cache-misses.
> >>
> >> I don't know if xa==NULL is even possible, but if it is, then I think
> >> this is a result of a driver mem_reg API usage bug.  And the mem-reg
> >> API is full of WARN's and error messages, exactly to push these kind of
> >> checks out of the fast-path.  There is no need for a BUG() call, as
> >> deref a NULL pointer will case an OOPS, that is easy to read and
> >> understand.
> >
> > Jesper, thanks for having a look! So, you're right that if xa==NULL
> > the driver is "broken/buggy" (as stated earlier!). I agree that
> > OOPSing on a NULL pointer is as good as a BUG!
> >
> > The applied patch adds a WARN_ON_ONCE, and I thought best practice was
> > that a buggy driver shouldn't crash the kernel... What is considered
> > best practices in these scenarios? *I'd* prefer an OOPS instead of
> > WARN_ON_ONCE, to catch that buggy driver. Again, that's me. I thought
> > that most people prefer not crashing, hence the patch. :-)
>
> In that case, lets send a revert for the patch with a proper analysis
> of why it is safe to omit the NULL check which should be placed as a
> comment right near the rhashtable_lookup().
>

I'll do that (as soon as I've double-checked so that I'm not lying)!


Björn

> Thanks,
> Daniel

^ permalink raw reply

* [PATCH net-next] net/tls: Always get number of sg entries for skb to be decrypted
From: Vakul Garg @ 2018-08-02 16:20 UTC (permalink / raw)
  To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg

Function decrypt_skb() made a bad assumption that number of sg entries
required for mapping skb to be decrypted would always be less than
MAX_SKB_FRAGS. The required count of sg entries for skb should always be
calculated. If they cannot fit in local array sgin_arr[], allocate them
from heap irrespective whether it is zero-copy case or otherwise. The
change also benefits the non-zero copy case as we could use sgin_arr[]
instead of always allocating sg entries from heap.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---

The said problem has been discussed with Dave Watson over mail list.

 net/tls/tls_sw.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ff3a6904a722..e2cf7aebb877 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -693,7 +693,7 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 	struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
 	struct scatterlist *sgin = &sgin_arr[0];
 	struct strp_msg *rxm = strp_msg(skb);
-	int ret, nsg = ARRAY_SIZE(sgin_arr);
+	int ret, nsg;
 	struct sk_buff *unused;
 
 	ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
@@ -703,12 +703,20 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 		return ret;
 
 	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
-	if (!sgout) {
-		nsg = skb_cow_data(skb, 0, &unused) + 1;
+
+	/* If required number of SG entries for skb are more than
+	 * sgin_arr elements, then dynamically allocate sg table.
+	 */
+	nsg = skb_cow_data(skb, 0, &unused) + 1;
+	if (nsg > ARRAY_SIZE(sgin_arr)) {
 		sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
-		sgout = sgin;
+		if (!sgin)
+			return -ENOMEM;
 	}
 
+	if (!sgout)
+		sgout = sgin;
+
 	sg_init_table(sgin, nsg);
 	sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
 
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH 1/1] selftest/net: fix protocol family to work for IPv4.
From: Eric Dumazet @ 2018-08-02 13:03 UTC (permalink / raw)
  To: Maninder Singh, davem, shuahkh
  Cc: netdev, linux-api, linux-kernel, edumazet, pankaj.m, a.sahrawat,
	Vaneet Narang
In-Reply-To: <20180802100201epcas5p390bb362ccabaa99087eff88af37bffb2~HCQZjTEZd0101301013epcas5p3C@epcas5p3.samsung.com>



On 08/02/2018 02:57 AM, Maninder Singh wrote:
> use actual protocol family passed by user rather than hardcoded
> AF_INTE6 to cerate sockets.
> current code is not working for IPv4.
> 
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> ---
>  tools/testing/selftests/net/tcp_mmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/tcp_mmap.c b/tools/testing/selftests/net/tcp_mmap.c
> index 77f7627..e8c5dff 100644
> --- a/tools/testing/selftests/net/tcp_mmap.c
> +++ b/tools/testing/selftests/net/tcp_mmap.c
> @@ -402,7 +402,7 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
>  
> -	fd = socket(AF_INET6, SOCK_STREAM, 0);
> +	fd = socket(cfg_family, SOCK_STREAM, 0);
>  	if (fd == -1) {
>  		perror("socket");
>  		exit(1);
> 


Darn, someone spotted my hidden attempt to kill IPv4 ;)

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: AW: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-02 13:05 UTC (permalink / raw)
  To: Marcel Hellwig, 'Eric Dumazet', Paolo Abeni,
	'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
	'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
  Cc: 'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <c54e9eee0add4f04b0ce61a1ae2c3f04@ZCOM03.mut-group.com>



On 08/02/2018 04:02 AM, Marcel Hellwig wrote:
> Hi everyone,
> 
> I did three things that evening.
> * I created a small dts file, so I could boot a 3.5.7 kernel.
> * Ported the pskb_expand_head from 3.5.7 to 3.4.113
> * Ported the whole skbuf.{c,h} file from 3.5.7 to 3.4.113

That seems bold :0)

Sorry, we only backport one fix at a time.

You can not simply copy whole files from one version to another.

> 
> Sadly enough the second and third one panicked as well, but interestingly not the first one (3.5.7 with a simple dts file). Now the question is: Is something changed outside of the skbuf (maybe in udp.{c,h}?) or is it because the dts uses a different approach of talking to the MAC of the lpc3250?
> 
> I may try a more recent kernel with a dts file (maybe a 4.x.x one), but I think that will be impossible to backport a patch for our 2.6 kernel :(
> 
> If anybody has any more ideas about why the dts kernel does not panic, you're very welcome.
> 
> Mit freundlichen Grüßen / With kind regards
> 
> Marcel Hellwig
> B. Sc. Informatik
> Entwickler
> 
> m-u-t GmbH
> Am Marienhof 2
> 22880 Wedel
> Germany
> 
> Phone:	+49 4103 9308 - 474
> Fax:  	+49 4103 9308 - 99
> mhellwig@mut-group.com
> 
> www.mut-group.com
> 
> Geschäftsführer (Managing Director): Fabian Peters
> Amtsgericht Pinneberg (Commercial Register No.): HRB 10304 PI
> USt-IdNr. (VAT-No.): DE228275390
> WEEE-Reg-Nr.: DE 72271808
> 
> 

^ permalink raw reply

* Re: [PATCH 1/1] selftest/net: fix FILE_SIZE for 32 bit architecture.
From: Eric Dumazet @ 2018-08-02 13:08 UTC (permalink / raw)
  To: Maninder Singh, davem, shuahkh
  Cc: netdev, linux-api, linux-kernel, edumazet, pankaj.m, a.sahrawat,
	Vaneet Narang
In-Reply-To: <20180802103616epcas5p48ec1e2ea3568b11683aa7b55254dffb0~HCuTv0d7l2131721317epcas5p4u@epcas5p4.samsung.com>



On 08/02/2018 03:31 AM, Maninder Singh wrote:
> FILE_SZ is defined as (1UL << 35), it will overflow
> for 32 bit system and logic will break.
> 
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> ---
>  tools/testing/selftests/net/tcp_mmap.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/tcp_mmap.c b/tools/testing/selftests/net/tcp_mmap.c
> index e8c5dff..1d6ca12 100644
> --- a/tools/testing/selftests/net/tcp_mmap.c
> +++ b/tools/testing/selftests/net/tcp_mmap.c
> @@ -85,7 +85,7 @@
>  #define MSG_ZEROCOPY    0x4000000
>  #endif
>  
> -#define FILE_SZ (1UL << 35)
> +#define FILE_SZ (1ULL << 35)
>  static int cfg_family = AF_INET6;
>  static socklen_t cfg_alen = sizeof(struct sockaddr_in6);
>  static int cfg_port = 8787;
> @@ -134,7 +134,7 @@ void hash_zone(void *zone, unsigned int length)
>  
>  void *child_thread(void *arg)
>  {
> -	unsigned long total_mmap = 0, total = 0;
> +	unsigned long long total_mmap = 0, total = 0;
>  	struct tcp_zerocopy_receive zc;
>  	unsigned long delta_usec;
>  	int flags = MAP_SHARED;
> @@ -316,7 +316,7 @@ int main(int argc, char *argv[])
>  {
>  	struct sockaddr_storage listenaddr, addr;
>  	unsigned int max_pacing_rate = 0;
> -	unsigned long total = 0;
> +	unsigned long long total = 0;
>  	char *host = NULL;
>  	int fd, c, on = 1;
>  	char *buffer;
> @@ -431,7 +431,7 @@ int main(int argc, char *argv[])
>  		zflg = 0;
>  	}
>  	while (total < FILE_SZ) {
> -		long wr = FILE_SZ - total;
> +		unsigned long long wr = FILE_SZ - total;
>  
>  		if (wr > chunk_size)
>  			wr = chunk_size;
> 

What about using more conventional size_t instead of "unsigned long long" ?

^ permalink raw reply

* Re: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-02 13:13 UTC (permalink / raw)
  To: David Laight, 'Marcel Hellwig', 'Eric Dumazet',
	'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
	'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
  Cc: 'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <d16f03bf47b24313ad9f2ff4e37d6274@AcuMS.aculab.com>



On 08/02/2018 02:17 AM, David Laight wrote:
> From: Marcel Hellwig
>> Sent: 01 August 2018 11:36
>>>> [<c0228adc>] (udp_recvmsg+0x284/0x33c) from [<c02306e0>] (inet_recvmsg+0x38/0x4c):
>> net/ipv4/udp.c:1234
>>>
>>>              sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>>>
>>> Unaligned access trap (virtual address c14fe63a), so either sin or ip_hdr(skb) are not on a 32bit
>> alignment
>>>
>>> Can you produce the disassembly of the trapping instruction ?
>>
>> https://gist.github.com/hellow554/6b11c6c0827d5db80a7e66f71f5636ff#file-net_uipv4_udp-lst-L1892-L1895
>>
>> 		sin->sin_addr.s_addr = ip_hdr(skb)->saddr;
>> c0228ad8:	e5943080 	ldr	r3, [r4, #128]	; 0x80
>> c0228adc:	e593300c 	ldr	r3, [r3, #12]
>> c0228ae0: 	e5823004	str	r3, [r2, #4]
> 
> There are actually 2 faults, difficult to quickly sort out the merged tracebacks.
> You are also running a rather old kernel: Linux version 3.4.113.
> 
> It may well be that whichever ethernet driver generated the misaligned frame
> has since been fixed.

A misalign frame driver problem would have faulted earlier in IP stack,
much before we perform the copy to user space in udp_recvmsg()

^ permalink raw reply

* RE: AW: PROBLEM: Kernel Oops in UDP stack
From: David Laight @ 2018-08-02 13:18 UTC (permalink / raw)
  To: 'Eric Dumazet', 'Marcel Hellwig',
	'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
	'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
  Cc: 'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <405319c1-2fa0-e1b5-7173-b19d9769d98a@gmail.com>

From: Eric Dumazet
> Sent: 02 August 2018 14:13
> 
> A misalign frame driver problem would have faulted earlier in IP stack,
> much before we perform the copy to user space in udp_recvmsg()

And my mailer failed to thread all the later responses :-(

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* Re: [PATCH v7 bpf-next 05/10] veth: Handle xdp_frames in xdp napi ring
From: Jesper Dangaard Brouer @ 2018-08-02 11:45 UTC (permalink / raw)
  To: Toshiaki Makita
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Jakub Kicinski,
	John Fastabend, brouer
In-Reply-To: <1533207314-2572-6-git-send-email-makita.toshiaki@lab.ntt.co.jp>

On Thu,  2 Aug 2018 19:55:09 +0900
Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:

> +	headroom = frame->data - delta - (void *)frame;

Your calculation of headroom is still adding an assumption that
xdp_frame is located in the top of data area, that is unnecessary.

The headroom can be calculated as:

 headroom = sizeof(struct xdp_frame) + frame->headroom - delta;

> +	skb = veth_build_skb(frame, headroom, len, 0);
> +	if (!skb) {
> +		xdp_return_frame(frame);
> +		goto err;
> +	}
> +
> +	xdp_scrub_frame(frame);

Thanks you for adding a xdp_scrub_frame() instead.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* [PATCH] ipv6: icmp: Updating pmtu for link local route
From: Georg Kohmann @ 2018-08-02 11:56 UTC (permalink / raw)
  To: netdev; +Cc: Georg Kohmann

When a ICMPV6_PKT_TOOBIG is received from a link local address the pmtu will
be updated on a route with an arbitrary interface index. Subsequent packets
sent back to the same link local address may therefore end up not
considering the updated pmtu.

Current behavior breaks TAHI v6LC4.1.4 Reduce PMTU On-link. Referring to RFC
1981: Section 3: "Note that Path MTU Discovery must be performed even in
cases where a node "thinks" a destination is attached to the same link as
itself. In a situation such as when a neighboring router acts as proxy [ND]
for some destination, the destination can to appear to be directly
connected but is in fact more than one hop away."

Using the interface index from the incoming ICMPV6_PKT_TOOBIG when updating
the pmtu.

Signed-off-by: Georg Kohmann <geokohma@cisco.com>
---
 net/ipv6/icmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 3ae2fbe..211db37 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -92,7 +92,7 @@ static void icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	struct net *net = dev_net(skb->dev);
 
 	if (type == ICMPV6_PKT_TOOBIG)
-		ip6_update_pmtu(skb, net, info, 0, 0, sock_net_uid(net, NULL));
+		ip6_update_pmtu(skb, net, info, skb->dev->ifindex, 0, sock_net_uid(net, NULL));
 	else if (type == NDISC_REDIRECT)
 		ip6_redirect(skb, net, skb->dev->ifindex, 0,
 			     sock_net_uid(net, NULL));
-- 
2.10.2

^ permalink raw reply related

* AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Marcel Hellwig @ 2018-08-02 13:57 UTC (permalink / raw)
  To: 'Eric Dumazet', David Laight,
	'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
	'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
  Cc: 'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <405319c1-2fa0-e1b5-7173-b19d9769d98a@gmail.com>

>> There are actually 2 faults, difficult to quickly sort out the merged tracebacks.
>> You are also running a rather old kernel: Linux version 3.4.113.
>> 
>> It may well be that whichever ethernet driver generated the misaligned 
>> frame has since been fixed.
>
>A misalign frame driver problem would have faulted earlier in IP stack, much before we perform the copy to user space in udp_recvmsg()
>

JFYI: we are talking about the lpc_eth driver[0] #57c10b6 , which is not the newest, but all newer did not fix a major problem (at least the commit messages are not screaming: WARNING, UNALIGNED MEMORY!). Is there a diagram/document how a ip packet travels down the code? From the MAC/phy driver to udp_recvmsg? It's not that obvious for me, but maybe it is something I can work with.


[0]: https://elixir.bootlin.com/linux/v3.4.113/source/drivers/net/ethernet/nxp/lpc_eth.c

Regards,
Marcel

^ permalink raw reply

* Re: [bug report] net/mlx5e: Gather all XDP pre-requisite checks in a single function
From: Leon Romanovsky @ 2018-08-02 12:31 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: tariqt, linux-rdma, linux-netdev, Saeed Mahameed
In-Reply-To: <20180802085449.hqlo6pqizuzbtliq@kili.mountain>

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

+netdev and Saeed

On Thu, Aug 02, 2018 at 11:54:49AM +0300, Dan Carpenter wrote:
> Hello Tariq Toukan,
>
> The patch 0ec13877ce95: "net/mlx5e: Gather all XDP pre-requisite
> checks in a single function" from Mar 12, 2018, leads to the
> following Smatch warning:
>
> 	drivers/net/ethernet/mellanox/mlx5/core/en_main.c:4284 mlx5e_xdp_set()
> 	error: uninitialized symbol 'err'.
>
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>   4214  static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
>   4215  {
>   4216          struct mlx5e_priv *priv = netdev_priv(netdev);
>   4217          struct bpf_prog *old_prog;
>   4218          bool reset, was_opened;
>   4219          int err;
>                 ^^^^^^^
> I always encourage people to remove unneeded initializers so that GCC
> can detect uninitialized variables, but it turns out that GCC misses a
> bunch of bugs?
>
>   4220          int i;
>   4221
>   4222          mutex_lock(&priv->state_lock);
>   4223
>   4224          if (prog) {
>   4225                  err = mlx5e_xdp_allowed(priv, prog);
>   4226                  if (err)
>   4227                          goto unlock;
>   4228          }
>   4229
>   4230          was_opened = test_bit(MLX5E_STATE_OPENED, &priv->state);
>   4231          /* no need for full reset when exchanging programs */
>   4232          reset = (!priv->channels.params.xdp_prog || !prog);
>   4233
>   4234          if (was_opened && reset)
>   4235                  mlx5e_close_locked(netdev);
>   4236          if (was_opened && !reset) {
>   4237                  /* num_channels is invariant here, so we can take the
>   4238                   * batched reference right upfront.
>   4239                   */
>   4240                  prog = bpf_prog_add(prog, priv->channels.num);
>   4241                  if (IS_ERR(prog)) {
>   4242                          err = PTR_ERR(prog);
>   4243                          goto unlock;
>   4244                  }
>   4245          }
>   4246
>   4247          /* exchange programs, extra prog reference we got from caller
>   4248           * as long as we don't fail from this point onwards.
>   4249           */
>   4250          old_prog = xchg(&priv->channels.params.xdp_prog, prog);
>   4251          if (old_prog)
>   4252                  bpf_prog_put(old_prog);
>   4253
>   4254          if (reset) /* change RQ type according to priv->xdp_prog */
>   4255                  mlx5e_set_rq_type(priv->mdev, &priv->channels.params);
>   4256
>   4257          if (was_opened && reset)
>   4258                  mlx5e_open_locked(netdev);
>   4259
>   4260          if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
>   4261                  goto unlock;
>                         ^^^^^^^^^^^
> In the original code this was a success path.
>
>   4262
>   4263          /* exchanging programs w/o reset, we update ref counts on behalf
>   4264           * of the channels RQs here.
>   4265           */
>   4266          for (i = 0; i < priv->channels.num; i++) {
>   4267                  struct mlx5e_channel *c = priv->channels.c[i];
>   4268
>   4269                  clear_bit(MLX5E_RQ_STATE_ENABLED, &c->rq.state);
>   4270                  napi_synchronize(&c->napi);
>   4271                  /* prevent mlx5e_poll_rx_cq from accessing rq->xdp_prog */
>   4272
>   4273                  old_prog = xchg(&c->rq.xdp_prog, prog);
>   4274
>   4275                  set_bit(MLX5E_RQ_STATE_ENABLED, &c->rq.state);
>   4276                  /* napi_schedule in case we have missed anything */
>   4277                  napi_schedule(&c->napi);
>   4278
>   4279                  if (old_prog)
>   4280                          bpf_prog_put(old_prog);
>   4281          }
>   4282
>   4283  unlock:
>   4284          mutex_unlock(&priv->state_lock);
>   4285          return err;
>                 ^^^^^^^^^^
>   4286  }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* general protection fault in smc_tx_prepared_sends
From: syzbot @ 2018-08-02 12:48 UTC (permalink / raw)
  To: davem, linux-kernel, linux-s390, netdev, syzkaller-bugs, ubraun

Hello,

syzbot found the following crash on:

HEAD commit:    6b4703768268 Merge branch 'fixes' of git://git.armlinux.or..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14d53fc2400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
dashboard link: https://syzkaller.appspot.com/bug?extid=5b2cece1a8ecb2ca77d8
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=119fea72400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1775828c400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5b2cece1a8ecb2ca77d8@syzkaller.appspotmail.com

8021q: adding VLAN 0 to HW filter on device team0
TCP: request_sock_TCP: Possible SYN flooding on port 20002. Sending  
cookies.  Check SNMP counters.
TCP: request_sock_TCP: Possible SYN flooding on port 20002. Sending  
cookies.  Check SNMP counters.
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
CPU: 0 PID: 6281 Comm: syz-executor186 Not tainted 4.18.0-rc7+ #173
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:smc_tx_prepared_sends+0x2c3/0x550 net/smc/smc_tx.h:27
Code: 48 89 f8 48 c1 e8 03 80 3c 10 00 0f 85 11 02 00 00 48 b8 00 00 00 00  
00 fc ff df 4d 8b 76 38 49 8d 7e 20 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84  
c0 74 08 3c 03 0f 8e de 01 00 00 41 8b 46 20 49 8d
RSP: 0018:ffff8801c8447560 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 1ffff10039088eae RCX: dffffc0000000000
RDX: 0000000000000004 RSI: 1ffff10039088eba RDI: 0000000000000020
RBP: ffff8801c8447738 R08: ffffed0039088ebb R09: ffffed0039088eba
R10: ffffed0039088eba R11: ffff8801c84475d7 R12: ffff8801c8447710
R13: ffff8801c84475d0 R14: 0000000000000000 R15: ffff8801c8447590
FS:  00007fada0c46700(0000) GS:ffff8801db000000(0000) knlGS:0000000000000000
TCP: request_sock_TCP: Possible SYN flooding on port 20002. Sending  
cookies.  Check SNMP counters.
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffdba80dc7c CR3: 00000001ae829000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
TCP: request_sock_TCP: Possible SYN flooding on port 20002. Sending  
cookies.  Check SNMP counters.
Call Trace:
kasan: CONFIG_KASAN_INLINE enabled
  smc_ioctl+0x36c/0xd90 net/smc/af_smc.c:1565
kasan: GPF could be caused by NULL-ptr deref or user memory access
  sock_do_ioctl+0xe4/0x3e0 net/socket.c:970
  sock_ioctl+0x30d/0x680 net/socket.c:1094
  vfs_ioctl fs/ioctl.c:46 [inline]
  file_ioctl fs/ioctl.c:500 [inline]
  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
  __do_sys_ioctl fs/ioctl.c:708 [inline]
  __se_sys_ioctl fs/ioctl.c:706 [inline]
  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x446a09
Code: e8 4c e7 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 3b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fada0c45db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000006dcc38 RCX: 0000000000446a09
RDX: 0000000020000140 RSI: 000000000000894b RDI: 0000000000000004
RBP: 00000000006dcc30 R08: 00007fada0c46700 R09: 0000000000000000
R10: 00007fada0c46700 R11: 0000000000000246 R12: 00000000006dcc3c
R13: 00007ffdba80e09f R14: 00007fada0c469c0 R15: 00000000006dcc30
Modules linked in:
Dumping ftrace buffer:
    (ftrace buffer empty)
general protection fault: 0000 [#2] SMP KASAN
---[ end trace 7a16431e05ebb360 ]---
CPU: 1 PID: 6317 Comm: syz-executor186 Tainted: G      D            
4.18.0-rc7+ #173
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:smc_tx_prepared_sends+0x2c3/0x550 net/smc/smc_tx.h:27
RIP: 0010:smc_tx_prepared_sends+0x2c3/0x550 net/smc/smc_tx.h:27
Code: 48 89 f8
Code:
48 c1 e8 03 80
48
3c 10 00 0f 85
89
11 02 00 00
f8
48 b8 00 00
48
00 00 00 fc ff
c1
df 4d 8b 76
e8
38 49 8d 7e 20
03
48 89 fa 48
80
c1 ea 03 <0f>
3c
b6 04 02 84
10
c0 74 08 3c 03
00
0f 8e de 01 00
0f
00 41 8b 46 20
85
49 8d
RSP: 0018:ffff8801c4c0f560 EFLAGS: 00010202
11
RAX: dffffc0000000000 RBX: 1ffff10038981eae RCX: dffffc0000000000
RDX: 0000000000000004 RSI: 1ffff10038981eba RDI: 0000000000000020
RBP: ffff8801c4c0f738 R08: ffffed0038981ebb R09: ffffed0038981eba
02
R10: ffffed0038981eba R11: ffff8801c4c0f5d7 R12: ffff8801c4c0f710
R13: ffff8801c4c0f5d0 R14: 0000000000000000 R15: ffff8801c4c0f590
FS:  00007fada0c46700(0000) GS:ffff8801db100000(0000) knlGS:0000000000000000
00
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f723479f9d4 CR3: 00000001b24d0000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
00
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
48
b8
00
00
00
00
00
  smc_ioctl+0x36c/0xd90 net/smc/af_smc.c:1565
fc
  sock_do_ioctl+0xe4/0x3e0 net/socket.c:970
ff
df
4d
8b
  sock_ioctl+0x30d/0x680 net/socket.c:1094
76
38
  vfs_ioctl fs/ioctl.c:46 [inline]
  file_ioctl fs/ioctl.c:500 [inline]
  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:684
49
8d
7e
20
  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
  __do_sys_ioctl fs/ioctl.c:708 [inline]
  __se_sys_ioctl fs/ioctl.c:706 [inline]
  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
48
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
89
fa
48
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x446a09
c1
Code: e8 4c
ea
e7 ff ff 48
03
83 c4 18 c3 0f
<0f>
1f 80 00 00
b6
00 00 48 89
04
f8 48 89 f7
02
48 89 d6 48 89
84
ca 4d 89 c2
c0
4d 89 c8 4c 8b
74
4c 24 08 0f
08
05 <48> 3d 01
3c
f0 ff ff 0f
03
83 3b 08 fc
0f
ff c3 66 2e
8e
0f 1f 84 00 00
de
00 00
RSP: 002b:00007fada0c45db8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
01
RAX: ffffffffffffffda RBX: 00000000006dcc38 RCX: 0000000000446a09
RDX: 0000000020000140 RSI: 000000000000894b RDI: 0000000000000004
RBP: 00000000006dcc30 R08: 00007fada0c46700 R09: 0000000000000000
00
R10: 00007fada0c46700 R11: 0000000000000246 R12: 00000000006dcc3c
R13: 00007ffdba80e09f R14: 00007fada0c469c0 R15: 00000000006dcc30
Modules linked in:
00
Dumping ftrace buffer:
    (ftrace buffer empty)
41
---[ end trace 7a16431e05ebb361 ]---
8b
RIP: 0010:smc_tx_prepared_sends+0x2c3/0x550 net/smc/smc_tx.h:27
46 20
Code:
49 8d
48
RSP: 0018:ffff8801c8447560 EFLAGS: 00010202
89
RAX: dffffc0000000000 RBX: 1ffff10039088eae RCX: dffffc0000000000
RDX: 0000000000000004 RSI: 1ffff10039088eba RDI: 0000000000000020
f8
RBP: ffff8801c8447738 R08: ffffed0039088ebb R09: ffffed0039088eba
R10: ffffed0039088eba R11: ffff8801c84475d7 R12: ffff8801c8447710
48
R13: ffff8801c84475d0 R14: 0000000000000000 R15: ffff8801c8447590
FS:  00007fada0c46700(0000) GS:ffff8801db000000(0000) knlGS:0000000000000000
c1
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007ffdba80dc7c CR3: 00000001ae829000 CR4: 00000000001406f0
e8
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
03


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* KASAN: slab-out-of-bounds Write in tls_push_record (2)
From: syzbot @ 2018-08-02 13:05 UTC (permalink / raw)
  To: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    44960f2a7b63 staging: ashmem: Fix SIGBUS crash when traver..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10f7ea72400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2dc0cd7c2eefb46f
dashboard link: https://syzkaller.appspot.com/bug?extid=3d650eba9d63b8de7478
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+3d650eba9d63b8de7478@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: slab-out-of-bounds in tls_fill_prepend include/net/tls.h:339  
[inline]
BUG: KASAN: slab-out-of-bounds in tls_push_record+0x1091/0x1400  
net/tls/tls_sw.c:239
Write of size 1 at addr ffff88019c5d8000 by task syz-executor2/11760

CPU: 0 PID: 11760 Comm: syz-executor2 Not tainted 4.18.0-rc7+ #172
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
  print_address_description+0x6c/0x20b mm/kasan/report.c:256
  kasan_report_error mm/kasan/report.c:354 [inline]
  kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
  __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
  tls_fill_prepend include/net/tls.h:339 [inline]
  tls_push_record+0x1091/0x1400 net/tls/tls_sw.c:239
  tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
  tls_handle_open_record net/tls/tls_main.c:164 [inline]
  tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
  inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
  __sock_release+0xd7/0x260 net/socket.c:600
  sock_close+0x19/0x20 net/socket.c:1151
  __fput+0x355/0x8b0 fs/file_table.c:209
  ____fput+0x15/0x20 fs/file_table.c:243
  task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
  tracehook_notify_resume include/linux/tracehook.h:192 [inline]
  exit_to_usermode_loop+0x313/0x370 arch/x86/entry/common.c:166
  prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
  syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
  do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4105c1
Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 34 19 00 00 c3 48  
83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48  
89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:0000000000a3feb0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000014 RCX: 00000000004105c1
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000013
RBP: 0000000000000010 R08: 0000000000000000 R09: 0000000000000001
R10: 0000000000a3fde0 R11: 0000000000000293 R12: 0000000000000000
R13: 000000000002fd87 R14: 0000000000000094 R15: badc0ffeebadface

Allocated by task 2597:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  kasan_kmalloc+0xc4/0xe0 mm/kasan/kasan.c:553
  kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
  kmem_cache_alloc+0x12e/0x760 mm/slab.c:3554
  getname_flags+0xd0/0x5a0 fs/namei.c:140
  user_path_at_empty+0x2d/0x50 fs/namei.c:2584
  user_path_at include/linux/namei.h:57 [inline]
  vfs_statx+0x129/0x210 fs/stat.c:185
  vfs_stat include/linux/fs.h:3102 [inline]
  __do_sys_newstat+0x8f/0x110 fs/stat.c:337
  __se_sys_newstat fs/stat.c:333 [inline]
  __x64_sys_newstat+0x54/0x80 fs/stat.c:333
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 2597:
  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
  set_track mm/kasan/kasan.c:460 [inline]
  __kasan_slab_free+0x11a/0x170 mm/kasan/kasan.c:521
  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
  __cache_free mm/slab.c:3498 [inline]
  kmem_cache_free+0x86/0x2d0 mm/slab.c:3756
  putname+0xf2/0x130 fs/namei.c:261
  filename_lookup+0x397/0x510 fs/namei.c:2330
  user_path_at_empty+0x40/0x50 fs/namei.c:2584
  user_path_at include/linux/namei.h:57 [inline]
  vfs_statx+0x129/0x210 fs/stat.c:185
  vfs_stat include/linux/fs.h:3102 [inline]
  __do_sys_newstat+0x8f/0x110 fs/stat.c:337
  __se_sys_newstat fs/stat.c:333 [inline]
  __x64_sys_newstat+0x54/0x80 fs/stat.c:333
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff88019c5d8980
  which belongs to the cache names_cache of size 4096
The buggy address is located 2432 bytes to the left of
  4096-byte region [ffff88019c5d8980, ffff88019c5d9980)
The buggy address belongs to the page:
page:ffffea0006717600 count:1 mapcount:0 mapping:ffff8801dad85dc0 index:0x0  
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0006470d88 ffffea0006717688 ffff8801dad85dc0
raw: 0000000000000000 ffff88019c5d8980 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
  ffff88019c5d7f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ffff88019c5d7f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff88019c5d8000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                    ^
  ffff88019c5d8080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  ffff88019c5d8100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.

^ permalink raw reply

* Re: AW: AW: PROBLEM: Kernel Oops in UDP stack
From: Eric Dumazet @ 2018-08-02 15:07 UTC (permalink / raw)
  To: Marcel Hellwig, 'Eric Dumazet', David Laight,
	'davem@davemloft.net', 'kuznet@ms2.inr.ac.ru',
	'yoshfuji@linux-ipv6.org', 'andrew@lunn.ch'
  Cc: 'netdev@vger.kernel.org',
	'linux-kernel@vger.kernel.org', Matthias Wystrik
In-Reply-To: <18341daf5b2c458db8e30299d6cddafc@ZCOM03.mut-group.com>



On 08/02/2018 06:57 AM, Marcel Hellwig wrote:
>>> There are actually 2 faults, difficult to quickly sort out the merged tracebacks.
>>> You are also running a rather old kernel: Linux version 3.4.113.
>>>
>>> It may well be that whichever ethernet driver generated the misaligned 
>>> frame has since been fixed.
>>
>> A misalign frame driver problem would have faulted earlier in IP stack, much before we perform the copy to user space in udp_recvmsg()
>>
> 
> JFYI: we are talking about the lpc_eth driver[0] #57c10b6 , which is not the newest, but all newer did not fix a major problem (at least the commit messages are not screaming: WARNING, UNALIGNED MEMORY!). Is there a diagram/document how a ip packet travels down the code? From the MAC/phy driver to udp_recvmsg? It's not that obvious for me, but maybe it is something I can work with.
> 
> 
> [0]: https://elixir.bootlin.com/linux/v3.4.113/source/drivers/net/ethernet/nxp/lpc_eth.c
> 
> Regards,
> Marcel
> 


Well, this driver does not use NET_IP_ALIGN reservation, meaning IP header is not 4-byte aligned.

No idea why mis-alignments are okay in IP layer, but not in UDP

You could try to patch it to use netdev_alloc_skb_ip_align() instead of dev_alloc_skb()

^ permalink raw reply

* Re: [PATCH v7 bpf-next 05/10] veth: Handle xdp_frames in xdp napi ring
From: Toshiaki Makita @ 2018-08-02 13:17 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Toshiaki Makita
  Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Jakub Kicinski,
	John Fastabend
In-Reply-To: <20180802134548.2230455b@redhat.com>

On 18/08/02 (木) 20:45, Jesper Dangaard Brouer wrote:
> On Thu,  2 Aug 2018 19:55:09 +0900
> Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> 
>> +	headroom = frame->data - delta - (void *)frame;
> 
> Your calculation of headroom is still adding an assumption that
> xdp_frame is located in the top of data area, that is unnecessary.
> 
> The headroom can be calculated as:
> 
>   headroom = sizeof(struct xdp_frame) + frame->headroom - delta;

Thanks. But I'm not sure I get what you are requesting.
Supposing xdp_frame is not located in the top of data area, what ensures 
that additional sizeof(struct xdp_frame) can be used?

Toshiaki Makita

^ permalink raw reply

* Re: UDP packets arriving on wrong sockets
From: Eric Dumazet @ 2018-08-02 13:20 UTC (permalink / raw)
  To: Andrew Cann, netdev
In-Reply-To: <20180802090505.GA29624@canndrew.org>



On 08/02/2018 02:05 AM, Andrew Cann wrote:
> Hi, 
> 
> I posted this on stackoverflow yesterday but I'm reposting it here since it got
> no response. Original post: https://stackoverflow.com/questions/51630337/udp-packets-arriving-on-wrong-sockets-on-linux
> 
> I have two UDP sockets bound to the same address and connected to addresses A
> and B. I have two more UDP sockets bound to A and B and not connected.
> 
> This is what my /proc/net/udp looks like (trimmed for readability):
> 
>       sl  local_address rem_address
>      3937: 0100007F:DD9C 0300007F:9910
>      3937: 0100007F:DD9C 0200007F:907D
>     16962: 0200007F:907D 00000000:0000
>     19157: 0300007F:9910 00000000:0000
> 
> According to connect(2): "If the socket sockfd is of type SOCK_DGRAM, then addr
> is the address to which datagrams are sent by default, *and the only address
> from which datagrams are received*."
> 
> For some reason, my connected sockets are receiving packets that were destined
> for each other. eg: The UDP socket connected to A sends a message to A, A then
> sends a reply back. The UDP socket connected to B sends a message to B, B then
> sends a reply back. But the reply from A arrives at the socket connected to B
> and the reply from B arrives at the socket connected to A.
> 
> Why on earth would this be happening? Note that it happens randomly - sometimes
> the replies arrive at the correct sockets and sometimes they don't. Is there
> any way to prevent this or any situation under which connect() is supposed to
> not work?
> 
> Any help explaining this would be hugely appreciated :)

Hi Andrew

Well, you should first give much more details, as there are thousands of different UDP stacks out there.

Documentation/admin-guide/reporting-bugs.rst

...
[4.1.] Kernel version (from /proc/version): 
...

Ideally you could give us a C reproducer, so that we can run it ourselves and fix the kernel bug if there is one.

This C reproducer could be part of an official patch, adding a test in tools/testing/selftests/net

Thanks !

^ permalink raw reply

* Re: UDP packets arriving on wrong sockets
From: Eric Dumazet @ 2018-08-02 13:35 UTC (permalink / raw)
  To: Eric Dumazet, Andrew Cann, netdev
In-Reply-To: <be3908e6-252f-eb10-a7ac-e2559e98dda6@gmail.com>



On 08/02/2018 06:20 AM, Eric Dumazet wrote:
> 
> Ideally you could give us a C reproducer, so that we can run it ourselves and fix the kernel bug if there is one.
> 
> This C reproducer could be part of an official patch, adding a test in tools/testing/selftests/net

Alternatively a test in Python would be accepted ;)

^ permalink raw reply

* Re: [PATCH v7 bpf-next 05/10] veth: Handle xdp_frames in xdp napi ring
From: Jesper Dangaard Brouer @ 2018-08-02 13:53 UTC (permalink / raw)
  To: Toshiaki Makita
  Cc: Toshiaki Makita, Alexei Starovoitov, Daniel Borkmann, netdev,
	Jakub Kicinski, John Fastabend, brouer
In-Reply-To: <4402804f-bf7b-9ca0-baa9-1415884b0a80@gmail.com>

On Thu, 2 Aug 2018 22:17:53 +0900
Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:

> On 18/08/02 (木) 20:45, Jesper Dangaard Brouer wrote:
> > On Thu,  2 Aug 2018 19:55:09 +0900
> > Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> wrote:
> >   
> >> +	headroom = frame->data - delta - (void *)frame;  
> > 
> > Your calculation of headroom is still adding an assumption that
> > xdp_frame is located in the top of data area, that is unnecessary.
> > 
> > The headroom can be calculated as:
> > 
> >   headroom = sizeof(struct xdp_frame) + frame->headroom - delta;  
> 
> Thanks. But I'm not sure I get what you are requesting.

I'm simply requesting you do not use the (void *)frame pointer address,
to calculate the headroom, as it can be calculated in another way.

> Supposing xdp_frame is not located in the top of data area, what ensures 
> that additional sizeof(struct xdp_frame) can be used?

The calculation in convert_to_xdp_frame() assures this.  If we later
add an xdp_frame that is not located in the top of data area, and want
to change the reserved headroom size, then we deal with it, and update
the code.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ 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