Re: VIRTIO_RING_F_INDIRECT_SIZE status

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Feb 28, 2023 at 12:49:00PM +0100, Christian Schoenebeck wrote:
> On Monday, February 27, 2023 6:41:12 PM CET Michael S. Tsirkin wrote:
> > On Mon, Feb 27, 2023 at 05:13:12PM +0100, Christian Schoenebeck wrote:
> > > On Monday, February 27, 2023 4:45:45 PM CET Stefan Hajnoczi wrote:
> > > > On Mon, Feb 27, 2023 at 02:53:55PM +0000, Afsa, Baptiste wrote:
> > > > > The issue that we have now, is that this limitation does not seem to be enforced
> > > > > in Linux virtio drivers today. I came across:
> > > > > 
> > > > > https://github.com/oasis-tcs/virtio-spec/issues/122
> > > > > 
> > > > > which looks like a good base for us to build upon, but I'm not sure what is the
> > > > > status with this issue. Do you know whether there is any plan regarding this?
> > > > 
> > > > CCing Christian regarding extending queue size limits.
> > > 
> > > Status on this issue: it was suspended in May last year. AFAICR Michael
> > > expressed the need to give some more thought about it, and a new virtio spec
> > > release was just ahead at that time.
> > > 
> > > Michael, suggestions how to bring this forward?
> > > 
> > > Best regards,
> > > Christian Schoenebeck
> > > 
> > 
> > How about a summary letter listing various options, pros and cons?
> 
> Actually I had submitted a draft for this feature (latest version linked
> above), and AFAICR Michael was the only person who expressed concerns from
> design perspective. Comments by other people were already just aboout precise
> wording, but not about the design itself.
> 
> We stopped the discussion at the point where Michael expressed the need to
> think more about it, but as his expressed concerns were a bit vague, I still
> don't see how I could bring this issue forward.
> 
> Best regards,
> Christian Schoenebeck
> 


So the last time there were two things:


1. bugs introduced during the packed ring work. For example:

	> 
	> I don't think so:
	> 
	>   2.6.5.3.1 Driver Requirements: Indirect Descriptors
	>   ..
	>   "A driver MUST NOT set both VIRTQ_DESC_F_INDIRECT and VIRTQ_DESC_F_NEXT in
	>   flags."
	> So as far as I can see it, the amount of direct descriptors is currently
	> always exactly one if an indirect table is used.
	> 
	> Best regards,
	> Christian Schoenebeck
	> 

	Oh. You are right.  Weirdly this text is not in packed-ring.tex - I
	think this and a bunch of other cases like this are an oversight,
	however we need to fix them first before adding features that assume
	they are fixed ...

we need to get them fixed before we poke at core ring otherwise it's a
mess - maybe the TC will accept just fixing this
without a feature bit, or maybe people will feel a feature bit
is required.



2. Given this is a lot of work I am trying to find a way to
make the impact bigger. In particular to cover the use-case
of limiting s/g to 1k while making queues deeper (with
or without indirect). For this I proposed:

	So I think that given this, we can limit the total number
	of non-indirect descriptors, including non-indirect ones
	in a chain + all the ones in indirect pointer table if any,
	and excluding the indirect descriptor itself, and this
	will address the issue you are describing here, right?

people seemed to be ok with this idea?



-- 
MST