[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v7] virtio-net: Add support for the flexible driver notification structure.
I can fix the patch according to Cornelia's and Michael's comments, or redefine the feature altogether (as I've mentioned below).
What do you think?
Does the new definition make more sense?
Thanks,
Vitaly
>-----Original Message-----
>From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-open.org> On Behalf Of Vitaly
>Mireyno
>Sent: Thursday, 26 March, 2020 14:19
>To: Cornelia Huck <cohuck@redhat.com>; Michael S. Tsirkin <mst@redhat.com>
>Cc: virtio-comment@lists.oasis-open.org; Jason Wang <jasowang@redhat.com>; Ariel Elior
><aelior@marvell.com>
>Subject: [EXT] [virtio-comment] Re: [PATCH v7] virtio-net: Add support for the flexible driver
>notification structure.
>
>External Email
>
>----------------------------------------------------------------------
>I can suggest changing the feature definition such that the value, that the driver sets in the vqn field,
>will be per-virtqueue (instead of per-device).
>The device can provide a per-virtqueue value to the driver in the PCI Common configuration structure
>(virtio_pci_common_cfg).
>This will probably make the feature more generic. It will also simplify the feature, as it will not depend
>on notify_off_multiplier anymore.
>
>Would it make more sense?
>
>
>>-----Original Message-----
>>From: Cornelia Huck <cohuck@redhat.com>
>>Sent: Thursday, 26 March, 2020 13:53
>>To: Michael S. Tsirkin <mst@redhat.com>
>>Cc: Vitaly Mireyno <vmireyno@marvell.com>;
>>virtio-comment@lists.oasis-open.org; Jason Wang <jasowang@redhat.com>;
>>Ariel Elior <aelior@marvell.com>
>>Subject: [EXT] Re: [PATCH v7] virtio-net: Add support for the flexible driver notification structure.
>>
>>External Email
>>
>>----------------------------------------------------------------------
>>On Thu, 26 Mar 2020 07:26:16 -0400
>>"Michael S. Tsirkin" <mst@redhat.com> wrote:
>>
>>> On Thu, Mar 26, 2020 at 10:43:20AM +0100, Cornelia Huck wrote:
>>> > On Wed, 18 Mar 2020 18:29:19 +0000
>>> > Vitaly Mireyno <vmireyno@marvell.com> wrote:
>>
>>> > > +
>>> > > +\begin{note}
>>> > > +If \field{notify_off_multiplier} > 0, the virtqueue number can
>>> > > +potentially be derived by the device from the Queue Notify
>>> > > +address, so \field{vqn} may be redundant.
>>>
>>> I feel this is confusing the reader more than it explains.
>>> the field is always there. It's either the virtqueue number or
>>> whatever we get from the device.
>>
>>"the virtqueue number can potentially be derived by the device from the
>>Queue Notify address, so obtaining the virtqueue number from \field{vqn} is not needed"
>>
>>?
>>
>>>
>>> > > +\end{note}
>>> > > +
>>(...)
>>> > > @@ -4180,6 +4205,33 @@ \subsubsection{Legacy Interface: Framing
>>> > > Requirements}\label{sec:Device See \ref{sec:Basic Facilities of
>>> > > a Virtio Device / Virtqueues / Message Framing}.
>>> > >
>>> > > +\subsubsection{Driver Notifications}\label{sec:Device Types /
>>> > > +Network Device / Device Operation / Driver Notifications} As
>>> > > +mentioned in section \ref{sec:Virtqueues / Driver
>>> > > +notifications}, when the driver is required to send an available
>>> > > +buffer notification to the device, it always sends the virtqueue
>>> > > +number to be notified. The method of delivering notifications is
>>> > > +transport specific. With the PCI transport, the device can
>>> > > +optionally provide a distinct per-virtqueue address for driver
>>> > > +notifications (Queue Notify address). In this case, the
>>> > > +virtqueue number can potentially be derived by the device from
>>> > > +the Queue Notify address. For devices,
>>that provide per-virtqueue Queue Notify addresses and are able to
>>derive the virtqueue number from that address, the virtqueue number in the notification data is
>redundant.
>>> >
>>> > I would keep the remark that the queue number might be redundant
>>> > with the PCI transport.
>>>
>>> I feel redundant is too strong a word. If anything sending data we
>>> just read from device back to it is definitely redundant.
>>> Spec says send it so we send it.
>>> Maybe say it can be ignored by the device?
>>
>>Maybe "not needed"?
>>
>>(see above)
>>
>>>
>>> > > +On the other hand, some devices can benefit from receiving
>>> > > +additional data with driver notifications. An example could be a
>>> > > +hardware device implementing multiple protocols (with virtio
>>> > > +being one of them), so the extra notification data could serve
>>> > > +as a notification type indication or
>>a protocol indication.
>>> >
>>> > Hm... what about replacing the whole text above with
>>> >
>>> > "Driver notifications (\ref{sec:Virtqueues / Driver notifications})
>>> > always contain at least the virtqueue number for which buffers are
>>> > available. Some devices can benefit from receiving additional data
>>> > with driver notifications. For example, a hardware device
>>> > implementing multiple protocols (with virtio being one of them)
>>> > could benefit from a notification type indication or a protocol indication."
>>>
>>>
>>> My problem with both versions is that it talks about "additional
>>> information" while in fact it's information that comes from the
>>> device itself.
>>
>>"per-device information", maybe?
>
>
>This publicly archived list offers a means to provide input to the OASIS Virtual I/O Device (VIRTIO) TC.
>
>In order to verify user consent to the Feedback License terms and to minimize spam in the list archive,
>subscription is required before posting.
>
>Subscribe: virtio-comment-subscribe@lists.oasis-open.org
>Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
>List help: virtio-comment-help@lists.oasis-open.org
>List archive: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-
>2Dopen.org_archives_virtio-
>2Dcomment_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=MD86W4SuL1LKe3rClaL0wwbDeAMforqLdcHFG42nHKg&s=NiLuajtoil7R--
>84VFef7ASGYAffizU84DMvw7_6ujg&e=
>Feedback License: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_who_ipr_feedback-
>5Flicense.pdf&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=MD86W4SuL1LKe3rClaL0wwbDeAMforqLdcHFG42nHKg&s=4wiTwfb7zEeWnG0yCAja-
>VdZOJkEsFUdbSXjy468z5U&e=
>List Guidelines: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_policies-2Dguidelines_mailing-
>2Dlists&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7
>xgI&m=MD86W4SuL1LKe3rClaL0wwbDeAMforqLdcHFG42nHKg&s=Ot4-
>d18NHluOvcC44go3ADsiXSLJpl_fQerACfadEWw&e=
>Committee: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_committees_virtio_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>Rdcevq01tbLQAw4A_NO7xgI&m=MD86W4SuL1LKe3rClaL0wwbDeAMforqLdcHFG42nHKg&s=Ff6uQ9iA-
>ntoanvVz2XIv6ybDxlM0Qfhj2_RhUrhWWU&e=
>Join OASIS: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_join_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQ
>Aw4A_NO7xgI&m=MD86W4SuL1LKe3rClaL0wwbDeAMforqLdcHFG42nHKg&s=WI26Krgtm4TiaX1ov9fU
>naZd4rcqApzJz-CmuZJ81SM&e=
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]