[ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.

Kevin Traynor ktraynor at redhat.com
Fri Jun 28 15:01:52 UTC 2019


On 28/06/2019 13:34, Ilya Maximets wrote:
> On 27.06.2019 14:12, Kevin Traynor wrote:
>> vhost tx retries can provide some mitigation against
>> dropped packets due to a temporarily slow guest/limited queue
>> size for an interface, but on the other hand when a system
>> is fully loaded those extra cycles retrying could mean
>> packets are dropped elsewhere.
>>
>> Up to now max vhost tx retries have been hardcoded, which meant
>> no tuning and no way to disable for debugging to see if extra
>> cycles spent retrying resulted in rx drops on some other
>> interface.
>>
>> Add an option to change the max retries, with a value of
>> 0 effectively disabling vhost tx retries.
>>
>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>> ---
>>  Documentation/topics/dpdk/vhost-user.rst | 25 ++++++++++++++++
>>  lib/netdev-dpdk.c                        | 36 +++++++++++++++++++++---
>>  vswitchd/vswitch.xml                     | 10 +++++++
>>  3 files changed, 67 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/topics/dpdk/vhost-user.rst b/Documentation/topics/dpdk/vhost-user.rst
>> index 3caa88231..d8508d8f8 100644
>> --- a/Documentation/topics/dpdk/vhost-user.rst
>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>> @@ -392,4 +392,29 @@ The default value is ``false``.
>>  .. _dpdk-testpmd:
>>  
>> +vhost-user-client tx retries config
>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> +
>> +For vhost-user-client interfaces, the max amount of retries can be changed from
>> +the default 8 by setting ``vhost-tx-retries``.
>> +
>> +The minimum is 0 which means there will be no retries and if any packets in
>> +each batch cannot be sent immediately they will be dropped. The maximum is 32,
>> +which would mean that after the first packet(s) in the batch was sent there
>> +could be a maximum of 32 more retries.
>> +
>> +Retries can help with avoiding packet loss when temporarily unable to send to a
>> +vhost interface because the virtqueue is full. However, spending more time
>> +retrying to send to one interface, will reduce the time available for rx/tx and
>> +processing packets on other interfaces, so some tuning may be required for best
>> +performance.
>> +
>> +Tx retries can be set for vhost-user-client ports::
>> +
>> +    $ ovs-vsctl set Interface vhost-client-1 options:vhost-tx-retries=0
>> +
>> +.. note::
>> +
>> + Configurable vhost tx retries are not supported with vhost-user ports.
> 
> Please, use at least 2 spaces for 'note' section indentation.
> 

sure

>> +
>>  DPDK in the Guest
>>  -----------------
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index 65161deaf..2befbf9a7 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -159,5 +159,10 @@ typedef uint16_t dpdk_port_t;
>>  #define DPDK_PORT_ID_FMT "%"PRIu16
>>  
>> -#define VHOST_ENQ_RETRY_NUM 8
>> +/* Minimum amount of vhost tx retries, effectively a disable. */
>> +#define VHOST_ENQ_RETRY_MIN 0
>> +/* Maximum amount of vhost tx retries. */
>> +#define VHOST_ENQ_RETRY_MAX 32
>> +/* Legacy default value for vhost tx retries. */
>> +#define VHOST_ENQ_RETRY_DEF 8
>>  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
>>  
>> @@ -409,5 +414,6 @@ struct netdev_dpdk {
>>          /* True if vHost device is 'up' and has been reconfigured at least once */
>>          bool vhost_reconfigured;
>> -        /* 3 pad bytes here. */
>> +        atomic_uint8_t vhost_tx_retries;
>> +        /* 2 pad bytes here. */
>>      );
>>  
>> @@ -1240,4 +1246,6 @@ vhost_common_construct(struct netdev *netdev)
>>      }
>>  
>> +    atomic_store_relaxed(&dev->vhost_tx_retries, VHOST_ENQ_RETRY_DEF);
>> +
>>      return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
>>                              DPDK_DEV_VHOST, socket_id);
>> @@ -1899,4 +1907,5 @@ netdev_dpdk_vhost_client_set_config(struct netdev *netdev,
>>      struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>>      const char *path;
>> +    int max_tx_retries, cur_max_tx_retries;
>>  
>>      ovs_mutex_lock(&dev->mutex);
>> @@ -1915,4 +1924,17 @@ netdev_dpdk_vhost_client_set_config(struct netdev *netdev,
>>          }
>>      }
>> +
>> +    max_tx_retries = smap_get_int(args, "vhost-tx-retries",
>> +                                  VHOST_ENQ_RETRY_DEF);
>> +    if (max_tx_retries < VHOST_ENQ_RETRY_MIN
>> +        || max_tx_retries > VHOST_ENQ_RETRY_MAX) {
>> +        max_tx_retries = VHOST_ENQ_RETRY_DEF;
>> +    }
>> +    atomic_read_relaxed(&dev->vhost_tx_retries, &cur_max_tx_retries);
>> +    if (max_tx_retries != cur_max_tx_retries) {
>> +        atomic_store_relaxed(&dev->vhost_tx_retries, max_tx_retries);
>> +        VLOG_INFO("Max Tx retries for vhost device '%s' set to %d",
>> +                  dev->up.name, max_tx_retries);
>> +    }
>>      ovs_mutex_unlock(&dev->mutex);
>>  
>> @@ -2322,4 +2344,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>      unsigned int dropped = 0;
>>      int i, retries = 0;
>> +    int max_retries = VHOST_ENQ_RETRY_MIN;
>>      int vid = netdev_dpdk_get_vid(dev);
>>  
>> @@ -2351,9 +2374,13 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>              /* Prepare for possible retry.*/
>>              cur_pkts = &cur_pkts[tx_pkts];
>> +            if (OVS_UNLIKELY(cnt && !retries)) {
>> +                /* Read max_retries before there may be some. */
>> +                atomic_read_relaxed(&dev->vhost_tx_retries, &max_retries);
>> +            }
>>          } else {
>>              /* No packets sent - do not retry.*/
>>              break;
>>          }
>> -    } while (cnt && (retries++ < VHOST_ENQ_RETRY_NUM));
>> +    } while (cnt && (retries++ < max_retries));
>>  
>>      rte_spinlock_unlock(&dev->tx_q[qid].tx_lock);
>> @@ -2361,5 +2388,6 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
>>      rte_spinlock_lock(&dev->stats_lock);
>>      netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts,
>> -                                         cnt + dropped, retries);
>> +                                         cnt + dropped,
>> +                                         max_retries ? retries : 0);
>>      rte_spinlock_unlock(&dev->stats_lock);
>>  
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 69fce7ffb..8353607ba 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -3120,4 +3120,14 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>>        </column>
>>  
>> +      <column name="options" key="vhost-tx-retries"
>> +              type='{"type": "integer", "minInteger": 0, "maxInteger": 32}'>
>> +        <p>
>> +          The value specifies the maximum amount of tx vhost retries that can
>> +          be made while trying to send a batch of packets to a dpdkvhostclient
> 
> s/dpdkvhostclient/dpdkvhostuserclient/ or /vhost-user/
> 

nice catch, thanks

>> +          interface.
>> +          Only supported by dpdkvhostuserclient interfaces.
> 
> Default value should be specified here.
> 

will add

>> +        </p>
>> +      </column>
>> +
>>        <column name="options" key="n_rxq_desc"
>>                type='{"type": "integer", "minInteger": 1, "maxInteger": 4096}'>
>>



More information about the dev mailing list