diff mbox series

[net-next,v2,13/14] virtio_net: small: optimize code

Message ID 20230418065327.72281-14-xuanzhuo@linux.alibaba.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series virtio_net: refactor xdp codes | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 18
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
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: 18 this patch: 18
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 25 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 April 18, 2023, 6:53 a.m. UTC 
Avoid the problem that some variables(headroom and so on) will repeat
the calculation when process xdp.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/net/virtio_net.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Jason Wang April 20, 2023, 6:32 a.m. UTC | #1 
On Tue, Apr 18, 2023 at 2:53 PM Xuan Zhuo <xuanzhuo@linux.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.

Thanks

>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.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
>
Xuan Zhuo April 20, 2023, 8:56 a.m. UTC | #2 
On Thu, 20 Apr 2023 14:32:37 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, Apr 18, 2023 at 2:53 PM Xuan Zhuo <xuanzhuo@linux.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@linux.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
> >
>
diff mbox series

Patch

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;