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 <ktray...@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}'> >> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev