[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: RE: [PATCH v2 1/2] virtio-net: Fix receive buffer size calculation text
Hi Cornelia, > From: Parav Pandit <parav@nvidia.com> > Sent: Thursday, February 1, 2024 11:27 AM > > Hi Cornelia, > > > From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis- > > open.org> On Behalf Of Parav Pandit > > Sent: Friday, January 19, 2024 5:56 PM > > > > Hi Cornelia, > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > Sent: Tuesday, January 16, 2024 6:59 PM > > > > > > On Tue, Jan 16, 2024 at 01:18:59PM +0000, Parav Pandit wrote: > > > > > > > > > From: Cornelia Huck <cohuck@redhat.com> > > > > > Sent: Tuesday, January 16, 2024 6:08 PM > > > > > To: Parav Pandit <parav@nvidia.com>; > > > > > virtio-comment@lists.oasis-open.org; > > > > > mst@redhat.com > > > > > Cc: Shahaf Shuler <shahafs@nvidia.com>; > > > > > xuanzhuo@linux.alibaba.com; yuri.benditovich@daynix.com > > > > > Subject: RE: [PATCH v2 1/2] virtio-net: Fix receive buffer size > > > > > calculation text > > > > > > > > > > On Tue, Jan 16 2024, Parav Pandit <parav@nvidia.com> wrote: > > > > > > > > > > >> From: Cornelia Huck <cohuck@redhat.com> > > > > > >> Sent: Tuesday, January 16, 2024 4:33 PM > > > > > >> To: Parav Pandit <parav@nvidia.com>; > > > > > >> virtio-comment@lists.oasis-open.org; > > > > > >> mst@redhat.com > > > > > >> Cc: Shahaf Shuler <shahafs@nvidia.com>; > > > > > >> xuanzhuo@linux.alibaba.com; yuri.benditovich@daynix.com > > > > > >> Subject: RE: [PATCH v2 1/2] virtio-net: Fix receive buffer > > > > > >> size calculation text > > > > > >> > > > > > >> On Tue, Jan 16 2024, Parav Pandit <parav@nvidia.com> wrote: > > > > > >> > > > > > >> >> From: Cornelia Huck <cohuck@redhat.com> > > > > > >> >> Sent: Monday, January 15, 2024 10:14 PM > > > > > >> > > > > > > >> >> On Mon, Jan 15 2024, Parav Pandit <parav@nvidia.com> wrote: > > > > > >> >> > +The driver MUST consider size of field \field{struct > > > > > >> >> > +virtio_net_hdr} > > > > > >> >> > +20 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated, and > > > > > >> >> > +12 bytes if > > > > > >> >> not. > > > > > >> >> > + > > > > > >> >> > > > > > >> >> Requiring the driver to consider the size of something to > > > > > >> >> be its actual size seems a bit odd :) I don't think we > > > > > >> >> need this, as the length can be derived from looking at > > > > > >> >> the definitions, and is already spelled out explicitly, if > > > > > >> >> you consider my suggestion > > > above. > > > > > >> > We need this because tx side also needs to refer to the > > > > > >> > virtio_net_hdr in > > > > > >> patch 2 to be same as that of the rx side. > > > > > >> > And hence, this normative sets base line for tx side too. > > > > > >> > Relying on rest of the > > > > > >> receive packet normative is not enough. > > > > > >> > > > > > >> Hm, why? If struct virtio_net_hdr is well-defined, its size > > > > > >> is well-defined as well, and we do not need to state it explictly? > > > > > > Because, > > > > > > the size of virtio_net_hdr is derived from the rx side features. > > > > > > Today there is no normative line that says that even though > > > > > > you are using A, > > > > > B, C Rx features, due to which your tx side virtio_net_hdr also changes. > > > > > > The 2nd patch in this series adds this explicit normative as > > > > > > explained in the > > > > > cover letter. > > > > > > > > > > Let's step back a bit. > > > > > > > > > > struct virtio_net_hdr is defined at the beginning of the "Device > > Operation" > > > > > section; the definition clearly says that the last three fields > > > > > depend on VIRTIO_NET_F_HASH_REPORT being negotiated. The device > > > and > > > > > the driver agree on whether HASH_REPORT is negotiated, and > > > > > therefore should also agree on the size of virtio_net_hdr? > > > > > > > > > Do you imply that device operation description is enough to not > > > > add > > > normative? > > > > If so, for this case and possibly new things if we write as device > > > > operation, > > > would it be enough? > > > > > > > > > Or is the problem that we did not state explicitly that the last > > > > > three fields of virtio_net_hdr do not exist without HASH_REPORT > > > > > (and are not merely invalid)? If yes, we should spell this out, > > > > > instead of adding normative statements about what the size of > > > > > virtio_net_hdr > > > should be considered to be. > > > > This suggested normative is added in this patch. > > > > > > > > > If virtio_net_hdr has a fixed size, we shouldn't need the second > > > > > patch, > > > either. > > > > The fact that HASH_REPORT is only for the rx, if we have to go > > > > back in time, > > > there is no need for the tx to force also to follow the rx virtio_net_hdr. > > > > There is no explicit normative indicating the virtio_net_hdr for > > > > the TX is > > > forced by the RX even though it has no relation to hash report. > > > > > > > > If you say device operation is enough, than I am sort of lost of > > > > when > > > normative is needed, and when device operation is enough. > > > > > > > > > Generally we start adding normatives when we see that something is > > unclear. > > > But I think generally I agree with Parav, if someone has the time to > > > write the normative, it's all good. > > > > Are you ok with the v3? > > I kept the clarification for tx and addressed rest of your and Xuan's > > comments in it. > > https://lists.oasis-open.org/archives/virtio-comment/202401/msg00070.h > > tml > > > > Its been nearly two weeks now. > I assume v3 is ok. > Can you please create the voting ballot? > I updated the github issue to refer to the v3 as well. > https://github.com/oasis-tcs/virtio-spec/issues/183 > https://lists.oasis-open.org/archives/virtio- > comment/202401/msg00070.html This thread [2] is waiting your response from 19th January 2023. Can you please either respond or open v3 [1] for vote? [1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00070.html [2] https://lore.kernel.org/virtio-comment/PH0PR12MB5481F74BF29A751E6BC249D4DC702@PH0PR12MB5481.namprd12.prod.outlook.com/
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]