[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 30, 2023 at 03:33:22PM -0400, Michael S. Tsirkin wrote:
> On Wed, May 24, 2023 at 04:12:46PM +0800, Heng Qi wrote:
> > On Wed, May 24, 2023 at 02:07:14AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, May 23, 2023 at 09:51:44PM +0800, Heng Qi wrote:
> > > > On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > > > > > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > > > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin
> > > > > > > > wrote:
> > > > > > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > > > > > 1) Add a feature bit to the virtio specification to tell
> > > > > > > > > > the sender that a fully
> > > > > > > > > > csumed packet must be sent.
> > > > > > > > >
> > > > > > > > > Who is the sender in this picture? The driver?
> > > > > > > >
> > > > > > > > The device or the driver.
> > > > > > > >
> > > > > > > > When the device is hw, the sender is more likely to be a device.
> > > > > > > > When the device is sw, the sender can be a device or a driver.
> > > > > > > >
> > > > > > > > But in general, this feature is inclined to constrain the
> > > > > > > > behavior of the device and
> > > > > > > > the driver from the receiving side.
> > > > > > >
> > > > > > > Based on above I am guessing you are talking about driver getting
> > > > > > > packets from device, I wish you used terms from virtio spec.
> > > > > >
> > > > > > Yes, I'm going to use the terminology of the virtio spec.
> > > > > >
> > > > > > >
> > > > > > > > For example:
> > > > > > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device
> > > > > > > > that you must send me a fully csumed packet.
> > > > > > > >
> > > > > > > > Then the specific implementation can be
> > > > > > > >
> > > > > > > > (1) the sender sends a fully csumed packet;
> > > > > > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the
> > > > > > > > device helps calculate the fully csum
> > > > > > > > (because the two parties in the communication are located
> > > > > > > > on the same host, the packet is trusted.).
> > > > > > > >
> > > > > > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the
> > > > > > > > driver will no longer receive any packets marked
> > > > > > > > CHECKSUM_PARTIAL.
> > > > > > > >
> > > > > > > > Thanks.
> > > > > > >
> > > > > > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> > > > > >
> > > > > > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device
> > > > > > can
> > > > > > receive a fully checksummed packet, we can no longer enjoy
> > > > > > the device's ability to validate the packet checksum. That is, the
> > > > > > value
> > > > > > of \field{flags} in the virtio_net_hdr structure is set to 0, which
> > > > > > means
> > > > > > that the packet received by the driver will not be marked as
> > > > > > VIRTIO_NET_HDR_F_DATA_VALID.
> > > > > >
> > > > > > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > > > > > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must
> > > > > > give the
> > > > > > driver a fully checksummed packet, and the packet is validated by
> > > > > > the
> > > > > > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> > > > > >
> > > > > > >
> > > > > > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > > > > > disables all offloads but you want to keep some of them?
> > > > > > >
> > > > > >
> > > > > > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is
> > > > > > needed
> > > > > > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are
> > > > > > negotiated,
> > > > > > then the driver may always receive packets marked as
> > > > > > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP
> > > > > > at the
> > > > > > same time.
> > > > >
> > > > > Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> > > > > VIRTIO_NET_HDR_F_DATA_VALID:
> > > >
> > > > We need to focus on what happens when the XDP program is loaded:
> > > >
> > > > The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
> > > > feature when loading XDP. The main reason for doing this is because
> > > > VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
> > > > VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
> > > > XDP programs, because we cannot guarantee that the csum_{start, offset}
> > > > fields are correct after XDP modifies the packets.
> > >
> > > What do you want the device to do with these packets?
> > > Just clear VIRTIO_NET_HDR_F_NEEDS_CSUM?
> > > Then ... why don't you have the driver clear that bit?
> >
> > Just clearing VIRTIO_NET_HDR_F_NEEDS_CSUM will cause the stack to drop
> > packets when validating the checksum.
> >
> >
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Wed, May 24, 2023 at 04:12:46PM +0800, Heng Qi wrote:
> On Wed, May 24, 2023 at 02:07:14AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 23, 2023 at 09:51:44PM +0800, Heng Qi wrote:
> > > On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > > > > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > > > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin
> > > > > > > wrote:
> > > > > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > > > > 1) Add a feature bit to the virtio specification to tell the
> > > > > > > > > sender that a fully
> > > > > > > > > csumed packet must be sent.
> > > > > > > >
> > > > > > > > Who is the sender in this picture? The driver?
> > > > > > >
> > > > > > > The device or the driver.
> > > > > > >
> > > > > > > When the device is hw, the sender is more likely to be a device.
> > > > > > > When the device is sw, the sender can be a device or a driver.
> > > > > > >
> > > > > > > But in general, this feature is inclined to constrain the
> > > > > > > behavior of the device and
> > > > > > > the driver from the receiving side.
> > > > > >
> > > > > > Based on above I am guessing you are talking about driver getting
> > > > > > packets from device, I wish you used terms from virtio spec.
> > > > >
> > > > > Yes, I'm going to use the terminology of the virtio spec.
> > > > >
> > > > > >
> > > > > > > For example:
> > > > > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that
> > > > > > > you must send me a fully csumed packet.
> > > > > > >
> > > > > > > Then the specific implementation can be
> > > > > > >
> > > > > > > (1) the sender sends a fully csumed packet;
> > > > > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the
> > > > > > > device helps calculate the fully csum
> > > > > > > (because the two parties in the communication are located on
> > > > > > > the same host, the packet is trusted.).
> > > > > > >
> > > > > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the
> > > > > > > driver will no longer receive any packets marked CHECKSUM_PARTIAL.
> > > > > > >
> > > > > > > Thanks.
> > > > > >
> > > > > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> > > > >
> > > > > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> > > > > receive a fully checksummed packet, we can no longer enjoy
> > > > > the device's ability to validate the packet checksum. That is, the
> > > > > value
> > > > > of \field{flags} in the virtio_net_hdr structure is set to 0, which
> > > > > means
> > > > > that the packet received by the driver will not be marked as
> > > > > VIRTIO_NET_HDR_F_DATA_VALID.
> > > > >
> > > > > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > > > > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give
> > > > > the
> > > > > driver a fully checksummed packet, and the packet is validated by the
> > > > > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> > > > >
> > > > > >
> > > > > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > > > > disables all offloads but you want to keep some of them?
> > > > > >
> > > > >
> > > > > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is
> > > > > needed
> > > > > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are
> > > > > negotiated,
> > > > > then the driver may always receive packets marked as
> > > > > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at
> > > > > the
> > > > > same time.
> > > >
> > > > Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> > > > VIRTIO_NET_HDR_F_DATA_VALID:
> > >
> > > We need to focus on what happens when the XDP program is loaded:
> > >
> > > The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
> > > feature when loading XDP. The main reason for doing this is because
> > > VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
> > > VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
> > > XDP programs, because we cannot guarantee that the csum_{start, offset}
> > > fields are correct after XDP modifies the packets.
> >
> > What do you want the device to do with these packets?
> > Just clear VIRTIO_NET_HDR_F_NEEDS_CSUM?
> > Then ... why don't you have the driver clear that bit?
>
> Just clearing VIRTIO_NET_HDR_F_NEEDS_CSUM will cause the stack to drop
> packets when validating the checksum.
>
> >From [1] we know that after negotiating VIRTIO_NET_F_GUEST_CSUM, the
> device can choose to save some work, thus generating packets carrying
> VIRTIO_NET_HDR_F_NEEDS_CSUM. The new feature just keeping the device from
> saving
> the work it's supposed to be doing. Of course, this will bring some
> device
Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Wed, May 24, 2023 at 02:07:14AM -0400, Michael S. Tsirkin wrote:
> On Tue, May 23, 2023 at 09:51:44PM +0800, Heng Qi wrote:
> > On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > > > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > > > 1) Add a feature bit to the virtio specification to tell the
> > > > > > > > sender that a fully
> > > > > > > > csumed packet must be sent.
> > > > > > >
> > > > > > > Who is the sender in this picture? The driver?
> > > > > >
> > > > > > The device or the driver.
> > > > > >
> > > > > > When the device is hw, the sender is more likely to be a device.
> > > > > > When the device is sw, the sender can be a device or a driver.
> > > > > >
> > > > > > But in general, this feature is inclined to constrain the behavior
> > > > > > of the device and
> > > > > > the driver from the receiving side.
> > > > >
> > > > > Based on above I am guessing you are talking about driver getting
> > > > > packets from device, I wish you used terms from virtio spec.
> > > >
> > > > Yes, I'm going to use the terminology of the virtio spec.
> > > >
> > > > >
> > > > > > For example:
> > > > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that
> > > > > > you must send me a fully csumed packet.
> > > > > >
> > > > > > Then the specific implementation can be
> > > > > >
> > > > > > (1) the sender sends a fully csumed packet;
> > > > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device
> > > > > > helps calculate the fully csum
> > > > > > (because the two parties in the communication are located on
> > > > > > the same host, the packet is trusted.).
> > > > > >
> > > > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the
> > > > > > driver will no longer receive any packets marked CHECKSUM_PARTIAL.
> > > > > >
> > > > > > Thanks.
> > > > >
> > > > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> > > >
> > > > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> > > > receive a fully checksummed packet, we can no longer enjoy
> > > > the device's ability to validate the packet checksum. That is, the value
> > > > of \field{flags} in the virtio_net_hdr structure is set to 0, which
> > > > means
> > > > that the packet received by the driver will not be marked as
> > > > VIRTIO_NET_HDR_F_DATA_VALID.
> > > >
> > > > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > > > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
> > > > driver a fully checksummed packet, and the packet is validated by the
> > > > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> > > >
> > > > >
> > > > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > > > disables all offloads but you want to keep some of them?
> > > > >
> > > >
> > > > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is
> > > > needed
> > > > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
> > > > then the driver may always receive packets marked as
> > > > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
> > > > same time.
> > >
> > > Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> > > VIRTIO_NET_HDR_F_DATA_VALID:
> >
> > We need to focus on what happens when the XDP program is loaded:
> >
> > The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
> > feature when loading XDP. The main reason for doing this is because
> > VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
> > VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
> > XDP programs, because we cannot guarantee that the csum_{start, offset}
> > fields are correct after XDP modifies the packets.
>
> What do you want the device to do with these packets?
> Just clear VIRTIO_NET_HDR_F_NEEDS_CSUM?
> Then ... why don't you have the driver clear that bit?
Just clearing VIRTIO_NET_HDR_F_NEEDS_CSUM will cause the stack to drop
packets when validating the checksum.
>From [1] we know that after negotiating VIRTIO_NET_F_GUEST_CSUM, the
device can choose to save some work, thus generating packets carrying
VIRTIO_NET_HDR_F_NEEDS_CSUM. The new feature just keeping the device from saving
the work it's supposed to be doing. Of course, this will bring some
device calculating overhead, but if the device has sufficient calculating
resources, it can offer the new feature.
[1] “The converse features are also available: a driver can save the
virtual device some work by negotiating these features. Note: For
example, a network packet transported between two guests on the same
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 09:51:44PM +0800, Heng Qi wrote:
> On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > > 1) Add a feature bit to the virtio specification to tell the
> > > > > > > sender that a fully
> > > > > > > csumed packet must be sent.
> > > > > >
> > > > > > Who is the sender in this picture? The driver?
> > > > >
> > > > > The device or the driver.
> > > > >
> > > > > When the device is hw, the sender is more likely to be a device.
> > > > > When the device is sw, the sender can be a device or a driver.
> > > > >
> > > > > But in general, this feature is inclined to constrain the behavior of
> > > > > the device and
> > > > > the driver from the receiving side.
> > > >
> > > > Based on above I am guessing you are talking about driver getting
> > > > packets from device, I wish you used terms from virtio spec.
> > >
> > > Yes, I'm going to use the terminology of the virtio spec.
> > >
> > > >
> > > > > For example:
> > > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you
> > > > > must send me a fully csumed packet.
> > > > >
> > > > > Then the specific implementation can be
> > > > >
> > > > > (1) the sender sends a fully csumed packet;
> > > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device
> > > > > helps calculate the fully csum
> > > > > (because the two parties in the communication are located on the
> > > > > same host, the packet is trusted.).
> > > > >
> > > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the
> > > > > driver will no longer receive any packets marked CHECKSUM_PARTIAL.
> > > > >
> > > > > Thanks.
> > > >
> > > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> > >
> > > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> > > receive a fully checksummed packet, we can no longer enjoy
> > > the device's ability to validate the packet checksum. That is, the value
> > > of \field{flags} in the virtio_net_hdr structure is set to 0, which means
> > > that the packet received by the driver will not be marked as
> > > VIRTIO_NET_HDR_F_DATA_VALID.
> > >
> > > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
> > > driver a fully checksummed packet, and the packet is validated by the
> > > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> > >
> > > >
> > > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > > disables all offloads but you want to keep some of them?
> > > >
> > >
> > > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is needed
> > > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
> > > then the driver may always receive packets marked as
> > > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
> > > same time.
> >
> > Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> > VIRTIO_NET_HDR_F_DATA_VALID:
>
> We need to focus on what happens when the XDP program is loaded:
>
> The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
> feature when loading XDP. The main reason for doing this is because
> VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
> VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
> XDP programs, because we cannot guarantee that the csum_{start, offset}
> fields are correct after XDP modifies the packets.
What do you want the device to do with these packets?
Just clear VIRTIO_NET_HDR_F_NEEDS_CSUM?
Then ... why don't you have the driver clear that bit?
> So in order for the driver to continue to enjoy the device's ability to
> validate packets while XDP is loaded, we need a new feature to tell the
> device not to deliver packets marked as VIRTIO_NET_HDR_F_NEEDS_CSUM (in
> other words, the device can only deliver packets marked as
> VIRTIO_NET_HDR_F_DATA_VALID).
>
> Thanks.
> > \item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
> > VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can be
> > set: if so, the packet checksum at offset \field{csum_offset}
> > from \field{csum_start} and any preceding checksums
> > have been validated. The checksum on the packet is incomplete and
> > if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
> > then \field{csum_start} and \field{csum_offset} indicate how to calculate
> > it
> > (see Packet Transmission point 1).
> >
> > Did you maybe mean if either feature is negotiated?
> >
> >
> >
> > > > Again please use virtio terminology
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 9:51 PM Heng Qi wrote:
>
> On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > > 1) Add a feature bit to the virtio specification to tell the
> > > > > > > sender that a fully
> > > > > > > csumed packet must be sent.
> > > > > >
> > > > > > Who is the sender in this picture? The driver?
> > > > >
> > > > > The device or the driver.
> > > > >
> > > > > When the device is hw, the sender is more likely to be a device.
> > > > > When the device is sw, the sender can be a device or a driver.
> > > > >
> > > > > But in general, this feature is inclined to constrain the behavior of
> > > > > the device and
> > > > > the driver from the receiving side.
> > > >
> > > > Based on above I am guessing you are talking about driver getting
> > > > packets from device, I wish you used terms from virtio spec.
> > >
> > > Yes, I'm going to use the terminology of the virtio spec.
> > >
> > > >
> > > > > For example:
> > > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you
> > > > > must send me a fully csumed packet.
> > > > >
> > > > > Then the specific implementation can be
> > > > >
> > > > > (1) the sender sends a fully csumed packet;
> > > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device
> > > > > helps calculate the fully csum
> > > > > (because the two parties in the communication are located on the
> > > > > same host, the packet is trusted.).
> > > > >
> > > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the
> > > > > driver will no longer receive any packets marked CHECKSUM_PARTIAL.
> > > > >
> > > > > Thanks.
> > > >
> > > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> > >
> > > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> > > receive a fully checksummed packet, we can no longer enjoy
> > > the device's ability to validate the packet checksum. That is, the value
> > > of \field{flags} in the virtio_net_hdr structure is set to 0, which means
> > > that the packet received by the driver will not be marked as
> > > VIRTIO_NET_HDR_F_DATA_VALID.
> > >
> > > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
> > > driver a fully checksummed packet, and the packet is validated by the
> > > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> > >
> > > >
> > > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > > disables all offloads but you want to keep some of them?
> > > >
> > >
> > > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is needed
> > > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
> > > then the driver may always receive packets marked as
> > > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
> > > same time.
> >
> > Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> > VIRTIO_NET_HDR_F_DATA_VALID:
>
> We need to focus on what happens when the XDP program is loaded:
>
> The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
> feature when loading XDP. The main reason for doing this is because
> VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
> VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
> XDP programs, because we cannot guarantee that the csum_{start, offset}
> fields are correct after XDP modifies the packets.
We can try to fix or workaround this. A rough idea is for example by
using a flow dissector? Anyhow the GSO of virtio-net was marked as
dodgy which means the csum_start/offset needs to be validated again in
the xmit path. Or we can monitor if the packet is modified or not, if
not, we don't need to zero vnet header?
Thanks
>
> So in order for the driver to continue to enjoy the device's ability to
> validate packets while XDP is loaded, we need a new feature to tell the
> device not to deliver packets marked as VIRTIO_NET_HDR_F_NEEDS_CSUM (in
> other words, the device can only deliver packets marked as
> VIRTIO_NET_HDR_F_DATA_VALID).
>
> Thanks.
>
> > \item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
> > VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can be
> > set: if so, the packet checksum at offset \field{csum_offset}
> > from \field{csum_start} and any preceding checksums
> > have been validated. The checksum on the packet is incomplete and
> > if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
> > then \field{csum_start} and \field{csum_offset} indicate how to calculate
> > it
> > (see
[virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 09:30:28AM -0400, Michael S. Tsirkin wrote:
> On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> > On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > > 1) Add a feature bit to the virtio specification to tell the sender
> > > > > > that a fully
> > > > > > csumed packet must be sent.
> > > > >
> > > > > Who is the sender in this picture? The driver?
> > > >
> > > > The device or the driver.
> > > >
> > > > When the device is hw, the sender is more likely to be a device.
> > > > When the device is sw, the sender can be a device or a driver.
> > > >
> > > > But in general, this feature is inclined to constrain the behavior of
> > > > the device and
> > > > the driver from the receiving side.
> > >
> > > Based on above I am guessing you are talking about driver getting
> > > packets from device, I wish you used terms from virtio spec.
> >
> > Yes, I'm going to use the terminology of the virtio spec.
> >
> > >
> > > > For example:
> > > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you
> > > > must send me a fully csumed packet.
> > > >
> > > > Then the specific implementation can be
> > > >
> > > > (1) the sender sends a fully csumed packet;
> > > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device
> > > > helps calculate the fully csum
> > > > (because the two parties in the communication are located on the
> > > > same host, the packet is trusted.).
> > > >
> > > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the driver
> > > > will no longer receive any packets marked CHECKSUM_PARTIAL.
> > > >
> > > > Thanks.
> > >
> > > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
> >
> > Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> > receive a fully checksummed packet, we can no longer enjoy
> > the device's ability to validate the packet checksum. That is, the value
> > of \field{flags} in the virtio_net_hdr structure is set to 0, which means
> > that the packet received by the driver will not be marked as
> > VIRTIO_NET_HDR_F_DATA_VALID.
> >
> > So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> > If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
> > driver a fully checksummed packet, and the packet is validated by the
> > device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
> >
> > >
> > > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > > disables all offloads but you want to keep some of them?
> > >
> >
> > No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is needed
> > in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
> > then the driver may always receive packets marked as
> > VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
> > same time.
>
> Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
> VIRTIO_NET_HDR_F_DATA_VALID:
We need to focus on what happens when the XDP program is loaded:
The current virtio-net needs to turn off the VIRTIO_NET_F_GUEST_CSUM
feature when loading XDP. The main reason for doing this is because
VIRTIO_NET_F_GUEST_CSUM allows to receive packets marked as
VIRTIO_NET_HDR_F_NEEDS_CSUM. Such packets are not compatible with
XDP programs, because we cannot guarantee that the csum_{start, offset}
fields are correct after XDP modifies the packets.
So in order for the driver to continue to enjoy the device's ability to
validate packets while XDP is loaded, we need a new feature to tell the
device not to deliver packets marked as VIRTIO_NET_HDR_F_NEEDS_CSUM (in
other words, the device can only deliver packets marked as
VIRTIO_NET_HDR_F_DATA_VALID).
Thanks.
> \item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
> VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can be
> set: if so, the packet checksum at offset \field{csum_offset}
> from \field{csum_start} and any preceding checksums
> have been validated. The checksum on the packet is incomplete and
> if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
> then \field{csum_start} and \field{csum_offset} indicate how to calculate it
> (see Packet Transmission point 1).
>
> Did you maybe mean if either feature is negotiated?
>
>
>
> > > Again please use virtio terminology not Linux. to help you out,
> > > in current linux, VIRTIO_NET_HDR_F_NEEDS_CSUM and
> > > VIRTIO_NET_HDR_F_DATA_VALID
> > > will set CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY respectively.
> > >
> >
> > Sure. Will do as you suggested.
> >
> > Thanks.
> >
> > >
> > > --
> > > MST
> > >
> > >
> > > -
> > > To unsubscribe, e-mail:
Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 05:18:20PM +0800, Heng Qi wrote:
> On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> > On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > > 1) Add a feature bit to the virtio specification to tell the sender
> > > > > that a fully
> > > > > csumed packet must be sent.
> > > >
> > > > Who is the sender in this picture? The driver?
> > >
> > > The device or the driver.
> > >
> > > When the device is hw, the sender is more likely to be a device.
> > > When the device is sw, the sender can be a device or a driver.
> > >
> > > But in general, this feature is inclined to constrain the behavior of the
> > > device and
> > > the driver from the receiving side.
> >
> > Based on above I am guessing you are talking about driver getting
> > packets from device, I wish you used terms from virtio spec.
>
> Yes, I'm going to use the terminology of the virtio spec.
>
> >
> > > For example:
> > > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you must
> > > send me a fully csumed packet.
> > >
> > > Then the specific implementation can be
> > >
> > > (1) the sender sends a fully csumed packet;
> > > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device helps
> > > calculate the fully csum
> > > (because the two parties in the communication are located on the same
> > > host, the packet is trusted.).
> > >
> > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the driver
> > > will no longer receive any packets marked CHECKSUM_PARTIAL.
> > >
> > > Thanks.
> >
> > This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
>
> Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
> receive a fully checksummed packet, we can no longer enjoy
> the device's ability to validate the packet checksum. That is, the value
> of \field{flags} in the virtio_net_hdr structure is set to 0, which means
> that the packet received by the driver will not be marked as
> VIRTIO_NET_HDR_F_DATA_VALID.
>
> So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
> If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
> driver a fully checksummed packet, and the packet is validated by the
> device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
>
> >
> > I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> > disables all offloads but you want to keep some of them?
> >
>
> No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is needed
> in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
> then the driver may always receive packets marked as
> VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
> same time.
Makes no sense to me. VIRTIO_NET_F_GUEST_CSUM set already allows
VIRTIO_NET_HDR_F_DATA_VALID:
\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} can be
set: if so, the packet checksum at offset \field{csum_offset}
from \field{csum_start} and any preceding checksums
have been validated. The checksum on the packet is incomplete and
if bit VIRTIO_NET_HDR_F_RSC_INFO is not set in \field{flags},
then \field{csum_start} and \field{csum_offset} indicate how to calculate it
(see Packet Transmission point 1).
Did you maybe mean if either feature is negotiated?
> > Again please use virtio terminology not Linux. to help you out,
> > in current linux, VIRTIO_NET_HDR_F_NEEDS_CSUM and
> > VIRTIO_NET_HDR_F_DATA_VALID
> > will set CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY respectively.
> >
>
> Sure. Will do as you suggested.
>
> Thanks.
>
> >
> > --
> > MST
> >
> >
> > -
> > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 03:15:37AM -0400, Michael S. Tsirkin wrote:
> On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote:
> > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote:
> > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote:
> > > > 1) Add a feature bit to the virtio specification to tell the sender
> > > > that a fully
> > > > csumed packet must be sent.
> > >
> > > Who is the sender in this picture? The driver?
> >
> > The device or the driver.
> >
> > When the device is hw, the sender is more likely to be a device.
> > When the device is sw, the sender can be a device or a driver.
> >
> > But in general, this feature is inclined to constrain the behavior of the
> > device and
> > the driver from the receiving side.
>
> Based on above I am guessing you are talking about driver getting
> packets from device, I wish you used terms from virtio spec.
Yes, I'm going to use the terminology of the virtio spec.
>
> > For example:
> > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you must
> > send me a fully csumed packet.
> >
> > Then the specific implementation can be
> >
> > (1) the sender sends a fully csumed packet;
> > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device helps
> > calculate the fully csum
> > (because the two parties in the communication are located on the same
> > host, the packet is trusted.).
> >
> > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the driver will
> > no longer receive any packets marked CHECKSUM_PARTIAL.
> >
> > Thanks.
>
> This is what clearing VIRTIO_NET_F_GUEST_CSUM does.
Yes, but with VIRTIO_NET_F_GUEST_CSUM cleared, although the device can
receive a fully checksummed packet, we can no longer enjoy
the device's ability to validate the packet checksum. That is, the value
of \field{flags} in the virtio_net_hdr structure is set to 0, which means
that the packet received by the driver will not be marked as
VIRTIO_NET_HDR_F_DATA_VALID.
So, we need a feature bit (let's say VIRTIO_NET_F_UNNECESSARY_CSUM).
If VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the device must give the
driver a fully checksummed packet, and the packet is validated by the
device with \field{flags} set to VIRTIO_NET_HDR_F_DATA_VALID.
>
> I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM
> disables all offloads but you want to keep some of them?
>
No, what I mean is that a feature VIRTIO_NET_F_UNNECESSARY_CSUM is needed
in addition to VIRTIO_NET_F_GUEST_CSUM, if both features are negotiated,
then the driver may always receive packets marked as
VIRTIO_NET_HDR_F_DATA_VALID, which means that we can now load XDP at the
same time.
> Again please use virtio terminology not Linux. to help you out,
> in current linux, VIRTIO_NET_HDR_F_NEEDS_CSUM and VIRTIO_NET_HDR_F_DATA_VALID
> will set CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY respectively.
>
Sure. Will do as you suggested.
Thanks.
>
> --
> MST
>
>
> -
> To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Tue, May 23, 2023 at 10:41:18AM +0800, Heng Qi wrote: > On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote: > > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote: > > > 1) Add a feature bit to the virtio specification to tell the sender that > > > a fully > > > csumed packet must be sent. > > > > Who is the sender in this picture? The driver? > > The device or the driver. > > When the device is hw, the sender is more likely to be a device. > When the device is sw, the sender can be a device or a driver. > > But in general, this feature is inclined to constrain the behavior of the > device and > the driver from the receiving side. Based on above I am guessing you are talking about driver getting packets from device, I wish you used terms from virtio spec. > For example: > VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you must > send me a fully csumed packet. > > Then the specific implementation can be > > (1) the sender sends a fully csumed packet; > (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device helps > calculate the fully csum > (because the two parties in the communication are located on the same > host, the packet is trusted.). > > In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the driver will > no longer receive any packets marked CHECKSUM_PARTIAL. > > Thanks. This is what clearing VIRTIO_NET_F_GUEST_CSUM does. I feel you are trying to say that clearing VIRTIO_NET_F_GUEST_CSUM disables all offloads but you want to keep some of them? Again please use virtio terminology not Linux. to help you out, in current linux, VIRTIO_NET_HDR_F_NEEDS_CSUM and VIRTIO_NET_HDR_F_DATA_VALID will set CHECKSUM_PARTIAL and CHECKSUM_UNNECESSARY respectively. -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Mon, May 22, 2023 at 03:10:05PM -0400, Michael S. Tsirkin wrote: > On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote: > > 1) Add a feature bit to the virtio specification to tell the sender that a > > fully > > csumed packet must be sent. > > Who is the sender in this picture? The driver? The device or the driver. When the device is hw, the sender is more likely to be a device. When the device is sw, the sender can be a device or a driver. But in general, this feature is inclined to constrain the behavior of the device and the driver from the receiving side. For example: VIRTIO_NET_F_UNNECESSARY_CSUM : The driver tells the device that you must send me a fully csumed packet. Then the specific implementation can be (1) the sender sends a fully csumed packet; (2) the receiver receives a CHECKSUM_PARTIAL packet, and the device helps calculate the fully csum (because the two parties in the communication are located on the same host, the packet is trusted.). In summary, if VIRTIO_NET_F_UNNECESSARY_CSUM is negotiated, the driver will no longer receive any packets marked CHECKSUM_PARTIAL. Thanks. > > -- > MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
On Mon, May 22, 2023 at 08:12:00PM +0800, Heng Qi wrote: > 1) Add a feature bit to the virtio specification to tell the sender that a > fully > csumed packet must be sent. Who is the sender in this picture? The driver? -- MST - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
