On Tue, Jun 25, 2019 at 03:57:24PM +0100, 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 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 <ktray...@redhat.com>
> ---
> Documentation/topics/dpdk/vhost-user.rst | 26 ++++++++++++++++++++++
> lib/netdev-dpdk.c | 28 +++++++++++++++++++++---
> vswitchd/vswitch.xml | 10 +++++++++
> 3 files changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/topics/dpdk/vhost-user.rst
> b/Documentation/topics/dpdk/vhost-user.rst
> index a25a64237..efee19b62 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -392,4 +392,30 @@ The default value is ``false``.
> .. _dpdk-testpmd:
>
> +vhost-user-client tx retries
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +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
> 31,
> +which would mean that after the first packet(s) in the batch was sent there
> +could be up to a separate retry for each of the remaining packets, until they
> +are all sent or no packet was sent during a retry.
> +
> +Retries can help with avoiding packet loss when temporarily unable to a vhost
... unable to <send> to a vhost interface?
> +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.
> +
> DPDK in the Guest
> -----------------
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 65161deaf..97a90d4a5 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -160,4 +160,5 @@ typedef uint16_t dpdk_port_t;
>
> #define VHOST_ENQ_RETRY_NUM 8
> +#define VHOST_MAX_ENQ_RETRY_NUM 31
> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
Maybe:
/* One retry for each packet in the batch */
#define VHOST_ENQ_RETRY_MAX ( NETDEV_MAX_BURST - 1 )
/* Minimum value to not retry */
#define VHOST_ENQ_RETRY_MIN 0
/* Arbitrary, debatable, though reasonable legacy value */
#define VHOST_ENQ_RETRY_DEF 8
and fix accordingly below...
>
> @@ -409,5 +410,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 +1242,6 @@ vhost_common_construct(struct netdev *netdev)
> }
>
> + atomic_store_relaxed(&dev->vhost_tx_retries, VHOST_ENQ_RETRY_NUM);
> +
> return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID,
> DPDK_DEV_VHOST, socket_id);
> @@ -1899,4 +1903,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 +1920,16 @@ netdev_dpdk_vhost_client_set_config(struct netdev
> *netdev,
> }
> }
> +
> + max_tx_retries = smap_get_int(args, "vhost-tx-retries",
> + VHOST_ENQ_RETRY_NUM);
> + if (max_tx_retries < 0 || max_tx_retries > VHOST_MAX_ENQ_RETRY_NUM) {
> + max_tx_retries = VHOST_ENQ_RETRY_NUM;
> + }
> + 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);
>
> @@ -2323,4 +2340,5 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int qid,
> int i, retries = 0;
> int vid = netdev_dpdk_get_vid(dev);
> + int max_retries = 0;
Perhaps declare retries and max_retries close to each other?
>
> qid = dev->tx_q[qid % netdev->n_txq].map;
> @@ -2351,9 +2369,12 @@ __netdev_dpdk_vhost_send(struct netdev *netdev, int
> qid,
> /* Prepare for possible retry.*/
> cur_pkts = &cur_pkts[tx_pkts];
> + if (OVS_UNLIKELY(cnt && !retries)) {
> + atomic_read_relaxed(&dev->vhost_tx_retries, &max_retries);
> + }
So, for vhost user the value is fixed to the default while vhost
user client we can tune. Here we only look at the value if there
are packets left to retry and it is the first time, ok.
> } 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 +2382,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);
Because retries is incremented even if it hasn't retried, ok.
> rte_spinlock_unlock(&dev->stats_lock);
>
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 69fce7ffb..2f47a2ceb 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": 31}'>
> + <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
> + interface. The max batch size is 32 packets.
> + Only supported by dpdkvhostuserclient interfaces.
> + </p>
> + </column>
> +
> <column name="options" key="n_rxq_desc"
> type='{"type": "integer", "minInteger": 1, "maxInteger":
> 4096}'>
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
