diff mbox series

[16/33] virtio_net: introduce virtnet_xdp_handler() to seprate the logic of run xdp

Message ID 20230202110058.130695-17-xuanzhuo@linux.alibaba.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series virtio-net: support AF_XDP zero copy | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 80 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xuan Zhuo Feb. 2, 2023, 11 a.m. UTC 
At present, we have two long similar logic to perform XDP Progs. And in
the implementation of XSK, we will have this need.

Therefore, this PATCH separates the code of executing XDP, which is
conducive to later maintenance and facilitates subsequent XSK for reuse.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio/main.c       | 53 +++++++++++++++++++++++++++++++++
 drivers/net/virtio/virtio_net.h | 11 +++++++
 2 files changed, 64 insertions(+)

Comments

Michael S. Tsirkin Feb. 3, 2023, 8:55 a.m. UTC | #1 
On Thu, Feb 02, 2023 at 07:00:41PM +0800, Xuan Zhuo wrote:
> At present, we have two long similar logic to perform XDP Progs. And in
> the implementation of XSK, we will have this need.
> 
> Therefore, this PATCH separates the code of executing XDP, which is
> conducive to later maintenance and facilitates subsequent XSK for reuse.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

So you first add a new function then move users over.
This means that it's hard during review to make sure
nothing is lost in translation.
Do the refactoring in a single patch instead.

> ---
>  drivers/net/virtio/main.c       | 53 +++++++++++++++++++++++++++++++++
>  drivers/net/virtio/virtio_net.h | 11 +++++++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> index 5683cb576474..9d4b84b23ef7 100644
> --- a/drivers/net/virtio/main.c
> +++ b/drivers/net/virtio/main.c
> @@ -478,6 +478,59 @@ static int virtnet_xdp_xmit(struct net_device *dev,
>  	return ret;
>  }
>  
> +int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> +			struct net_device *dev,
> +			unsigned int *xdp_xmit,
> +			struct virtnet_rq_stats *stats)
> +{
> +	struct xdp_frame *xdpf;
> +	int err;
> +	u32 act;
> +
> +	act = bpf_prog_run_xdp(xdp_prog, xdp);
> +	stats->xdp_packets++;
> +
> +	switch (act) {
> +	case XDP_PASS:
> +		return VIRTNET_XDP_RES_PASS;
> +
> +	case XDP_TX:
> +		stats->xdp_tx++;
> +		xdpf = xdp_convert_buff_to_frame(xdp);
> +		if (unlikely(!xdpf))
> +			return VIRTNET_XDP_RES_DROP;
> +
> +		err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
> +		if (unlikely(!err)) {
> +			xdp_return_frame_rx_napi(xdpf);
> +		} else if (unlikely(err < 0)) {
> +			trace_xdp_exception(dev, xdp_prog, act);
> +			return VIRTNET_XDP_RES_DROP;
> +		}
> +
> +		*xdp_xmit |= VIRTIO_XDP_TX;
> +		return VIRTNET_XDP_RES_CONSUMED;
> +
> +	case XDP_REDIRECT:
> +		stats->xdp_redirects++;
> +		err = xdp_do_redirect(dev, xdp, xdp_prog);
> +		if (err)
> +			return VIRTNET_XDP_RES_DROP;
> +
> +		*xdp_xmit |= VIRTIO_XDP_REDIR;
> +		return VIRTNET_XDP_RES_CONSUMED;
> +
> +	default:
> +		bpf_warn_invalid_xdp_action(dev, xdp_prog, act);
> +		fallthrough;
> +	case XDP_ABORTED:
> +		trace_xdp_exception(dev, xdp_prog, act);
> +		fallthrough;
> +	case XDP_DROP:
> +		return VIRTNET_XDP_RES_DROP;
> +	}
> +}
> +
>  static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
>  {
>  	return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> index 8bf31429ae28..af3e7e817f9e 100644
> --- a/drivers/net/virtio/virtio_net.h
> +++ b/drivers/net/virtio/virtio_net.h
> @@ -22,6 +22,12 @@
>  #include <net/net_failover.h>
>  #include <net/xdp_sock_drv.h>
>  
> +enum {
> +	VIRTNET_XDP_RES_PASS,
> +	VIRTNET_XDP_RES_DROP,
> +	VIRTNET_XDP_RES_CONSUMED,
> +};
> +
>  #define VIRTIO_XDP_FLAG	BIT(0)
>  
>  struct virtnet_info {
> @@ -262,4 +268,9 @@ static void __free_old_xmit(struct send_queue *sq, bool in_napi,
>  		stats->packets++;
>  	}
>  }
> +
> +int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> +			struct net_device *dev,
> +			unsigned int *xdp_xmit,
> +			struct virtnet_rq_stats *stats);
>  #endif
> -- 
> 2.32.0.3.g01195cf9f
Xuan Zhuo Feb. 3, 2023, 9:01 a.m. UTC | #2 
On Fri, 3 Feb 2023 03:55:26 -0500, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Feb 02, 2023 at 07:00:41PM +0800, Xuan Zhuo wrote:
> > At present, we have two long similar logic to perform XDP Progs. And in
> > the implementation of XSK, we will have this need.
> >
> > Therefore, this PATCH separates the code of executing XDP, which is
> > conducive to later maintenance and facilitates subsequent XSK for reuse.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> So you first add a new function then move users over.
> This means that it's hard during review to make sure
> nothing is lost in translation.
> Do the refactoring in a single patch instead.

I agree.

Thanks.

>
> > ---
> >  drivers/net/virtio/main.c       | 53 +++++++++++++++++++++++++++++++++
> >  drivers/net/virtio/virtio_net.h | 11 +++++++
> >  2 files changed, 64 insertions(+)
> >
> > diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
> > index 5683cb576474..9d4b84b23ef7 100644
> > --- a/drivers/net/virtio/main.c
> > +++ b/drivers/net/virtio/main.c
> > @@ -478,6 +478,59 @@ static int virtnet_xdp_xmit(struct net_device *dev,
> >  	return ret;
> >  }
> >
> > +int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> > +			struct net_device *dev,
> > +			unsigned int *xdp_xmit,
> > +			struct virtnet_rq_stats *stats)
> > +{
> > +	struct xdp_frame *xdpf;
> > +	int err;
> > +	u32 act;
> > +
> > +	act = bpf_prog_run_xdp(xdp_prog, xdp);
> > +	stats->xdp_packets++;
> > +
> > +	switch (act) {
> > +	case XDP_PASS:
> > +		return VIRTNET_XDP_RES_PASS;
> > +
> > +	case XDP_TX:
> > +		stats->xdp_tx++;
> > +		xdpf = xdp_convert_buff_to_frame(xdp);
> > +		if (unlikely(!xdpf))
> > +			return VIRTNET_XDP_RES_DROP;
> > +
> > +		err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
> > +		if (unlikely(!err)) {
> > +			xdp_return_frame_rx_napi(xdpf);
> > +		} else if (unlikely(err < 0)) {
> > +			trace_xdp_exception(dev, xdp_prog, act);
> > +			return VIRTNET_XDP_RES_DROP;
> > +		}
> > +
> > +		*xdp_xmit |= VIRTIO_XDP_TX;
> > +		return VIRTNET_XDP_RES_CONSUMED;
> > +
> > +	case XDP_REDIRECT:
> > +		stats->xdp_redirects++;
> > +		err = xdp_do_redirect(dev, xdp, xdp_prog);
> > +		if (err)
> > +			return VIRTNET_XDP_RES_DROP;
> > +
> > +		*xdp_xmit |= VIRTIO_XDP_REDIR;
> > +		return VIRTNET_XDP_RES_CONSUMED;
> > +
> > +	default:
> > +		bpf_warn_invalid_xdp_action(dev, xdp_prog, act);
> > +		fallthrough;
> > +	case XDP_ABORTED:
> > +		trace_xdp_exception(dev, xdp_prog, act);
> > +		fallthrough;
> > +	case XDP_DROP:
> > +		return VIRTNET_XDP_RES_DROP;
> > +	}
> > +}
> > +
> >  static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
> >  {
> >  	return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
> > diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
> > index 8bf31429ae28..af3e7e817f9e 100644
> > --- a/drivers/net/virtio/virtio_net.h
> > +++ b/drivers/net/virtio/virtio_net.h
> > @@ -22,6 +22,12 @@
> >  #include <net/net_failover.h>
> >  #include <net/xdp_sock_drv.h>
> >
> > +enum {
> > +	VIRTNET_XDP_RES_PASS,
> > +	VIRTNET_XDP_RES_DROP,
> > +	VIRTNET_XDP_RES_CONSUMED,
> > +};
> > +
> >  #define VIRTIO_XDP_FLAG	BIT(0)
> >
> >  struct virtnet_info {
> > @@ -262,4 +268,9 @@ static void __free_old_xmit(struct send_queue *sq, bool in_napi,
> >  		stats->packets++;
> >  	}
> >  }
> > +
> > +int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
> > +			struct net_device *dev,
> > +			unsigned int *xdp_xmit,
> > +			struct virtnet_rq_stats *stats);
> >  #endif
> > --
> > 2.32.0.3.g01195cf9f
>
diff mbox series

Patch

diff --git a/drivers/net/virtio/main.c b/drivers/net/virtio/main.c
index 5683cb576474..9d4b84b23ef7 100644
--- a/drivers/net/virtio/main.c
+++ b/drivers/net/virtio/main.c
@@ -478,6 +478,59 @@  static int virtnet_xdp_xmit(struct net_device *dev,
 	return ret;
 }
 
+int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
+			struct net_device *dev,
+			unsigned int *xdp_xmit,
+			struct virtnet_rq_stats *stats)
+{
+	struct xdp_frame *xdpf;
+	int err;
+	u32 act;
+
+	act = bpf_prog_run_xdp(xdp_prog, xdp);
+	stats->xdp_packets++;
+
+	switch (act) {
+	case XDP_PASS:
+		return VIRTNET_XDP_RES_PASS;
+
+	case XDP_TX:
+		stats->xdp_tx++;
+		xdpf = xdp_convert_buff_to_frame(xdp);
+		if (unlikely(!xdpf))
+			return VIRTNET_XDP_RES_DROP;
+
+		err = virtnet_xdp_xmit(dev, 1, &xdpf, 0);
+		if (unlikely(!err)) {
+			xdp_return_frame_rx_napi(xdpf);
+		} else if (unlikely(err < 0)) {
+			trace_xdp_exception(dev, xdp_prog, act);
+			return VIRTNET_XDP_RES_DROP;
+		}
+
+		*xdp_xmit |= VIRTIO_XDP_TX;
+		return VIRTNET_XDP_RES_CONSUMED;
+
+	case XDP_REDIRECT:
+		stats->xdp_redirects++;
+		err = xdp_do_redirect(dev, xdp, xdp_prog);
+		if (err)
+			return VIRTNET_XDP_RES_DROP;
+
+		*xdp_xmit |= VIRTIO_XDP_REDIR;
+		return VIRTNET_XDP_RES_CONSUMED;
+
+	default:
+		bpf_warn_invalid_xdp_action(dev, xdp_prog, act);
+		fallthrough;
+	case XDP_ABORTED:
+		trace_xdp_exception(dev, xdp_prog, act);
+		fallthrough;
+	case XDP_DROP:
+		return VIRTNET_XDP_RES_DROP;
+	}
+}
+
 static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
 {
 	return vi->xdp_enabled ? VIRTIO_XDP_HEADROOM : 0;
diff --git a/drivers/net/virtio/virtio_net.h b/drivers/net/virtio/virtio_net.h
index 8bf31429ae28..af3e7e817f9e 100644
--- a/drivers/net/virtio/virtio_net.h
+++ b/drivers/net/virtio/virtio_net.h
@@ -22,6 +22,12 @@ 
 #include <net/net_failover.h>
 #include <net/xdp_sock_drv.h>
 
+enum {
+	VIRTNET_XDP_RES_PASS,
+	VIRTNET_XDP_RES_DROP,
+	VIRTNET_XDP_RES_CONSUMED,
+};
+
 #define VIRTIO_XDP_FLAG	BIT(0)
 
 struct virtnet_info {
@@ -262,4 +268,9 @@  static void __free_old_xmit(struct send_queue *sq, bool in_napi,
 		stats->packets++;
 	}
 }
+
+int virtnet_xdp_handler(struct bpf_prog *xdp_prog, struct xdp_buff *xdp,
+			struct net_device *dev,
+			unsigned int *xdp_xmit,
+			struct virtnet_rq_stats *stats);
 #endif