[ovs-dev] [PATCH] netdev-dpdk: New option 'hw_strip_crc' fields for DPDK interfaces

wenxu wenx05124561 at 163.com
Sat Oct 14 02:33:21 UTC 2017



Hi Sugesh, 


I agree with you !  If there is hw_strip_crc info in the rte_eth_dev_info, it can be configured dynamic. If all the DPDK NICs can support the hw_crc_strip, it can be enable default. Maybe this can be check by DPDK guys?







At 2017-10-13 18:28:46, "Chandran, Sugesh" <sugesh.chandran at intel.com> wrote:
>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
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list