Re: [virtio-comment] [PATCH v4 1/1] virtio-net: Define per-packet hash reporting feature

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

On Thu, Feb 20, 2020 at 04:11:51PM +0800, Jason Wang wrote:
> 
> On 2020/2/20 下午3:41, Yuri Benditovich wrote:
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> >     *From: *"Jason Wang" <jasowang@redhat.com>
> >     *To: *"Michael S. Tsirkin" <mst@redhat.com>
> >     *Cc: *"Yuri Benditovich" <yuri.benditovich@daynix.com>,
> >     virtio-comment@lists.oasis-open.org
> >     *Sent: *Thursday, February 20, 2020 8:38:50 AM
> >     *Subject: *Re: [virtio-comment] [PATCH v4 1/1] virtio-net: Define
> >     per-packet hash reporting feature
> > 
> > 
> >     On 2020/2/19 下午10:23, Michael S. Tsirkin wrote:
> >     > On Wed, Feb 19, 2020 at 08:27:10PM +0800, Jason Wang wrote:
> >     >> On 2020/2/19 下午3:53, Yuri Benditovich wrote:
> >     >>>    The device MUST set
> >     \field{rss_max_indirection_table_length} to at least 128, if it offers
> >     >>>    VIRTIO_NET_F_RSS.
> >     >>> @@ -3195,6 +3180,8 @@ \subsection{Device
> >     Operation}\label{sec:Device Types / Network Device / Device O
> >     >>>            le16 csum_start;
> >     >>>            le16 csum_offset;
> >     >>>            le16 num_buffers;
> >     >>> +        le32 hash_value; (Only if VIRTIO_NET_F_HASH_REPORT
> >     negotiated)
> >     >>> +        le16 hash_type;  (Only if VIRTIO_NET_F_HASH_REPORT
> >     negotiated)
> >     >>>    };
> >     >>>    \end{lstlisting}
> >     >> A question here:
> >     >>
> >     >> Consider we introduce VIRTIO_NET_F_FEATURE_INFORMATION in the
> >     future:
> >     >>
> >     >> le32 hash_type; // VIRTIO_NET_F_HASH_REPORT
> >     >> le32 feature_information; // VIRTIO_NET_F_FEATURE_INFORMATION
> >     >>
> >     >> What happens if HASH_REPORT is not negotiated, I believe we
> >     expect a stable
> >     >> ABI(offset) here for feature_information?
> >     >>
> >     >> Thanks
> >     >>
> >     > We'll have to decide at that point ... any better ideas?
> > 
> > 
> >     Not sure but something that is self descriptive? (which could be an
> >     overkill for fields that only need few bytes).
> > 
> > The problem is that the driver typically wants to know the header size
> > from the beginning to configure SG table.
> 
> 
> Yes, so still the above example. Consider a driver only support
> VIRTIO_NET_F_FEATURE_INFORMATION, how to determine the size of the vnet
> header?
> 
> Stable offset is simpler but may waste space.

So we have:
                dev->needed_headroom = vi->hdr_len;

simply put we are going to waste space on the max header size anyway.
Dynamically sizing the header for each packet is pointless,
won't save memory.

> 
> > So any self-descriptive layout seems good but in practice not so usable,
> > IMO.
> 
> 
> I think then it can calculate the header length based on the feature
> negotiated.
> 
> hdr_len = sizeof(vnet_hdr) + VIRTIO_NET_F_HASH_REPORT ? 4 : 0 +
> VIRTIO_NET_F_FEAUTER_INFORMATION ? 4 : 0
> 
> Thanks


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://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/