netdev - Re: [PATCH net-next v2 13/14] virtio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1681980971.1167793-1-xuanzhuo@linux.alibaba.com>
Date:   Thu, 20 Apr 2023 16:56:11 +0800
From:   Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
To:     Jason Wang <jasowang@...hat.com>
Cc:     netdev@...r.kernel.org, "Michael S. Tsirkin" <mst@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        virtualization@...ts.linux-foundation.org, bpf@...r.kernel.org
Subject: Re: [PATCH net-next v2 13/14] virtio_net: small: optimize code

On Thu, 20 Apr 2023 14:32:37 +0800, Jason Wang <jasowang@...hat.com> wrote:
> On Tue, Apr 18, 2023 at 2:53 PM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
> >
> > Avoid the problem that some variables(headroom and so on) will repeat
> > the calculation when process xdp.
>
> While at it, if we agree to use separate code paths for building skbs.
>
> It would be better to have a helper for building skb for non XDP
> cases, then we can hide those calculation there.


Yes, we can introduce one helper, then receive_small will be more simple.
But these calculation can not shared with xdp case, because xdp case needs to
get these vars before running xdp.

Then the code "copy vnet hdr" and "set metadata" should stay in their own
function.

Only the virtnet_build_skb()[build_skb, skb_reserve, skb_put] can be shared.

static struct sk_buff *virtnet_build_skb(void *buf, unsigned int buflen,
					 unsigned int headroom,
					 unsigned int len)
{
	struct sk_buff *skb;

	skb = build_skb(buf, buflen);
	if (!skb)
		return NULL;

	skb_reserve(skb, headroom);
	skb_put(skb, len);

	return skb;
}

static struct sk_buff *receive_small_build_skb(struct virtnet_info *vi,
					       unsigned int xdp_headroom,
					       void *buf,
					       unsigned int len)
{
	unsigned int header_offset;
	unsigned int headroom;
	unsigned int buflen;
	struct sk_buff *skb;

	header_offset = VIRTNET_RX_PAD + xdp_headroom;
	headroom = vi->hdr_len + header_offset;
	buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
		SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

	skb = virtnet_build_skb(buf, buflen, headroom, len);
	if (unlikely(!skb))
		return NULL;

	buf += header_offset;
	memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);

	return skb;
}

Thanks

>
> Thanks
>
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> > ---
> >  drivers/net/virtio_net.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f6f5903face2..5a5636178bd3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1040,11 +1040,10 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >         struct sk_buff *skb;
> >         struct bpf_prog *xdp_prog;
> >         unsigned int xdp_headroom = (unsigned long)ctx;
> > -       unsigned int header_offset = VIRTNET_RX_PAD + xdp_headroom;
> > -       unsigned int headroom = vi->hdr_len + header_offset;
> > -       unsigned int buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
> > -                             SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> >         struct page *page = virt_to_head_page(buf);
> > +       unsigned int header_offset;
> > +       unsigned int headroom;
> > +       unsigned int buflen;
> >
> >         len -= vi->hdr_len;
> >         stats->bytes += len;
> > @@ -1072,6 +1071,11 @@ static struct sk_buff *receive_small(struct net_device *dev,
> >         rcu_read_unlock();
> >
> >  skip_xdp:
> > +       header_offset = VIRTNET_RX_PAD + xdp_headroom;
> > +       headroom = vi->hdr_len + header_offset;
> > +       buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
> > +               SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> > +
> >         skb = build_skb(buf, buflen);
> >         if (!skb)
> >                 goto err;
> > --
> > 2.32.0.3.g01195cf9f
> >
>