Re: [PATCH v3 1/4] Add virtio Admin virtqueue

[Cornelia Huck <cohuck@redhat.com>]

On Tue, Feb 08 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Feb 08, 2022 at 03:59:13PM +0100, Cornelia Huck wrote:
>> On Tue, Feb 08 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> 
>> > On Tue, Feb 08, 2022 at 01:32:12PM +0000, Parav Pandit wrote:
>> >> 
>> >> > From: Cornelia Huck <cohuck@redhat.com>
>> >> > Sent: Tuesday, February 8, 2022 6:50 PM
>> >> > 
>> >> > On Tue, Feb 08 2022, Parav Pandit <parav@nvidia.com> wrote:
>> >> > 
>> >> > >> From: Michael S. Tsirkin <mst@redhat.com>
>> >> > >> Sent: Tuesday, February 8, 2022 12:13 PM
>> >> > >
>> >> > >> On Tue, Feb 08, 2022 at 06:25:41AM +0000, Parav Pandit wrote:
>> >> > >> >
>> >> > >> > > From: Michael S. Tsirkin <mst@redhat.com>
>> >> > >> > > Sent: Monday, February 7, 2022 4:09 PM
>> >> > >> > >
>> >> > >> > > Next, trying to think about scalable iov extensions. So we will
>> >> > >> > > have groups of VFs and then SFs as the next level.
>> >> > >> > > How does one differentiate between the two?
>> >> > >> > > Maybe reserve a field for "destination type"?
>> >> > >> > >
>> >> > >> > We already discussed this in v2.
>> >> > >> > SF will have different identification than 16-bits. And no one
>> >> > >> > knows what
>> >> > >> that would be.
>> >> > >> > We just cannot reserve some arbitrary bytes for unknown.
>> >> > >> > You suggested in v2 to reserve 4 bytes for sf_id, and I explained
>> >> > >> > you that 4
>> >> > >> bytes may not be enough.
>> >> > >> >
>> >> > >> > Whether SFs are on top of VFs or SFs are on top of PFs or both is
>> >> > >> > completely
>> >> > >> different spec.
>> >> > >> > Whether PF will manage SFs of the VFs or it will be done nested
>> >> > >> > manner by
>> >> > >> VF etc is a completely different discussion than what is being proposed here.
>> >> > >> > Whether PF will manage the SF is yet another big question. We work
>> >> > >> > with
>> >> > >> users and they dislike this.
>> >> > >> > To address it, some OS has a different management interface (not
>> >> > >> > visible to
>> >> > >> PF) for SF life cycle even though SFs are anchored on a PF.
>> >> > >> >
>> >> > >> > So SF/iov extension discussion has long way to go for community to
>> >> > >> > first
>> >> > >> understand the use cases before crafting some extension.
>> >> > >> >
>> >> > >> > So lets not complicate and mix things here for a blurry definition
>> >> > >> > of scalable
>> >> > >> iov/sf extension.
>> >> > >>
>> >> > >> Some reserved bytes won't hurt. 2 bytes for type gives us 64k types,
>> >> > >> sounds like that should be enough.
>> >> > > It doesn't stop there.
>> >> > > Mentioning some destination type, interrupt type, etc also requires reserving
>> >> > bytes for different device id type, interrupt type and more.
>> >> > > We past this stage long ago after discussing this in v1 at [1].
>> >> > > It is just better and cleaner to define a different structure to describe SF/iov
>> >> > and its configuration.
>> >> > 
>> >> > I have the feeling that we might be overcomplicating this. We have some
>> >> > groups of targets (a device, a group, that more complicated SF thingy), and we
>> >> > want to distinguish between them. That's easy enough to cover via some kind of
>> >> > enum-equivalent (0 == same dev, 1 == target a dev id, 2 == target a group id, 3
>> >> > == multi-layer target) and some spec how 1 and 2 should look like (as I'd expect
>> >> > them to be common for many different things). 
>> >> Do we have a concrete example of a command that can be targeted for same device and a target device, which requires differentiating their destination? If so, lets discuss and then it make sense to add for the well-defined use case.
>> >
>> > So e.g. things like controlling NIC's MAC can reasonably be part of
>> > the same device.
>> 
>> Yes, that would be an example.
>> 
>> I might have been a bit too vague about what I had been thinking
>> about. Let's do a sketch (intentionally without concrete sizes):
>> 
>> +-------------------------------------------------------+
>> | command                                               |
>> +-------------------------------------------------------+
>> | target type (0 - self, 1 - dev id, 2 - group id, ...  |
>> +-------------------------------------------------------+
>> | dev id                                                |
>> +-------------------------------------------------------+
>> | group id                                              |
>> +-------------------------------------------------------+
>> | command-specific data                                 |
>> +-------------------------------------------------------+
>> | response part                                         |
>> +-------------------------------------------------------+
>> 
>> 'dev id' would be valid for 'target type' == 1, 'group id' would be
>> valid for 'target type' == 2. Alternatively, 'dev id' and 'group id'
>> could be a single 'target id' field; if there's nothing better to use,
>> it can simply contain a uuid.
>
> I am not sure why do we have both dev id and group id.
> They are never used together right?

Right, we can certainly use a single field for both.

> Maybe just have an id length field if we can't agree on
> how much space to reserve.

If we think that 64 bit should be able to accommodate everything, I'd
say just go with that, no need to make it overly complicated.