All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Heng Qi <hengqi@linux.alibaba.com>
Cc: virtio-dev@lists.oasis-open.org,
	virtio-comment@lists.oasis-open.org,
	Jason Wang <jasowang@redhat.com>, Parav Pandit <parav@nvidia.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
Date: Tue, 23 May 2023 09:30:28 -0400	[thread overview]
Message-ID: <20230523092237-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20230523091820.GC23504@h68b04307.sqa.eu95>

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-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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/


WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Heng Qi <hengqi@linux.alibaba.com>
Cc: virtio-dev@lists.oasis-open.org,
	virtio-comment@lists.oasis-open.org,
	Jason Wang <jasowang@redhat.com>, Parav Pandit <parav@nvidia.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>
Subject: Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net
Date: Tue, 23 May 2023 09:30:28 -0400	[thread overview]
Message-ID: <20230523092237-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20230523091820.GC23504@h68b04307.sqa.eu95>

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-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


  reply	other threads:[~2023-05-23 13:30 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-22 12:12 [virtio-comment] [Proposal] Relationship between XDP and rx-csum in virtio-net Heng Qi
2023-05-22 12:12 ` [virtio-dev] " Heng Qi
2023-05-22 19:10 ` [virtio-comment] " Michael S. Tsirkin
2023-05-22 19:10   ` [virtio-dev] " Michael S. Tsirkin
2023-05-23  2:41   ` [virtio-comment] " Heng Qi
2023-05-23  2:41     ` [virtio-dev] " Heng Qi
2023-05-23  7:15     ` [virtio-comment] " Michael S. Tsirkin
2023-05-23  7:15       ` [virtio-dev] " Michael S. Tsirkin
2023-05-23  9:18       ` [virtio-comment] " Heng Qi
2023-05-23  9:18         ` Heng Qi
2023-05-23 13:30         ` Michael S. Tsirkin [this message]
2023-05-23 13:30           ` Michael S. Tsirkin
2023-05-23 13:51           ` [virtio-comment] " Heng Qi
2023-05-23 13:51             ` [virtio-dev] " Heng Qi
2023-05-24  4:09             ` Jason Wang
2023-05-24  4:09               ` [virtio-dev] " Jason Wang
2023-05-24  5:07               ` [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-nety Heng Qi
2023-05-24  5:07                 ` [virtio-dev] " Heng Qi
2023-05-24  5:52                 ` Jason Wang
2023-05-24  5:52                   ` [virtio-dev] " Jason Wang
2023-05-24  7:24                   ` [virtio-comment] " Heng Qi
2023-05-24  7:24                     ` Heng Qi
2023-05-26  7:58                     ` [virtio-comment] " Heng Qi
2023-05-26  7:58                       ` [virtio-dev] " Heng Qi
2023-05-24  5:17               ` [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [Proposal] Relationship between XDP and rx-csum in virtio-net Heng Qi
2023-05-24  5:17                 ` Heng Qi
2023-05-24  6:07             ` Michael S. Tsirkin
2023-05-24  6:07               ` [virtio-dev] " Michael S. Tsirkin
2023-05-24  8:12               ` [virtio-comment] " Heng Qi
2023-05-24  8:12                 ` Heng Qi
2023-05-30 19:33                 ` [virtio-comment] " Michael S. Tsirkin
2023-05-30 19:33                   ` [virtio-dev] " Michael S. Tsirkin
2023-05-31  6:57                   ` Heng Qi
2023-05-31  6:57                     ` [virtio-dev] " Heng Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230523092237-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=hengqi@linux.alibaba.com \
    --cc=jasowang@redhat.com \
    --cc=parav@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.