[PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

The patch adds a basic XDP processing to xen-netfront driver.

We ran an XDP program for an RX response received from netback
driver. Also we request xen-netback to adjust data offset for
bpf_xdp_adjust_head() header space for custom headers.

v4:
- added verbose patch descriprion
- don't expose the XDP headroom offset to the domU guest
- add a modparam to netback to toggle XDP offset
- don't process jumbo frames for now

v3:
- added XDP_TX support (tested with xdping echoserver)
- added XDP_REDIRECT support (tested with modified xdp_redirect_kern)
- moved xdp negotiation to xen-netback

v2:
- avoid data copying while passing to XDP
- tell xen-netback that we need the headroom space

Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
 drivers/net/xen-netback/common.h  |   2 +
 drivers/net/xen-netback/netback.c |   7 ++
 drivers/net/xen-netback/rx.c      |   7 +-
 drivers/net/xen-netback/xenbus.c  |  28 +++++
 drivers/net/xen-netfront.c        | 240 +++++++++++++++++++++++++++++++++++++-
 5 files changed, 282 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb..4a148d6 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -280,6 +280,7 @@ struct xenvif {
 	u8 ip_csum:1;
 	u8 ipv6_csum:1;
 	u8 multicast_control:1;
+	u8 xdp_enabled:1;
 
 	/* Is this interface disabled? True when backend discovers
 	 * frontend is rogue.
@@ -395,6 +396,7 @@ static inline pending_ring_idx_t nr_pending_reqs(struct xenvif_queue *queue)
 irqreturn_t xenvif_interrupt(int irq, void *dev_id);
 
 extern bool separate_tx_rx_irq;
+extern bool provides_xdp_headroom;
 
 extern unsigned int rx_drain_timeout_msecs;
 extern unsigned int rx_stall_timeout_msecs;
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 315dfc6..6dfca72 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -96,6 +96,13 @@
 module_param_named(hash_cache_size, xenvif_hash_cache_size, uint, 0644);
 MODULE_PARM_DESC(hash_cache_size, "Number of flows in the hash cache");
 
+/* The module parameter tells that we have to put data
+ * for xen-netfront with the XDP_PACKET_HEADROOM offset
+ * needed for XDP processing
+ */
+bool provides_xdp_headroom = true;
+module_param(provides_xdp_headroom, bool, 0644);
+
 static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
 			       u8 status);
 
diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
index ef58870..aba826b 100644
--- a/drivers/net/xen-netback/rx.c
+++ b/drivers/net/xen-netback/rx.c
@@ -33,6 +33,11 @@
 #include <xen/xen.h>
 #include <xen/events.h>
 
+static inline int xenvif_rx_xdp_offset(struct xenvif *vif)
+{
+	return (vif->xdp_enabled ? XDP_PACKET_HEADROOM : 0);
+}
+
 static bool xenvif_rx_ring_slots_available(struct xenvif_queue *queue)
 {
 	RING_IDX prod, cons;
@@ -356,7 +361,7 @@ static void xenvif_rx_data_slot(struct xenvif_queue *queue,
 				struct xen_netif_rx_request *req,
 				struct xen_netif_rx_response *rsp)
 {
-	unsigned int offset = 0;
+	unsigned int offset = xenvif_rx_xdp_offset(queue->vif);
 	unsigned int flags;
 
 	do {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 286054b..7c0450e 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -393,6 +393,20 @@ static void set_backend_state(struct backend_info *be,
 	}
 }
 
+static void read_xenbus_frontend_xdp(struct backend_info *be,
+				      struct xenbus_device *dev)
+{
+	struct xenvif *vif = be->vif;
+	unsigned int val;
+	int err;
+
+	err = xenbus_scanf(XBT_NIL, dev->otherend,
+			   "feature-xdp", "%u", &val);
+	if (err < 0)
+		return;
+	vif->xdp_enabled = val;
+}
+
 /**
  * Callback received when the frontend's state changes.
  */
@@ -417,6 +431,11 @@ static void frontend_changed(struct xenbus_device *dev,
 		set_backend_state(be, XenbusStateConnected);
 		break;
 
+	case XenbusStateReconfiguring:
+		read_xenbus_frontend_xdp(be, dev);
+		xenbus_switch_state(dev, XenbusStateReconfigured);
+		break;
+
 	case XenbusStateClosing:
 		set_backend_state(be, XenbusStateClosing);
 		break;
@@ -1036,6 +1055,15 @@ static int netback_probe(struct xenbus_device *dev,
 			goto abort_transaction;
 		}
 
+		/* we can adjust a headroom for netfront XDP processing */
+		err = xenbus_printf(xbt, dev->nodename,
+				    "feature-xdp-headroom", "%d",
+				    !!provides_xdp_headroom);
+		if (err) {
+			message = "writing feature-xdp-headroom";
+			goto abort_transaction;
+		}
+
 		/* We don't support rx-flip path (except old guests who
 		 * don't grok this feature flag).
 		 */
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 482c6c8..c06ae57 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -44,6 +44,8 @@
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <net/ip.h>
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
 
 #include <xen/xen.h>
 #include <xen/xenbus.h>
@@ -102,6 +104,8 @@ struct netfront_queue {
 	char name[QUEUE_NAME_SIZE]; /* DEVNAME-qN */
 	struct netfront_info *info;
 
+	struct bpf_prog __rcu *xdp_prog;
+
 	struct napi_struct napi;
 
 	/* Split event channels support, tx_* == rx_* when using
@@ -144,6 +148,8 @@ struct netfront_queue {
 	struct sk_buff *rx_skbs[NET_RX_RING_SIZE];
 	grant_ref_t gref_rx_head;
 	grant_ref_t grant_rx_ref[NET_RX_RING_SIZE];
+
+	struct xdp_rxq_info xdp_rxq;
 };
 
 struct netfront_info {
@@ -159,6 +165,8 @@ struct netfront_info {
 	struct netfront_stats __percpu *rx_stats;
 	struct netfront_stats __percpu *tx_stats;
 
+	bool netback_has_xdp_headroom;
+
 	atomic_t rx_gso_checksum_fixup;
 };
 
@@ -167,6 +175,9 @@ struct netfront_rx_info {
 	struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX - 1];
 };
 
+static int xennet_xdp_xmit(struct net_device *dev, int n,
+			   struct xdp_frame **frames, u32 flags);
+
 static void skb_entry_set_link(union skb_entry *list, unsigned short id)
 {
 	list->link = id;
@@ -406,7 +417,8 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
 			queue->grant_tx_ref[id] = GRANT_INVALID_REF;
 			queue->grant_tx_page[id] = NULL;
 			add_id_to_freelist(&queue->tx_skb_freelist, queue->tx_skbs, id);
-			dev_kfree_skb_irq(skb);
+			if (skb)
+				dev_kfree_skb_irq(skb);
 		}
 
 		queue->tx.rsp_cons = prod;
@@ -778,6 +790,52 @@ static int xennet_get_extras(struct netfront_queue *queue,
 	return err;
 }
 
+u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,
+		   struct xen_netif_rx_response *rx, struct bpf_prog *prog,
+		   struct xdp_buff *xdp)
+{
+	struct xdp_frame *xdpf;
+	u32 len = rx->status;
+	u32 act = XDP_PASS;
+	int err;
+
+	xdp->data_hard_start = page_address(pdata);
+	xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
+	xdp_set_data_meta_invalid(xdp);
+	xdp->data_end = xdp->data + len;
+	xdp->rxq = &queue->xdp_rxq;
+	xdp->handle = 0;
+
+	act = bpf_prog_run_xdp(prog, xdp);
+	switch (act) {
+	case XDP_TX:
+		xdpf = convert_to_xdp_frame(xdp);
+		err = xennet_xdp_xmit(queue->info->netdev, 1,
+				&xdpf, 0);
+		if (unlikely(err < 0))
+			trace_xdp_exception(queue->info->netdev, prog, act);
+		break;
+	case XDP_REDIRECT:
+		err = xdp_do_redirect(queue->info->netdev, xdp, prog);
+		if (unlikely(err))
+			trace_xdp_exception(queue->info->netdev, prog, act);
+		xdp_do_flush();
+		break;
+	case XDP_PASS:
+	case XDP_DROP:
+		break;
+
+	case XDP_ABORTED:
+		trace_xdp_exception(queue->info->netdev, prog, act);
+		break;
+
+	default:
+		bpf_warn_invalid_xdp_action(act);
+	}
+
+	return act;
+}
+
 static int xennet_get_responses(struct netfront_queue *queue,
 				struct netfront_rx_info *rinfo, RING_IDX rp,
 				struct sk_buff_head *list)
@@ -792,6 +850,9 @@ static int xennet_get_responses(struct netfront_queue *queue,
 	int slots = 1;
 	int err = 0;
 	unsigned long ret;
+	struct bpf_prog *xdp_prog;
+	struct xdp_buff xdp;
+	u32 verdict;
 
 	if (rx->flags & XEN_NETRXF_extra_info) {
 		err = xennet_get_extras(queue, extras, rp);
@@ -827,6 +888,21 @@ static int xennet_get_responses(struct netfront_queue *queue,
 
 		gnttab_release_grant_reference(&queue->gref_rx_head, ref);
 
+		rcu_read_lock();
+		xdp_prog = rcu_dereference(queue->xdp_prog);
+		if (xdp_prog && !(rx->flags & XEN_NETRXF_more_data)) {
+			/* currently only a single page contains data */
+			WARN_ON_ONCE(skb_shinfo(skb)->nr_frags != 1);
+			verdict = xennet_run_xdp(queue,
+				       skb_frag_page(&skb_shinfo(skb)->frags[0]),
+				       rx, xdp_prog, &xdp);
+			if (verdict != XDP_PASS && verdict != XDP_TX) {
+				err = -EINVAL;
+				goto next;
+			}
+
+		}
+		rcu_read_unlock();
 		__skb_queue_tail(list, skb);
 
 next:
@@ -1261,6 +1337,144 @@ static void xennet_poll_controller(struct net_device *dev)
 }
 #endif
 
+#define NETBACK_XDP_HEADROOM_DISABLE	0
+#define NETBACK_XDP_HEADROOM_ENABLE	1
+
+static int talk_to_netback_xdp(struct netfront_info *np, int xdp)
+{
+	int err;
+
+	err = xenbus_printf(XBT_NIL, np->xbdev->nodename,
+			     "feature-xdp", "%u", xdp);
+	if (err)
+		pr_debug("Error writing feature-xdp\n");
+
+	return err;
+}
+
+static int xennet_xdp_set(struct net_device *dev, struct bpf_prog *prog,
+			struct netlink_ext_ack *extack)
+{
+	struct netfront_info *np = netdev_priv(dev);
+	struct bpf_prog *old_prog;
+	unsigned int i, err;
+
+	if (!np->netback_has_xdp_headroom)
+		return 0;
+
+	old_prog = rtnl_dereference(np->queues[0].xdp_prog);
+	if (!old_prog && !prog)
+		return 0;
+
+	if (prog)
+		bpf_prog_add(prog, dev->real_num_tx_queues);
+
+	for (i = 0; i < dev->real_num_tx_queues; ++i)
+		rcu_assign_pointer(np->queues[i].xdp_prog, prog);
+
+	if (old_prog)
+		for (i = 0; i < dev->real_num_tx_queues; ++i)
+			bpf_prog_put(old_prog);
+
+	err = talk_to_netback_xdp(np, old_prog ? NETBACK_XDP_HEADROOM_DISABLE:
+				  NETBACK_XDP_HEADROOM_ENABLE);
+	if (err)
+		return err;
+
+	xenbus_switch_state(np->xbdev, XenbusStateReconfiguring);
+
+	return 0;
+}
+
+static u32 xennet_xdp_query(struct net_device *dev)
+{
+	struct netfront_info *np = netdev_priv(dev);
+	unsigned int num_queues = dev->real_num_tx_queues;
+	unsigned int i;
+	struct netfront_queue *queue;
+	const struct bpf_prog *xdp_prog;
+
+	for (i = 0; i < num_queues; ++i) {
+		queue = &np->queues[i];
+		xdp_prog = rtnl_dereference(queue->xdp_prog);
+		if (xdp_prog)
+			return xdp_prog->aux->id;
+	}
+
+	return 0;
+}
+
+static int xennet_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+	switch (xdp->command) {
+	case XDP_SETUP_PROG:
+		return xennet_xdp_set(dev, xdp->prog, xdp->extack);
+	case XDP_QUERY_PROG:
+		xdp->prog_id = xennet_xdp_query(dev);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int xennet_xdp_xmit_one(struct net_device *dev, struct xdp_frame *xdpf)
+{
+	struct netfront_info *np = netdev_priv(dev);
+	struct netfront_stats *tx_stats = this_cpu_ptr(np->tx_stats);
+	struct netfront_queue *queue = NULL;
+	unsigned int num_queues = dev->real_num_tx_queues;
+	unsigned long flags;
+	int notify;
+	struct xen_netif_tx_request *tx;
+
+	queue = &np->queues[smp_processor_id() % num_queues];
+
+	spin_lock_irqsave(&queue->tx_lock, flags);
+
+	tx = xennet_make_first_txreq(queue, NULL,
+				     virt_to_page(xdpf->data),
+				     offset_in_page(xdpf->data),
+				     xdpf->len);
+
+	RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&queue->tx, notify);
+	if (notify)
+		notify_remote_via_irq(queue->tx_irq);
+
+	u64_stats_update_begin(&tx_stats->syncp);
+	tx_stats->bytes += xdpf->len;
+	tx_stats->packets++;
+	u64_stats_update_end(&tx_stats->syncp);
+
+	xennet_tx_buf_gc(queue);
+
+	spin_unlock_irqrestore(&queue->tx_lock, flags);
+	return 0;
+}
+
+static int xennet_xdp_xmit(struct net_device *dev, int n,
+			   struct xdp_frame **frames, u32 flags)
+{
+	int drops = 0;
+	int i, err;
+
+	if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+		return -EINVAL;
+
+	for (i = 0; i < n; i++) {
+		struct xdp_frame *xdpf = frames[i];
+
+		if (!xdpf)
+			continue;
+		err = xennet_xdp_xmit_one(dev, xdpf);
+		if (err) {
+			xdp_return_frame_rx_napi(xdpf);
+			drops++;
+		}
+	}
+
+	return n - drops;
+}
+
 static const struct net_device_ops xennet_netdev_ops = {
 	.ndo_open            = xennet_open,
 	.ndo_stop            = xennet_close,
@@ -1272,6 +1486,8 @@ static void xennet_poll_controller(struct net_device *dev)
 	.ndo_fix_features    = xennet_fix_features,
 	.ndo_set_features    = xennet_set_features,
 	.ndo_select_queue    = xennet_select_queue,
+	.ndo_bpf            = xennet_xdp,
+	.ndo_xdp_xmit	    = xennet_xdp_xmit,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = xennet_poll_controller,
 #endif
@@ -1779,6 +1995,21 @@ static int xennet_create_queues(struct netfront_info *info,
 			break;
 		}
 
+		ret = xdp_rxq_info_reg(&queue->xdp_rxq, queue->info->netdev,
+				       queue->id);
+		if (ret) {
+			dev_err(&info->xbdev->dev, "xdp_rxq_info_reg failed\n");
+			return -EINVAL;
+		}
+
+		ret = xdp_rxq_info_reg_mem_model(&queue->xdp_rxq,
+						 MEM_TYPE_PAGE_ORDER0, NULL);
+		if (ret) {
+			dev_err(&info->xbdev->dev, "xdp_rxq_info_reg_mem_model failed\n");
+			return -EINVAL;
+		}
+
+
 		netif_napi_add(queue->info->netdev, &queue->napi,
 			       xennet_poll, 64);
 		if (netif_running(info->netdev))
@@ -1825,6 +2056,8 @@ static int talk_to_netback(struct xenbus_device *dev,
 		goto out_unlocked;
 	}
 
+	info->netback_has_xdp_headroom = xenbus_read_unsigned(info->xbdev->otherend,
+							      "feature-xdp-headroom", 0);
 	rtnl_lock();
 	if (info->queues)
 		xennet_destroy_queues(info);
@@ -1959,6 +2192,8 @@ static int xennet_connect(struct net_device *dev)
 	err = talk_to_netback(np->xbdev, np);
 	if (err)
 		return err;
+	if (np->netback_has_xdp_headroom)
+		pr_info("backend supports XDP headroom\n");
 
 	/* talk_to_netback() sets the correct number of queues */
 	num_queues = dev->real_num_tx_queues;
@@ -2019,7 +2254,10 @@ static void netback_changed(struct xenbus_device *dev,
 	case XenbusStateInitialising:
 	case XenbusStateInitialised:
 	case XenbusStateReconfiguring:
+		break;
 	case XenbusStateReconfigured:
+		xenbus_switch_state(dev, XenbusStateConnected);
+		break;
 	case XenbusStateUnknown:
 		break;
 
-- 
1.8.3.1

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jesper Dangaard Brouer]

On Mon, 16 Mar 2020 16:09:36 +0300
Denis Kirjanov <kda@linux-powerpc.org> wrote:

> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 482c6c8..c06ae57 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
[...]
> @@ -778,6 +790,52 @@ static int xennet_get_extras(struct netfront_queue *queue,
>  	return err;
>  }
>  
> +u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,
> +		   struct xen_netif_rx_response *rx, struct bpf_prog *prog,
> +		   struct xdp_buff *xdp)
> +{
> +	struct xdp_frame *xdpf;
> +	u32 len = rx->status;
> +	u32 act = XDP_PASS;
> +	int err;
> +
> +	xdp->data_hard_start = page_address(pdata);
> +	xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
> +	xdp_set_data_meta_invalid(xdp);
> +	xdp->data_end = xdp->data + len;
> +	xdp->rxq = &queue->xdp_rxq;
> +	xdp->handle = 0;
> +
> +	act = bpf_prog_run_xdp(prog, xdp);
> +	switch (act) {
> +	case XDP_TX:
> +		xdpf = convert_to_xdp_frame(xdp);
> +		err = xennet_xdp_xmit(queue->info->netdev, 1,
> +				&xdpf, 0);

Strange line wrap, I don't think this is needed, please fix.


> +		if (unlikely(err < 0))
> +			trace_xdp_exception(queue->info->netdev, prog, act);
> +		break;
> +	case XDP_REDIRECT:
> +		err = xdp_do_redirect(queue->info->netdev, xdp, prog);

What is the frame size of the packet memory?


> +		if (unlikely(err))
> +			trace_xdp_exception(queue->info->netdev, prog, act);
> +		xdp_do_flush();
> +		break;
> +	case XDP_PASS:
> +	case XDP_DROP:
> +		break;
> +
> +	case XDP_ABORTED:
> +		trace_xdp_exception(queue->info->netdev, prog, act);
> +		break;
> +
> +	default:
> +		bpf_warn_invalid_xdp_action(act);
> +	}
> +
> +	return act;
> +}
> +

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

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jesper Dangaard Brouer]

On Mon, 16 Mar 2020 16:09:36 +0300
Denis Kirjanov <kda@linux-powerpc.org> wrote:

> v3:
> - added XDP_TX support (tested with xdping echoserver)
> - added XDP_REDIRECT support (tested with modified xdp_redirect_kern)

Please test XDP_REDIRECT with xdp_redirect_cpu from samples/bpf/.

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

RE: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Paul Durrant]

> -----Original Message-----
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Sent: 16 March 2020 13:10
> To: netdev@vger.kernel.org
> Cc: jgross@suse.com; ilias.apalodimas@linaro.org; wei.liu@kernel.org; paul@xen.org; Denis Kirjanov
> <kda@linux-powerpc.org>
> Subject: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront
> 
> The patch adds a basic XDP processing to xen-netfront driver.
> 
> We ran an XDP program for an RX response received from netback
> driver. Also we request xen-netback to adjust data offset for
> bpf_xdp_adjust_head() header space for custom headers.
> 
> v4:
> - added verbose patch descriprion
> - don't expose the XDP headroom offset to the domU guest
> - add a modparam to netback to toggle XDP offset
> - don't process jumbo frames for now
> 
> v3:
> - added XDP_TX support (tested with xdping echoserver)
> - added XDP_REDIRECT support (tested with modified xdp_redirect_kern)
> - moved xdp negotiation to xen-netback
> 
> v2:
> - avoid data copying while passing to XDP
> - tell xen-netback that we need the headroom space
> 
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> ---
>  drivers/net/xen-netback/common.h  |   2 +
>  drivers/net/xen-netback/netback.c |   7 ++
>  drivers/net/xen-netback/rx.c      |   7 +-
>  drivers/net/xen-netback/xenbus.c  |  28 +++++
>  drivers/net/xen-netfront.c        | 240 +++++++++++++++++++++++++++++++++++++-
>  5 files changed, 282 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 05847eb..4a148d6 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -280,6 +280,7 @@ struct xenvif {
>  	u8 ip_csum:1;
>  	u8 ipv6_csum:1;
>  	u8 multicast_control:1;
> +	u8 xdp_enabled:1;
> 
>  	/* Is this interface disabled? True when backend discovers
>  	 * frontend is rogue.
> @@ -395,6 +396,7 @@ static inline pending_ring_idx_t nr_pending_reqs(struct xenvif_queue *queue)
>  irqreturn_t xenvif_interrupt(int irq, void *dev_id);
> 
>  extern bool separate_tx_rx_irq;
> +extern bool provides_xdp_headroom;
> 
>  extern unsigned int rx_drain_timeout_msecs;
>  extern unsigned int rx_stall_timeout_msecs;
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 315dfc6..6dfca72 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -96,6 +96,13 @@
>  module_param_named(hash_cache_size, xenvif_hash_cache_size, uint, 0644);
>  MODULE_PARM_DESC(hash_cache_size, "Number of flows in the hash cache");
> 
> +/* The module parameter tells that we have to put data
> + * for xen-netfront with the XDP_PACKET_HEADROOM offset
> + * needed for XDP processing
> + */
> +bool provides_xdp_headroom = true;
> +module_param(provides_xdp_headroom, bool, 0644);
> +
>  static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
>  			       u8 status);
> 
> diff --git a/drivers/net/xen-netback/rx.c b/drivers/net/xen-netback/rx.c
> index ef58870..aba826b 100644
> --- a/drivers/net/xen-netback/rx.c
> +++ b/drivers/net/xen-netback/rx.c
> @@ -33,6 +33,11 @@
>  #include <xen/xen.h>
>  #include <xen/events.h>
> 
> +static inline int xenvif_rx_xdp_offset(struct xenvif *vif)
> +{
> +	return (vif->xdp_enabled ? XDP_PACKET_HEADROOM : 0);
> +}
> +

No need for the brackets here.

[snip]
> @@ -167,6 +175,9 @@ struct netfront_rx_info {
>  	struct xen_netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX - 1];
>  };
> 
> +static int xennet_xdp_xmit(struct net_device *dev, int n,
> +			   struct xdp_frame **frames, u32 flags);
> +
>  static void skb_entry_set_link(union skb_entry *list, unsigned short id)
>  {
>  	list->link = id;
> @@ -406,7 +417,8 @@ static void xennet_tx_buf_gc(struct netfront_queue *queue)
>  			queue->grant_tx_ref[id] = GRANT_INVALID_REF;
>  			queue->grant_tx_page[id] = NULL;
>  			add_id_to_freelist(&queue->tx_skb_freelist, queue->tx_skbs, id);
> -			dev_kfree_skb_irq(skb);
> +			if (skb)
> +				dev_kfree_skb_irq(skb);

Can you drop this? I said previously that it is unnecessary.

  Paul

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 16.03.20 14:09, Denis Kirjanov wrote:
> The patch adds a basic XDP processing to xen-netfront driver.
> 
> We ran an XDP program for an RX response received from netback
> driver. Also we request xen-netback to adjust data offset for
> bpf_xdp_adjust_head() header space for custom headers.

This is in no way a "verbose patch descriprion".

I'm missing:

- Why are you doing this. "Add XDP support" is not enough, for such
   a change I'd like to see some performance numbers to get an idea
   of the improvement to expect, or which additional functionality
   for the user is available.

- A short description for me as a Xen maintainer with only basic
   networking know-how, what XDP programs are about (a link to some
   more detailed doc is enough, of course) and how the interface
   is working (especially for switching between XDP mode and normal
   SKB processing).

- A proper description of the netfront/netback communication when
   enabling or disabling XDP mode (who is doing what, is silencing
   of the virtual adapter required, ...).

- Reasoning why the suggested changes of frontend and backend state
   are no problem for special cases like hot-remove of an interface or
   live migration or suspend of the guest.

Finally I'd like to ask you to split up the patch into a netfront and
a netback one.


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/16/20, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Mon, 16 Mar 2020 16:09:36 +0300
> Denis Kirjanov <kda@linux-powerpc.org> wrote:
>
>> v3:
>> - added XDP_TX support (tested with xdping echoserver)
>> - added XDP_REDIRECT support (tested with modified xdp_redirect_kern)
>
> Please test XDP_REDIRECT with xdp_redirect_cpu from samples/bpf/.

./xdp_redirect_cpu --dev eth0 --prog xdp_cpu_map0 --cpu 1

Running XDP/eBPF prog_name:xdp_cpu_map0
XDP-cpumap      CPU:to  pps            drop-pps    extra-info
XDP-RX          0       5,964          0           0
XDP-RX          total   5,964          0
cpumap-enqueue    0:1   5,964          0           1.00       bulk-average
cpumap-enqueue  sum:1   5,964          0           1.00       bulk-average
cpumap_kthread  1       5,963          0           5,939      sched
cpumap_kthread  total   5,963          0           5,939      sched-sum
redirect_err    total   0              0
xdp_exception   total   0              0

I can see increase in RES:
RES:     129814     484285     198201     359270   Rescheduling interrupts


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

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/16/20, Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Mon, 16 Mar 2020 16:09:36 +0300
> Denis Kirjanov <kda@linux-powerpc.org> wrote:
>
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> index 482c6c8..c06ae57 100644
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
> [...]
>> @@ -778,6 +790,52 @@ static int xennet_get_extras(struct netfront_queue
>> *queue,
>>  	return err;
>>  }
>>
>> +u32 xennet_run_xdp(struct netfront_queue *queue, struct page *pdata,
>> +		   struct xen_netif_rx_response *rx, struct bpf_prog *prog,
>> +		   struct xdp_buff *xdp)
>> +{
>> +	struct xdp_frame *xdpf;
>> +	u32 len = rx->status;
>> +	u32 act = XDP_PASS;
>> +	int err;
>> +
>> +	xdp->data_hard_start = page_address(pdata);
>> +	xdp->data = xdp->data_hard_start + XDP_PACKET_HEADROOM;
>> +	xdp_set_data_meta_invalid(xdp);
>> +	xdp->data_end = xdp->data + len;
>> +	xdp->rxq = &queue->xdp_rxq;
>> +	xdp->handle = 0;
>> +
>> +	act = bpf_prog_run_xdp(prog, xdp);
>> +	switch (act) {
>> +	case XDP_TX:
>> +		xdpf = convert_to_xdp_frame(xdp);
>> +		err = xennet_xdp_xmit(queue->info->netdev, 1,
>> +				&xdpf, 0);
>
> Strange line wrap, I don't think this is needed, please fix.
>
>
>> +		if (unlikely(err < 0))
>> +			trace_xdp_exception(queue->info->netdev, prog, act);
>> +		break;
>> +	case XDP_REDIRECT:
>> +		err = xdp_do_redirect(queue->info->netdev, xdp, prog);
>
> What is the frame size of the packet memory?

It's XEN_PAGE_SIZE (4k)

>
>
>> +		if (unlikely(err))
>> +			trace_xdp_exception(queue->info->netdev, prog, act);
>> +		xdp_do_flush();
>> +		break;
>> +	case XDP_PASS:
>> +	case XDP_DROP:
>> +		break;
>> +
>> +	case XDP_ABORTED:
>> +		trace_xdp_exception(queue->info->netdev, prog, act);
>> +		break;
>> +
>> +	default:
>> +		bpf_warn_invalid_xdp_action(act);
>> +	}
>> +
>> +	return act;
>> +}
>> +
>
> --
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer
>
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
> On 16.03.20 14:09, Denis Kirjanov wrote:
>> The patch adds a basic XDP processing to xen-netfront driver.
>>
>> We ran an XDP program for an RX response received from netback
>> driver. Also we request xen-netback to adjust data offset for
>> bpf_xdp_adjust_head() header space for custom headers.
>
> This is in no way a "verbose patch descriprion".
>
> I'm missing:
>
> - Why are you doing this. "Add XDP support" is not enough, for such
>    a change I'd like to see some performance numbers to get an idea
>    of the improvement to expect, or which additional functionality
>    for the user is available.
Ok, I'll try to measure  some numbers.

>
> - A short description for me as a Xen maintainer with only basic
>    networking know-how, what XDP programs are about (a link to some
>    more detailed doc is enough, of course) and how the interface
>    is working (especially for switching between XDP mode and normal
>    SKB processing).

You can search for the "A practical introduction to XDP" tutorial.
Actually there is a lot of information available regarding XDP, you
can easily find it.

>
> - A proper description of the netfront/netback communication when
>    enabling or disabling XDP mode (who is doing what, is silencing
>    of the virtual adapter required, ...).
Currently we need only a header offset from netback driver so that we can put
custom encapsulation header if required and that's done using xen bus
state switching,
so that:
- netback tells that it can adjust the header offset
- netfront part reads it
>
> - Reasoning why the suggested changes of frontend and backend state
>    are no problem for special cases like hot-remove of an interface or
>    live migration or suspend of the guest.

I've put the code to talk_to_netback which is called "when first
setting up, and when resuming"
If you see a problem with that please share.

>
> Finally I'd like to ask you to split up the patch into a netfront and
> a netback one.

Ok, will do.

Thanks!
>
>
> Juergen
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 18.03.20 13:50, Denis Kirjanov wrote:
> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>
>>> We ran an XDP program for an RX response received from netback
>>> driver. Also we request xen-netback to adjust data offset for
>>> bpf_xdp_adjust_head() header space for custom headers.
>>
>> This is in no way a "verbose patch descriprion".
>>
>> I'm missing:
>>
>> - Why are you doing this. "Add XDP support" is not enough, for such
>>     a change I'd like to see some performance numbers to get an idea
>>     of the improvement to expect, or which additional functionality
>>     for the user is available.
> Ok, I'll try to measure  some numbers.
> 
>>
>> - A short description for me as a Xen maintainer with only basic
>>     networking know-how, what XDP programs are about (a link to some
>>     more detailed doc is enough, of course) and how the interface
>>     is working (especially for switching between XDP mode and normal
>>     SKB processing).
> 
> You can search for the "A practical introduction to XDP" tutorial.
> Actually there is a lot of information available regarding XDP, you
> can easily find it.
> 
>>
>> - A proper description of the netfront/netback communication when
>>     enabling or disabling XDP mode (who is doing what, is silencing
>>     of the virtual adapter required, ...).
> Currently we need only a header offset from netback driver so that we can put
> custom encapsulation header if required and that's done using xen bus
> state switching,
> so that:
> - netback tells that it can adjust the header offset
> - netfront part reads it

Yes, but how is this synchronized with currently running network load?
Assume you are starting without XDP being active and then you are
activating it. How is the synchronization done from which request on
the XDP headroom is available?

>>
>> - Reasoning why the suggested changes of frontend and backend state
>>     are no problem for special cases like hot-remove of an interface or
>>     live migration or suspend of the guest.
> 
> I've put the code to talk_to_netback which is called "when first
> setting up, and when resuming"
> If you see a problem with that please share.

What happens if a migration is just starting when netfront has switched
its state to "Reconfigured"? Will the new device activation at the
target host work as desired? In which state will XDP be on the frontend
after that (i.e. is a direct state transition on the frontend from
"Reconfigured" to "Initializing" fine)? It should be spelled out in the
commit message that this scenario has been thought of and that it will
work.

> 
>>
>> Finally I'd like to ask you to split up the patch into a netfront and
>> a netback one.
> 
> Ok, will do.

Thanks.


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
> On 18.03.20 13:50, Denis Kirjanov wrote:
>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>
>>>> We ran an XDP program for an RX response received from netback
>>>> driver. Also we request xen-netback to adjust data offset for
>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>
>>> This is in no way a "verbose patch descriprion".
>>>
>>> I'm missing:
>>>
>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>     a change I'd like to see some performance numbers to get an idea
>>>     of the improvement to expect, or which additional functionality
>>>     for the user is available.
>> Ok, I'll try to measure  some numbers.
>>
>>>
>>> - A short description for me as a Xen maintainer with only basic
>>>     networking know-how, what XDP programs are about (a link to some
>>>     more detailed doc is enough, of course) and how the interface
>>>     is working (especially for switching between XDP mode and normal
>>>     SKB processing).
>>
>> You can search for the "A practical introduction to XDP" tutorial.
>> Actually there is a lot of information available regarding XDP, you
>> can easily find it.
>>
>>>
>>> - A proper description of the netfront/netback communication when
>>>     enabling or disabling XDP mode (who is doing what, is silencing
>>>     of the virtual adapter required, ...).
>> Currently we need only a header offset from netback driver so that we can
>> put
>> custom encapsulation header if required and that's done using xen bus
>> state switching,
>> so that:
>> - netback tells that it can adjust the header offset
>> - netfront part reads it
>
> Yes, but how is this synchronized with currently running network load?
> Assume you are starting without XDP being active and then you are
> activating it. How is the synchronization done from which request on
> the XDP headroom is available?

Hi Jurgen,

basically XDP is activated when you've assigned an xdp program to the
networking device.
Assigning an xdp program means that we have to adjust a pointer which
is RCU protected.

>
>>>
>>> - Reasoning why the suggested changes of frontend and backend state
>>>     are no problem for special cases like hot-remove of an interface or
>>>     live migration or suspend of the guest.
>>
>> I've put the code to talk_to_netback which is called "when first
>> setting up, and when resuming"
>> If you see a problem with that please share.
>
> What happens if a migration is just starting when netfront has switched
> its state to "Reconfigured"? Will the new device activation at the
> target host work as desired? In which state will XDP be on the frontend
> after that (i.e. is a direct state transition on the frontend from
> "Reconfigured" to "Initializing" fine)? It should be spelled out in the
> commit message that this scenario has been thought of and that it will
> work.
Well, that definitely means that I have to play with that scenario.

Moreover, I've found a problem with memory consumption while testing a
pktgen load with
xen-netfront.

Thank!
>
>>
>>>
>>> Finally I'd like to ask you to split up the patch into a netfront and
>>> a netback one.
>>
>> Ok, will do.
>
> Thanks.
>
>
> Juergen
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 23.03.20 11:15, Denis Kirjanov wrote:
> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>
>>>>> We ran an XDP program for an RX response received from netback
>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>
>>>> This is in no way a "verbose patch descriprion".
>>>>
>>>> I'm missing:
>>>>
>>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>>      a change I'd like to see some performance numbers to get an idea
>>>>      of the improvement to expect, or which additional functionality
>>>>      for the user is available.
>>> Ok, I'll try to measure  some numbers.
>>>
>>>>
>>>> - A short description for me as a Xen maintainer with only basic
>>>>      networking know-how, what XDP programs are about (a link to some
>>>>      more detailed doc is enough, of course) and how the interface
>>>>      is working (especially for switching between XDP mode and normal
>>>>      SKB processing).
>>>
>>> You can search for the "A practical introduction to XDP" tutorial.
>>> Actually there is a lot of information available regarding XDP, you
>>> can easily find it.
>>>
>>>>
>>>> - A proper description of the netfront/netback communication when
>>>>      enabling or disabling XDP mode (who is doing what, is silencing
>>>>      of the virtual adapter required, ...).
>>> Currently we need only a header offset from netback driver so that we can
>>> put
>>> custom encapsulation header if required and that's done using xen bus
>>> state switching,
>>> so that:
>>> - netback tells that it can adjust the header offset
>>> - netfront part reads it
>>
>> Yes, but how is this synchronized with currently running network load?
>> Assume you are starting without XDP being active and then you are
>> activating it. How is the synchronization done from which request on
>> the XDP headroom is available?
> 
> Hi Jurgen,
> 
> basically XDP is activated when you've assigned an xdp program to the
> networking device.
> Assigning an xdp program means that we have to adjust a pointer which
> is RCU protected.

This doesn't answer my question.

You have basically two communication channels: the state of the frontend
and backend for activation/deactivation of XDP, and the ring pages with
the rx and tx requests and responses. How is the synchronization between
those two channels done? So how does the other side know which of the
packets in flight will then have XDP on or off?


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
> On 23.03.20 11:15, Denis Kirjanov wrote:
>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>
>>>>>> We ran an XDP program for an RX response received from netback
>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>
>>>>> This is in no way a "verbose patch descriprion".
>>>>>
>>>>> I'm missing:
>>>>>
>>>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>>>      a change I'd like to see some performance numbers to get an idea
>>>>>      of the improvement to expect, or which additional functionality
>>>>>      for the user is available.
>>>> Ok, I'll try to measure  some numbers.
>>>>
>>>>>
>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>      networking know-how, what XDP programs are about (a link to some
>>>>>      more detailed doc is enough, of course) and how the interface
>>>>>      is working (especially for switching between XDP mode and normal
>>>>>      SKB processing).
>>>>
>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>> Actually there is a lot of information available regarding XDP, you
>>>> can easily find it.
>>>>
>>>>>
>>>>> - A proper description of the netfront/netback communication when
>>>>>      enabling or disabling XDP mode (who is doing what, is silencing
>>>>>      of the virtual adapter required, ...).
>>>> Currently we need only a header offset from netback driver so that we
>>>> can
>>>> put
>>>> custom encapsulation header if required and that's done using xen bus
>>>> state switching,
>>>> so that:
>>>> - netback tells that it can adjust the header offset
>>>> - netfront part reads it
>>>
>>> Yes, but how is this synchronized with currently running network load?
>>> Assume you are starting without XDP being active and then you are
>>> activating it. How is the synchronization done from which request on
>>> the XDP headroom is available?
>>
>> Hi Jurgen,
>>
>> basically XDP is activated when you've assigned an xdp program to the
>> networking device.
>> Assigning an xdp program means that we have to adjust a pointer which
>> is RCU protected.
>
> This doesn't answer my question.
>
> You have basically two communication channels: the state of the frontend
> and backend for activation/deactivation of XDP, and the ring pages with
> the rx and tx requests and responses. How is the synchronization between
> those two channels done? So how does the other side know which of the
> packets in flight will then have XDP on or off?

Right,
that's done in xen-netback using xenbus state:
- in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback to
adjust offset for an RX response.
-xen-netback reads the value from xenstore and adjusts the offset for
xen-netback
in xenvif_rx_data_slot() using vif->xdp_enabled flag.


>
>
> Juergen
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 23.03.20 11:49, Denis Kirjanov wrote:
> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>
>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>
>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>
>>>>>> I'm missing:
>>>>>>
>>>>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>>>>       a change I'd like to see some performance numbers to get an idea
>>>>>>       of the improvement to expect, or which additional functionality
>>>>>>       for the user is available.
>>>>> Ok, I'll try to measure  some numbers.
>>>>>
>>>>>>
>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>       networking know-how, what XDP programs are about (a link to some
>>>>>>       more detailed doc is enough, of course) and how the interface
>>>>>>       is working (especially for switching between XDP mode and normal
>>>>>>       SKB processing).
>>>>>
>>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>>> Actually there is a lot of information available regarding XDP, you
>>>>> can easily find it.
>>>>>
>>>>>>
>>>>>> - A proper description of the netfront/netback communication when
>>>>>>       enabling or disabling XDP mode (who is doing what, is silencing
>>>>>>       of the virtual adapter required, ...).
>>>>> Currently we need only a header offset from netback driver so that we
>>>>> can
>>>>> put
>>>>> custom encapsulation header if required and that's done using xen bus
>>>>> state switching,
>>>>> so that:
>>>>> - netback tells that it can adjust the header offset
>>>>> - netfront part reads it
>>>>
>>>> Yes, but how is this synchronized with currently running network load?
>>>> Assume you are starting without XDP being active and then you are
>>>> activating it. How is the synchronization done from which request on
>>>> the XDP headroom is available?
>>>
>>> Hi Jurgen,
>>>
>>> basically XDP is activated when you've assigned an xdp program to the
>>> networking device.
>>> Assigning an xdp program means that we have to adjust a pointer which
>>> is RCU protected.
>>
>> This doesn't answer my question.
>>
>> You have basically two communication channels: the state of the frontend
>> and backend for activation/deactivation of XDP, and the ring pages with
>> the rx and tx requests and responses. How is the synchronization between
>> those two channels done? So how does the other side know which of the
>> packets in flight will then have XDP on or off?
> 
> Right,
> that's done in xen-netback using xenbus state:
> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback to
> adjust offset for an RX response.
> -xen-netback reads the value from xenstore and adjusts the offset for
> xen-netback
> in xenvif_rx_data_slot() using vif->xdp_enabled flag.

And before that all in-flight requests in the ring pages are being
processed and no new requests are guaranteed to be enqueued?


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
> On 23.03.20 11:49, Denis Kirjanov wrote:
>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>>
>>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>>
>>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>>
>>>>>>> I'm missing:
>>>>>>>
>>>>>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>>>>>       a change I'd like to see some performance numbers to get an
>>>>>>> idea
>>>>>>>       of the improvement to expect, or which additional
>>>>>>> functionality
>>>>>>>       for the user is available.
>>>>>> Ok, I'll try to measure  some numbers.
>>>>>>
>>>>>>>
>>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>>       networking know-how, what XDP programs are about (a link to
>>>>>>> some
>>>>>>>       more detailed doc is enough, of course) and how the interface
>>>>>>>       is working (especially for switching between XDP mode and
>>>>>>> normal
>>>>>>>       SKB processing).
>>>>>>
>>>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>>>> Actually there is a lot of information available regarding XDP, you
>>>>>> can easily find it.
>>>>>>
>>>>>>>
>>>>>>> - A proper description of the netfront/netback communication when
>>>>>>>       enabling or disabling XDP mode (who is doing what, is
>>>>>>> silencing
>>>>>>>       of the virtual adapter required, ...).
>>>>>> Currently we need only a header offset from netback driver so that we
>>>>>> can
>>>>>> put
>>>>>> custom encapsulation header if required and that's done using xen bus
>>>>>> state switching,
>>>>>> so that:
>>>>>> - netback tells that it can adjust the header offset
>>>>>> - netfront part reads it
>>>>>
>>>>> Yes, but how is this synchronized with currently running network load?
>>>>> Assume you are starting without XDP being active and then you are
>>>>> activating it. How is the synchronization done from which request on
>>>>> the XDP headroom is available?
>>>>
>>>> Hi Jurgen,
>>>>
>>>> basically XDP is activated when you've assigned an xdp program to the
>>>> networking device.
>>>> Assigning an xdp program means that we have to adjust a pointer which
>>>> is RCU protected.
>>>
>>> This doesn't answer my question.
>>>
>>> You have basically two communication channels: the state of the frontend
>>> and backend for activation/deactivation of XDP, and the ring pages with
>>> the rx and tx requests and responses. How is the synchronization between
>>> those two channels done? So how does the other side know which of the
>>> packets in flight will then have XDP on or off?
>>
>> Right,
>> that's done in xen-netback using xenbus state:
>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback to
>> adjust offset for an RX response.
>> -xen-netback reads the value from xenstore and adjusts the offset for
>> xen-netback
>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>
> And before that all in-flight requests in the ring pages are being
> processed and no new requests are guaranteed to be enqueued?

Actually I don't see the need to sync these requests since that all we
have to do is to copy
data with specified offset:
with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
with xdd->enabled=0: copy without the offset

Thanks!

>
>
> Juergen
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 30.03.20 14:16, Denis Kirjanov wrote:
> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>> On 23.03.20 11:49, Denis Kirjanov wrote:
>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>>>
>>>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>>>
>>>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>>>
>>>>>>>> I'm missing:
>>>>>>>>
>>>>>>>> - Why are you doing this. "Add XDP support" is not enough, for such
>>>>>>>>        a change I'd like to see some performance numbers to get an
>>>>>>>> idea
>>>>>>>>        of the improvement to expect, or which additional
>>>>>>>> functionality
>>>>>>>>        for the user is available.
>>>>>>> Ok, I'll try to measure  some numbers.
>>>>>>>
>>>>>>>>
>>>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>>>        networking know-how, what XDP programs are about (a link to
>>>>>>>> some
>>>>>>>>        more detailed doc is enough, of course) and how the interface
>>>>>>>>        is working (especially for switching between XDP mode and
>>>>>>>> normal
>>>>>>>>        SKB processing).
>>>>>>>
>>>>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>>>>> Actually there is a lot of information available regarding XDP, you
>>>>>>> can easily find it.
>>>>>>>
>>>>>>>>
>>>>>>>> - A proper description of the netfront/netback communication when
>>>>>>>>        enabling or disabling XDP mode (who is doing what, is
>>>>>>>> silencing
>>>>>>>>        of the virtual adapter required, ...).
>>>>>>> Currently we need only a header offset from netback driver so that we
>>>>>>> can
>>>>>>> put
>>>>>>> custom encapsulation header if required and that's done using xen bus
>>>>>>> state switching,
>>>>>>> so that:
>>>>>>> - netback tells that it can adjust the header offset
>>>>>>> - netfront part reads it
>>>>>>
>>>>>> Yes, but how is this synchronized with currently running network load?
>>>>>> Assume you are starting without XDP being active and then you are
>>>>>> activating it. How is the synchronization done from which request on
>>>>>> the XDP headroom is available?
>>>>>
>>>>> Hi Jurgen,
>>>>>
>>>>> basically XDP is activated when you've assigned an xdp program to the
>>>>> networking device.
>>>>> Assigning an xdp program means that we have to adjust a pointer which
>>>>> is RCU protected.
>>>>
>>>> This doesn't answer my question.
>>>>
>>>> You have basically two communication channels: the state of the frontend
>>>> and backend for activation/deactivation of XDP, and the ring pages with
>>>> the rx and tx requests and responses. How is the synchronization between
>>>> those two channels done? So how does the other side know which of the
>>>> packets in flight will then have XDP on or off?
>>>
>>> Right,
>>> that's done in xen-netback using xenbus state:
>>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback to
>>> adjust offset for an RX response.
>>> -xen-netback reads the value from xenstore and adjusts the offset for
>>> xen-netback
>>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>>
>> And before that all in-flight requests in the ring pages are being
>> processed and no new requests are guaranteed to be enqueued?
> 
> Actually I don't see the need to sync these requests since that all we
> have to do is to copy
> data with specified offset:
> with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
> with xdd->enabled=0: copy without the offset

Isn't that racy?

In xennet_xdp_set() you set queue->xdp_prog and then you change the
state to Reconfiguring. From the time queue->xdp_prog is set you'll
do the Xdp processing in xennet_get_responses(), even if the response
you are working on doesn't have the headroom you need, as the backend
didn't create it with headroom (it needs some time until it has seen
the new state and could react on it by sending _new_ responses with
headroom).

Or am I missing something about Xdp programs?


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/30/20, Jürgen Groß <jgross@suse.com> wrote:
> On 30.03.20 14:16, Denis Kirjanov wrote:
>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>> On 23.03.20 11:49, Denis Kirjanov wrote:
>>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>>>>
>>>>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>>>>
>>>>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>>>>
>>>>>>>>> I'm missing:
>>>>>>>>>
>>>>>>>>> - Why are you doing this. "Add XDP support" is not enough, for
>>>>>>>>> such
>>>>>>>>>        a change I'd like to see some performance numbers to get an
>>>>>>>>> idea
>>>>>>>>>        of the improvement to expect, or which additional
>>>>>>>>> functionality
>>>>>>>>>        for the user is available.
>>>>>>>> Ok, I'll try to measure  some numbers.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>>>>        networking know-how, what XDP programs are about (a link to
>>>>>>>>> some
>>>>>>>>>        more detailed doc is enough, of course) and how the
>>>>>>>>> interface
>>>>>>>>>        is working (especially for switching between XDP mode and
>>>>>>>>> normal
>>>>>>>>>        SKB processing).
>>>>>>>>
>>>>>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>>>>>> Actually there is a lot of information available regarding XDP, you
>>>>>>>> can easily find it.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> - A proper description of the netfront/netback communication when
>>>>>>>>>        enabling or disabling XDP mode (who is doing what, is
>>>>>>>>> silencing
>>>>>>>>>        of the virtual adapter required, ...).
>>>>>>>> Currently we need only a header offset from netback driver so that
>>>>>>>> we
>>>>>>>> can
>>>>>>>> put
>>>>>>>> custom encapsulation header if required and that's done using xen
>>>>>>>> bus
>>>>>>>> state switching,
>>>>>>>> so that:
>>>>>>>> - netback tells that it can adjust the header offset
>>>>>>>> - netfront part reads it
>>>>>>>
>>>>>>> Yes, but how is this synchronized with currently running network
>>>>>>> load?
>>>>>>> Assume you are starting without XDP being active and then you are
>>>>>>> activating it. How is the synchronization done from which request on
>>>>>>> the XDP headroom is available?
>>>>>>
>>>>>> Hi Jurgen,
>>>>>>
>>>>>> basically XDP is activated when you've assigned an xdp program to the
>>>>>> networking device.
>>>>>> Assigning an xdp program means that we have to adjust a pointer which
>>>>>> is RCU protected.
>>>>>
>>>>> This doesn't answer my question.
>>>>>
>>>>> You have basically two communication channels: the state of the
>>>>> frontend
>>>>> and backend for activation/deactivation of XDP, and the ring pages
>>>>> with
>>>>> the rx and tx requests and responses. How is the synchronization
>>>>> between
>>>>> those two channels done? So how does the other side know which of the
>>>>> packets in flight will then have XDP on or off?
>>>>
>>>> Right,
>>>> that's done in xen-netback using xenbus state:
>>>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback
>>>> to
>>>> adjust offset for an RX response.
>>>> -xen-netback reads the value from xenstore and adjusts the offset for
>>>> xen-netback
>>>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>>>
>>> And before that all in-flight requests in the ring pages are being
>>> processed and no new requests are guaranteed to be enqueued?
>>
>> Actually I don't see the need to sync these requests since that all we
>> have to do is to copy
>> data with specified offset:
>> with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
>> with xdd->enabled=0: copy without the offset
>
> Isn't that racy?
>
> In xennet_xdp_set() you set queue->xdp_prog and then you change the
> state to Reconfiguring. From the time queue->xdp_prog is set you'll
> do the Xdp processing in xennet_get_responses(), even if the response
> you are working on doesn't have the headroom you need, as the backend
> didn't create it with headroom (it needs some time until it has seen
> the new state and could react on it by sending _new_ responses with
> headroom).

Ah, I see. You mean that we have to wait until XenbusStateReconfigured
is set in
xen-netfront and only after that it's safe to process packets.

>
> Or am I missing something about Xdp programs?
>
>
> Juergen
>

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Jürgen Groß]

On 30.03.20 15:09, Denis Kirjanov wrote:
> On 3/30/20, Jürgen Groß <jgross@suse.com> wrote:
>> On 30.03.20 14:16, Denis Kirjanov wrote:
>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>> On 23.03.20 11:49, Denis Kirjanov wrote:
>>>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>>>>>
>>>>>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>>>>>
>>>>>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>>>>>
>>>>>>>>>> I'm missing:
>>>>>>>>>>
>>>>>>>>>> - Why are you doing this. "Add XDP support" is not enough, for
>>>>>>>>>> such
>>>>>>>>>>         a change I'd like to see some performance numbers to get an
>>>>>>>>>> idea
>>>>>>>>>>         of the improvement to expect, or which additional
>>>>>>>>>> functionality
>>>>>>>>>>         for the user is available.
>>>>>>>>> Ok, I'll try to measure  some numbers.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>>>>>         networking know-how, what XDP programs are about (a link to
>>>>>>>>>> some
>>>>>>>>>>         more detailed doc is enough, of course) and how the
>>>>>>>>>> interface
>>>>>>>>>>         is working (especially for switching between XDP mode and
>>>>>>>>>> normal
>>>>>>>>>>         SKB processing).
>>>>>>>>>
>>>>>>>>> You can search for the "A practical introduction to XDP" tutorial.
>>>>>>>>> Actually there is a lot of information available regarding XDP, you
>>>>>>>>> can easily find it.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - A proper description of the netfront/netback communication when
>>>>>>>>>>         enabling or disabling XDP mode (who is doing what, is
>>>>>>>>>> silencing
>>>>>>>>>>         of the virtual adapter required, ...).
>>>>>>>>> Currently we need only a header offset from netback driver so that
>>>>>>>>> we
>>>>>>>>> can
>>>>>>>>> put
>>>>>>>>> custom encapsulation header if required and that's done using xen
>>>>>>>>> bus
>>>>>>>>> state switching,
>>>>>>>>> so that:
>>>>>>>>> - netback tells that it can adjust the header offset
>>>>>>>>> - netfront part reads it
>>>>>>>>
>>>>>>>> Yes, but how is this synchronized with currently running network
>>>>>>>> load?
>>>>>>>> Assume you are starting without XDP being active and then you are
>>>>>>>> activating it. How is the synchronization done from which request on
>>>>>>>> the XDP headroom is available?
>>>>>>>
>>>>>>> Hi Jurgen,
>>>>>>>
>>>>>>> basically XDP is activated when you've assigned an xdp program to the
>>>>>>> networking device.
>>>>>>> Assigning an xdp program means that we have to adjust a pointer which
>>>>>>> is RCU protected.
>>>>>>
>>>>>> This doesn't answer my question.
>>>>>>
>>>>>> You have basically two communication channels: the state of the
>>>>>> frontend
>>>>>> and backend for activation/deactivation of XDP, and the ring pages
>>>>>> with
>>>>>> the rx and tx requests and responses. How is the synchronization
>>>>>> between
>>>>>> those two channels done? So how does the other side know which of the
>>>>>> packets in flight will then have XDP on or off?
>>>>>
>>>>> Right,
>>>>> that's done in xen-netback using xenbus state:
>>>>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback
>>>>> to
>>>>> adjust offset for an RX response.
>>>>> -xen-netback reads the value from xenstore and adjusts the offset for
>>>>> xen-netback
>>>>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>>>>
>>>> And before that all in-flight requests in the ring pages are being
>>>> processed and no new requests are guaranteed to be enqueued?
>>>
>>> Actually I don't see the need to sync these requests since that all we
>>> have to do is to copy
>>> data with specified offset:
>>> with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
>>> with xdd->enabled=0: copy without the offset
>>
>> Isn't that racy?
>>
>> In xennet_xdp_set() you set queue->xdp_prog and then you change the
>> state to Reconfiguring. From the time queue->xdp_prog is set you'll
>> do the Xdp processing in xennet_get_responses(), even if the response
>> you are working on doesn't have the headroom you need, as the backend
>> didn't create it with headroom (it needs some time until it has seen
>> the new state and could react on it by sending _new_ responses with
>> headroom).
> 
> Ah, I see. You mean that we have to wait until XenbusStateReconfigured
> is set in
> xen-netfront and only after that it's safe to process packets.

Right. That is the problem of using a different communication channel
for enabling/disabling XDP.


Juergen

Re: [PATCH net-next v4] xen networking: add basic XDP support for xen-netfront

[Denis Kirjanov]

On 3/30/20, Jürgen Groß <jgross@suse.com> wrote:
> On 30.03.20 15:09, Denis Kirjanov wrote:
>> On 3/30/20, Jürgen Groß <jgross@suse.com> wrote:
>>> On 30.03.20 14:16, Denis Kirjanov wrote:
>>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>> On 23.03.20 11:49, Denis Kirjanov wrote:
>>>>>> On 3/23/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>> On 23.03.20 11:15, Denis Kirjanov wrote:
>>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>>> On 18.03.20 13:50, Denis Kirjanov wrote:
>>>>>>>>>> On 3/18/20, Jürgen Groß <jgross@suse.com> wrote:
>>>>>>>>>>> On 16.03.20 14:09, Denis Kirjanov wrote:
>>>>>>>>>>>> The patch adds a basic XDP processing to xen-netfront driver.
>>>>>>>>>>>>
>>>>>>>>>>>> We ran an XDP program for an RX response received from netback
>>>>>>>>>>>> driver. Also we request xen-netback to adjust data offset for
>>>>>>>>>>>> bpf_xdp_adjust_head() header space for custom headers.
>>>>>>>>>>>
>>>>>>>>>>> This is in no way a "verbose patch descriprion".
>>>>>>>>>>>
>>>>>>>>>>> I'm missing:
>>>>>>>>>>>
>>>>>>>>>>> - Why are you doing this. "Add XDP support" is not enough, for
>>>>>>>>>>> such
>>>>>>>>>>>         a change I'd like to see some performance numbers to get
>>>>>>>>>>> an
>>>>>>>>>>> idea
>>>>>>>>>>>         of the improvement to expect, or which additional
>>>>>>>>>>> functionality
>>>>>>>>>>>         for the user is available.
>>>>>>>>>> Ok, I'll try to measure  some numbers.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> - A short description for me as a Xen maintainer with only basic
>>>>>>>>>>>         networking know-how, what XDP programs are about (a link
>>>>>>>>>>> to
>>>>>>>>>>> some
>>>>>>>>>>>         more detailed doc is enough, of course) and how the
>>>>>>>>>>> interface
>>>>>>>>>>>         is working (especially for switching between XDP mode
>>>>>>>>>>> and
>>>>>>>>>>> normal
>>>>>>>>>>>         SKB processing).
>>>>>>>>>>
>>>>>>>>>> You can search for the "A practical introduction to XDP"
>>>>>>>>>> tutorial.
>>>>>>>>>> Actually there is a lot of information available regarding XDP,
>>>>>>>>>> you
>>>>>>>>>> can easily find it.
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> - A proper description of the netfront/netback communication
>>>>>>>>>>> when
>>>>>>>>>>>         enabling or disabling XDP mode (who is doing what, is
>>>>>>>>>>> silencing
>>>>>>>>>>>         of the virtual adapter required, ...).
>>>>>>>>>> Currently we need only a header offset from netback driver so
>>>>>>>>>> that
>>>>>>>>>> we
>>>>>>>>>> can
>>>>>>>>>> put
>>>>>>>>>> custom encapsulation header if required and that's done using xen
>>>>>>>>>> bus
>>>>>>>>>> state switching,
>>>>>>>>>> so that:
>>>>>>>>>> - netback tells that it can adjust the header offset
>>>>>>>>>> - netfront part reads it
>>>>>>>>>
>>>>>>>>> Yes, but how is this synchronized with currently running network
>>>>>>>>> load?
>>>>>>>>> Assume you are starting without XDP being active and then you are
>>>>>>>>> activating it. How is the synchronization done from which request
>>>>>>>>> on
>>>>>>>>> the XDP headroom is available?
>>>>>>>>
>>>>>>>> Hi Jurgen,
>>>>>>>>
>>>>>>>> basically XDP is activated when you've assigned an xdp program to
>>>>>>>> the
>>>>>>>> networking device.
>>>>>>>> Assigning an xdp program means that we have to adjust a pointer
>>>>>>>> which
>>>>>>>> is RCU protected.
>>>>>>>
>>>>>>> This doesn't answer my question.
>>>>>>>
>>>>>>> You have basically two communication channels: the state of the
>>>>>>> frontend
>>>>>>> and backend for activation/deactivation of XDP, and the ring pages
>>>>>>> with
>>>>>>> the rx and tx requests and responses. How is the synchronization
>>>>>>> between
>>>>>>> those two channels done? So how does the other side know which of
>>>>>>> the
>>>>>>> packets in flight will then have XDP on or off?
>>>>>>
>>>>>> Right,
>>>>>> that's done in xen-netback using xenbus state:
>>>>>> - in xennet_xdp_set() we call xenbus_switch_state to tell xen-netback
>>>>>> to
>>>>>> adjust offset for an RX response.
>>>>>> -xen-netback reads the value from xenstore and adjusts the offset for
>>>>>> xen-netback
>>>>>> in xenvif_rx_data_slot() using vif->xdp_enabled flag.
>>>>>
>>>>> And before that all in-flight requests in the ring pages are being
>>>>> processed and no new requests are guaranteed to be enqueued?
>>>>
>>>> Actually I don't see the need to sync these requests since that all we
>>>> have to do is to copy
>>>> data with specified offset:
>>>> with xdp->enabled=1: copy with the offset XDP_PACKET_HEADROOM
>>>> with xdd->enabled=0: copy without the offset
>>>
>>> Isn't that racy?
>>>
>>> In xennet_xdp_set() you set queue->xdp_prog and then you change the
>>> state to Reconfiguring. From the time queue->xdp_prog is set you'll
>>> do the Xdp processing in xennet_get_responses(), even if the response
>>> you are working on doesn't have the headroom you need, as the backend
>>> didn't create it with headroom (it needs some time until it has seen
>>> the new state and could react on it by sending _new_ responses with
>>> headroom).
>>
>> Ah, I see. You mean that we have to wait until XenbusStateReconfigured
>> is set in
>> xen-netfront and only after that it's safe to process packets.
>
> Right. That is the problem of using a different communication channel
> for enabling/disabling XDP.

Ok, I'll update my patch.

Thanks for review!

>
>
> Juergen
>