[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] Re: [PATCH v4 1/2] virtio-net: update description for VIRTIO_NET_F_GUEST_CSUM.
On Mon, Dec 4, 2023 at 3:37âPM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> å 2023/12/4 äå3:18, Jason Wang åé:
> > 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.
>
> Yes, you are right, so I used "device *can*" here. Which packet types
> the device recognizes or validates
> depends on the device's implementation. This is also the current
> practice of GUEST_CSUM.
>
> >
> >> and the driver or stack does not need to
> >> validate these packets.
> > Ok, so I think there're something that is subtle here,
>
> Ok, I see.
>
> > 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.
>
> Maybe the description below would be less subtle?
> +\item[VIRTIO_NET_F_GUEST_CSUM (1)] Driver handles packets with partial
> checksum or full checksum.
I'd suggest to leave it as is. As I didn't find any issue since even
with DATA_VALID. Did you?
> +\item[VIRTIO_NET_F_GUEST_FULL_CSUM (64)] The driver handles packets
> with full checksum,
> and the device optionally validates the packet's checksum.
Or maybe something like (not a native speaker)
The driver handles packets with full checksum which the device has
already validated.
Thanks
>
> Thanks!
>
> >
> > 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/
> >>
>
>
> 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/
>
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]