[ovs-dev] [PATCH] netdev-dpdk: New option 'hw_strip_crc' fields for DPDK interfaces
Chandran, Sugesh
sugesh.chandran at intel.com
Fri Oct 13 10:28:46 UTC 2017
Hi Wenxu,
Thank you for the patch.
I had reviewed the patch and I have same comment as Zhenyu.
As you said there are so many other configs. Its not a good practice to offer configuration choices for some of them.
I don’t think user see any difference in performance/functionality either its enabled/disabled.
My understanding is, this will let user to reconfigure the port again with hw_strip_crc when they see a failure.
Instead of having an option for user. How about check the support first and enable/disable accordingly. I think DPDK doesn’t provide this information in its rte_eth_dev_info now. However its worth to check the possibility in DPDK to report hw_crc_strip feature support with other flags.
It looks similar to how rx checksum offload is being handled in OVS-DPDK.
Also if all the DPDK NICs can support the hw_crc_strip, it make sense to keep it enable as a default choice.
What do you think?
Regards
_Sugesh
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of wenxu
> Sent: Tuesday, October 10, 2017 3:33 AM
> To: Gao Zhenyu <sysugaozhenyu at gmail.com>
> Cc: ovs dev <dev at openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: New option 'hw_strip_crc' fields
> for DPDK interfaces
>
> Hi Zhenyu,
>
>
> I think add a option maybe better.
>
>
> 1. It keep unchange the action with default value is false 2. It can be try one
> more time but if there some other different config, there should be more times
> to try 3. For some user who want to enable this feature, it can be configurable
>
>
>
>
> 在 2017-10-09 21:13:12，"Gao Zhenyu" <sysugaozhenyu at gmail.com> 写道：
>
> Thanks for working on it.
>
>
> I want to know if we can have a patch to try one more time if we hit the
> hw_strip_crc error since we had introduce many options in ovs-dpdk now. (Just
> like function dpdk_eth_dev_queue_setup, it figures out queue number by
> retrying)
>
>
> Thanks
> Zhenyu Gao
>
>
> 2017-10-09 17:09 GMT+08:00 <wenxu at ucloud.cn>:
> From: wenxu <wenxu at ucloud.cn>
>
> Some vf driver like i40evf can't disable the hw_strip_crc, with err
> 'dpdk|ERR|i40evf_dev_configure(): VF can't disable HW CRC Strip'
> so hw_strip_crc feature should can be configurable
>
> Signed-off-by: wenxu <wenxu at ucloud.cn>
> ---
> NEWS | 2 ++
> lib/netdev-dpdk.c | 30 ++++++++++++++++++++++++++++++
> vswitchd/vswitch.xml | 9 +++++++++
> 3 files changed, 41 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index a3c1a02..d913847 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -5,6 +5,8 @@ Post-v2.8.0
> chassis "hostname" in addition to a chassis "name".
> - Linux kernel 4.13
> * Add support for compiling OVS with the latest Linux 4.13 kernel
> + - DPDK:
> + * New option 'hw_strip_crc' fields for DPDK interfaces
>
> v2.8.0 - xx xxx xxxx
> ---------------------
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index c60f46f..23c2a72
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -402,11 +402,15 @@ struct netdev_dpdk {
> int requested_n_rxq;
> int requested_rxq_size;
> int requested_txq_size;
> + bool requested_hw_strip_crc;
>
> /* Number of rx/tx descriptors for physical devices */
> int rxq_size;
> int txq_size;
>
> + /*crc strip feature config*/
> + bool hw_strip_crc;
> +
> /* Socket ID detected when vHost device is brought up */
> int requested_socket_id;
>
> @@ -700,6 +704,7 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
> int n_rxq, int n_txq)
>
> conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> + conf.rxmode.hw_strip_crc = dev->hw_strip_crc;
> /* A device may report more queues than it makes available (this has
> * been observed for Intel xl710, which reserves some of them for
> * SRIOV): rte_eth_*_queue_setup will fail if a queue is not @@ -914,6
> +919,8 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no,
> dev->requested_n_txq = NR_QUEUE;
> dev->requested_rxq_size = NIC_PORT_DEFAULT_RXQ_SIZE;
> dev->requested_txq_size = NIC_PORT_DEFAULT_TXQ_SIZE;
> + dev->requested_hw_strip_crc = false;
> + dev->hw_strip_crc = false;
>
> /* Initialize the flow control to NULL */
> memset(&dev->fc_conf, 0, sizeof dev->fc_conf); @@ -1192,6 +1199,11 @@
> netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args)
> } else {
> smap_add(args, "rx_csum_offload", "false");
> }
> + if (dev->hw_strip_crc) {
> + smap_add(args, "hw_strip_crc", "true");
> + } else {
> + smap_add(args, "hw_strip_crc", "false");
> + }
> }
> ovs_mutex_unlock(&dev->mutex);
>
> @@ -1255,6 +1267,19 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev,
> const struct smap *args) }
>
> static void
> +dpdk_set_hw_strip_crc_config(struct netdev_dpdk *dev, const struct smap
> *args)
> + OVS_REQUIRES(dev->mutex)
> +{
> + int new_hw_strip_crc;
> +
> + new_hw_strip_crc = smap_get_bool(args, "hw_strip_crc", false);
> + if (new_hw_strip_crc != dev->requested_hw_strip_crc) {
> + dev->requested_hw_strip_crc = new_hw_strip_crc;
> + netdev_request_reconfigure(&dev->up);
> + }
> +}
> +
> +static void
> dpdk_process_queue_size(struct netdev *netdev, const struct smap *args,
> const char *flag, int default_size, int *new_size) { @@ -1290,6
> +1315,8 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap
> *args,
>
> dpdk_set_rxq_config(dev, args);
>
> + dpdk_set_hw_strip_crc_config(dev, args);
> +
> dpdk_process_queue_size(netdev, args, "n_rxq_desc",
> NIC_PORT_DEFAULT_RXQ_SIZE,
> &dev->requested_rxq_size); @@ -3196,6 +3223,7 @@
> netdev_dpdk_reconfigure(struct netdev *netdev)
> if (netdev->n_txq == dev->requested_n_txq
> && netdev->n_rxq == dev->requested_n_rxq
> && dev->mtu == dev->requested_mtu
> + && dev->hw_strip_crc == dev->requested_hw_strip_crc
> && dev->rxq_size == dev->requested_rxq_size
> && dev->txq_size == dev->requested_txq_size
> && dev->socket_id == dev->requested_socket_id) { @@ -3217,6 +3245,8
> @@ netdev_dpdk_reconfigure(struct netdev *netdev)
> dev->rxq_size = dev->requested_rxq_size;
> dev->txq_size = dev->requested_txq_size;
>
> + dev->hw_strip_crc = dev->requested_hw_strip_crc;
> +
> rte_free(dev->tx_q);
> err = dpdk_eth_dev_init(dev);
> dev->tx_q = netdev_dpdk_alloc_txq(netdev->n_txq);
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index
> 074535b..d1e7a6b 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -2589,6 +2589,15 @@
> </p>
> </column>
>
> + <column name="options" key="hw_strip_crc"
> + type='{"type": "bool"}'>
> + <p>
> + Specifies DPDK interfaces whether enable hw_strip_crc feature or not.
> + If not specified, the default is false.
> + Not supported by DPDK vHost interfaces.
> + </p>
> + </column>
> +
> <column name="other_config" key="pmd-rxq-affinity">
> <p>Specifies mapping of RX queues of this interface to CPU cores.</p>
> <p>Value should be set in the following form:</p>
> --
> 1.8.3.1
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list