Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-20 Thread Jason Wang
On Wed, Jun 19, 2024 at 11:44 PM Heng Qi  wrote:
>
>
> 在 2024/6/19 下午11:08, Jakub Kicinski 写道:
> > On Wed, 19 Jun 2024 10:02:58 +0800 Heng Qi wrote:
>  Currently we do not allow RXCUSM to be disabled.
> >>> You don't have to disable checksuming in the device.
> >> Yes, it is up to the device itself to decide whether to validate checksum.
> >> What I mean is that we don't allow users to disable the driver's
> >> NETIF_F_RXCSUM flag.
> > I understand. What I'm suggesting is that you send a follow up patch
> > that allows it.

Exactly my point as well.

>
> OK, will do it.
>
> Thanks.

Thanks

>
>




Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-19 Thread Heng Qi



在 2024/6/19 下午11:08, Jakub Kicinski 写道:

On Wed, 19 Jun 2024 10:02:58 +0800 Heng Qi wrote:

Currently we do not allow RXCUSM to be disabled.

You don't have to disable checksuming in the device.

Yes, it is up to the device itself to decide whether to validate checksum.
What I mean is that we don't allow users to disable the driver's
NETIF_F_RXCSUM flag.

I understand. What I'm suggesting is that you send a follow up patch
that allows it.


OK, will do it.

Thanks.





Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-19 Thread Jakub Kicinski
On Wed, 19 Jun 2024 10:02:58 +0800 Heng Qi wrote:
> > > Currently we do not allow RXCUSM to be disabled.  
> > 
> > You don't have to disable checksuming in the device.  
> 
> Yes, it is up to the device itself to decide whether to validate checksum.
> What I mean is that we don't allow users to disable the driver's
> NETIF_F_RXCSUM flag.

I understand. What I'm suggesting is that you send a follow up patch
that allows it.



Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-18 Thread Heng Qi
On Tue, 18 Jun 2024 18:15:16 -0700, Jakub Kicinski  wrote:
> On Tue, 18 Jun 2024 11:09:02 +0800 Heng Qi wrote:
> > > (Should we manually do checksum if RXCUSM is disabled?)
> > >   
> > 
> > Currently we do not allow RXCUSM to be disabled.
> 
> You don't have to disable checksuming in the device.

Yes, it is up to the device itself to decide whether to validate checksum.
What I mean is that we don't allow users to disable the driver's
NETIF_F_RXCSUM flag.

> Just ignore VIRTIO_NET_HDR_F_DATA_VALID if user cleared NETIF_F_RXCSUM.

Right.

Thanks.

> I know some paranoid workloads which do actually want the kernel to
> calculate the checksum.
> 



Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-18 Thread Jakub Kicinski
On Tue, 18 Jun 2024 11:09:02 +0800 Heng Qi wrote:
> > (Should we manually do checksum if RXCUSM is disabled?)
> >   
> 
> Currently we do not allow RXCUSM to be disabled.

You don't have to disable checksuming in the device.
Just ignore VIRTIO_NET_HDR_F_DATA_VALID if user cleared NETIF_F_RXCSUM.
I know some paranoid workloads which do actually want the kernel to
calculate the checksum.



Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-17 Thread Heng Qi
On Tue, 18 Jun 2024 11:01:27 +0800, Jason Wang  wrote:
> On Mon, Jun 17, 2024 at 9:15 PM Heng Qi  wrote:
> >
> > In virtio spec 0.95, VIRTIO_NET_F_GUEST_CSUM was designed to handle
> > partially checksummed packets, and the validation of fully checksummed
> > packets by the device is independent of VIRTIO_NET_F_GUEST_CSUM
> > negotiation. However, the specification erroneously stated:
> >
> >   "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set flags
> >to zero and SHOULD supply a fully checksummed packet to the driver."
> >
> > This statement is inaccurate because even without VIRTIO_NET_F_GUEST_CSUM
> > negotiation, the device can still set the VIRTIO_NET_HDR_F_DATA_VALID flag.
> > Essentially, the device can facilitate the validation of these packets'
> > checksums - a process known as RX checksum offloading - removing the need
> > for the driver to do so.
> >
> > This scenario is currently not implemented in the driver and requires
> > correction. The necessary specification correction[1] has been made and
> > approved in the virtio TC vote.
> > [1] 
> > https://lists.oasis-open.org/archives/virtio-comment/202401/msg00011.html
> >
> > Fixes: 4f49129be6fa ("virtio-net: Set RXCSUM feature if GUEST_CSUM is 
> > available")
> > Signed-off-by: Heng Qi 
> > ---
> 
> Acked-by: Jason Wang 
> 
> (Should we manually do checksum if RXCUSM is disabled?)
> 

Currently we do not allow RXCUSM to be disabled.

Thanks.

> Thanks
> 



Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-17 Thread Jason Wang
On Mon, Jun 17, 2024 at 9:15 PM Heng Qi  wrote:
>
> In virtio spec 0.95, VIRTIO_NET_F_GUEST_CSUM was designed to handle
> partially checksummed packets, and the validation of fully checksummed
> packets by the device is independent of VIRTIO_NET_F_GUEST_CSUM
> negotiation. However, the specification erroneously stated:
>
>   "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set flags
>to zero and SHOULD supply a fully checksummed packet to the driver."
>
> This statement is inaccurate because even without VIRTIO_NET_F_GUEST_CSUM
> negotiation, the device can still set the VIRTIO_NET_HDR_F_DATA_VALID flag.
> Essentially, the device can facilitate the validation of these packets'
> checksums - a process known as RX checksum offloading - removing the need
> for the driver to do so.
>
> This scenario is currently not implemented in the driver and requires
> correction. The necessary specification correction[1] has been made and
> approved in the virtio TC vote.
> [1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00011.html
>
> Fixes: 4f49129be6fa ("virtio-net: Set RXCSUM feature if GUEST_CSUM is 
> available")
> Signed-off-by: Heng Qi 
> ---

Acked-by: Jason Wang 

(Should we manually do checksum if RXCUSM is disabled?)

Thanks




Re: [PATCH 1/2] virtio_net: checksum offloading handling fix

2024-06-17 Thread Jiri Pirko
Mon, Jun 17, 2024 at 03:15:23PM CEST, [email protected] wrote:
>In virtio spec 0.95, VIRTIO_NET_F_GUEST_CSUM was designed to handle
>partially checksummed packets, and the validation of fully checksummed
>packets by the device is independent of VIRTIO_NET_F_GUEST_CSUM
>negotiation. However, the specification erroneously stated:
>
>  "If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set flags
>   to zero and SHOULD supply a fully checksummed packet to the driver."
>
>This statement is inaccurate because even without VIRTIO_NET_F_GUEST_CSUM
>negotiation, the device can still set the VIRTIO_NET_HDR_F_DATA_VALID flag.
>Essentially, the device can facilitate the validation of these packets'
>checksums - a process known as RX checksum offloading - removing the need
>for the driver to do so.
>
>This scenario is currently not implemented in the driver and requires
>correction. The necessary specification correction[1] has been made and
>approved in the virtio TC vote.
>[1] https://lists.oasis-open.org/archives/virtio-comment/202401/msg00011.html
>
>Fixes: 4f49129be6fa ("virtio-net: Set RXCSUM feature if GUEST_CSUM is 
>available")
>Signed-off-by: Heng Qi 

Reviewed-by: Jiri Pirko