[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