netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	John Fastabend <john.fastabend@gmail.com>
Cc: john.r.fastabend@intel.com, netdev@vger.kernel.org,
	alexei.starovoitov@gmail.com, daniel@iogearbox.net
Subject: Re: [net PATCH v5 6/6] virtio_net: XDP support for adjust_head
Date: Thu, 19 Jan 2017 11:05:40 +0800	[thread overview]
Message-ID: <40e9a46a-1115-c21a-49f3-fdedb0a34101@redhat.com> (raw)
In-Reply-To: <20170118171259-mutt-send-email-mst@kernel.org>



On 2017年01月18日 23:15, Michael S. Tsirkin wrote:
> On Tue, Jan 17, 2017 at 02:22:59PM -0800, John Fastabend wrote:
>> Add support for XDP adjust head by allocating a 256B header region
>> that XDP programs can grow into. This is only enabled when a XDP
>> program is loaded.
>>
>> In order to ensure that we do not have to unwind queue headroom push
>> queue setup below bpf_prog_add. It reads better to do a prog ref
>> unwind vs another queue setup call.
>>
>> At the moment this code must do a full reset to ensure old buffers
>> without headroom on program add or with headroom on program removal
>> are not used incorrectly in the datapath. Ideally we would only
>> have to disable/enable the RX queues being updated but there is no
>> API to do this at the moment in virtio so use the big hammer. In
>> practice it is likely not that big of a problem as this will only
>> happen when XDP is enabled/disabled changing programs does not
>> require the reset. There is some risk that the driver may either
>> have an allocation failure or for some reason fail to correctly
>> negotiate with the underlying backend in this case the driver will
>> be left uninitialized. I have not seen this ever happen on my test
>> systems and for what its worth this same failure case can occur
>> from probe and other contexts in virtio framework.
>>
>> Signed-off-by: John Fastabend<john.r.fastabend@intel.com>
> I've been thinking about it - can't we drop
> old buffers without the head room which were posted before
> xdp attached?
>
> Avoiding the reset would be much nicer.
>
> Thoughts?
>

As been discussed before, device may use them in the same time so it's 
not safe. Or do you mean detect them after xdp were set and drop the 
buffer without head room, this looks sub-optimal.

Thanks

  reply	other threads:[~2017-01-19  3:22 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-17 22:19 [net PATCH v5 0/6] virtio_net XDP fixes and adjust_header support John Fastabend
2017-01-17 22:19 ` [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive John Fastabend
2017-01-18 15:48   ` Michael S. Tsirkin
2017-01-23 21:08   ` Michael S. Tsirkin
2017-01-23 21:57     ` John Fastabend
2017-01-24 19:43     ` David Miller
2017-01-24 20:08       ` Michael S. Tsirkin
2017-01-24 20:11         ` David Miller
2017-01-24 20:54           ` Michael S. Tsirkin
2017-01-25  2:57         ` Jason Wang
2017-01-25  3:23           ` Michael S. Tsirkin
2017-01-25  4:02             ` John Fastabend
2017-01-25  5:46               ` Jason Wang
2017-01-25 14:47                 ` Michael S. Tsirkin
2017-01-25 14:45               ` Michael S. Tsirkin
2017-01-17 22:20 ` [net PATCH v5 2/6] virtio_net: wrap rtnl_lock in test for calling with lock already held John Fastabend
2017-01-17 22:21 ` [net PATCH v5 3/6] virtio_net: factor out xdp handler for readability John Fastabend
2017-01-18 15:48   ` Michael S. Tsirkin
2017-01-17 22:21 ` [net PATCH v5 4/6] virtio_net: remove duplicate queue pair binding in XDP John Fastabend
2017-01-18 15:49   ` Michael S. Tsirkin
2017-01-17 22:22 ` [net PATCH v5 5/6] virtio_net: refactor freeze/restore logic into virtnet reset logic John Fastabend
2017-01-18 15:50   ` Michael S. Tsirkin
2017-01-17 22:22 ` [net PATCH v5 6/6] virtio_net: XDP support for adjust_head John Fastabend
2017-01-18  3:35   ` Jason Wang
2017-01-18 15:15   ` Michael S. Tsirkin
2017-01-19  3:05     ` Jason Wang [this message]
2017-01-19 21:11       ` Michael S. Tsirkin
2017-01-20  3:26         ` Jason Wang
2017-01-20  3:39           ` John Fastabend
2017-01-20  3:38         ` John Fastabend
2017-01-20 16:59         ` David Laight
2017-01-20 17:48           ` Michael S. Tsirkin
2017-01-22  2:51             ` Jason Wang
2017-01-22  4:14               ` John Fastabend
2017-01-23 17:02                 ` Michael S. Tsirkin
2017-01-23 19:22   ` Michael S. Tsirkin
2017-01-23 20:09     ` Michael S. Tsirkin
2017-01-23 22:12       ` John Fastabend
2017-01-23 22:28         ` Michael S. Tsirkin
2017-01-18 15:48 ` [net PATCH v5 0/6] virtio_net XDP fixes and adjust_header support Michael S. Tsirkin

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=40e9a46a-1115-c21a-49f3-fdedb0a34101@redhat.com \
    --to=jasowang@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=john.r.fastabend@intel.com \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    /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).