[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH v4 1/1] virtio-net: Define per-packet hash reporting feature
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
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]