netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: kvm@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	virtualization@lists.linux-foundation.org,
	Sasha Levin <levinsasha928@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	avi@redhat.com, Thomas Lendacky <tahm@linux.vnet.ibm.com>
Subject: Re: [PATCH 0/3] virtio-net: inline header support
Date: Thu, 11 Oct 2012 13:04:31 +0200	[thread overview]
Message-ID: <20121011110430.GF5552@redhat.com> (raw)
In-Reply-To: <87k3ux98oc.fsf@rustcorp.com.au>

On Thu, Oct 11, 2012 at 10:33:31AM +1030, Rusty Russell wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> > Il 09/10/2012 06:59, Rusty Russell ha scritto:
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >>> Il 05/10/2012 07:43, Rusty Russell ha scritto:
> >>>> That's good.  But virtio_blk's scsi command is insoluble AFAICT.  As I
> >>>> said to Anthony, the best rules are "always" and "never", so I'd really
> >>>> rather not have to grandfather that in.
> >>>
> >>> It is, but we can add a rule that if the (transport) flag
> >>> VIRTIO_RING_F_ANY_HEADER_SG is set, the cdb field is always 32 bytes in
> >>> virtio-blk.
> >> 
> >> Could we do that?  It's the cmd length I'm concerned about; is it always
> >> 32 in practice for some reason?
> >
> > It is always 32 or less except in very obscure cases that are pretty
> > much confined to iSCSI.  We don't care about the obscure cases, and the
> > extra bytes don't hurt.
> >
> > BTW, 32 is the default cdb_size used by virtio-scsi.
> >
> >> Currently qemu does:
> >> 
> >>     struct sg_io_hdr hdr;
> >>     memset(&hdr, 0, sizeof(struct sg_io_hdr));
> >>     hdr.interface_id = 'S';
> >>     hdr.cmd_len = req->elem.out_sg[1].iov_len;
> >>     hdr.cmdp = req->elem.out_sg[1].iov_base;
> >>     hdr.dxfer_len = 0;
> >> 
> >> If it's a command which expects more output data, there's no way to
> >> guess where the boundary is between that command and the data.
> >
> > Yep, so I understood the problem right.
> 
> OK.  Well, Anthony wants qemu to be robust in this regard, so I am
> tempted to rework all the qemu drivers to handle arbitrary layouts.
> They could use a good audit anyway.

I agree here. Still trying to understand whether we can agree to use
a feature bit for this, or not.

> This would become a glaring exception, but I'm tempted to fix it to 32
> bytes at the same time as we get the new pci layout (ie. for the virtio
> 1.0 spec).

But this isn't a virtio-pci only issue, is it?
qemu has s390 bus with same limmitation.
How can we tie it to pci layout?

Maybe what you mean is to use a transport feature for this
and tie *that* to new layout in case of pci?



> The Linux driver would carefully be backwards compatible, of
> course, and the spec would document why.
> 
> Cheers,
> Rusty.

  reply	other threads:[~2012-10-11 11:04 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-28  9:26 [PATCH 0/3] virtio-net: inline header support Michael S. Tsirkin
2012-09-28  9:26 ` [PATCH 1/3] virtio: add API to query ring capacity Michael S. Tsirkin
2012-09-28  9:26 ` [PATCH 2/3] virtio-net: correct capacity math on ring full Michael S. Tsirkin
2012-10-04  0:24   ` Rusty Russell
2012-09-28  9:26 ` [PATCH 3/3] virtio-net: put virtio net header inline with data Michael S. Tsirkin
2012-10-03  6:44 ` [PATCH 0/3] virtio-net: inline header support Rusty Russell
2012-10-03  7:10   ` Rusty Russell
2012-10-04  1:24   ` Anthony Liguori
2012-10-04  3:34     ` Rusty Russell
2012-10-04  4:29       ` Anthony Liguori
2012-10-04  7:44         ` Rusty Russell
2012-10-05  7:47           ` Paolo Bonzini
2012-10-08 21:31       ` Michael S. Tsirkin
2012-10-04  1:35   ` Anthony Liguori
2012-10-04  5:17     ` Rusty Russell
2012-10-08 20:41   ` Michael S. Tsirkin
     [not found] ` <87vces2gxq.fsf__45058.6618776017$1349247807$gmane$org@rustcorp.com.au>
2012-10-03 10:53   ` Paolo Bonzini
2012-10-04  0:11     ` Rusty Russell
2012-10-04  7:09       ` Paolo Bonzini
2012-10-04 12:51         ` Rusty Russell
2012-10-04 13:23           ` Paolo Bonzini
2012-10-05  5:43             ` Rusty Russell
     [not found]             ` <87391t1nkq.fsf__40391.6521034718$1349505001$gmane$org@rustcorp.com.au>
2012-10-06 12:54               ` Paolo Bonzini
2012-10-09  4:59                 ` Rusty Russell
2012-10-09  7:27                   ` Paolo Bonzini
2012-10-11  0:03                     ` Rusty Russell
2012-10-11 11:04                       ` Michael S. Tsirkin [this message]
2012-10-11 22:37                         ` Rusty Russell
2012-10-12  7:38                           ` Paolo Bonzini
2012-10-12 11:52                           ` Cornelia Huck

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=20121011110430.GF5552@redhat.com \
    --to=mst@redhat.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rusty@rustcorp.com.au \
    --cc=tahm@linux.vnet.ibm.com \
    --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 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).