Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Michael S. Tsirkin]

On Wed, Feb 08, 2017 at 08:39:13AM -0800, John Fastabend wrote:
> [...]
> 
> > However, I came up with a new idea for the future and I'd like to show
> > where I'm going.  The idea is that we don't use s/g buffers on RX, so we
> > have a pointer per descriptor untapped.  So we can allow users to stick
> > their own pointer in there, if they promise not to use s/g on this vq.
> > With a full extra pointer to play with, we can go wild.
> 
> I looked at this quickly it seems like it would work and allow us to avoid
> the reset. However, it seems like a lot of churn to avoid a single reset.
> I don't see the reset itself as being that bad of an operation. I agree the
> reset is not ideal though.
> 
> Are there any other use cases for this other than XDP?

Well in fact this would allow reducing MERGEABLE_BUFFER_ALIGN
to L1_CACHE_BYTES so we save space per packet for regular
networking.

The idea to use build_skb would also benefit accordingly.

I guess ndo_set_rx_headroom could benefit if we were to implement that.



> > 
> > Take a look but it doesn't even build yet.
> > Need to roll it out to all devices etc.
> > 
> > --->
> > 
> 
> [...]

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[John Fastabend]

[...]

> However, I came up with a new idea for the future and I'd like to show
> where I'm going.  The idea is that we don't use s/g buffers on RX, so we
> have a pointer per descriptor untapped.  So we can allow users to stick
> their own pointer in there, if they promise not to use s/g on this vq.
> With a full extra pointer to play with, we can go wild.

I looked at this quickly it seems like it would work and allow us to avoid
the reset. However, it seems like a lot of churn to avoid a single reset.
I don't see the reset itself as being that bad of an operation. I agree the
reset is not ideal though.

Are there any other use cases for this other than XDP?

> 
> Take a look but it doesn't even build yet.
> Need to roll it out to all devices etc.
> 
> --->
> 

[...]

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[David Miller]

From: "Michael S. Tsirkin" 
Date: Tue, 7 Feb 2017 06:15:13 +0200

> On Thu, Feb 02, 2017 at 07:14:05PM -0800, John Fastabend wrote:
>> This series adds adjust head support for virtio. The following is my
>> test setup. I use qemu + virtio as follows,
>> 
>> ./x86_64-softmmu/qemu-system-x86_64 \
>>   -hda /var/lib/libvirt/images/Fedora-test0.img \
>>   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
>>   -device 
>> virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9
>> 
>> In order to use XDP with virtio until LRO is supported TSO must be
>> turned off in the host. The important fields in the above command line
>> are the following,
>> 
>>   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off
>> 
>> Also note it is possible to conusme more queues than can be supported
>> because when XDP is enabled for retransmit XDP attempts to use a queue
>> per cpu. My standard queue count is 'queues=4'.
>> 
>> After loading the VM I run the relevant XDP test programs in,
>> 
>>   ./sammples/bpf
>> 
>> For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
>> with iperf (-d option to get bidirectional traffic), ping, and pktgen.
>> I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
>> the normal traffic path to the stack continues to work with XDP loaded.
>> 
>> It would be great to automate this soon. At the moment I do it by hand
>> which is starting to get tedious.
>> 
>> v2: original series dropped trace points after merge.
> 
> So I'd say ok, let's go ahead and merge this for now.

Ok, done.  Thanks everyone.

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Michael S. Tsirkin]

On Thu, Feb 02, 2017 at 07:14:05PM -0800, John Fastabend wrote:
> This series adds adjust head support for virtio. The following is my
> test setup. I use qemu + virtio as follows,
> 
> ./x86_64-softmmu/qemu-system-x86_64 \
>   -hda /var/lib/libvirt/images/Fedora-test0.img \
>   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
>   -device 
> virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9
> 
> In order to use XDP with virtio until LRO is supported TSO must be
> turned off in the host. The important fields in the above command line
> are the following,
> 
>   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off
> 
> Also note it is possible to conusme more queues than can be supported
> because when XDP is enabled for retransmit XDP attempts to use a queue
> per cpu. My standard queue count is 'queues=4'.
> 
> After loading the VM I run the relevant XDP test programs in,
> 
>   ./sammples/bpf
> 
> For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
> with iperf (-d option to get bidirectional traffic), ping, and pktgen.
> I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
> the normal traffic path to the stack continues to work with XDP loaded.
> 
> It would be great to automate this soon. At the moment I do it by hand
> which is starting to get tedious.
> 
> v2: original series dropped trace points after merge.

So I'd say ok, let's go ahead and merge this for now.

However, I came up with a new idea for the future and I'd like to show
where I'm going.  The idea is that we don't use s/g buffers on RX, so we
have a pointer per descriptor untapped.  So we can allow users to stick
their own pointer in there, if they promise not to use s/g on this vq.
With a full extra pointer to play with, we can go wild.

Take a look but it doesn't even build yet.
Need to roll it out to all devices etc.

--->

Signed-off-by: Michael S. Tsirkin 

--

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa..b59e95e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -263,6 +263,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
unsigned int out_sgs,
unsigned int in_sgs,
void *data,
+   void *ctx,
gfp_t gfp)
 {
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -275,6 +276,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
START_USE(vq);
 
BUG_ON(data == NULL);
+   BUG_ON(ctx && vq->indirect);
 
if (unlikely(vq->broken)) {
END_USE(vq);
@@ -389,6 +391,8 @@ static inline int virtqueue_add(struct virtqueue *_vq,
vq->desc_state[head].data = data;
if (indirect)
vq->desc_state[head].indir_desc = desc;
+   if (ctx)
+   vq->desc_state[head].indir_desc = ctx;
 
/* Put entry in available array (but don't update avail->idx until they
 * do sync). */
@@ -461,7 +465,8 @@ int virtqueue_add_sgs(struct virtqueue *_vq,
for (sg = sgs[i]; sg; sg = sg_next(sg))
total_sg++;
}
-   return virtqueue_add(_vq, sgs, total_sg, out_sgs, in_sgs, data, gfp);
+   return virtqueue_add(_vq, sgs, total_sg, out_sgs, in_sgs,
+data, NULL, gfp);
 }
 EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
 
@@ -483,7 +488,7 @@ int virtqueue_add_outbuf(struct virtqueue *vq,
 void *data,
 gfp_t gfp)
 {
-   return virtqueue_add(vq, , num, 1, 0, data, gfp);
+   return virtqueue_add(vq, , num, 1, 0, data, NULL, gfp);
 }
 EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
 
@@ -505,7 +510,31 @@ int virtqueue_add_inbuf(struct virtqueue *vq,
void *data,
gfp_t gfp)
 {
-   return virtqueue_add(vq, , num, 0, 1, data, gfp);
+   return virtqueue_add(vq, , num, 0, 1, data, NULL, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_inbuf);
+
+/**
+ * virtqueue_add_inbuf_ctx - expose input buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: scatterlist (must be well-formed and terminated!)
+ * @num: the number of entries in @sg writable by other side
+ * @data: the token identifying the buffer.
+ * @ctx: extra context for the token
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
+ */
+int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
+   struct scatterlist *sg, unsigned int num,
+   void *data,
+   void *ctx,
+   gfp_t gfp)
+{
+   return virtqueue_add(vq, 

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[David Miller]

From: "Michael S. Tsirkin" 
Date: Mon, 6 Feb 2017 06:39:54 +0200

> Well the point is to avoid resets completely, at the cost of extra 256 bytes
> for packets > 128 bytes on ppc (64k pages) only.
> 
> Found a volunteer so I hope to have this idea tested on ppc Tuesday.
> 
> And really all we need to know is confirm whether this:
> -#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
> +#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT (PAGE_SHIFT / 2 + 1)
> 
> affects performance in a measureable way.
> 
> So I would rather wait another day. But the patches themselves
> look correct, from that POV.
> 
> Acked-by: Michael S. Tsirkin 
> 
> but I would prefer that you waited another day for a Tested-by from me too.

Ok.

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Jason Wang]

On 2017年02月06日 12:39, Michael S. Tsirkin wrote:

On Sun, Feb 05, 2017 at 05:36:34PM -0500, David Miller wrote:

From: John Fastabend 
Date: Thu, 02 Feb 2017 19:14:05 -0800


This series adds adjust head support for virtio. The following is my
test setup. I use qemu + virtio as follows,

./x86_64-softmmu/qemu-system-x86_64 \
   -hda /var/lib/libvirt/images/Fedora-test0.img \
   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
   -device 
virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9

In order to use XDP with virtio until LRO is supported TSO must be
turned off in the host. The important fields in the above command line
are the following,

   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off

Also note it is possible to conusme more queues than can be supported
because when XDP is enabled for retransmit XDP attempts to use a queue
per cpu. My standard queue count is 'queues=4'.

After loading the VM I run the relevant XDP test programs in,

   ./sammples/bpf

For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
with iperf (-d option to get bidirectional traffic), ping, and pktgen.
I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
the normal traffic path to the stack continues to work with XDP loaded.

It would be great to automate this soon. At the moment I do it by hand
which is starting to get tedious.

v2: original series dropped trace points after merge.

Michael, I just want to apply this right now.

I don't think haggling over whether to allocate the adjust_head area
unconditionally or not is a blocker for this series going in.  That
can be addressed trivially in a follow-on patch.

FYI it would just mean we revert most of this patchset except patches 2 and 3 
though.


We want these new reset paths tested as much as possible and each day
we delay this series is detrimental towards that goal.

Thanks.

Well the point is to avoid resets completely, at the cost of extra 256 bytes
for packets > 128 bytes on ppc (64k pages) only.

Found a volunteer so I hope to have this idea tested on ppc Tuesday.

And really all we need to know is confirm whether this:
-#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
+#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT (PAGE_SHIFT / 2 + 1)

affects performance in a measureable way.


Ok, but we still need to drop some packets with this way I believe, and 
does it work if we allow to change the size of headroom in the future?


Thanks



So I would rather wait another day. But the patches themselves
look correct, from that POV.

Acked-by: Michael S. Tsirkin 

but I would prefer that you waited another day for a Tested-by from me too.

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Michael S. Tsirkin]

On Sun, Feb 05, 2017 at 05:36:34PM -0500, David Miller wrote:
> From: John Fastabend 
> Date: Thu, 02 Feb 2017 19:14:05 -0800
> 
> > This series adds adjust head support for virtio. The following is my
> > test setup. I use qemu + virtio as follows,
> > 
> > ./x86_64-softmmu/qemu-system-x86_64 \
> >   -hda /var/lib/libvirt/images/Fedora-test0.img \
> >   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
> >   -device 
> > virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9
> > 
> > In order to use XDP with virtio until LRO is supported TSO must be
> > turned off in the host. The important fields in the above command line
> > are the following,
> > 
> >   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off
> > 
> > Also note it is possible to conusme more queues than can be supported
> > because when XDP is enabled for retransmit XDP attempts to use a queue
> > per cpu. My standard queue count is 'queues=4'.
> > 
> > After loading the VM I run the relevant XDP test programs in,
> > 
> >   ./sammples/bpf
> > 
> > For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
> > with iperf (-d option to get bidirectional traffic), ping, and pktgen.
> > I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
> > the normal traffic path to the stack continues to work with XDP loaded.
> > 
> > It would be great to automate this soon. At the moment I do it by hand
> > which is starting to get tedious.
> > 
> > v2: original series dropped trace points after merge.
> 
> Michael, I just want to apply this right now.
> 
> I don't think haggling over whether to allocate the adjust_head area
> unconditionally or not is a blocker for this series going in.  That
> can be addressed trivially in a follow-on patch.

FYI it would just mean we revert most of this patchset except patches 2 and 3 
though.

> We want these new reset paths tested as much as possible and each day
> we delay this series is detrimental towards that goal.
> 
> Thanks.

Well the point is to avoid resets completely, at the cost of extra 256 bytes
for packets > 128 bytes on ppc (64k pages) only.

Found a volunteer so I hope to have this idea tested on ppc Tuesday.

And really all we need to know is confirm whether this:
-#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT ((PAGE_SHIFT + 1) / 2)
+#define MERGEABLE_BUFFER_MIN_ALIGN_SHIFT (PAGE_SHIFT / 2 + 1)

affects performance in a measureable way.

So I would rather wait another day. But the patches themselves
look correct, from that POV.

Acked-by: Michael S. Tsirkin 

but I would prefer that you waited another day for a Tested-by from me too.

-- 
MST

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[David Miller]

From: John Fastabend 
Date: Thu, 02 Feb 2017 19:14:05 -0800

> This series adds adjust head support for virtio. The following is my
> test setup. I use qemu + virtio as follows,
> 
> ./x86_64-softmmu/qemu-system-x86_64 \
>   -hda /var/lib/libvirt/images/Fedora-test0.img \
>   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
>   -device 
> virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9
> 
> In order to use XDP with virtio until LRO is supported TSO must be
> turned off in the host. The important fields in the above command line
> are the following,
> 
>   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off
> 
> Also note it is possible to conusme more queues than can be supported
> because when XDP is enabled for retransmit XDP attempts to use a queue
> per cpu. My standard queue count is 'queues=4'.
> 
> After loading the VM I run the relevant XDP test programs in,
> 
>   ./sammples/bpf
> 
> For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
> with iperf (-d option to get bidirectional traffic), ping, and pktgen.
> I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
> the normal traffic path to the stack continues to work with XDP loaded.
> 
> It would be great to automate this soon. At the moment I do it by hand
> which is starting to get tedious.
> 
> v2: original series dropped trace points after merge.

Michael, I just want to apply this right now.

I don't think haggling over whether to allocate the adjust_head area
unconditionally or not is a blocker for this series going in.  That
can be addressed trivially in a follow-on patch.

We want these new reset paths tested as much as possible and each day
we delay this series is detrimental towards that goal.

Thanks.

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Jakub Kicinski]

On Thu, 02 Feb 2017 19:14:05 -0800, John Fastabend wrote:
> This series adds adjust head support for virtio. ...

XDP bits look good to me too!

Re: [net-next PATCH v2 0/5] XDP adjust head support for virtio

[Alexei Starovoitov]

On 2/2/17 7:14 PM, John Fastabend wrote:

This series adds adjust head support for virtio. The following is my
test setup. I use qemu + virtio as follows,

./x86_64-softmmu/qemu-system-x86_64 \
   -hda /var/lib/libvirt/images/Fedora-test0.img \
   -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
   -device 
virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9

In order to use XDP with virtio until LRO is supported TSO must be
turned off in the host. The important fields in the above command line
are the following,

   guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off


thank you for sharing the command line!
Pretty hard to figure this out.
Too bad you dropped the patch that allows dynamic switch off of LRO.
Long term I don't see why guest shouldn't be allowed to turn that knob.


Also note it is possible to conusme more queues than can be supported
because when XDP is enabled for retransmit XDP attempts to use a queue
per cpu. My standard queue count is 'queues=4'.

After loading the VM I run the relevant XDP test programs in,

   ./sammples/bpf

For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
with iperf (-d option to get bidirectional traffic), ping, and pktgen.
I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
the normal traffic path to the stack continues to work with XDP loaded.


same here.
xdp testing requires two physical machines with specific nics,
so hard to automate.
At least the virtio+xdp gives us ability to test the programs
automatically. So virtio+xdp will get the most test coverage and
all hw nics will be using it as a yardstick. Very important to
make it easy to use.

For bpf and generic xdp bits:
Acked-by: Alexei Starovoitov 

[net-next PATCH v2 0/5] XDP adjust head support for virtio

[John Fastabend]

This series adds adjust head support for virtio. The following is my
test setup. I use qemu + virtio as follows,

./x86_64-softmmu/qemu-system-x86_64 \
  -hda /var/lib/libvirt/images/Fedora-test0.img \
  -m 4096  -enable-kvm -smp 2 -netdev tap,id=hn0,queues=4,vhost=on \
  -device 
virtio-net-pci,netdev=hn0,mq=on,guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off,vectors=9

In order to use XDP with virtio until LRO is supported TSO must be
turned off in the host. The important fields in the above command line
are the following,

  guest_tso4=off,guest_tso6=off,guest_ecn=off,guest_ufo=off

Also note it is possible to conusme more queues than can be supported
because when XDP is enabled for retransmit XDP attempts to use a queue
per cpu. My standard queue count is 'queues=4'.

After loading the VM I run the relevant XDP test programs in,

  ./sammples/bpf

For this series I tested xdp1, xdp2, and xdp_tx_iptunnel. I usually test
with iperf (-d option to get bidirectional traffic), ping, and pktgen.
I also have a modified xdp1 that returns XDP_PASS on any packet to ensure
the normal traffic path to the stack continues to work with XDP loaded.

It would be great to automate this soon. At the moment I do it by hand
which is starting to get tedious.

v2: original series dropped trace points after merge.

---

John Fastabend (5):
  virtio_net: wrap rtnl_lock in test for calling with lock already held
  virtio_net: factor out xdp handler for readability
  virtio_net: remove duplicate queue pair binding in XDP
  virtio_net: refactor freeze/restore logic into virtnet reset logic
  virtio_net: XDP support for adjust_head


 drivers/net/virtio_net.c |  338 ++
 drivers/virtio/virtio.c  |   42 +++---
 include/linux/virtio.h   |4 +
 3 files changed, 247 insertions(+), 137 deletions(-)

--
Signature