[ovs-dev] [RFC PATCH v1] net-dpdk: Introducing TX tcp HW checksum offload support for DPDK pnic

Chandran, Sugesh sugesh.chandran at intel.com
Mon Jun 19 09:26:41 UTC 2017


Hi Zhenyu,

Thank you for working on this,
I have couple of questions in this patch.

Regards
_Sugesh

> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of Zhenyu Gao
> Sent: Friday, June 16, 2017 1:54 PM
> To: blp at ovn.org; u9012063 at gmail.com; ktraynor at redhat.com; Kavanagh,
> Mark B <mark.b.kavanagh at intel.com>; dev at openvswitch.org
> Subject: [ovs-dev] [RFC PATCH v1] net-dpdk: Introducing TX tcp HW
> checksum offload support for DPDK pnic
> 
> This patch introduce TX tcp-checksum offload support for DPDK pnic.
> The feature is disabled by default and can be enabled by setting tx-
> checksum-offload, which like:
> ovs-vsctl set Interface dpdk-eth3 \
>  options:tx-checksum-offload=true
> ---
>  lib/netdev-dpdk.c    | 112
> +++++++++++++++++++++++++++++++++++++++++++++++----
>  vswitchd/vswitch.xml |  13 ++++--
>  2 files changed, 115 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bba4de3..5a68a48
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -32,6 +32,7 @@
>  #include <rte_mbuf.h>
>  #include <rte_meter.h>
>  #include <rte_virtio_net.h>
> +#include <rte_ip.h>
> 
>  #include "dirs.h"
>  #include "dp-packet.h"
> @@ -328,6 +329,7 @@ struct ingress_policer {
> 
>  enum dpdk_hw_ol_features {
>      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
> +    NETDEV_TX_CHECKSUM_OFFLOAD = 1 << 1,
>  };
> 
>  struct netdev_dpdk {
> @@ -649,6 +651,8 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev, int n_rxq, int n_txq)
>      int diag = 0;
>      int i;
>      struct rte_eth_conf conf = port_conf;
> +    struct rte_eth_txconf *txconf;
> +    struct rte_eth_dev_info dev_info;
> 
>      if (dev->mtu > ETHER_MTU) {
>          conf.rxmode.jumbo_frame = 1;
> @@ -676,9 +680,16 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev, int n_rxq, int n_txq)
>              break;
>          }
> 
> +        rte_eth_dev_info_get(dev->port_id, &dev_info);
> +        txconf = &dev_info.default_txconf;
> +        if (dev->hw_ol_features & NETDEV_TX_CHECKSUM_OFFLOAD) {
> +            /*Enable tx offload feature on pnic*/
> +            txconf->txq_flags = 0;
> +        }
> +
>          for (i = 0; i < n_txq; i++) {
>              diag = rte_eth_tx_queue_setup(dev->port_id, i, dev->txq_size,
> -                                          dev->socket_id, NULL);
> +                                          dev->socket_id, txconf);
>              if (diag) {
>                  VLOG_INFO("Interface %s txq(%d) setup error: %s",
>                            dev->up.name, i, rte_strerror(-diag)); @@ -724,11 +735,15 @@
> dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)  {
>      struct rte_eth_dev_info info;
>      bool rx_csum_ol_flag = false;
> +    bool tx_csum_ol_flag = false;
>      uint32_t rx_chksm_offload_capa = DEV_RX_OFFLOAD_UDP_CKSUM |
>                                       DEV_RX_OFFLOAD_TCP_CKSUM |
>                                       DEV_RX_OFFLOAD_IPV4_CKSUM;
> +    uint32_t tx_chksm_offload_capa = DEV_TX_OFFLOAD_TCP_CKSUM;
[Sugesh] Any reason, why this patch does only the TCP checksum offload?? The command line option says tx_checksum offload (it could be mistakenly considered for full checksum offload).

> +
>      rte_eth_dev_info_get(dev->port_id, &info);
>      rx_csum_ol_flag = (dev->hw_ol_features &
> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> +    tx_csum_ol_flag = (dev->hw_ol_features &
> + NETDEV_TX_CHECKSUM_OFFLOAD) != 0;
> 
>      if (rx_csum_ol_flag &&
>          (info.rx_offload_capa & rx_chksm_offload_capa) != @@ -736,9 +751,15
> @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
>          VLOG_WARN_ONCE("Rx checksum offload is not supported on device
> %"PRIu8,
>                         dev->port_id);
>          dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> -        return;
> +    } else if (tx_csum_ol_flag &&
> +               (info.tx_offload_capa & tx_chksm_offload_capa) !=
> +                tx_chksm_offload_capa) {
> +        VLOG_WARN_ONCE("Tx checksum offload is not supported on device
> %"PRIu8,
> +                       dev->port_id);
> +        dev->hw_ol_features &= ~NETDEV_TX_CHECKSUM_OFFLOAD;
> +    } else {
> +        netdev_request_reconfigure(&dev->up);
>      }
> -    netdev_request_reconfigure(&dev->up);
>  }
> 
> --

[Sugesh] What is the performance improvement offered with this feature? Do you have any numbers to share?
I think DPDK uses non-vector functions when Tx checksum offload is enabled. Will it give enough performance improvement to mitigate that cost?

Finally Rx checksum offload is going to be a default option (there wont be any configuration option to enable/disable, Kevin's patch for the support is already acked and waiting to merge).  Similarly can't we enable it by default when it is supported?


  
> 1.8.3.1

> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list