On Tue, 24 Sep 2024 15:35:03 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> On Tue, Sep 24, 2024 at 9:32 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> >
> > Because the af-xdp will introduce a new xmit type, so I refactor the
> > xmit type mechanism first.
> >
> > In general, pointers are aligned to 4 or 8 bytes.
>
> I think this needs some clarification, the alignment seems to depend
> on the lowest common multiple between the alignments of all struct
> members. So we know both xdp_frame and sk_buff are at least 4 bytes
> aligned.
>
> If we want to reuse the lowest bit of pointers in AF_XDP, the
> alignment of the data structure should be at least 4 bytes.
YES, for AF_XDP. See more in #10.
>
> > If it is aligned to 4
> > bytes, then only two bits are free for a pointer. But there are 4 types
> > here, so we can't use bits to distinguish them. And 2 bits is enough for
> > 4 types:
> >
> > 00 for skb
> > 01 for SKB_ORPHAN
> > 10 for XDP
> > 11 for af-xdp tx
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > ---
> > drivers/net/virtio_net.c | 90 +++++++++++++++++++++++-----------------
> > 1 file changed, 51 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 630e5b21ad69..41a5ea9b788d 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -45,9 +45,6 @@ module_param(napi_tx, bool, 0644);
> > #define VIRTIO_XDP_TX BIT(0)
> > #define VIRTIO_XDP_REDIR BIT(1)
> >
> > -#define VIRTIO_XDP_FLAG BIT(0)
> > -#define VIRTIO_ORPHAN_FLAG BIT(1)
> > -
> > /* RX packet size EWMA. The average packet size is used to determine the packet
> > * buffer size when refilling RX rings. As the entire RX ring may be refilled
> > * at once, the weight is chosen so that the EWMA will be insensitive to short-
> > @@ -512,34 +509,35 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
> > struct page *page, void *buf,
> > int len, int truesize);
> >
> > -static bool is_xdp_frame(void *ptr)
> > -{
> > - return (unsigned long)ptr & VIRTIO_XDP_FLAG;
> > -}
> > +enum virtnet_xmit_type {
> > + VIRTNET_XMIT_TYPE_SKB,
> > + VIRTNET_XMIT_TYPE_SKB_ORPHAN,
> > + VIRTNET_XMIT_TYPE_XDP,
> > +};
> >
> > -static void *xdp_to_ptr(struct xdp_frame *ptr)
> > -{
> > - return (void *)((unsigned long)ptr | VIRTIO_XDP_FLAG);
> > -}
> > +/* We use the last two bits of the pointer to distinguish the xmit type. */
> > +#define VIRTNET_XMIT_TYPE_MASK (BIT(0) | BIT(1))
> >
> > -static struct xdp_frame *ptr_to_xdp(void *ptr)
> > +static enum virtnet_xmit_type virtnet_xmit_ptr_strip(void **ptr)
>
> Nit: not a native speaker but I think something like pack/unpack might
> be better.
Will fix.
Thanks
>
> With those changes.
>
> Acked-by: Jason Wang <jasowang@xxxxxxxxxx>
>
> Thanks
>
