From: Stefan Hajnoczi <stefanha@redhat.com> To: "Alex Bennée" <alex.bennee@linaro.org> Cc: virtio-dev@lists.oasis-open.org, "Michael S. Tsirkin" <mst@redhat.com>, viresh.kumar@linaro.org, qemu-devel@nongnu.org, rust-vmm@lists.opendev.org, Jiang Liu <gerry@linux.alibaba.com>, stratos-dev@op-lists.linaro.org Subject: Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation Date: Mon, 1 Mar 2021 16:35:51 +0000 [thread overview] Message-ID: <YD0X58hj+al5uPWk@stefanha-x1.localdomain> (raw) In-Reply-To: <87czwjjdf7.fsf@linaro.org> [-- Attachment #1: Type: text/plain, Size: 4817 bytes --] On Mon, Mar 01, 2021 at 11:38:47AM +0000, Alex Bennée wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote: > >> +However as the protocol negotiation something that only occurs between > > > > Missing "is". Shortening the sentence fixes that without losing clarity: > > s/something that/negotiation/ > > > >> +parts of the backend implementation it is permissible to for the master > > > > "vhost-user device backend" is often used to refer to the slave (to > > avoid saying the word "slave") but "backend" is being used in a > > different sense here. That is confusing. > > > >> +to mask the feature bit from the guest. > > > > I think this sentence effectively says "the master MAY mask the > > VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That > > is not really accurate since VIRTIO devices do not advertise this > > feature bit and so it can never be negotiated through the VIRTIO feature > > negotiation process. > > > > How about referring to the details from the VIRTIO 1.1 specification > > instead. Something like this: > > > > Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature > > bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits > > <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_. > > VIRTIO devices do not advertise this feature bit and therefore VIRTIO > > drivers cannot negotiate it. > > > > This reserved feature bit was reused by the vhost-user protocol to add > > vhost-user protocol feature negotiation in a backwards compatible > > fashion. Old vhost-user master and slave implementations continue to > > work even though they are not aware of vhost-user protocol feature > > negotiation. > > OK - so does that mean that feature bit will remain UNUSED for ever > more? It's unlikely to be repurposed in VIRTIO. It can never be used by VIRTIO in a situation that overlaps with vhost-user. That leaves cases that don't overlap with vhost-user but that is unlikely too since the bit had a previous meaning (before vhost-user) and repurposing it would cause confusion for very old drivers or devices. > What about other feature bits? Is it permissible for the > master/requester/vhost-user front-end/QEMU to filter any other feature > bits the slave/vhost-user backend/daemon may offer from being read by > the guest driver when it reads the feature bits? Yes, the vhost-user frontend can decide how it wants to expose VHOST_USER_GET_FEATURES feature bits on the VIRTIO device: 1. Pass-through. Allow the vhost-user device backend to control the feature bit. 2. Disabling. Clear a feature bit because it cannot be supported for some reason (e.g. VIRTIO 1.1 packed vrings are not implemented and therefore enabling them would prevent live migration). 3. Enabling. Enable a feature bit that does not rely on vhost-user device backend support. For example, message-signalled interrupts for virtio-mmio. > > > > >> As noted for the > >> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and > >> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a > >> +final ``VHOST_USER_SET_FEATURES`` comes from the guest. > > > > I couldn't find any place where vhost-user.rst states that > > VHOST_USER_SET_PROTOCOL_FEATURES has to come before > > VHOST_USER_SET_FEATURES? > > > > The only order I found was: > > > > 1. VHOST_USER_GET_FEATURES to determine whether protocol features are > > supported. > > 2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits. > > 3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits. > > 4. Using functionality that depends on enabled protocol feature bits. > > > > Is the purpose of this sentence to add a new requirement to the spec > > that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before > > VHOST_USER_SET_FEATURES"? > > No I don't want to add a new sequence requirement. But if SET_FEATURES > doesn't acknowledge the VHOST_USER_F_PROTOCOL_FEATURES bit should that > stop the processing of > VHOST_USER_GET_PROTOCOL_FEATURES/VHOST_USER_SET_PROTOCOL_FEATURES > messages? AFAICT SET_FEATURES should be irrelevant to the negotiation of > the PROTOCOL_FEATURES right? I agree, the value of VHOST_USER_F_PROTOCOL_FEATURES in VHOST_USER_SET_FEATURES does not matter according to the spec: Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in ``VHOST_USER_GET_FEATURES``. Since it does not mention "set in VHOST_USER_SET_FEATURES" we have to assume existing vhost-user device backends do not care whether the vhost-user frontend includes the bit in VHOST_USER_SET_FEATURES or not. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Stefan Hajnoczi <stefanha@redhat.com> To: "Alex Bennée" <alex.bennee@linaro.org> Cc: qemu-devel@nongnu.org, rust-vmm@lists.opendev.org, stratos-dev@op-lists.linaro.org, virtio-dev@lists.oasis-open.org, viresh.kumar@linaro.org, Jiang Liu <gerry@linux.alibaba.com>, "Michael S. Tsirkin" <mst@redhat.com> Subject: Re: [virtio-dev] [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation Date: Mon, 1 Mar 2021 16:35:51 +0000 [thread overview] Message-ID: <YD0X58hj+al5uPWk@stefanha-x1.localdomain> (raw) In-Reply-To: <87czwjjdf7.fsf@linaro.org> [-- Attachment #1: Type: text/plain, Size: 4817 bytes --] On Mon, Mar 01, 2021 at 11:38:47AM +0000, Alex Bennée wrote: > Stefan Hajnoczi <stefanha@redhat.com> writes: > > On Fri, Feb 26, 2021 at 11:16:19AM +0000, Alex Bennée wrote: > >> +However as the protocol negotiation something that only occurs between > > > > Missing "is". Shortening the sentence fixes that without losing clarity: > > s/something that/negotiation/ > > > >> +parts of the backend implementation it is permissible to for the master > > > > "vhost-user device backend" is often used to refer to the slave (to > > avoid saying the word "slave") but "backend" is being used in a > > different sense here. That is confusing. > > > >> +to mask the feature bit from the guest. > > > > I think this sentence effectively says "the master MAY mask the > > VHOST_USER_F_PROTOCOL_FEATURES bit from the VIRTIO feature bits". That > > is not really accurate since VIRTIO devices do not advertise this > > feature bit and so it can never be negotiated through the VIRTIO feature > > negotiation process. > > > > How about referring to the details from the VIRTIO 1.1 specification > > instead. Something like this: > > > > Note that VHOST_USER_F_PROTOCOL_FEATURES is the UNUSED (30) feature > > bit defined in `VIRTIO 1.1 6.3 Legacy Interface: Reserved Feature Bits > > <https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virtio-v1.1-cs01.html#x1-4130003>`_. > > VIRTIO devices do not advertise this feature bit and therefore VIRTIO > > drivers cannot negotiate it. > > > > This reserved feature bit was reused by the vhost-user protocol to add > > vhost-user protocol feature negotiation in a backwards compatible > > fashion. Old vhost-user master and slave implementations continue to > > work even though they are not aware of vhost-user protocol feature > > negotiation. > > OK - so does that mean that feature bit will remain UNUSED for ever > more? It's unlikely to be repurposed in VIRTIO. It can never be used by VIRTIO in a situation that overlaps with vhost-user. That leaves cases that don't overlap with vhost-user but that is unlikely too since the bit had a previous meaning (before vhost-user) and repurposing it would cause confusion for very old drivers or devices. > What about other feature bits? Is it permissible for the > master/requester/vhost-user front-end/QEMU to filter any other feature > bits the slave/vhost-user backend/daemon may offer from being read by > the guest driver when it reads the feature bits? Yes, the vhost-user frontend can decide how it wants to expose VHOST_USER_GET_FEATURES feature bits on the VIRTIO device: 1. Pass-through. Allow the vhost-user device backend to control the feature bit. 2. Disabling. Clear a feature bit because it cannot be supported for some reason (e.g. VIRTIO 1.1 packed vrings are not implemented and therefore enabling them would prevent live migration). 3. Enabling. Enable a feature bit that does not rely on vhost-user device backend support. For example, message-signalled interrupts for virtio-mmio. > > > > >> As noted for the > >> +``VHOST_USER_GET_PROTOCOL_FEATURES`` and > >> +``VHOST_USER_SET_PROTOCOL_FEATURES`` messages this occurs before a > >> +final ``VHOST_USER_SET_FEATURES`` comes from the guest. > > > > I couldn't find any place where vhost-user.rst states that > > VHOST_USER_SET_PROTOCOL_FEATURES has to come before > > VHOST_USER_SET_FEATURES? > > > > The only order I found was: > > > > 1. VHOST_USER_GET_FEATURES to determine whether protocol features are > > supported. > > 2. VHOST_USER_GET_PROTOCOL_FEATURES to fetch available protocol feature bits. > > 3. VHOST_USER_SET_PROTOCOL_FEATURES to set protocol feature bits. > > 4. Using functionality that depends on enabled protocol feature bits. > > > > Is the purpose of this sentence to add a new requirement to the spec > > that "VHOST_USER_SET_PROTOCOL_FEATURES MUST be sent before > > VHOST_USER_SET_FEATURES"? > > No I don't want to add a new sequence requirement. But if SET_FEATURES > doesn't acknowledge the VHOST_USER_F_PROTOCOL_FEATURES bit should that > stop the processing of > VHOST_USER_GET_PROTOCOL_FEATURES/VHOST_USER_SET_PROTOCOL_FEATURES > messages? AFAICT SET_FEATURES should be irrelevant to the negotiation of > the PROTOCOL_FEATURES right? I agree, the value of VHOST_USER_F_PROTOCOL_FEATURES in VHOST_USER_SET_FEATURES does not matter according to the spec: Only legal if feature bit ``VHOST_USER_F_PROTOCOL_FEATURES`` is present in ``VHOST_USER_GET_FEATURES``. Since it does not mention "set in VHOST_USER_SET_FEATURES" we have to assume existing vhost-user device backends do not care whether the vhost-user frontend includes the bit in VHOST_USER_SET_FEATURES or not. Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-03-01 16:37 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-02-26 11:16 [VHOST USER SPEC PATCH] vhost-user.rst: add clarifying language about protocol negotiation Alex Bennée 2021-02-26 11:16 ` [virtio-dev] " Alex Bennée 2021-02-26 11:21 ` no-reply 2021-03-01 11:05 ` [virtio-dev] " Stefan Hajnoczi 2021-03-01 11:05 ` Stefan Hajnoczi 2021-03-01 11:38 ` Alex Bennée 2021-03-01 11:38 ` Alex Bennée 2021-03-01 16:35 ` Stefan Hajnoczi [this message] 2021-03-01 16:35 ` Stefan Hajnoczi 2021-03-01 17:18 ` Michael S. Tsirkin 2021-03-01 17:18 ` Michael S. Tsirkin
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=YD0X58hj+al5uPWk@stefanha-x1.localdomain \ --to=stefanha@redhat.com \ --cc=alex.bennee@linaro.org \ --cc=gerry@linux.alibaba.com \ --cc=mst@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=rust-vmm@lists.opendev.org \ --cc=stratos-dev@op-lists.linaro.org \ --cc=viresh.kumar@linaro.org \ --cc=virtio-dev@lists.oasis-open.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: linkBe 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.