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

[Heng Qi <hengqi@linux.alibaba.com>]

在 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.

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/
>>


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/