On 5/29/23 21:56, Mike Pattrick wrote:
> On Wed, May 24, 2023 at 8:56 AM Ilya Maximets <[email protected]> wrote:
>>
>> On 5/17/23 05:11, Mike Pattrick wrote:
>>> From: Flavio Leitner <[email protected]>
>>>
>>> The netdev receiving packets is supposed to provide the flags
>>> indicating if the L4 checksum was verified and it is OK or BAD,
>>> otherwise the stack will check when appropriate by software.
>>>
>>> If the packet comes with good checksum, then postpone the
>>> checksum calculation to the egress device if needed.
>>>
>>> When encapsulate a packet with that flag, set the checksum
>>> of the inner L4 header since that is not yet supported.
>>>
>>> Calculate the L4 checksum when the packet is going to be sent
>>> over a device that doesn't support the feature.
>>>
>>> Linux tap devices allows enabling L3 and L4 offload, so this
>>> patch enables the feature. However, Linux socket interface
>>> remains disabled because the API doesn't allow enabling
>>> those two features without enabling TSO too.
>>>
>>> Signed-off-by: Flavio Leitner <[email protected]>
>>> Co-authored-by: Mike Pattrick <[email protected]>
>>> Signed-off-by: Mike Pattrick <[email protected]>
>>>
>>> ---
>>>  Since v9:
>>>   - Extended miniflow_extract changes into avx512 code
>>>   - Formatting changes
>>>   - Note that we cannot currently enable checksum offloading in
>>>     CONFIGURE_VETH_OFFLOADS for check-system-userspace as
>>>     netdev-linux.c currently only parses the vnet header if TSO
>>>     is enabled.
>>>  Since v10:
>>>   - No change
>>>  Since v11:
>>>   - Added AVX512 IPv6 checksum offload support.
>>>   - Improved error messages and logging.
>>>  Since v12:
>>>   - Added missing mutex annotations
>>>
>>> Signed-off-by: Mike Pattrick <[email protected]>
>>> ---
>>>  lib/conntrack.c                  |  15 +-
>>>  lib/dp-packet.c                  |  25 ++++
>>>  lib/dp-packet.h                  |  78 +++++++++-
>>>  lib/dpif-netdev-extract-avx512.c |  62 +++++++-
>>>  lib/flow.c                       |  23 +++
>>>  lib/netdev-dpdk.c                | 176 +++++++++++++++-------
>>>  lib/netdev-linux.c               | 243 +++++++++++++++++++++----------
>>>  lib/netdev-native-tnl.c          |  32 +---
>>>  lib/netdev.c                     |  46 ++----
>>>  lib/odp-execute-avx512.c         |  88 ++++++-----
>>>  lib/packets.c                    | 175 +++++++++++++++++-----
>>>  lib/packets.h                    |   3 +
>>>  12 files changed, 688 insertions(+), 278 deletions(-)
>>>
>>
>> <snip>
>>
>>> @@ -1403,7 +1409,6 @@ static int
>>>  netdev_linux_batch_rxq_recv_tap(struct netdev_rxq_linux *rx, int mtu,
>>>                                  struct dp_packet_batch *batch)
>>>  {
>>> -    int virtio_net_hdr_size;
>>>      ssize_t retval;
>>>      size_t std_len;
>>>      int iovlen;
>>> @@ -1413,16 +1418,14 @@ netdev_linux_batch_rxq_recv_tap(struct 
>>> netdev_rxq_linux *rx, int mtu,
>>>          /* Use the buffer from the allocated packet below to receive MTU
>>>           * sized packets and an aux_buf for extra TSO data. */
>>>          iovlen = IOV_TSO_SIZE;
>>> -        virtio_net_hdr_size = sizeof(struct virtio_net_hdr);
>>>      } else {
>>>          /* Use only the buffer from the allocated packet. */
>>>          iovlen = IOV_STD_SIZE;
>>> -        virtio_net_hdr_size = 0;
>>>      }
>>>
>>>      /* The length here needs to be accounted in the same way when the
>>>       * aux_buf is allocated so that it can be prepended to TSO buffer. */
>>> -    std_len = virtio_net_hdr_size + VLAN_ETH_HEADER_LEN + mtu;
>>> +    std_len = sizeof(struct virtio_net_hdr) + VLAN_ETH_HEADER_LEN + mtu;
>>>      for (i = 0; i < NETDEV_MAX_BURST; i++) {
>>>          struct dp_packet *buffer;
>>>          struct dp_packet *pkt;
>>> @@ -1462,7 +1465,7 @@ netdev_linux_batch_rxq_recv_tap(struct 
>>> netdev_rxq_linux *rx, int mtu,
>>>              pkt = buffer;
>>>          }
>>>
>>> -        if (virtio_net_hdr_size && netdev_linux_parse_vnet_hdr(pkt)) {
>>> +        if (netdev_linux_parse_vnet_hdr(pkt)) {
>>
>> If TUNSETOFFLOAD failed, we will not have a vnet header, right?
> 
> The vnet header is linked to TUNSETIFF not TUNSETOFFLOAD. Support for
> this was added in 2.6.34 (Feb 2010):
> https://github.com/torvalds/linux/commit/b9fb9ee07e67fce0b7bfd517a48710465706c30a#diff-75b86939050dfaa1707f33042b068bd819e6cd34efae37535046e648a4ecd413R529
> 
> That said, I notice the releases FAQ says:
> 
> "Open vSwitch userspace is not sensitive to the Linux kernel version.
> It should build against almost any kernel, certainly against 2.6.32
> and later."
> 
> This statement is probably still true, it might be able to build but
> wouldn't run properly if a userspace tap port was used. I'll update
> that line in the documentation to reflect this.

Another option is to check if the kernel supports VNET_HDR with
TUNGETFEATURES and not use it if not supported.  TUNGETFEATURES
is available since 2.6.27.

> 
> 
> Thanks,
> M
> 
> N.B. For posterity, and because I tracked it down anyways, the support
> for TUNSETOFFLOAD was added in 3.11 in 2013
> https://github.com/torvalds/linux/commit/2be5c76794b0e570aa87b012df5ac864ce668a74
> 
>>
>>>              struct netdev *netdev_ = netdev_rxq_get_netdev(&rx->up);
>>>              struct netdev_linux *netdev = netdev_linux_cast(netdev_);
>>>
>>> @@ -1611,7 +1614,7 @@ netdev_linux_sock_batch_send(int sock, int ifindex, 
>>> bool tso, int mtu,
>>>   * on other interface types because we attach a socket filter to the rx
>>>   * socket. */
>>>  static int
>>> -netdev_linux_tap_batch_send(struct netdev *netdev_, bool tso, int mtu,
>>> +netdev_linux_tap_batch_send(struct netdev *netdev_, int mtu,
>>>                              struct dp_packet_batch *batch)
>>>  {
>>>      struct netdev_linux *netdev = netdev_linux_cast(netdev_);
>>> @@ -1632,9 +1635,7 @@ netdev_linux_tap_batch_send(struct netdev *netdev_, 
>>> bool tso, int mtu,
>>>          ssize_t retval;
>>>          int error;
>>>
>>> -        if (tso) {
>>> -            netdev_linux_prepend_vnet_hdr(packet, mtu);
>>> -        }
>>> +        netdev_linux_prepend_vnet_hdr(packet, mtu);
>>
>> Is it allowed to add vnet header if TUNSETOFFLOAD failed?
>>
>>>
>>>          size = dp_packet_size(packet);
>>>          do {
>>> @@ -1765,7 +1766,7 @@ netdev_linux_send(struct netdev *netdev_, int qid 
>>> OVS_UNUSED,
>>>
>>>          error = netdev_linux_sock_batch_send(sock, ifindex, tso, mtu, 
>>> batch);
>>>      } else {
>>> -        error = netdev_linux_tap_batch_send(netdev_, tso, mtu, batch);
>>> +        error = netdev_linux_tap_batch_send(netdev_, mtu, batch);
>>>      }
>>>      if (error) {
>>>          if (error == ENOBUFS) {
>>
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to