netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Shrijeet Mukherjee <shrijeet@gmail.com>
Cc: mst@redhat.com, tom@herbertland.com, netdev@vger.kernel.org,
	shm@cumulusnetworks.com, roopa@cumulusnetworks.com,
	nikolay@cumulusnetworks.com
Subject: Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
Date: Sun, 23 Oct 2016 09:38:08 -0700	[thread overview]
Message-ID: <20161023093808.161535f3@xeon-e3> (raw)
In-Reply-To: <1477109243-29520-1-git-send-email-shrijeet@gmail.com>

Overall, I am glad to see XDP support more widely available. Minor stuff
in implementation.

>  
> +/* this function is not called from the receive_buf path directly as
> + * we want to use the page model for rx merge buffer and big buffers
> + * and not use the fast path for driving skb's around
> + */
> +static inline u32 do_xdp_prog(struct virtnet_info *vi,
> +			      struct receive_queue *rq,
> +			      void *buf, int offset, int len)
> +{

Do not mark non-trivial static functions as inline. The compiler will
do it anyway if it thinks it is appropriate.

+static int virtnet_xdp_query(struct net_device *dev)

Use bool here.

@@ -366,13 +420,22 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
...

 	if (unlikely(!curr_skb))
 		goto err_skb;
+
 	while (--num_buf) {
 		int num_skb_frags;

@@ -1878,7 +2000,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 		if (virtnet_change_mtu(dev, mtu))
 			__virtio_clear_bit(vdev, VIRTIO_NET_F_MTU);
 	}
-
 	if (vi->any_header_sg)
 		dev->needed_headroom = vi->hdr_len;

This parts of the patch is white space creep. I.e other edits you did
that stayed around.

Do you have any performance numbers? Does the receive into pages hurt
the non XDP performance?

  reply	other threads:[~2016-10-23 16:37 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-22  4:07 [PATCH net-next RFC WIP] Patch for XDP support for virtio_net Shrijeet Mukherjee
2016-10-23 16:38 ` Stephen Hemminger [this message]
2016-10-24  1:51   ` Shrijeet Mukherjee
2016-10-25  1:10     ` Alexei Starovoitov
2016-10-25 17:36 ` Jakub Kicinski
2016-10-26 13:52 ` Jesper Dangaard Brouer
2016-10-26 16:36   ` Michael S. Tsirkin
2016-10-26 16:52     ` David Miller
2016-10-26 17:07       ` Michael S. Tsirkin
2016-10-26 17:11         ` David Miller
2016-10-27  8:55           ` Jesper Dangaard Brouer
2016-10-27 21:09             ` John Fastabend
2016-10-27 21:30               ` Michael S. Tsirkin
2016-10-27 21:42                 ` David Miller
2016-10-27 22:25                   ` Michael S. Tsirkin
2016-10-28  1:35                     ` David Miller
2016-10-28  1:43                       ` Alexander Duyck
2016-10-28  2:10                         ` David Miller
2016-10-28 15:56                           ` John Fastabend
2016-10-28 16:18                             ` Jakub Kicinski
2016-10-28 18:22                               ` Alexei Starovoitov
2016-10-28 20:35                                 ` Alexander Duyck
2016-10-28 20:42                                   ` Jakub Kicinski
2016-10-28 20:36                                 ` Jakub Kicinski
2016-10-29  3:51                                 ` Shrijeet Mukherjee
2016-10-29 11:25                                   ` Thomas Graf
2016-11-02 14:27                                     ` Jesper Dangaard Brouer
2016-11-03  1:28                                       ` Shrijeet Mukherjee
2016-11-03  4:11                                         ` Michael S. Tsirkin
2016-11-03  6:44                                           ` John Fastabend
2016-11-03 22:20                                             ` John Fastabend
2016-11-03 22:42                                             ` Michael S. Tsirkin
2016-11-03 23:29                                               ` John Fastabend
2016-11-04  0:34                                                 ` Michael S. Tsirkin
2016-11-04 23:05                                                   ` John Fastabend
2016-11-06  6:50                                                     ` Michael S. Tsirkin
2016-10-28 17:11                             ` David Miller
2016-10-30 22:53                               ` Michael S. Tsirkin
2016-11-02 14:01                               ` Jesper Dangaard Brouer
2016-11-02 16:06                                 ` Alexander Duyck
2016-10-28  0:02               ` Shrijeet Mukherjee
2016-10-28  0:46                 ` Shrijeet Mukherjee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161023093808.161535f3@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=shm@cumulusnetworks.com \
    --cc=shrijeet@gmail.com \
    --cc=tom@herbertland.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).