netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Schoenebeck <linux_oss@crudebyte.com>
To: Dominique Martinet <asmadeus@codewreck.org>,
	Eric Van Hensbergen <ericvh@gmail.com>
Cc: Greg Kurz <groug@kaod.org>, Latchesar Ionkov <lucho@ionkov.net>,
	Nikolay Kichukov <nikolay@oldum.net>,
	netdev@vger.kernel.org, v9fs-developer@lists.sourceforge.net
Subject: Re: [PATCH v4 00/12] remove msize limit in virtio transport
Date: Fri, 08 Jul 2022 15:00:45 +0200	[thread overview]
Message-ID: <7969175.Y4Flz6HuuJ@silver> (raw)
In-Reply-To: <YsgXtBsfLEQ9dFux@codewreck.org>

On Freitag, 8. Juli 2022 13:40:36 CEST Dominique Martinet wrote:
> Christian Schoenebeck wrote on Fri, Jul 08, 2022 at 01:18:40PM +0200:
> > On Freitag, 8. Juli 2022 04:26:40 CEST Eric Van Hensbergen wrote:
[...]
> https://github.com/kvmtool/kvmtool indeed has a 9p server, I think I
> used to run it ages ago.
> I'll give it a fresh spin, thanks for the reminder.
> 
> For this one it defines VIRTQUEUE_NUM to 128, so not quite 1024.

Yes, and it does *not* limit client supplied 'msize' either. It just always 
sends the same 'msize' value as-is back to client. :/ So I would expect it to 
error (or worse) if client tries msize > 512kB.

> > > > I found https://github.com/moby/hyperkit for OSX but that doesn't
> > > > really
> > > > help me, and can't see much else relevant in a quick search
> > 
> > So that appears to be a 9p (@virtio-PCI) client for xhyve,
> 
> oh the 9p part is client code?
> the readme says it's a server:
> "It includes a complete hypervisor, based on xhyve/bhyve"
> but I can't run it anyway, so I didn't check very hard.

Hmm, I actually just interpreted this for it to be a client:

fprintf(stderr, "virtio-9p: unexpected EOF writing to server-- did the 9P 
server crash?\n");

But looking at it again, it seems you are right, at least I see that it also 
handles even 9p message type numbers, but only Twrite and Tflush? I don't see 
any Tversion or msize handling in general. [shrug]

> > with max. 256kB buffers <=> max. 68 virtio descriptors (memory segments) 
[1]:
> huh...
> 
> Well, as long as msize is set I assume it'll work out anyway?

If server would limit 'msize' appropriately, yes. But as the kvmtool example 
shows, that should probably not taken for granted.

> How does virtio queue size work with e.g. parallel messages?

Simple question, complicated to answer.

From virtio spec PoV (and current virtio <= v1.2), the negotiated virtio queue 
size defines both the max. amount of parallel (round-trip) messages *and* the 
max. amount of virtio descriptors (memory segments) of *all* currently active/
parallel messages in total. I "think" because of virtio's origin for 
virtualized network devices?

So yes, if you are very strict what the virtio spec <= v1.2 says, and say you 
have a virtio queue size of 128 (e.g. hard coded by QEMU, kvmtool), and say 
client would send out the first 9p request with 128 memory segments, then the 
next (i.e. second) parallel 9p request sent to server would already exceed the 
theoretically allowed max. amount of virtio descriptors.

But in practice, I don't see that this theoretical limitation would exist in 
actual 9p virtio server implementations. At least in all server 
implementations I saw so far, they all seem to handle the max. virtio 
descriptors amount for each request separately.

> Anyway, even if the negotiation part gets done servers won't all get
> implemented in a day, so we need to think of other servers a bit..

OTOH kernel should have the name of the hypervisor/emulator somewhere, right? 
So Linux client's max. virtio descriptors could probably made dependent on 
that name?

Best regards,
Christian Schoenebeck



      reply	other threads:[~2022-07-08 13:00 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-30 13:23 [PATCH v4 00/12] remove msize limit in virtio transport Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 12/12] net/9p: allocate appropriate reduced message buffers Christian Schoenebeck
2022-04-02 14:05   ` Dominique Martinet
2022-04-03 11:29     ` Christian Schoenebeck
2022-04-03 12:37       ` Dominique Martinet
2022-04-03 14:00         ` Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 08/12] net/9p: limit 'msize' to KMALLOC_MAX_SIZE for all transports Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 04/12] 9p/trans_virtio: introduce struct virtqueue_sg Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 01/12] net/9p: show error message if user 'msize' cannot be satisfied Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 10/12] 9p: add P9_ERRMAX for 9p2000 and 9p2000.u Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 11/12] net/9p: add p9_msg_buf_size() Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 06/12] 9p/trans_virtio: support larger msize values Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 02/12] 9p/trans_virtio: separate allocation of scatter gather list Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 09/12] net/9p: split message size argument into 't_size' and 'r_size' pair Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 07/12] 9p/trans_virtio: resize sg lists to whatever is possible Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 05/12] net/9p: add trans_maxsize to struct p9_client Christian Schoenebeck
2021-12-30 13:23 ` [PATCH v4 03/12] 9p/trans_virtio: turn amount of sg lists into runtime info Christian Schoenebeck
2022-01-20 22:43 ` [PATCH v4 00/12] remove msize limit in virtio transport Nikolay Kichukov
2022-01-22 13:34   ` Christian Schoenebeck
2022-01-24 10:21     ` Nikolay Kichukov
2022-01-24 11:07       ` Dominique Martinet
2022-01-24 11:57         ` Christian Schoenebeck
2022-01-24 12:56           ` Dominique Martinet
2022-01-24 13:55             ` Christian Schoenebeck
2022-01-25  8:45           ` Nikolay Kichukov
2022-05-24  8:10         ` Nikolay Kichukov
2022-05-24 11:29           ` Christian Schoenebeck
2022-07-07 14:30 ` Christian Schoenebeck
     [not found]   ` <CAFkjPT=GAoViYd0E7CZQDq3ZjhmYT0DsBytfZXnE10JL0P8O-Q@mail.gmail.com>
2022-07-08  1:15     ` Dominique Martinet
     [not found]       ` <CAFkjPTngeFh=0mPVW-Yf1Sxkxp_HDNUeANndoYN3-eU9_rGLuQ@mail.gmail.com>
2022-07-08 11:18         ` Christian Schoenebeck
2022-07-08 11:40           ` Dominique Martinet
2022-07-08 13:00             ` Christian Schoenebeck [this message]

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=7969175.Y4Flz6HuuJ@silver \
    --to=linux_oss@crudebyte.com \
    --cc=asmadeus@codewreck.org \
    --cc=ericvh@gmail.com \
    --cc=groug@kaod.org \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@oldum.net \
    --cc=v9fs-developer@lists.sourceforge.net \
    /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).