Re: [virtio-comment] Re: [PATCH v4 1/2] virtio-net: update description for VIRTIO_NET_F_GUEST_CSUM.

[Jason Wang <jasowang@redhat.com>]

On Fri, Dec 1, 2023 at 3:16âPM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> å 2023/12/1 äå3:05, Jason Wang åé:
> > On Fri, Dec 1, 2023 at 2:30âPM Heng Qi <hengqi@linux.alibaba.com> wrote:
> >>
> >>
> >> å 2023/12/1 äå2:24, Heng Qi åé:
> >>>
> >>> å 2023/12/1 äå1:18, Jason Wang åé:
> >>>> On Wed, Nov 29, 2023 at 4:23âPM Heng Qi <hengqi@linux.alibaba.com>
> >>>> wrote:
> >>>>>
> >>>>> å 2023/11/29 äå4:00, Jason Wang åé:
> >>>>>> On Tue, Nov 28, 2023 at 4:08âPM Heng Qi <hengqi@linux.alibaba.com>
> >>>>>> wrote:
> >>>>>>> To prevent readers from misunderstanding that the driver can
> >>>>>>> only handles packets with partial checksum when
> >>>>>>> VIRTIO_NET_F_GUEST_CSUM is negotiated, we update the description.
> >>>>>>>
> >>>>>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> >>>>>>> ---
> >>>>>>>     device-types/net/description.tex | 2 +-
> >>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/device-types/net/description.tex
> >>>>>>> b/device-types/net/description.tex
> >>>>>>> index aff5e08..529f470 100644
> >>>>>>> --- a/device-types/net/description.tex
> >>>>>>> +++ b/device-types/net/description.tex
> >>>>>>> @@ -38,7 +38,7 @@ \subsection{Feature bits}\label{sec:Device Types
> >>>>>>> / Network Device / Feature bits
> >>>>>>>     \begin{description}
> >>>>>>>     \item[VIRTIO_NET_F_CSUM (0)] Device handles packets with
> >>>>>>> partial checksum offload.
> >>>>>>>
> >>>>>>> -\item[VIRTIO_NET_F_GUEST_CSUM (1)] Driver handles packets with
> >>>>>>> partial checksum.
> >>>>>>> +\item[VIRTIO_NET_F_GUEST_CSUM (1)] Driver handles packets with
> >>>>>>> partial checksum or full checksum.
> >>>>>> So patch 2 said
> >>>>>>
> >>>>>> "
> >>>>>> +\item[VIRTIO_NET_F_GUEST_FULL_CSUM (64)] Driver handles packets with
> >>>>>> full checksum.
> >>>>>>     \end{description}
> >>>>>> "
> >>>>>>
> >>>>>> Is there any difference between the two "full checksum" here?
> >>>>> There's no difference.
> >>>>>
> >>>>> The core is that VIRTIO_NET_F_GUEST_FULL_CSUM means that the driver
> >>>>> "can
> >>>>> only" handle packets with full checksum.
> >>>> This seems to be odd.
> >>>>
> >>>> Driver can always handle packet with full checksum, no?
> >>> Yes.
> >>>
> >>>> I meant it
> >>>> will be then to be functional equivalent to !
> >>>> VIRTIO_NET_F_GUEST_FULL_CSUM?
> >>> Are you referring to
> >>> "functional equivalent to !VIRTIO_NET_F_GUEST_CSUM" ?
> >> Sorry, this is a typo. I meant
> >>
> >> Are you referring to
> >> "functional equivalent to !VIRTIO_NET_F_GUEST_FULL_CSUM" ?
> >>
> >>> If so, I think it's no.
> >>>
> >>> Maybe a description similar to the following would be more clearer:
> >>>
> >>> +\item[VIRTIO_NET_F_GUEST_FULL_CSUM (64)] Driver does not handle
> >>> packets with partial checksum.
> > I may miss something here, but what's the difference between
> >
> > VIRTIO_NET_F_GUEST_FULL_CSUM
> >
> > and
> >
> > !VIRTIO_NET_F_GUEST_CSUM?
>
>  From the device perspective:
>
> If !VIRTIO_NET_F_GUEST_CSUM, the device delivers packets with full
> checksum to the driver,
> but the device can not validate the checksum for these packets. That is,
> the flags in virtio-net-hdr
> will not contain _DATA_VALID, and the driver or stack needs to validate
> these packets.
>
> If VIRTIO_NET_F_GUEST_FULL_CSUM, the device delivers packets with full
> checksum to the driver,
> and the device can validate the checksum for these packets. That is, the
> flags in virtio-net-hdr
> will contain _DATA_VALID,

I think DATA_VALID is optional here as device can't recognize all type
of protocols.

> and the driver or stack does not need to
> validate these packets.

Ok, so I think there're something that is subtle here, and that's why
I'm asking here:

1) "Driver does not handle packets with partial checksum" is not
accurate, !GUEST_CUSM also fit for this definition.
2) "Driver handles packets with full checksum" is kind of ambiguous as
it doesn't say whether or not the packet has been validated or not.

Thanks

>
> Thanks!
>
> >
> > Thanks
> >
> >>> Thanks!
> >>>
> >>>> Thanks
> >>>>
> >>>>> Thanks!
> >>>>>
> >>>>>> Thanks
> >>>>>>
> >>>>>>> \item[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS (2)] Control channel offloads
> >>>>>>>             reconfiguration support.
> >>>>>>> --
> >>>>>>> 2.19.1.6.gb485710b
> >>>>>>>
> >>>
> >>> 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/
>
>
> 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/
>