[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 5:06âPM Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>
>
> å 2023/12/4 äå4:49, Jason Wang åé:
> > 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.
>
> OK, I'm not inclined to update the description for classic features
> either, if that's okay with others.
>
> > 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.
>
> Packets which are not validated by the device can still be received by
> the driver. This is basic
> and this feature does not break this.
It doesn't break. It doesn't exclude that.
See how GUEST_CSUM defined:
Driver handles packets with partial checksum.
Thanks
>
> Thanks!
>
> >
> > 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]