On Fri, 3 Feb 2023 03:55:26 -0500, "Michael S. Tsirkin" <mst@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx>
>
> 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
>
