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 02:41:03AM -0500, 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.
> So any self-descriptive layout seems good but in practice not so usable, IMO.

We should also look at packing things more tightly down the road.




> 
> 
>     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/
> 
> 
>