[ovs-dev] [PATCH v4 3/3] netdev-dpdk: Enable vhost-tx-retries config.
Eelco Chaudron
echaudro at redhat.com
Thu Jul 4 10:16:13 UTC 2019
On 3 Jul 2019, at 18:07, Ian Stokes wrote:
> On 7/2/2019 1:32 AM, 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.
>>
>
> Thanks for the patch Kevin. Tests ok for me. I realize your on leave
> but there's a few minor comments below. If there are no objection by
> those who have either acked or are reviewing I can add them on commit.
Your two suggested changes are fine by me!
>> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
>> Acked-by: Eelco Chaudron <echaudro at redhat.com>
>> Acked-by: Flavio Leitner <fbl at sysclose.org>
>> ---
>> Documentation/topics/dpdk/vhost-user.rst | 28 +++++++++++++++++
>> lib/netdev-dpdk.c | 39
>> +++++++++++++++++++++---
>> vswitchd/vswitch.xml | 12 ++++++++
>
> Probably a minor entry in NEWS for this additional feature needed
> also. As Kevin is on leave this can be added on commit if there is no
> objection.
>
>> 3 files changed, 75 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/topics/dpdk/vhost-user.rst
>> b/Documentation/topics/dpdk/vhost-user.rst
>> index 368f44520..724aa62f6 100644
>> --- a/Documentation/topics/dpdk/vhost-user.rst
>> +++ b/Documentation/topics/dpdk/vhost-user.rst
>> @@ -351,4 +351,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 ``tx-retries-max``.
>> +
>> +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 max can be set for vhost-user-client ports::
>> +
>> + $ ovs-vsctl set Interface vhost-client-1
>> options:tx-retries-max=0
>
> Minor, the commit message enables vhost-tx-retries, but the config
> option is tx-retries-max. I think vhost-tx-retries comes from the
> previous revision of the patch. This can be changed on commit if there
> is no objection.
>
> Regards
> Ian
>
>> +
>> +.. note::
>> +
>> + Configurable vhost tx retries are not supported with vhost-user
>> ports.
>> +
>> DPDK in the Guest
>> -----------------
>> @@ -496,4 +521,7 @@ packets can be sent, it may mean the guest is not
>> accepting packets, so there
>> are no (more) retries.
>> +For information about configuring the maximum amount of tx retries
>> for
>> +vhost-user-client interfaces see `vhost-user-client tx retries
>> config`_.
>> +
>> .. note::
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>> index d3e02d389..b8592962f 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -165,5 +165,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)
>> @@ -418,5 +423,7 @@ 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_max;
>> + /* 2 pad bytes here. */
>> );
>> @@ -1262,4 +1269,6 @@ vhost_common_construct(struct netdev *netdev)
>> }
>> + atomic_store_relaxed(&dev->vhost_tx_retries_max,
>> VHOST_ENQ_RETRY_DEF);
>> +
>> return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
>> DPDK_DEV_VHOST, socket_id);
>> @@ -1922,4 +1931,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);
>> @@ -1938,4 +1948,17 @@ netdev_dpdk_vhost_client_set_config(struct
>> netdev *netdev,
>> }
>> }
>> +
>> + max_tx_retries = smap_get_int(args, "tx-retries-max",
>> + 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_max,
>> &cur_max_tx_retries);
>> + if (max_tx_retries != cur_max_tx_retries) {
>> + atomic_store_relaxed(&dev->vhost_tx_retries_max,
>> 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);
>> @@ -2351,4 +2374,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);
>> @@ -2380,9 +2404,16 @@ __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 as there are packets not sent
>> + * and no retries have already occurred.
>> + */
>> + atomic_read_relaxed(&dev->vhost_tx_retries_max,
>> &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);
>> @@ -2391,5 +2422,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev,
>> int qid,
>> netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts,
>> total_pkts,
>> cnt + dropped);
>> - dev->tx_retries += MIN(retries, VHOST_ENQ_RETRY_NUM);
>> + dev->tx_retries += MIN(retries, max_retries);
>> rte_spinlock_unlock(&dev->stats_lock);
>> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
>> index 69fce7ffb..5b4c13b64 100644
>> --- a/vswitchd/vswitch.xml
>> +++ b/vswitchd/vswitch.xml
>> @@ -3120,4 +3120,16 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
>> type=patch options:peer=p1 \
>> </column>
>> + <column name="options" key="tx-retries-max"
>> + type='{"type": "integer", "minInteger": 0,
>> "maxInteger": 32}'>
>> + <p>
>> + The value specifies the maximum amount of vhost tx retries
>> that can
>> + be made while trying to send a batch of packets to an
>> interface.
>> + Only supported by dpdkvhostuserclient interfaces.
>> + </p>
>> + <p>
>> + Default value is 8.
>> + </p>
>> + </column>
>> +
>> <column name="options" key="n_rxq_desc"
>> type='{"type": "integer", "minInteger": 1,
>> "maxInteger": 4096}'>
>>
More information about the dev
mailing list