From: "Michael S. Tsirkin" <mst@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Cong Wang <amwang@redhat.com>,
qemu-devel@nongnu.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
Dave Jones <davej@redhat.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH] virtio-net: put virtio net header inline with data
Date: Sun, 9 Jun 2013 10:11:27 +0300 [thread overview]
Message-ID: <20130609071127.GC13082@redhat.com> (raw)
In-Reply-To: <87hahar7ms.fsf@rustcorp.com.au>
On Fri, Jun 07, 2013 at 11:42:43AM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > For small packets we can simplify xmit processing by linearizing buffers
> > with the header: most packets seem to have enough head room we can use
> > for this purpose.
> >
> > Since some older hypervisors (e.g. qemu before version 1.5)
> > required that header is the first s/g element,
> > we need a feature bit for this.
>
> OK, we know this is horrible. But I will sleep better knowing that we
> this feature need never make it into a final 1.0 spec, since it can be
> assumed at that point...
Nod. Though if we want to require this for all devices,
virtio-blk scsi command passthrough will need to change -
I sent a spec patch a while ago
virtio-spec: add field for scsi command size
any comments on it?
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > + if (vi->mergeable_rx_bufs)
> > + hdr_len = sizeof hdr->mhdr;
> > + else
> > + hdr_len = sizeof hdr->hdr;
> > +
> > + can_push = vi->any_header_sg &&
> > + !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
> > + !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;
>
> Idle thought: how often does this fail?
I think it's mostly doesn't fail in my testing.
It's probably a good idea to add a counter here, then
if it starts triggering we can optimize.
I think things like skb_header_cloned depend on guest config
really, e.g. tcpdump running on the interface in guest can cause this.
> Would it suck if we copied
> headers which didn't let us prepend data?
I think it will - copies are generally best avoided,
and header is easily 1K of data.
> Or could we bump
> dev->hard_header_len appropriately?
Needs some thought, though from experience it's a pain.
> Thanks,
> Rusty.
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: "David S. Miller" <davem@davemloft.net>,
Jason Wang <jasowang@redhat.com>, Cong Wang <amwang@redhat.com>,
Amos Kong <akong@redhat.com>, Dave Jones <davej@redhat.com>,
virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
qemu-devel@nongnu.org
Subject: Re: [PATCH] virtio-net: put virtio net header inline with data
Date: Sun, 9 Jun 2013 10:11:27 +0300 [thread overview]
Message-ID: <20130609071127.GC13082@redhat.com> (raw)
In-Reply-To: <87hahar7ms.fsf@rustcorp.com.au>
On Fri, Jun 07, 2013 at 11:42:43AM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > For small packets we can simplify xmit processing by linearizing buffers
> > with the header: most packets seem to have enough head room we can use
> > for this purpose.
> >
> > Since some older hypervisors (e.g. qemu before version 1.5)
> > required that header is the first s/g element,
> > we need a feature bit for this.
>
> OK, we know this is horrible. But I will sleep better knowing that we
> this feature need never make it into a final 1.0 spec, since it can be
> assumed at that point...
Nod. Though if we want to require this for all devices,
virtio-blk scsi command passthrough will need to change -
I sent a spec patch a while ago
virtio-spec: add field for scsi command size
any comments on it?
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > + if (vi->mergeable_rx_bufs)
> > + hdr_len = sizeof hdr->mhdr;
> > + else
> > + hdr_len = sizeof hdr->hdr;
> > +
> > + can_push = vi->any_header_sg &&
> > + !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
> > + !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;
>
> Idle thought: how often does this fail?
I think it's mostly doesn't fail in my testing.
It's probably a good idea to add a counter here, then
if it starts triggering we can optimize.
I think things like skb_header_cloned depend on guest config
really, e.g. tcpdump running on the interface in guest can cause this.
> Would it suck if we copied
> headers which didn't let us prepend data?
I think it will - copies are generally best avoided,
and header is easily 1K of data.
> Or could we bump
> dev->hard_header_len appropriately?
Needs some thought, though from experience it's a pain.
> Thanks,
> Rusty.
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: Cong Wang <amwang@redhat.com>,
qemu-devel@nongnu.org, netdev@vger.kernel.org,
Jason Wang <jasowang@redhat.com>,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
Dave Jones <davej@redhat.com>, Amos Kong <akong@redhat.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [Qemu-devel] [PATCH] virtio-net: put virtio net header inline with data
Date: Sun, 9 Jun 2013 10:11:27 +0300 [thread overview]
Message-ID: <20130609071127.GC13082@redhat.com> (raw)
In-Reply-To: <87hahar7ms.fsf@rustcorp.com.au>
On Fri, Jun 07, 2013 at 11:42:43AM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > For small packets we can simplify xmit processing by linearizing buffers
> > with the header: most packets seem to have enough head room we can use
> > for this purpose.
> >
> > Since some older hypervisors (e.g. qemu before version 1.5)
> > required that header is the first s/g element,
> > we need a feature bit for this.
>
> OK, we know this is horrible. But I will sleep better knowing that we
> this feature need never make it into a final 1.0 spec, since it can be
> assumed at that point...
Nod. Though if we want to require this for all devices,
virtio-blk scsi command passthrough will need to change -
I sent a spec patch a while ago
virtio-spec: add field for scsi command size
any comments on it?
> > pr_debug("%s: xmit %p %pM\n", vi->dev->name, skb, dest);
> > + if (vi->mergeable_rx_bufs)
> > + hdr_len = sizeof hdr->mhdr;
> > + else
> > + hdr_len = sizeof hdr->hdr;
> > +
> > + can_push = vi->any_header_sg &&
> > + !((unsigned long)skb->data & (__alignof__(*hdr) - 1)) &&
> > + !skb_header_cloned(skb) && skb_headroom(skb) >= hdr_len;
>
> Idle thought: how often does this fail?
I think it's mostly doesn't fail in my testing.
It's probably a good idea to add a counter here, then
if it starts triggering we can optimize.
I think things like skb_header_cloned depend on guest config
really, e.g. tcpdump running on the interface in guest can cause this.
> Would it suck if we copied
> headers which didn't let us prepend data?
I think it will - copies are generally best avoided,
and header is easily 1K of data.
> Or could we bump
> dev->hard_header_len appropriately?
Needs some thought, though from experience it's a pain.
> Thanks,
> Rusty.
next prev parent reply other threads:[~2013-06-09 7:11 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-06 9:55 [PATCH] virtio-net: put virtio net header inline with data Michael S. Tsirkin
2013-06-06 9:55 ` [Qemu-devel] " Michael S. Tsirkin
2013-06-06 19:59 ` Jesse Larrew
2013-06-06 19:59 ` Jesse Larrew
2013-06-06 20:09 ` Dave Jones
2013-06-06 20:09 ` Dave Jones
2013-06-06 20:09 ` Dave Jones
2013-06-06 20:18 ` Jesse Larrew
2013-06-06 20:18 ` Jesse Larrew
2013-06-07 2:12 ` Rusty Russell
2013-06-07 2:12 ` [Qemu-devel] " Rusty Russell
2013-06-09 7:11 ` Michael S. Tsirkin [this message]
2013-06-09 7:11 ` Michael S. Tsirkin
2013-06-09 7:11 ` Michael S. Tsirkin
2013-06-07 2:52 ` Jason Wang
2013-06-07 2:52 ` [Qemu-devel] " Jason Wang
2013-06-07 2:52 ` Jason Wang
2013-06-09 6:52 ` Michael S. Tsirkin
2013-06-09 6:52 ` [Qemu-devel] " Michael S. Tsirkin
2013-06-09 6:52 ` Michael S. Tsirkin
-- strict thread matches above, loose matches on Subject: below --
2013-07-08 10:12 Michael S. Tsirkin
2013-07-08 10:12 ` Michael S. Tsirkin
2013-07-09 2:16 ` Rusty Russell
2013-07-09 2:16 ` Rusty Russell
2013-07-09 5:18 ` Michael S. Tsirkin
2013-07-09 5:18 ` Michael S. Tsirkin
2013-07-09 8:08 ` Rusty Russell
2013-07-09 8:08 ` Rusty Russell
2013-07-10 4:38 ` David Miller
2013-07-10 4:38 ` David Miller
2013-07-15 1:43 ` Rusty Russell
2013-07-15 1:43 ` Rusty Russell
2013-07-16 19:33 ` David Miller
2013-07-16 19:33 ` David Miller
2013-07-17 0:08 ` Rusty Russell
2013-07-17 0:08 ` Rusty Russell
2013-07-17 5:00 ` Michael S. Tsirkin
2013-07-17 5:00 ` Michael S. Tsirkin
2013-07-17 5:05 ` David Miller
2013-07-17 5:05 ` David Miller
2013-07-17 6:02 ` Rusty Russell
2013-07-17 6:02 ` Rusty Russell
2013-07-24 19:44 ` Michael S. Tsirkin
2013-07-24 19:44 ` Michael S. Tsirkin
2013-07-10 6:14 ` Michael S. Tsirkin
2013-07-10 6:14 ` Michael S. Tsirkin
2013-07-15 1:40 ` Rusty Russell
2013-07-15 1:40 ` Rusty Russell
2013-07-11 13:00 ` Michael S. Tsirkin
2013-07-11 13:00 ` Michael S. Tsirkin
2013-07-12 5:57 ` Rusty Russell
2013-07-12 5:57 ` Rusty Russell
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=20130609071127.GC13082@redhat.com \
--to=mst@redhat.com \
--cc=amwang@redhat.com \
--cc=davej@redhat.com \
--cc=davem@davemloft.net \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=qemu-devel@nongnu.org \
--cc=rusty@rustcorp.com.au \
--cc=virtualization@lists.linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.