Re: [ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.
On 28/06/2019 15:39, Ian Stokes wrote: > On 6/27/2019 12:12 PM, 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 >> --- >> 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: > I'm still testing this but a minor suggestion below. > > in patch 2 of the series you introduce a section 'vhost tx retries' and > you specify the behavior and number of default retries. It would be nice > to link that section to this section below to flag to users that the > default can be changed. It should make navigation easy between the two > points as currently they are separated in the doc by other configuration > sections. > > Something like > > --- a/Documentation/topics/dpdk/vhost-user.rst > +++ b/Documentation/topics/dpdk/vhost-user.rst > @@ -82,8 +82,10 @@ When sending a batch of packets to a vhost-user or > vhost-user-client interface, > it may happen that some but not all of the packets in the batch are > able to be > sent to the guest. This is often because there is not enough free > descriptors > in the virtqueue for all the packets in the batch to be sent. In this case > -there will be a retry, with a default maximum of 8 occurring. If at any > time no > -packets can be sent, it may mean the guest is not accepting packets, so > there > +there will be a retry, with a default maximum of 8 occurring, for > information > +regarding how this can be configured please refer to > +`vhost-user-client tx retries config`_. If at any time no packets can > be sent, > +it may mean the guest is not accepting packets, so there > are no (more) retries. > sure, I can add some sort of link between the sections > Ian > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.
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
>> ---
>> 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(>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(>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(>vhost_tx_retries, _max_tx_retries);
>> +if (max_tx_retries != cur_max_tx_retries) {
>> +atomic_store_relaxed(>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(>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
Re: [ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.
On 6/27/2019 12:12 PM, 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
---
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:
I'm still testing this but a minor suggestion below.
in patch 2 of the series you introduce a section 'vhost tx retries' and
you specify the behavior and number of default retries. It would be nice
to link that section to this section below to flag to users that the
default can be changed. It should make navigation easy between the two
points as currently they are separated in the doc by other configuration
sections.
Something like
--- a/Documentation/topics/dpdk/vhost-user.rst
+++ b/Documentation/topics/dpdk/vhost-user.rst
@@ -82,8 +82,10 @@ When sending a batch of packets to a vhost-user or
vhost-user-client interface,
it may happen that some but not all of the packets in the batch are
able to be
sent to the guest. This is often because there is not enough free
descriptors
in the virtqueue for all the packets in the batch to be sent. In this case
-there will be a retry, with a default maximum of 8 occurring. If at any
time no
-packets can be sent, it may mean the guest is not accepting packets, so
there
+there will be a retry, with a default maximum of 8 occurring, for
information
+regarding how this can be configured please refer to
+`vhost-user-client tx retries config`_. If at any time no packets can
be sent,
+it may mean the guest is not accepting packets, so there
are no (more) retries.
Ian
+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.
+
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(>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(>mutex);
@@ -1915,4
Re: [ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.
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
> ---
> 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.
> +
> 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(>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(>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(>vhost_tx_retries, _max_tx_retries);
> +if (max_tx_retries != cur_max_tx_retries) {
> +atomic_store_relaxed(>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(>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.*/
>
Re: [ovs-dev] [PATCH v3 4/4] netdev-dpdk: Enable vhost-tx-retries config.
On Thu, Jun 27, 2019 at 12:12:32PM +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 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 > --- Acked-by: Flavio Leitner Thanks Kevin! fbl ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
