virtio-comment

 View Only
  • 1.  [PATCH v7 0/2] checksum corrections and additions

    Posted 12-28-2023 07:46
    Patch 1 corrects a historical error for the virtio spec.

    Patch 2 ensures that the device only delivers fully checksummed packets
    to the driver, which is useful for XDP scenarios.

    Heng Qi (2):
    virtio-net: correct conditions for devices to validate packet checksum
    virtio-net: device does not deliver partially checksummed packet and
    may validate the checksum

    device-types/net/description.tex | 76 ++++++++++++++++++++++++++++-----
    device-types/net/device-conformance.tex | 1 +
    2 files changed, 67 insertions(+), 10 deletions(-)

    --
    1.8.3.1




  • 2.  [PATCH v7 1/2] virtio-net: correct conditions for devices to validate packet checksum

    Posted 12-28-2023 07:46
    There is a historical error in virtio spec:
    "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."

    Currently in Linux and virtio-related implementations, the device validates
    the packet checksum and sets DATA_VALID regardless of whether
    VIRTIO_NET_F_GUEST_CSUM is negotiated.

    Please refer to the following summary and thread[1] for details and reasons:

    Summary:
    1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with partially
    checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM is mapped
    to NETIF_F_RXCSUM.
    GUEST_CSUM only indicates whether the driver handles partially checksummed packets.
    When XDP is loaded, the offload of GUEST_CSUM will be disabled, which means that
    packets have NEEDS_CSUM set will be dropped, but packets have DATA_VALID set will
    still be received because NETIF_F_RXCSUM still exists.

    2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it was actually expected
    that rx checksum offload (the driver can set CHECKSUM_UNNECESSARY) had nothing to do
    with whether GUEST_CSUM was negotiated. But due to an error, below desctiption was
    added incorrectly:
    "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."

    3. We now hope to correct this error. Let the setting of DATA_VALID not be controlled by
    whether GUEST_CSUM is negotiated, but only controlled by whether rx checksum offload is
    enabled on the OS side. The state of this rx checksum offload is also not aware of the device.

    4 NETIF_RXCSUM corresponding to rx checksum offload should be added to dev->hw_features.
    When the user turns off rx checksum offload through ethtool -K, neither NEEDS_CSUM nor
    DATA_VALID should be taken care of, that is, all packets will be CHECKSUM_NONE.

    5. GUEST_FULLY_CSUM is added to disable the offload of GUEST_CSUM:
    When a packet have NEEDS_CSUM set is received, it is either dropped or a fully checksummed
    packet is calculated.
    When the corresponding offload of the GUEST_FULLY_CSUM is disabled, it is as if only
    GUEST_CSUM was negotiated.

    [1] https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
    [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")

    Suggested-by: Jason Wang <jasowang@redhat.com>
    Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    ---
    device-types/net/description.tex | 8 +++-----
    1 file changed, 3 insertions(+), 5 deletions(-)

    diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    index aff5e08..a5210f2 100644
    --- a/device-types/net/description.tex
    +++ b/device-types/net/description.tex
    @@ -723,8 +723,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    \field{num_buffers} is one, then the entire packet will be
    contained within this buffer, immediately following the struct
    virtio_net_hdr.
    -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
    - VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    +\item The VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    set: if so, device has validated the packet checksum.
    In case of multiple encapsulated protocols, one level of checksums
    has been validated.
    @@ -783,7 +782,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    packet together, such that at least \field{num_buffers} are
    observed by driver as used.

    -If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set
    +If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MAY set
    \field{flags} to zero and SHOULD supply a fully checksummed
    packet to the driver.

    @@ -842,8 +841,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    not less than the length of the headers, including the transport
    header.

    -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
    -device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    +The device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    \field{flags}, if so, the device MUST validate the packet
    checksum (in case of multiple encapsulated protocols, one level
    of checksums is validated).
    --
    1.8.3.1




  • 3.  Re: [PATCH v7 1/2] virtio-net: correct conditions for devices to validate packet checksum

    Posted 01-02-2024 01:49
    On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >
    > There is a historical error in virtio spec:
    > "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."
    >
    > Currently in Linux and virtio-related implementations, the device validates
    > the packet checksum and sets DATA_VALID regardless of whether
    > VIRTIO_NET_F_GUEST_CSUM is negotiated.
    >
    > Please refer to the following summary and thread[1] for details and reasons:
    >
    > Summary:
    > 1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with partially
    > checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM is mapped
    > to NETIF_F_RXCSUM.
    > GUEST_CSUM only indicates whether the driver handles partially checksummed packets.
    > When XDP is loaded, the offload of GUEST_CSUM will be disabled, which means that
    > packets have NEEDS_CSUM set will be dropped, but packets have DATA_VALID set will
    > still be received because NETIF_F_RXCSUM still exists.
    >
    > 2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it was actually expected
    > that rx checksum offload (the driver can set CHECKSUM_UNNECESSARY) had nothing to do
    > with whether GUEST_CSUM was negotiated. But due to an error, below desctiption was
    > added incorrectly:
    > "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."
    >
    > 3. We now hope to correct this error. Let the setting of DATA_VALID not be controlled by
    > whether GUEST_CSUM is negotiated, but only controlled by whether rx checksum offload is
    > enabled on the OS side. The state of this rx checksum offload is also not aware of the device.
    >
    > 4 NETIF_RXCSUM corresponding to rx checksum offload should be added to dev->hw_features.
    > When the user turns off rx checksum offload through ethtool -K, neither NEEDS_CSUM nor
    > DATA_VALID should be taken care of, that is, all packets will be CHECKSUM_NONE.
    >
    > 5. GUEST_FULLY_CSUM is added to disable the offload of GUEST_CSUM:
    > When a packet have NEEDS_CSUM set is received, it is either dropped or a fully checksummed
    > packet is calculated.
    > When the corresponding offload of the GUEST_FULLY_CSUM is disabled, it is as if only
    > GUEST_CSUM was negotiated.
    >
    > [1] https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
    > [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")
    >
    > Suggested-by: Jason Wang <jasowang@redhat.com>
    > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    > ---
    > device-types/net/description.tex | 8 +++-----
    > 1 file changed, 3 insertions(+), 5 deletions(-)
    >
    > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    > index aff5e08..a5210f2 100644
    > --- a/device-types/net/description.tex
    > +++ b/device-types/net/description.tex
    > @@ -723,8 +723,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    > \field{num_buffers} is one, then the entire packet will be
    > contained within this buffer, immediately following the struct
    > virtio_net_hdr.
    > -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
    > - VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    > +\item The VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    > set: if so, device has validated the packet checksum.
    > In case of multiple encapsulated protocols, one level of checksums
    > has been validated.
    > @@ -783,7 +782,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    > packet together, such that at least \field{num_buffers} are
    > observed by driver as used.
    >
    > -If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set
    > +If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MAY set
    > \field{flags} to zero and SHOULD supply a fully checksummed
    > packet to the driver.

    I think it would more clearer if we just drop the flags part:

    "
    If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device SHOULD supply
    a fully checksummed packet to the driver.
    "

    >
    > @@ -842,8 +841,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    > not less than the length of the headers, including the transport
    > header.
    >
    > -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
    > -device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    > +The device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    > \field{flags}, if so, the device MUST validate the packet
    > checksum (in case of multiple encapsulated protocols, one level
    > of checksums is validated).

    And drop this part.

    Thanks

    > --
    > 1.8.3.1
    >




  • 4.  Re: [virtio-comment] Re: [PATCH v7 1/2] virtio-net: correct conditions for devices to validate packet checksum

    Posted 01-02-2024 03:06


    ? 2024/1/2 ??9:48, Jason Wang ??:
    > On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >> There is a historical error in virtio spec:
    >> "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."
    >>
    >> Currently in Linux and virtio-related implementations, the device validates
    >> the packet checksum and sets DATA_VALID regardless of whether
    >> VIRTIO_NET_F_GUEST_CSUM is negotiated.
    >>
    >> Please refer to the following summary and thread[1] for details and reasons:
    >>
    >> Summary:
    >> 1. GUEST_CSUM at virtio spec 0.95 is intended to be compatible with partially
    >> checksummed packets (NEEDS_CSUM <-> CHECKSUM_PARTIAL). So GUEST_CSUM is mapped
    >> to NETIF_F_RXCSUM.
    >> GUEST_CSUM only indicates whether the driver handles partially checksummed packets.
    >> When XDP is loaded, the offload of GUEST_CSUM will be disabled, which means that
    >> packets have NEEDS_CSUM set will be dropped, but packets have DATA_VALID set will
    >> still be received because NETIF_F_RXCSUM still exists.
    >>
    >> 2. When DATA_VALID was added to Linux in 2011[2] and virtio1.0, it was actually expected
    >> that rx checksum offload (the driver can set CHECKSUM_UNNECESSARY) had nothing to do
    >> with whether GUEST_CSUM was negotiated. But due to an error, below desctiption was
    >> added incorrectly:
    >> "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."
    >>
    >> 3. We now hope to correct this error. Let the setting of DATA_VALID not be controlled by
    >> whether GUEST_CSUM is negotiated, but only controlled by whether rx checksum offload is
    >> enabled on the OS side. The state of this rx checksum offload is also not aware of the device.
    >>
    >> 4 NETIF_RXCSUM corresponding to rx checksum offload should be added to dev->hw_features.
    >> When the user turns off rx checksum offload through ethtool -K, neither NEEDS_CSUM nor
    >> DATA_VALID should be taken care of, that is, all packets will be CHECKSUM_NONE.
    >>
    >> 5. GUEST_FULLY_CSUM is added to disable the offload of GUEST_CSUM:
    >> When a packet have NEEDS_CSUM set is received, it is either dropped or a fully checksummed
    >> packet is calculated.
    >> When the corresponding offload of the GUEST_FULLY_CSUM is disabled, it is as if only
    >> GUEST_CSUM was negotiated.
    >>
    >> [1] https://lists.oasis-open.org/archives/virtio-comment/202312/msg00135.html
    >> [2] 10a8d94a9574 ("virtio_net: introduce VIRTIO_NET_HDR_F_DATA_VALID")
    >>
    >> Suggested-by: Jason Wang <jasowang@redhat.com>
    >> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    >> ---
    >> device-types/net/description.tex | 8 +++-----
    >> 1 file changed, 3 insertions(+), 5 deletions(-)
    >>
    >> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    >> index aff5e08..a5210f2 100644
    >> --- a/device-types/net/description.tex
    >> +++ b/device-types/net/description.tex
    >> @@ -723,8 +723,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    >> \field{num_buffers} is one, then the entire packet will be
    >> contained within this buffer, immediately following the struct
    >> virtio_net_hdr.
    >> -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
    >> - VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    >> +\item The VIRTIO_NET_HDR_F_DATA_VALID bit in \field{flags} can be
    >> set: if so, device has validated the packet checksum.
    >> In case of multiple encapsulated protocols, one level of checksums
    >> has been validated.
    >> @@ -783,7 +782,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    >> packet together, such that at least \field{num_buffers} are
    >> observed by driver as used.
    >>
    >> -If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MUST set
    >> +If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device MAY set
    >> \field{flags} to zero and SHOULD supply a fully checksummed
    >> packet to the driver.
    > I think it would more clearer if we just drop the flags part:
    >
    > "
    > If VIRTIO_NET_F_GUEST_CSUM is not negotiated, the device SHOULD supply
    > a fully checksummed packet to the driver.
    > "
    >
    >> @@ -842,8 +841,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    >> not less than the length of the headers, including the transport
    >> header.
    >>
    >> -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
    >> -device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    >> +The device MAY set the VIRTIO_NET_HDR_F_DATA_VALID bit in
    >> \field{flags}, if so, the device MUST validate the packet
    >> checksum (in case of multiple encapsulated protocols, one level
    >> of checksums is validated).
    > And drop this part.

    Will be modified in the next version.

    Thanks!

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




  • 5.  [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 12-28-2023 07:46
    virtio-net works in a virtualized system and is somewhat different from
    physical nics. One of the differences is that to save virtio device
    resources, rx may receive partially checksummed packets. However, XDP may
    cause partially checksummed packets to be dropped.
    So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    (to disable the tx checksum offload).

    In addition, implementation of some performant devices always do not generate
    partially checksummed packets, but the standard driver still cannot load XDP
    when VIRTIO_NET_F_GUEST_CSUM is there.

    This patch lets the device to supply fully checksummed packets to the driver.

    A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    situation, which provides the driver with configurable offload.
    If the offload is enabled, then the device must deliver fully
    checksummed packets to the driver and may validate the checksum.

    Use case example:
    If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    is retained if the device has validated its checksum, resulting in the guest
    not needing to validate the checksum again. This is useful for guests:
    1. Bring the driver advantages such as cpu savings.
    2. For devices that do not generate partially checksummed packets themselves,
    XDP can be loaded in the driver without modifying the hardware behavior.

    Several solutions have been discussed in the previous proposal[1].
    After historical discussion, we have tried the method proposed by Jason[2],
    but some complex scenarios and challenges are difficult to deal with.
    We now return to the method suggested in [1].

    [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/

    Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    ---
    v6->v7:
    - FULLY_CSUM no longer depends on GUEST_CSUM.
    - The description related to DATA_VALID has nothing to do with
    whether GUEST_CSUM is negotiated or not. @Jason

    v5->v6:
    - Rewrite and clarify patch description. @Michael

    v4->v5:
    - Remove the modification to the GUEST_CSUM. @Jason
    - The description of this feature has been reorganized for greater clarity. @Michael

    v3->v4:
    - Streamline some repetitive descriptions. @Jason
    - Add how features should work, when to be enabled, and overhead. @Jason @Michael

    v2->v3:
    - Add a section named "Driver Handles Fully Checksummed Packets"
    and more descriptions. @Michael

    v1->v2:
    - Modify full checksum functionality as a configurable offload
    that is initially turned off. @Jason

    device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    device-types/net/device-conformance.tex | 1 +
    2 files changed, 64 insertions(+), 5 deletions(-)

    diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    index a5210f2..d4a7329 100644
    --- a/device-types/net/description.tex
    +++ b/device-types/net/description.tex
    @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    device with the same MAC address.

    \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    +
    +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    + to the driver and may validate the checksum.
    \end{description}

    \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.

    \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    the same system might not need checksumming at all, nor segmentation,
    if both guests are amenable.}
    The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    - checksummed packets can be received, and if it can do that then
    + checksummed packets can be received (provided that the offload of
    + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    everything else.

    +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    +
    +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    +benefit from the device's ability to calculate and validate the checksum.
    +
    +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    +and its offload is enabled, the device behaves as follows:
    +\begin{itemize}
    + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    + \item The device may validate the packet checksum before delivering it.
    +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    +in \field{flags} is set: in case of multiple encapsulated protocols, one
    +level of checksums has been validated.
    + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    +\end{itemize}
    +
    +The packet types that the device recognizes and can verify are independent of
    +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.
    +
    +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    +and SCTP (Stream Control Transmission Protocol).
    +A fully checksummed packet's checksum field for each of the above protocols
    +is set to a calculated value that covers the transport header and payload
    +(TCP or UDP involves the additional pseudo header) of the packet.
    +
    +Delivering fully checksummed packets rather than partially
    +checksummed packets incurs additional overhead for the device.
    +The overhead varies from device to device, for example the overhead of
    +calculating and validating the packet checksum is a few microseconds
    +for some hardware devices.
    +
    +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    +which when enabled means that the device delivers fully checksummed packets
    +to the driver and may validate the checksum.
    +\begin{note}
    +Unlike other offloads, this offload is disabled even after
    +VIRTIO_NET_F_GUEST_FULLY_CSUM has been negotiated.
    +The offload has to be enabled when, for example, eXpress Data
    +Path (XDP) in Linux is active.
    +\end{note}
    +
    +\devicenormative{\subsubsection}{Device Delivers Fully Checksummed Packets}{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    +
    +If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, its offload is disabled
    +until the VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET command with the
    +VIRTIO_NET_F_GUEST_FULLY_CSUM bit set completes successfully.
    +
    \subsection{Device Operation}\label{sec:Device Types / Network Device / Device Operation}

    Packets are transmitted by placing them in the
    @@ -746,7 +800,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    number of coalesced TCP segments in \field{csum_start} field and
    number of duplicated ACK segments in \field{csum_offset} field
    and sets bit VIRTIO_NET_HDR_F_RSC_INFO in \field{flags}.
    -\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated, the
    +\item If the VIRTIO_NET_F_GUEST_CSUM feature was negotiated but the
    + VIRTIO_NET_F_GUEST_FULLY_CSUM feature was not 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
    @@ -804,8 +859,9 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    device MUST set the VIRTIO_NET_HDR_GSO_ECN bit in
    \field{gso_type}.

    -If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the
    -device MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
    +If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated but
    +the VIRTIO_NET_F_GUEST_FULLY_CSUM feature has not been negotiated,
    +the device MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
    \field{flags}, if so:
    \begin{enumerate}
    \item the device MUST validate the packet checksum at
    @@ -825,7 +881,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
    been negotiated, the device MUST set \field{gso_type} to
    VIRTIO_NET_HDR_GSO_NONE.

    -If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, then
    +If the VIRTIO_NET_F_GUEST_FULLY_CSUM feature has not been negotiated and
    +\field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, then
    the device MUST also set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
    \field{flags} MUST set \field{gso_size} to indicate the desired MSS.
    If VIRTIO_NET_F_RSC_EXT was negotiated, the device MUST also
    @@ -1631,6 +1688,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
    #define VIRTIO_NET_F_GUEST_UFO 10
    #define VIRTIO_NET_F_GUEST_USO4 54
    #define VIRTIO_NET_F_GUEST_USO6 55
    +#define VIRTIO_NET_F_GUEST_FULLY_CSUM 64

    #define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5
    #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0
    diff --git a/device-types/net/device-conformance.tex b/device-types/net/device-conformance.tex
    index 52526e4..43b3921 100644
    --- a/device-types/net/device-conformance.tex
    +++ b/device-types/net/device-conformance.tex
    @@ -16,4 +16,5 @@
    \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
    \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
    \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Device Statistics}
    +\item \ref{devicenormative:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    \end{itemize}
    --
    1.8.3.1




  • 6.  Re: [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 01-02-2024 01:58
    On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >
    > virtio-net works in a virtualized system and is somewhat different from
    > physical nics. One of the differences is that to save virtio device
    > resources, rx may receive partially checksummed packets. However, XDP may
    > cause partially checksummed packets to be dropped.
    > So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    > When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    > (to disable the tx checksum offload).
    >
    > In addition, implementation of some performant devices always do not generate
    > partially checksummed packets, but the standard driver still cannot load XDP
    > when VIRTIO_NET_F_GUEST_CSUM is there.
    >
    > This patch lets the device to supply fully checksummed packets to the driver.
    >
    > A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    > situation, which provides the driver with configurable offload.
    > If the offload is enabled, then the device must deliver fully

    For offload, did you mean rx csum offload?

    > checksummed packets to the driver and may validate the checksum.
    >
    > Use case example:
    > If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    > after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > is retained if the device has validated its checksum, resulting in the guest
    > not needing to validate the checksum again. This is useful for guests:
    > 1. Bring the driver advantages such as cpu savings.
    > 2. For devices that do not generate partially checksummed packets themselves,
    > XDP can be loaded in the driver without modifying the hardware behavior.
    >
    > Several solutions have been discussed in the previous proposal[1].
    > After historical discussion, we have tried the method proposed by Jason[2],
    > but some complex scenarios and challenges are difficult to deal with.
    > We now return to the method suggested in [1].
    >
    > [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    > [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/
    >
    > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    > ---
    > v6->v7:
    > - FULLY_CSUM no longer depends on GUEST_CSUM.
    > - The description related to DATA_VALID has nothing to do with
    > whether GUEST_CSUM is negotiated or not. @Jason
    >
    > v5->v6:
    > - Rewrite and clarify patch description. @Michael
    >
    > v4->v5:
    > - Remove the modification to the GUEST_CSUM. @Jason
    > - The description of this feature has been reorganized for greater clarity. @Michael
    >
    > v3->v4:
    > - Streamline some repetitive descriptions. @Jason
    > - Add how features should work, when to be enabled, and overhead. @Jason @Michael
    >
    > v2->v3:
    > - Add a section named "Driver Handles Fully Checksummed Packets"
    > and more descriptions. @Michael
    >
    > v1->v2:
    > - Modify full checksum functionality as a configurable offload
    > that is initially turned off. @Jason
    >
    > device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    > device-types/net/device-conformance.tex | 1 +
    > 2 files changed, 64 insertions(+), 5 deletions(-)
    >
    > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    > index a5210f2..d4a7329 100644
    > --- a/device-types/net/description.tex
    > +++ b/device-types/net/description.tex
    > @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    > device with the same MAC address.
    >
    > \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    > +
    > +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    > + to the driver and may validate the checksum.
    > \end{description}
    >
    > \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    > @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    > \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    > \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    > \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    > +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
    >
    > \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    > \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    > @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > the same system might not need checksumming at all, nor segmentation,
    > if both guests are amenable.}
    > The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    > - checksummed packets can be received, and if it can do that then
    > + checksummed packets can be received (provided that the offload of
    > + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    > the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    > VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    > and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    > @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    > everything else.
    >
    > +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    > +
    > +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    > +benefit from the device's ability to calculate and validate the checksum.
    > +
    > +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    > +and its offload is enabled, the device behaves as follows:
    > +\begin{itemize}
    > + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    > +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    > + \item The device may validate the packet checksum before delivering it.
    > +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > +in \field{flags} is set: in case of multiple encapsulated protocols, one
    > +level of checksums has been validated.
    > + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    > +\end{itemize}

    I may miss something, but this seems the existing behaviour of the
    devices regardless of FULLY_CSUM?

    > +
    > +The packet types that the device recognizes and can verify are independent of
    > +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.

    Right, so I don't see why this became part of this patch.

    > +
    > +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    > +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    > +and SCTP (Stream Control Transmission Protocol).
    > +A fully checksummed packet's checksum field for each of the above protocols
    > +is set to a calculated value that covers the transport header and payload
    > +(TCP or UDP involves the additional pseudo header) of the packet.
    > +
    > +Delivering fully checksummed packets rather than partially
    > +checksummed packets incurs additional overhead for the device.
    > +The overhead varies from device to device, for example the overhead of
    > +calculating and validating the packet checksum is a few microseconds
    > +for some hardware devices.
    > +
    > +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    > +which when enabled means that the device delivers fully checksummed packets
    > +to the driver and may validate the checksum.
    > +\begin{note}

    This is the functional equivalent to disable GUEST_CSUM. So I guess
    what I'm missing is the difference between:

    1) enable FULLY_CUSM

    and

    2) disable GUEST_CSUM

    It looks to me 1) try to keep e.g HW_GRO (GUEST_TSOX) work? Note that
    GRO produce partial csum usually, not sure it can work correctly.

    Thanks




  • 7.  Re: [virtio-comment] Re: [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 01-02-2024 03:34


    ? 2024/1/2 ??9:58, Jason Wang ??:
    > On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >> virtio-net works in a virtualized system and is somewhat different from
    >> physical nics. One of the differences is that to save virtio device
    >> resources, rx may receive partially checksummed packets. However, XDP may
    >> cause partially checksummed packets to be dropped.
    >> So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    >> When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    >> (to disable the tx checksum offload).
    >>
    >> In addition, implementation of some performant devices always do not generate
    >> partially checksummed packets, but the standard driver still cannot load XDP
    >> when VIRTIO_NET_F_GUEST_CSUM is there.
    >>
    >> This patch lets the device to supply fully checksummed packets to the driver.
    >>
    >> A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    >> situation, which provides the driver with configurable offload.
    >> If the offload is enabled, then the device must deliver fully
    > For offload, did you mean rx csum offload?

    I mean FULLY_CSUM's offload.

    >
    >> checksummed packets to the driver and may validate the checksum.
    >>
    >> Use case example:
    >> If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    >> after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    >> is retained if the device has validated its checksum, resulting in the guest
    >> not needing to validate the checksum again. This is useful for guests:
    >> 1. Bring the driver advantages such as cpu savings.
    >> 2. For devices that do not generate partially checksummed packets themselves,
    >> XDP can be loaded in the driver without modifying the hardware behavior.
    >>
    >> Several solutions have been discussed in the previous proposal[1].
    >> After historical discussion, we have tried the method proposed by Jason[2],
    >> but some complex scenarios and challenges are difficult to deal with.
    >> We now return to the method suggested in [1].
    >>
    >> [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    >> [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/
    >>
    >> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    >> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    >> ---
    >> v6->v7:
    >> - FULLY_CSUM no longer depends on GUEST_CSUM.
    >> - The description related to DATA_VALID has nothing to do with
    >> whether GUEST_CSUM is negotiated or not. @Jason
    >>
    >> v5->v6:
    >> - Rewrite and clarify patch description. @Michael
    >>
    >> v4->v5:
    >> - Remove the modification to the GUEST_CSUM. @Jason
    >> - The description of this feature has been reorganized for greater clarity. @Michael
    >>
    >> v3->v4:
    >> - Streamline some repetitive descriptions. @Jason
    >> - Add how features should work, when to be enabled, and overhead. @Jason @Michael
    >>
    >> v2->v3:
    >> - Add a section named "Driver Handles Fully Checksummed Packets"
    >> and more descriptions. @Michael
    >>
    >> v1->v2:
    >> - Modify full checksum functionality as a configurable offload
    >> that is initially turned off. @Jason
    >>
    >> device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    >> device-types/net/device-conformance.tex | 1 +
    >> 2 files changed, 64 insertions(+), 5 deletions(-)
    >>
    >> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    >> index a5210f2..d4a7329 100644
    >> --- a/device-types/net/description.tex
    >> +++ b/device-types/net/description.tex
    >> @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    >> device with the same MAC address.
    >>
    >> \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    >> +
    >> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    >> + to the driver and may validate the checksum.
    >> \end{description}
    >>
    >> \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    >> @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    >> \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    >> \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    >> \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    >> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
    >>
    >> \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    >> \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    >> @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    >> the same system might not need checksumming at all, nor segmentation,
    >> if both guests are amenable.}
    >> The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    >> - checksummed packets can be received, and if it can do that then
    >> + checksummed packets can be received (provided that the offload of
    >> + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    >> the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    >> VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    >> and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    >> @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    >> A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    >> everything else.
    >>
    >> +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    >> +
    >> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    >> +benefit from the device's ability to calculate and validate the checksum.
    >> +
    >> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    >> +and its offload is enabled, the device behaves as follows:
    >> +\begin{itemize}
    >> + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    >> +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    >> + \item The device may validate the packet checksum before delivering it.
    >> +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    >> +in \field{flags} is set: in case of multiple encapsulated protocols, one
    >> +level of checksums has been validated.
    >> + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    >> +\end{itemize}
    > I may miss something, but this seems the existing behaviour of the
    > devices regardless of FULLY_CSUM?

    Yes Jason.
    I think that we have the fix from the first patch, our requirements have
    been met.
    But back to the second patch itself, on some devices, turning off
    GUEST_CSUM offload
    may mean turning off tx checksum offload, but GUEST_FULLY_CSUM does not
    have this requirement.
    GRO_HW will also be turned off.

    >
    >> +
    >> +The packet types that the device recognizes and can verify are independent of
    >> +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.
    > Right, so I don't see why this became part of this patch.
    >
    >> +
    >> +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    >> +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    >> +and SCTP (Stream Control Transmission Protocol).
    >> +A fully checksummed packet's checksum field for each of the above protocols
    >> +is set to a calculated value that covers the transport header and payload
    >> +(TCP or UDP involves the additional pseudo header) of the packet.
    >> +
    >> +Delivering fully checksummed packets rather than partially
    >> +checksummed packets incurs additional overhead for the device.
    >> +The overhead varies from device to device, for example the overhead of
    >> +calculating and validating the packet checksum is a few microseconds
    >> +for some hardware devices.
    >> +
    >> +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    >> +which when enabled means that the device delivers fully checksummed packets
    >> +to the driver and may validate the checksum.
    >> +\begin{note}
    > This is the functional equivalent to disable GUEST_CSUM. So I guess
    > what I'm missing is the difference between:
    >
    > 1) enable FULLY_CUSM
    >
    > and
    >
    > 2) disable GUEST_CSUM
    >
    > It looks to me 1) try to keep e.g HW_GRO (GUEST_TSOX) work? Note that
    > GRO produce partial csum usually, not sure it can work correctly.

    When GUEST_CSUM is turned off, GRO_HW offload will be turned off.
    Meaning the hardware will not merge the packets.
    IIRC, after the packets have been merged, the packets should have full
    checksums and have been verified:

           if (!(features & NETIF_F_RXCSUM)) {
                    /* NETIF_F_GRO_HW implies doing RXCSUM since every packet
                     * successfully merged by hardware must also have the
                     * checksum verified by hardware.  If the user does not
                     * want to enable RXCSUM, logically, we should disable
    GRO_HW.
                     */
                    if (features & NETIF_F_GRO_HW) {
                            netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since
    no RXCSUM feature.\n");
                            features &= ~NETIF_F_GRO_HW;
                    }
            }


    Thanks!

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




  • 8.  Re: [virtio-comment] Re: [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 01-02-2024 05:57
    On Tue, Jan 2, 2024 at 11:33?AM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >
    >
    >
    > ? 2024/1/2 ??9:58, Jason Wang ??:
    > > On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    > >> virtio-net works in a virtualized system and is somewhat different from
    > >> physical nics. One of the differences is that to save virtio device
    > >> resources, rx may receive partially checksummed packets. However, XDP may
    > >> cause partially checksummed packets to be dropped.
    > >> So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    > >> When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    > >> (to disable the tx checksum offload).
    > >>
    > >> In addition, implementation of some performant devices always do not generate
    > >> partially checksummed packets, but the standard driver still cannot load XDP
    > >> when VIRTIO_NET_F_GUEST_CSUM is there.
    > >>
    > >> This patch lets the device to supply fully checksummed packets to the driver.
    > >>
    > >> A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    > >> situation, which provides the driver with configurable offload.
    > >> If the offload is enabled, then the device must deliver fully
    > > For offload, did you mean rx csum offload?
    >
    > I mean FULLY_CSUM's offload.
    >
    > >
    > >> checksummed packets to the driver and may validate the checksum.
    > >>
    > >> Use case example:
    > >> If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    > >> after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > >> is retained if the device has validated its checksum, resulting in the guest
    > >> not needing to validate the checksum again. This is useful for guests:
    > >> 1. Bring the driver advantages such as cpu savings.
    > >> 2. For devices that do not generate partially checksummed packets themselves,
    > >> XDP can be loaded in the driver without modifying the hardware behavior.
    > >>
    > >> Several solutions have been discussed in the previous proposal[1].
    > >> After historical discussion, we have tried the method proposed by Jason[2],
    > >> but some complex scenarios and challenges are difficult to deal with.
    > >> We now return to the method suggested in [1].
    > >>
    > >> [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    > >> [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/
    > >>
    > >> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    > >> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    > >> ---
    > >> v6->v7:
    > >> - FULLY_CSUM no longer depends on GUEST_CSUM.
    > >> - The description related to DATA_VALID has nothing to do with
    > >> whether GUEST_CSUM is negotiated or not. @Jason
    > >>
    > >> v5->v6:
    > >> - Rewrite and clarify patch description. @Michael
    > >>
    > >> v4->v5:
    > >> - Remove the modification to the GUEST_CSUM. @Jason
    > >> - The description of this feature has been reorganized for greater clarity. @Michael
    > >>
    > >> v3->v4:
    > >> - Streamline some repetitive descriptions. @Jason
    > >> - Add how features should work, when to be enabled, and overhead. @Jason @Michael
    > >>
    > >> v2->v3:
    > >> - Add a section named "Driver Handles Fully Checksummed Packets"
    > >> and more descriptions. @Michael
    > >>
    > >> v1->v2:
    > >> - Modify full checksum functionality as a configurable offload
    > >> that is initially turned off. @Jason
    > >>
    > >> device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    > >> device-types/net/device-conformance.tex | 1 +
    > >> 2 files changed, 64 insertions(+), 5 deletions(-)
    > >>
    > >> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    > >> index a5210f2..d4a7329 100644
    > >> --- a/device-types/net/description.tex
    > >> +++ b/device-types/net/description.tex
    > >> @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    > >> device with the same MAC address.
    > >>
    > >> \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    > >> +
    > >> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    > >> + to the driver and may validate the checksum.
    > >> \end{description}
    > >>
    > >> \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    > >> @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    > >> \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >> \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >> \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
    > >>
    > >> \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    > >> \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    > >> @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > >> the same system might not need checksumming at all, nor segmentation,
    > >> if both guests are amenable.}
    > >> The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    > >> - checksummed packets can be received, and if it can do that then
    > >> + checksummed packets can be received (provided that the offload of
    > >> + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    > >> the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    > >> VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    > >> and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    > >> @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > >> A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    > >> everything else.
    > >>
    > >> +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    > >> +
    > >> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    > >> +benefit from the device's ability to calculate and validate the checksum.
    > >> +
    > >> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    > >> +and its offload is enabled, the device behaves as follows:
    > >> +\begin{itemize}
    > >> + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    > >> +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    > >> + \item The device may validate the packet checksum before delivering it.
    > >> +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > >> +in \field{flags} is set: in case of multiple encapsulated protocols, one
    > >> +level of checksums has been validated.
    > >> + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    > >> +\end{itemize}
    > > I may miss something, but this seems the existing behaviour of the
    > > devices regardless of FULLY_CSUM?
    >
    > Yes Jason.
    > I think that we have the fix from the first patch, our requirements have
    > been met.
    > But back to the second patch itself, on some devices, turning off
    > GUEST_CSUM offload
    > may mean turning off tx checksum offload,

    For tx did you mean guest? If yes, it should be HOST_CSUM I think.

    Or you actually mean the tx csum on the host?

    > but GUEST_FULLY_CSUM does not
    > have this requirement.
    > GRO_HW will also be turned off.
    >
    > >
    > >> +
    > >> +The packet types that the device recognizes and can verify are independent of
    > >> +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.
    > > Right, so I don't see why this became part of this patch.
    > >
    > >> +
    > >> +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    > >> +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    > >> +and SCTP (Stream Control Transmission Protocol).
    > >> +A fully checksummed packet's checksum field for each of the above protocols
    > >> +is set to a calculated value that covers the transport header and payload
    > >> +(TCP or UDP involves the additional pseudo header) of the packet.
    > >> +
    > >> +Delivering fully checksummed packets rather than partially
    > >> +checksummed packets incurs additional overhead for the device.
    > >> +The overhead varies from device to device, for example the overhead of
    > >> +calculating and validating the packet checksum is a few microseconds
    > >> +for some hardware devices.
    > >> +
    > >> +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    > >> +which when enabled means that the device delivers fully checksummed packets
    > >> +to the driver and may validate the checksum.
    > >> +\begin{note}
    > > This is the functional equivalent to disable GUEST_CSUM. So I guess
    > > what I'm missing is the difference between:
    > >
    > > 1) enable FULLY_CUSM
    > >
    > > and
    > >
    > > 2) disable GUEST_CSUM
    > >
    > > It looks to me 1) try to keep e.g HW_GRO (GUEST_TSOX) work? Note that
    > > GRO produce partial csum usually, not sure it can work correctly.
    >
    > When GUEST_CSUM is turned off, GRO_HW offload will be turned off.

    It's the GUEST_TSO4/6 that is actually turned off. So this needs more thought:

    1) Reuse GUEST_TSO4/6 but have a mode to have full checksum
    2) New feature like GUEST_GRO, and claims that the packet boundary
    needs to be reserved there

    My understanding is, 1) may end up being somewhat ambiguous since TSO
    usually have partial checksum.

    What's your thought?

    > Meaning the hardware will not merge the packets.

    Right.

    > IIRC, after the packets have been merged, the packets should have full
    > checksums and have been verified:
    >
    > if (!(features & NETIF_F_RXCSUM)) {
    > /* NETIF_F_GRO_HW implies doing RXCSUM since every packet
    > * successfully merged by hardware must also have the
    > * checksum verified by hardware. If the user does not
    > * want to enable RXCSUM, logically, we should disable
    > GRO_HW.
    > */
    > if (features & NETIF_F_GRO_HW) {
    > netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since
    > no RXCSUM feature.\n");
    > features &= ~NETIF_F_GRO_HW;
    > }
    > }
    >

    Thanks




  • 9.  Re: [virtio-comment] Re: [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 01-02-2024 06:34


    ? 2024/1/2 ??1:56, Jason Wang ??:
    > On Tue, Jan 2, 2024 at 11:33?AM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >>
    >>
    >> ? 2024/1/2 ??9:58, Jason Wang ??:
    >>> On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >>>> virtio-net works in a virtualized system and is somewhat different from
    >>>> physical nics. One of the differences is that to save virtio device
    >>>> resources, rx may receive partially checksummed packets. However, XDP may
    >>>> cause partially checksummed packets to be dropped.
    >>>> So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    >>>> When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    >>>> (to disable the tx checksum offload).
    >>>>
    >>>> In addition, implementation of some performant devices always do not generate
    >>>> partially checksummed packets, but the standard driver still cannot load XDP
    >>>> when VIRTIO_NET_F_GUEST_CSUM is there.
    >>>>
    >>>> This patch lets the device to supply fully checksummed packets to the driver.
    >>>>
    >>>> A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    >>>> situation, which provides the driver with configurable offload.
    >>>> If the offload is enabled, then the device must deliver fully
    >>> For offload, did you mean rx csum offload?
    >> I mean FULLY_CSUM's offload.
    >>
    >>>> checksummed packets to the driver and may validate the checksum.
    >>>>
    >>>> Use case example:
    >>>> If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    >>>> after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    >>>> is retained if the device has validated its checksum, resulting in the guest
    >>>> not needing to validate the checksum again. This is useful for guests:
    >>>> 1. Bring the driver advantages such as cpu savings.
    >>>> 2. For devices that do not generate partially checksummed packets themselves,
    >>>> XDP can be loaded in the driver without modifying the hardware behavior.
    >>>>
    >>>> Several solutions have been discussed in the previous proposal[1].
    >>>> After historical discussion, we have tried the method proposed by Jason[2],
    >>>> but some complex scenarios and challenges are difficult to deal with.
    >>>> We now return to the method suggested in [1].
    >>>>
    >>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    >>>> [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/
    >>>>
    >>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    >>>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    >>>> ---
    >>>> v6->v7:
    >>>> - FULLY_CSUM no longer depends on GUEST_CSUM.
    >>>> - The description related to DATA_VALID has nothing to do with
    >>>> whether GUEST_CSUM is negotiated or not. @Jason
    >>>>
    >>>> v5->v6:
    >>>> - Rewrite and clarify patch description. @Michael
    >>>>
    >>>> v4->v5:
    >>>> - Remove the modification to the GUEST_CSUM. @Jason
    >>>> - The description of this feature has been reorganized for greater clarity. @Michael
    >>>>
    >>>> v3->v4:
    >>>> - Streamline some repetitive descriptions. @Jason
    >>>> - Add how features should work, when to be enabled, and overhead. @Jason @Michael
    >>>>
    >>>> v2->v3:
    >>>> - Add a section named "Driver Handles Fully Checksummed Packets"
    >>>> and more descriptions. @Michael
    >>>>
    >>>> v1->v2:
    >>>> - Modify full checksum functionality as a configurable offload
    >>>> that is initially turned off. @Jason
    >>>>
    >>>> device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    >>>> device-types/net/device-conformance.tex | 1 +
    >>>> 2 files changed, 64 insertions(+), 5 deletions(-)
    >>>>
    >>>> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    >>>> index a5210f2..d4a7329 100644
    >>>> --- a/device-types/net/description.tex
    >>>> +++ b/device-types/net/description.tex
    >>>> @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    >>>> device with the same MAC address.
    >>>>
    >>>> \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    >>>> +
    >>>> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    >>>> + to the driver and may validate the checksum.
    >>>> \end{description}
    >>>>
    >>>> \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    >>>> @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    >>>> \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    >>>> \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    >>>> \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    >>>> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
    >>>>
    >>>> \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    >>>> \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    >>>> @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    >>>> the same system might not need checksumming at all, nor segmentation,
    >>>> if both guests are amenable.}
    >>>> The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    >>>> - checksummed packets can be received, and if it can do that then
    >>>> + checksummed packets can be received (provided that the offload of
    >>>> + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    >>>> the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    >>>> VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    >>>> and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    >>>> @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    >>>> A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    >>>> everything else.
    >>>>
    >>>> +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    >>>> +
    >>>> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    >>>> +benefit from the device's ability to calculate and validate the checksum.
    >>>> +
    >>>> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    >>>> +and its offload is enabled, the device behaves as follows:
    >>>> +\begin{itemize}
    >>>> + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    >>>> +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    >>>> + \item The device may validate the packet checksum before delivering it.
    >>>> +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    >>>> +in \field{flags} is set: in case of multiple encapsulated protocols, one
    >>>> +level of checksums has been validated.
    >>>> + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    >>>> +\end{itemize}
    >>> I may miss something, but this seems the existing behaviour of the
    >>> devices regardless of FULLY_CSUM?
    >> Yes Jason.
    >> I think that we have the fix from the first patch, our requirements have
    >> been met.
    >> But back to the second patch itself, on some devices, turning off
    >> GUEST_CSUM offload
    >> may mean turning off tx checksum offload,
    > For tx did you mean guest? If yes, it should be HOST_CSUM I think.
    >
    > Or you actually mean the tx csum on the host?
    >
    >> but GUEST_FULLY_CSUM does not
    >> have this requirement.
    >> GRO_HW will also be turned off.
    >>
    >>>> +
    >>>> +The packet types that the device recognizes and can verify are independent of
    >>>> +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.
    >>> Right, so I don't see why this became part of this patch.
    >>>
    >>>> +
    >>>> +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    >>>> +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    >>>> +and SCTP (Stream Control Transmission Protocol).
    >>>> +A fully checksummed packet's checksum field for each of the above protocols
    >>>> +is set to a calculated value that covers the transport header and payload
    >>>> +(TCP or UDP involves the additional pseudo header) of the packet.
    >>>> +
    >>>> +Delivering fully checksummed packets rather than partially
    >>>> +checksummed packets incurs additional overhead for the device.
    >>>> +The overhead varies from device to device, for example the overhead of
    >>>> +calculating and validating the packet checksum is a few microseconds
    >>>> +for some hardware devices.
    >>>> +
    >>>> +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    >>>> +which when enabled means that the device delivers fully checksummed packets
    >>>> +to the driver and may validate the checksum.
    >>>> +\begin{note}
    >>> This is the functional equivalent to disable GUEST_CSUM. So I guess
    >>> what I'm missing is the difference between:
    >>>
    >>> 1) enable FULLY_CUSM
    >>>
    >>> and
    >>>
    >>> 2) disable GUEST_CSUM
    >>>
    >>> It looks to me 1) try to keep e.g HW_GRO (GUEST_TSOX) work? Note that
    >>> GRO produce partial csum usually, not sure it can work correctly.
    >> When GUEST_CSUM is turned off, GRO_HW offload will be turned off.
    > It's the GUEST_TSO4/6 that is actually turned off. So this needs more thought:
    >
    > 1) Reuse GUEST_TSO4/6 but have a mode to have full checksum
    > 2) New feature like GUEST_GRO, and claims that the packet boundary
    > needs to be reserved there
    >
    > My understanding is, 1) may end up being somewhat ambiguous since TSO
    > usually have partial checksum.
    >
    > What's your thought?


    I think the reason why GUEST_TSO4/6 etc. rely on GUEST_CSUM is that the
    overhead of segmenting and re-merging
    packets can be saved between two vms on the same host.

    Currently, disabling GUEST_CSUM happens when XDP is loading, and XDP
    does conflict with GUEST_TSO4/6.
    Therefore, in this scenario, when we disable GUEST_CSUM, GUEST_TSO4/6
    must also be disabled.

    In non-XDP scenarios, we don't need to disable GUEST_CSUM (and the
    current Linux virtio driver does not provide a way to turn it off),
    and they work well.

    Designing solutions such as (1)/(2) can indeed be one of the future
    directions, but there does not seem to be a strong motivation(at least
    in the Alibaba cloud scenario).

    Therefore, I plan to push only the first fix patch in the next version,
    and fix the bug in Linux that \field{flags} with the DATA_VALID set is
    ignored when XDP is loaded.

    Thanks a lot!

    >
    >> Meaning the hardware will not merge the packets.
    > Right.
    >
    >> IIRC, after the packets have been merged, the packets should have full
    >> checksums and have been verified:
    >>
    >> if (!(features & NETIF_F_RXCSUM)) {
    >> /* NETIF_F_GRO_HW implies doing RXCSUM since every packet
    >> * successfully merged by hardware must also have the
    >> * checksum verified by hardware. If the user does not
    >> * want to enable RXCSUM, logically, we should disable
    >> GRO_HW.
    >> */
    >> if (features & NETIF_F_GRO_HW) {
    >> netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since
    >> no RXCSUM feature.\n");
    >> features &= ~NETIF_F_GRO_HW;
    >> }
    >> }
    >>
    > Thanks




  • 10.  Re: [virtio-comment] Re: [PATCH v7 2/2] virtio-net: device does not deliver partially checksummed packet and may validate the checksum

    Posted 01-03-2024 02:31
    On Tue, Jan 2, 2024 at 2:33?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    >
    >
    >
    > ? 2024/1/2 ??1:56, Jason Wang ??:
    > > On Tue, Jan 2, 2024 at 11:33?AM Heng Qi <hengqi@linux.alibaba.com> wrote:
    > >>
    > >>
    > >> ? 2024/1/2 ??9:58, Jason Wang ??:
    > >>> On Thu, Dec 28, 2023 at 3:46?PM Heng Qi <hengqi@linux.alibaba.com> wrote:
    > >>>> virtio-net works in a virtualized system and is somewhat different from
    > >>>> physical nics. One of the differences is that to save virtio device
    > >>>> resources, rx may receive partially checksummed packets. However, XDP may
    > >>>> cause partially checksummed packets to be dropped.
    > >>>> So XDP loading currently conflicts with the feature VIRTIO_NET_F_GUEST_CSUM.
    > >>>> When XDP is loaded, the offload of VIRTIO_NET_F_GUEST_CSUM is disabled
    > >>>> (to disable the tx checksum offload).
    > >>>>
    > >>>> In addition, implementation of some performant devices always do not generate
    > >>>> partially checksummed packets, but the standard driver still cannot load XDP
    > >>>> when VIRTIO_NET_F_GUEST_CSUM is there.
    > >>>>
    > >>>> This patch lets the device to supply fully checksummed packets to the driver.
    > >>>>
    > >>>> A new feature VIRTIO_NET_F_GUEST_FULLY_CSUM is added to solve the above
    > >>>> situation, which provides the driver with configurable offload.
    > >>>> If the offload is enabled, then the device must deliver fully
    > >>> For offload, did you mean rx csum offload?
    > >> I mean FULLY_CSUM's offload.
    > >>
    > >>>> checksummed packets to the driver and may validate the checksum.
    > >>>>
    > >>>> Use case example:
    > >>>> If VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated and the offload is enabled,
    > >>>> after XDP processes a fully checksummed packet, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > >>>> is retained if the device has validated its checksum, resulting in the guest
    > >>>> not needing to validate the checksum again. This is useful for guests:
    > >>>> 1. Bring the driver advantages such as cpu savings.
    > >>>> 2. For devices that do not generate partially checksummed packets themselves,
    > >>>> XDP can be loaded in the driver without modifying the hardware behavior.
    > >>>>
    > >>>> Several solutions have been discussed in the previous proposal[1].
    > >>>> After historical discussion, we have tried the method proposed by Jason[2],
    > >>>> but some complex scenarios and challenges are difficult to deal with.
    > >>>> We now return to the method suggested in [1].
    > >>>>
    > >>>> [1] https://lists.oasis-open.org/archives/virtio-dev/202305/msg00291.html
    > >>>> [2] 20230628030506.2213-1-hengqi@linux.alibaba.com/">https://lore.kernel.org/all/20230628030506.2213-1-hengqi@linux.alibaba.com/
    > >>>>
    > >>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
    > >>>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
    > >>>> ---
    > >>>> v6->v7:
    > >>>> - FULLY_CSUM no longer depends on GUEST_CSUM.
    > >>>> - The description related to DATA_VALID has nothing to do with
    > >>>> whether GUEST_CSUM is negotiated or not. @Jason
    > >>>>
    > >>>> v5->v6:
    > >>>> - Rewrite and clarify patch description. @Michael
    > >>>>
    > >>>> v4->v5:
    > >>>> - Remove the modification to the GUEST_CSUM. @Jason
    > >>>> - The description of this feature has been reorganized for greater clarity. @Michael
    > >>>>
    > >>>> v3->v4:
    > >>>> - Streamline some repetitive descriptions. @Jason
    > >>>> - Add how features should work, when to be enabled, and overhead. @Jason @Michael
    > >>>>
    > >>>> v2->v3:
    > >>>> - Add a section named "Driver Handles Fully Checksummed Packets"
    > >>>> and more descriptions. @Michael
    > >>>>
    > >>>> v1->v2:
    > >>>> - Modify full checksum functionality as a configurable offload
    > >>>> that is initially turned off. @Jason
    > >>>>
    > >>>> device-types/net/description.tex | 68 ++++++++++++++++++++++++++++++---
    > >>>> device-types/net/device-conformance.tex | 1 +
    > >>>> 2 files changed, 64 insertions(+), 5 deletions(-)
    > >>>>
    > >>>> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
    > >>>> index a5210f2..d4a7329 100644
    > >>>> --- a/device-types/net/description.tex
    > >>>> +++ b/device-types/net/description.tex
    > >>>> @@ -122,6 +122,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
    > >>>> device with the same MAC address.
    > >>>>
    > >>>> \item[VIRTIO_NET_F_SPEED_DUPLEX(63)] Device reports speed and duplex.
    > >>>> +
    > >>>> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM (64)] Device delivers fully checksummed packets
    > >>>> + to the driver and may validate the checksum.
    > >>>> \end{description}
    > >>>>
    > >>>> \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
    > >>>> @@ -136,6 +139,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
    > >>>> \item[VIRTIO_NET_F_GUEST_UFO] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >>>> \item[VIRTIO_NET_F_GUEST_USO4] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >>>> \item[VIRTIO_NET_F_GUEST_USO6] Requires VIRTIO_NET_F_GUEST_CSUM.
    > >>>> +\item[VIRTIO_NET_F_GUEST_FULLY_CSUM] Requires VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
    > >>>>
    > >>>> \item[VIRTIO_NET_F_HOST_TSO4] Requires VIRTIO_NET_F_CSUM.
    > >>>> \item[VIRTIO_NET_F_HOST_TSO6] Requires VIRTIO_NET_F_CSUM.
    > >>>> @@ -383,7 +387,8 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > >>>> the same system might not need checksumming at all, nor segmentation,
    > >>>> if both guests are amenable.}
    > >>>> The VIRTIO_NET_F_GUEST_CSUM feature indicates that partially
    > >>>> - checksummed packets can be received, and if it can do that then
    > >>>> + checksummed packets can be received (provided that the offload of
    > >>>> + VIRTIO_NET_F_GUEST_FULLY_CSUM is disabled), and if it can do that then
    > >>>> the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
    > >>>> VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_USO4
    > >>>> and VIRTIO_NET_F_GUEST_USO6 are the input equivalents of the features described above.
    > >>>> @@ -398,6 +403,55 @@ \subsection{Device Initialization}\label{sec:Device Types / Network Device / Dev
    > >>>> A truly minimal driver would only accept VIRTIO_NET_F_MAC and ignore
    > >>>> everything else.
    > >>>>
    > >>>> +\subsubsection{Device Delivers Fully Checksummed Packets}\label{sec:Device Types / Network Device / Device Initialization / Device Delivers Fully Checksummed Packets}
    > >>>> +
    > >>>> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated, the driver can
    > >>>> +benefit from the device's ability to calculate and validate the checksum.
    > >>>> +
    > >>>> +If the feature VIRTIO_NET_F_GUEST_FULLY_CSUM is negotiated
    > >>>> +and its offload is enabled, the device behaves as follows:
    > >>>> +\begin{itemize}
    > >>>> + \item The device delivers a fully checksummed packet to the driver rather than a partially checksummed packet.
    > >>>> +Partially checksummed packets come from TCP/UDP protocols \ref{devicenormative:Device Types / Network Device / Device Operation / Processing of Packets}.
    > >>>> + \item The device may validate the packet checksum before delivering it.
    > >>>> +If the packet checksum has been verified, the VIRTIO_NET_HDR_F_DATA_VALID bit
    > >>>> +in \field{flags} is set: in case of multiple encapsulated protocols, one
    > >>>> +level of checksums has been validated.
    > >>>> + \item The device can not set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags}.
    > >>>> +\end{itemize}
    > >>> I may miss something, but this seems the existing behaviour of the
    > >>> devices regardless of FULLY_CSUM?
    > >> Yes Jason.
    > >> I think that we have the fix from the first patch, our requirements have
    > >> been met.
    > >> But back to the second patch itself, on some devices, turning off
    > >> GUEST_CSUM offload
    > >> may mean turning off tx checksum offload,
    > > For tx did you mean guest? If yes, it should be HOST_CSUM I think.
    > >
    > > Or you actually mean the tx csum on the host?
    > >
    > >> but GUEST_FULLY_CSUM does not
    > >> have this requirement.
    > >> GRO_HW will also be turned off.
    > >>
    > >>>> +
    > >>>> +The packet types that the device recognizes and can verify are independent of
    > >>>> +whether the offload of VIRTIO_NET_F_GUEST_FULLY_CSUM is enabled.
    > >>> Right, so I don't see why this became part of this patch.
    > >>>
    > >>>> +
    > >>>> +Specific transport protocols that may have VIRTIO_NET_HDR_F_DATA_VALID set
    > >>>> +in \field{flags} include TCP, UDP, GRE (Generic Routing Encapsulation),
    > >>>> +and SCTP (Stream Control Transmission Protocol).
    > >>>> +A fully checksummed packet's checksum field for each of the above protocols
    > >>>> +is set to a calculated value that covers the transport header and payload
    > >>>> +(TCP or UDP involves the additional pseudo header) of the packet.
    > >>>> +
    > >>>> +Delivering fully checksummed packets rather than partially
    > >>>> +checksummed packets incurs additional overhead for the device.
    > >>>> +The overhead varies from device to device, for example the overhead of
    > >>>> +calculating and validating the packet checksum is a few microseconds
    > >>>> +for some hardware devices.
    > >>>> +
    > >>>> +The feature VIRTIO_NET_F_GUEST_FULLY_CSUM has a corresponding offload \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration},
    > >>>> +which when enabled means that the device delivers fully checksummed packets
    > >>>> +to the driver and may validate the checksum.
    > >>>> +\begin{note}
    > >>> This is the functional equivalent to disable GUEST_CSUM. So I guess
    > >>> what I'm missing is the difference between:
    > >>>
    > >>> 1) enable FULLY_CUSM
    > >>>
    > >>> and
    > >>>
    > >>> 2) disable GUEST_CSUM
    > >>>
    > >>> It looks to me 1) try to keep e.g HW_GRO (GUEST_TSOX) work? Note that
    > >>> GRO produce partial csum usually, not sure it can work correctly.
    > >> When GUEST_CSUM is turned off, GRO_HW offload will be turned off.
    > > It's the GUEST_TSO4/6 that is actually turned off. So this needs more thought:
    > >
    > > 1) Reuse GUEST_TSO4/6 but have a mode to have full checksum
    > > 2) New feature like GUEST_GRO, and claims that the packet boundary
    > > needs to be reserved there
    > >
    > > My understanding is, 1) may end up being somewhat ambiguous since TSO
    > > usually have partial checksum.
    > >
    > > What's your thought?
    >
    >
    > I think the reason why GUEST_TSO4/6 etc. rely on GUEST_CSUM is that the
    > overhead of segmenting and re-merging
    > packets can be saved between two vms on the same host.
    >
    > Currently, disabling GUEST_CSUM happens when XDP is loading, and XDP
    > does conflict with GUEST_TSO4/6.
    > Therefore, in this scenario, when we disable GUEST_CSUM, GUEST_TSO4/6
    > must also be disabled.
    >
    > In non-XDP scenarios, we don't need to disable GUEST_CSUM (and the
    > current Linux virtio driver does not provide a way to turn it off),
    > and they work well.
    >
    > Designing solutions such as (1)/(2) can indeed be one of the future
    > directions, but there does not seem to be a strong motivation(at least
    > in the Alibaba cloud scenario).
    >
    > Therefore, I plan to push only the first fix patch in the next version,
    > and fix the bug in Linux that \field{flags} with the DATA_VALID set is
    > ignored when XDP is loaded.
    >
    > Thanks a lot!

    That's fine.

    Thanks

    >
    > >
    > >> Meaning the hardware will not merge the packets.
    > > Right.
    > >
    > >> IIRC, after the packets have been merged, the packets should have full
    > >> checksums and have been verified:
    > >>
    > >> if (!(features & NETIF_F_RXCSUM)) {
    > >> /* NETIF_F_GRO_HW implies doing RXCSUM since every packet
    > >> * successfully merged by hardware must also have the
    > >> * checksum verified by hardware. If the user does not
    > >> * want to enable RXCSUM, logically, we should disable
    > >> GRO_HW.
    > >> */
    > >> if (features & NETIF_F_GRO_HW) {
    > >> netdev_dbg(dev, "Dropping NETIF_F_GRO_HW since
    > >> no RXCSUM feature.\n");
    > >> features &= ~NETIF_F_GRO_HW;
    > >> }
    > >> }
    > >>
    > > Thanks
    >