On Tue, Jan 24, 2017 at 08:02:29PM -0800, John Fastabend wrote:

On 17-01-24 07:23 PM, Michael S. Tsirkin wrote:

quoted

On Wed, Jan 25, 2017 at 10:57:12AM +0800, Jason Wang wrote:

quoted

On 2017年01月25日 04:08, Michael S. Tsirkin wrote:

quoted

On Tue, Jan 24, 2017 at 02:43:28PM -0500, David Miller wrote:

quoted

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Mon, 23 Jan 2017 23:08:35 +0200

quoted

On Tue, Jan 17, 2017 at 02:19:50PM -0800, John Fastabend wrote:

quoted

In the small buffer case during driver unload we currently use
put_page instead of dev_kfree_skb. Resolve this by adding a check
for virtnet mode when checking XDP queue type. Also name the
function so that the code reads correctly to match the additional
check.

Fixes: bb91accf2733 ("virtio-net: XDP support for small buffers")
Signed-off-by: John Fastabend <redacted>
Acked-by: Jason Wang <jasowang@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

I think we definitely want this one in -net as it's
a bugfix.

This whole series is a bug fix, we must have adjust_header XDP
support in the virtio_net driver before v4.10 goes out, it is
a requires base feature for XDP.

I have to say device resets outside probe have a huge potential
to uncover hypervisor bugs.

Maybe not if it reuses most of current codes? Since we've already used them
in sleep or hibernation?

Thanks

Except almost no one uses sleep or hybernate with VMs.  I'm not saying
it's a bad idea, just that it needs a lot of testing before release and
we won't get enough if we merge at this point.

Then it would seem like a good thing to have another user of these paths and
find the bugs versus letting them sit there for some poor folks who do use
sleep/hybernate.

Absolutely. But -rc6 is not the time to test waters IMO.

quoted

quoted

quoted

  I am rather uncomfortable
doing that after -rc1.

How about a module option to disable it by default?
We can then ship a partial implementation in 4.10
and work on completing it in 4.11.

Ugh I would prefer to avoid module options. This will only happen if users
push XDP program into driver anyways.

Again I agree, it's an idea for a stopgap measure so we can have
something in 4.10 - and also assuming that 256b headroom is a must.

quoted

To clarify, I'm thinking an option similar to enable_xdp,
and have all packets have a 256 byte headroom for 4.10.

An option where? In QEMU side, in driver? Is the reset really that bad, coming
from a hardware driver side lots of configuration changes can cause resets. I
agree its not overly elegant but could follow on patches be used to make it
prettier if possible.

Again I agree and it's not that bad it's just not something we should
do past rc5.

I know folks prefer to avoid tuning knobs but I think exposing the headroom
configuration to users might not be a bad idea. After all these same users are
already programming maps and ebpf codes. A simple tuning knob should not be a
big deal and reasonable defaults would of course be used. That is a net-next
debate though.

No arguments from my side here.

quoted

Consider our options for 4.11.

Finally just to point out here are the drivers with XDP support on latest
net tree,

	mlx/mlx5
	mlx/mlx4
	qlogic/qede
	netronome/nfp
	virtio_net

And here is the list of adjust header support,

	mlx/mlx4

Above seems to imply an interface for userspace to detect the amount
of head space would be benefitial.

So we currently have the same feature gap on all the other drivers except one.
Although I do not think that is a very good excuse. Lets figure out what we
should do about virtio.

Thanks,
John

If we can simply defer adjust_head patches to 4.11 then that's fine.

-- 
MST