[ovs-dev] [PATCHv9 1/2] userspace: Enable TSO support for non-DPDK.

[William Tu]

On Thu, May 14, 2020 at 01:42:21PM +0200, Ilya Maximets wrote:
> On 3/24/20 11:10 PM, William Tu wrote:
> > This patch enables TSO support for non-DPDK use cases, and
> > also add check-system-tso testsuite. Before TSO, we have to
> > disable checksum offload, allowing the kernel to calculate the
> > TCP/UDP packet checsum. With TSO, we can skip the checksum
> > validation by enabling checksum offload, and with large packet
> > size, we see better performance.
> > 
> > Consider container to container use cases:
> >   iperf3 -c (ns0) -> veth peer -> OVS -> veth peer -> iperf3 -s (ns1)
> > And I got around 6Gbps, similar to TSO with DPDK-enabled.
> > 
> > Signed-off-by: William Tu <u9012063 at gmail.com>
> > Acked-by: Flavio Leitner <fbl at sysclose.org>
> > 
> 
> This version looks good except the '(DPDK doesn't support)' comment.
> It's misleading.  DPDK supports partial offloading.
> 
> And a couple of minor style/grammar suggestions:
> ---
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index 4c127e759..0430cca8e 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -47,6 +47,7 @@ enum OVS_PACKED_ENUM dp_packet_source {
>  };
>  
>  #define DP_PACKET_CONTEXT_SIZE 64
> +
>  #ifdef DPDK_NETDEV
>  #define DEF_OL_FLAG(NAME, DPDK_DEF, GENERIC_DEF) NAME = DPDK_DEF
>  #else
> @@ -58,7 +59,7 @@ enum dp_packet_offload_mask {
>      /* Value 0 is not used. */
>      /* Is the 'rss_hash' valid? */
>      DEF_OL_FLAG(DP_PACKET_OL_RSS_HASH, PKT_RX_RSS_HASH, 0x1),
> -    /* Is the 'flow_mark' valid? (DPDK does not support) */
> +    /* Is the 'flow_mark' valid? */
>      DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, PKT_RX_FDIR_ID, 0x2),
>      /* Bad L4 checksum in the packet. */
>      DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, PKT_RX_L4_CKSUM_BAD, 0x4),
> @@ -80,20 +81,20 @@ enum dp_packet_offload_mask {
>      DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CKSUM, PKT_TX_UDP_CKSUM, 0x400),
>      /* Offload SCTP checksum. */
>      DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, PKT_TX_SCTP_CKSUM, 0x800),
> -    /* Adding new field requres adding to DP_PACKET_OL_SUPPORTED_MASK */
> +    /* Adding new field requires adding to DP_PACKET_OL_SUPPORTED_MASK. */
>  };
>  
> -#define DP_PACKET_OL_SUPPORTED_MASK (DP_PACKET_OL_RSS_HASH  |       \
> -                                     DP_PACKET_OL_FLOW_MARK |       \
> -                                     DP_PACKET_OL_RX_L4_CKSUM_BAD | \
> -                                     DP_PACKET_OL_RX_IP_CKSUM_BAD | \
> -                                     DP_PACKET_OL_RX_L4_CKSUM_GOOD| \
> -                                     DP_PACKET_OL_RX_IP_CKSUM_GOOD| \
> -                                     DP_PACKET_OL_TX_TCP_SEG |      \
> -                                     DP_PACKET_OL_TX_IPV4 |         \
> -                                     DP_PACKET_OL_TX_IPV6 |         \
> -                                     DP_PACKET_OL_TX_TCP_CKSUM |    \
> -                                     DP_PACKET_OL_TX_UDP_CKSUM |    \
> +#define DP_PACKET_OL_SUPPORTED_MASK (DP_PACKET_OL_RSS_HASH         | \
> +                                     DP_PACKET_OL_FLOW_MARK        | \
> +                                     DP_PACKET_OL_RX_L4_CKSUM_BAD  | \
> +                                     DP_PACKET_OL_RX_IP_CKSUM_BAD  | \
> +                                     DP_PACKET_OL_RX_L4_CKSUM_GOOD | \
> +                                     DP_PACKET_OL_RX_IP_CKSUM_GOOD | \
> +                                     DP_PACKET_OL_TX_TCP_SEG       | \
> +                                     DP_PACKET_OL_TX_IPV4          | \
> +                                     DP_PACKET_OL_TX_IPV6          | \
> +                                     DP_PACKET_OL_TX_TCP_CKSUM     | \
> +                                     DP_PACKET_OL_TX_UDP_CKSUM     | \
>                                       DP_PACKET_OL_TX_SCTP_CKSUM)
>  
>  #define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \
> @@ -888,7 +889,7 @@ dp_packet_batch_reset_cutlen(struct dp_packet_batch *batch)
>  }
>  
>  /* Returns the RSS hash of the packet 'p'.  Note that the returned value is
> - * correct only if 'dp_packet_rss_valid(p)' returns true */
> + * correct only if 'dp_packet_rss_valid(p)' returns 'true'. */
>  static inline uint32_t
>  dp_packet_get_rss_hash(const struct dp_packet *p)
>  {
> ---
> 
> 
> With above diff applied for the series:
> Acked-by: Ilya Maximets <i.maximets at ovn.org>

Applied to master, thanks!