| Message ID | 20230504173606.262663-3-ktraynor@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | netdev-dpdk: Apply device rx/tx descriptor limits. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Thu, May 4, 2023 at 7:36 PM Kevin Traynor <ktraynor@redhat.com> wrote: > > By default OVS configures 2048 descriptors for tx and rx queues > on DPDK devices. It also allows the user to configure those values. > > If the values used are not acceptable to the device then queue setup > would fail. > > The device exposes it's max/min/alignment requirements and OVS > applies some limits also. Use these to ensure an acceptable value > is used for the number of descriptors on a device tx/rx. > > If the default or user value is not acceptable, adjust to a suitable > value and log. > > Reported-at: https://bugzilla.redhat.com/2119876 > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > --- > lib/netdev-dpdk.c | 53 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 42 insertions(+), 11 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index 2d9afc493..0ea6055bc 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -1911,8 +1911,27 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) > static void > dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, > - const char *flag, int default_size, int *new_size) > + struct rte_eth_dev_info *info, bool is_rx) > { > - int queue_size = smap_get_int(args, flag, default_size); > + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + struct rte_eth_desc_lim *lim; > + int default_size, queue_size; > + int *requested_size; > > + if (is_rx) { > + default_size = NIC_PORT_DEFAULT_RXQ_SIZE; > + queue_size = smap_get_int(args, "n_rxq_desc", default_size); > + requested_size = &dev->requested_rxq_size; > + lim = info ? &info->rx_desc_lim : NULL; > + > + } else { > + default_size = NIC_PORT_DEFAULT_TXQ_SIZE; > + queue_size = smap_get_int(args, "n_txq_desc", default_size); > + requested_size = &dev->requested_txq_size; > + lim = info ? &info->tx_desc_lim : NULL; > + } > + > + *requested_size = queue_size; Well, if no adjustement is done, then we may miss a reconfigure request. So maybe we should store the "old" requested value before storing it. > + > + /* Check for OVS limits. */ > if (queue_size <= 0 || queue_size > NIC_PORT_MAX_Q_SIZE > || !is_pow2(queue_size)) { > @@ -1920,6 +1939,18 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, > } > > - if (queue_size != *new_size) { > - *new_size = queue_size; > + if (lim) { > + /* Check for device limits. */ > + if (lim->nb_align) { > + queue_size = ROUND_UP(queue_size, lim->nb_align); > + } > + queue_size = MIN(queue_size, lim->nb_max); > + queue_size = MAX(queue_size, lim->nb_min); > + } > + > + if (*requested_size != queue_size) { To go with my comment above, we don't want to display the log below when no adjustement takes place. > + VLOG_INFO("Interface %s cannot set %s descriptor size to %d. " > + "Adjusted to %d.", dev->up.name, is_rx ? "rx": "tx" , > + *requested_size, queue_size); > + *requested_size = queue_size; > netdev_request_reconfigure(netdev); > } > @@ -1938,7 +1969,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > {RTE_ETH_FC_RX_PAUSE, RTE_ETH_FC_FULL } > }; > + struct rte_eth_dev_info info; > const char *new_devargs; > const char *vf_mac; > int err = 0; > + int ret; > > ovs_mutex_lock(&dpdk_mutex); > @@ -1947,11 +1980,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > dpdk_set_rxq_config(dev, args); > > - dpdk_process_queue_size(netdev, args, "n_rxq_desc", > - NIC_PORT_DEFAULT_RXQ_SIZE, > - &dev->requested_rxq_size); > - dpdk_process_queue_size(netdev, args, "n_txq_desc", > - NIC_PORT_DEFAULT_TXQ_SIZE, > - &dev->requested_txq_size); > - > new_devargs = smap_get(args, "dpdk-devargs"); > > @@ -2073,4 +2099,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > } > > + ret = rte_eth_dev_info_get(dev->port_id, &info); > + > + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, true); > + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, false); > + > out: > ovs_mutex_unlock(&dev->mutex); > -- > 2.40.1 > HTH.
On 10/05/2023 16:19, David Marchand wrote: > On Thu, May 4, 2023 at 7:36 PM Kevin Traynor <ktraynor@redhat.com> wrote: >> >> By default OVS configures 2048 descriptors for tx and rx queues >> on DPDK devices. It also allows the user to configure those values. >> >> If the values used are not acceptable to the device then queue setup >> would fail. >> >> The device exposes it's max/min/alignment requirements and OVS >> applies some limits also. Use these to ensure an acceptable value >> is used for the number of descriptors on a device tx/rx. >> >> If the default or user value is not acceptable, adjust to a suitable >> value and log. >> >> Reported-at: https://bugzilla.redhat.com/2119876 >> Signed-off-by: Kevin Traynor <ktraynor@redhat.com> >> --- >> lib/netdev-dpdk.c | 53 +++++++++++++++++++++++++++++++++++++---------- >> 1 file changed, 42 insertions(+), 11 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index 2d9afc493..0ea6055bc 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -1911,8 +1911,27 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) >> static void >> dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, >> - const char *flag, int default_size, int *new_size) >> + struct rte_eth_dev_info *info, bool is_rx) >> { >> - int queue_size = smap_get_int(args, flag, default_size); >> + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); >> + struct rte_eth_desc_lim *lim; >> + int default_size, queue_size; >> + int *requested_size; >> >> + if (is_rx) { >> + default_size = NIC_PORT_DEFAULT_RXQ_SIZE; >> + queue_size = smap_get_int(args, "n_rxq_desc", default_size); >> + requested_size = &dev->requested_rxq_size; >> + lim = info ? &info->rx_desc_lim : NULL; >> + >> + } else { >> + default_size = NIC_PORT_DEFAULT_TXQ_SIZE; >> + queue_size = smap_get_int(args, "n_txq_desc", default_size); >> + requested_size = &dev->requested_txq_size; >> + lim = info ? &info->tx_desc_lim : NULL; >> + } >> + >> + *requested_size = queue_size; > > Well, if no adjustement is done, then we may miss a reconfigure request. > So maybe we should store the "old" requested value before storing it. > Yes, thanks for catching this - I tested the complex path and missed this on the easy one :/ > >> + >> + /* Check for OVS limits. */ >> if (queue_size <= 0 || queue_size > NIC_PORT_MAX_Q_SIZE >> || !is_pow2(queue_size)) { >> @@ -1920,6 +1939,18 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, >> } >> >> - if (queue_size != *new_size) { >> - *new_size = queue_size; >> + if (lim) { >> + /* Check for device limits. */ >> + if (lim->nb_align) { >> + queue_size = ROUND_UP(queue_size, lim->nb_align); >> + } >> + queue_size = MIN(queue_size, lim->nb_max); >> + queue_size = MAX(queue_size, lim->nb_min); >> + } >> + >> + if (*requested_size != queue_size) { > > To go with my comment above, we don't want to display the log below > when no adjustement takes place. > Ack. I made a few small changes around logging in v4, I will add to v4 cover letter. >> + VLOG_INFO("Interface %s cannot set %s descriptor size to %d. " >> + "Adjusted to %d.", dev->up.name, is_rx ? "rx": "tx" , >> + *requested_size, queue_size); >> + *requested_size = queue_size; >> netdev_request_reconfigure(netdev); >> } >> @@ -1938,7 +1969,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, >> {RTE_ETH_FC_RX_PAUSE, RTE_ETH_FC_FULL } >> }; >> + struct rte_eth_dev_info info; >> const char *new_devargs; >> const char *vf_mac; >> int err = 0; >> + int ret; >> >> ovs_mutex_lock(&dpdk_mutex); >> @@ -1947,11 +1980,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, >> dpdk_set_rxq_config(dev, args); >> >> - dpdk_process_queue_size(netdev, args, "n_rxq_desc", >> - NIC_PORT_DEFAULT_RXQ_SIZE, >> - &dev->requested_rxq_size); >> - dpdk_process_queue_size(netdev, args, "n_txq_desc", >> - NIC_PORT_DEFAULT_TXQ_SIZE, >> - &dev->requested_txq_size); >> - >> new_devargs = smap_get(args, "dpdk-devargs"); >> >> @@ -2073,4 +2099,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, >> } >> >> + ret = rte_eth_dev_info_get(dev->port_id, &info); >> + >> + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, true); >> + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, false); >> + >> out: >> ovs_mutex_unlock(&dev->mutex); >> -- >> 2.40.1 >> > > HTH. > >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 2d9afc493..0ea6055bc 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -1911,8 +1911,27 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) static void dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, - const char *flag, int default_size, int *new_size) + struct rte_eth_dev_info *info, bool is_rx) { - int queue_size = smap_get_int(args, flag, default_size); + struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); + struct rte_eth_desc_lim *lim; + int default_size, queue_size; + int *requested_size; + if (is_rx) { + default_size = NIC_PORT_DEFAULT_RXQ_SIZE; + queue_size = smap_get_int(args, "n_rxq_desc", default_size); + requested_size = &dev->requested_rxq_size; + lim = info ? &info->rx_desc_lim : NULL; + + } else { + default_size = NIC_PORT_DEFAULT_TXQ_SIZE; + queue_size = smap_get_int(args, "n_txq_desc", default_size); + requested_size = &dev->requested_txq_size; + lim = info ? &info->tx_desc_lim : NULL; + } + + *requested_size = queue_size; + + /* Check for OVS limits. */ if (queue_size <= 0 || queue_size > NIC_PORT_MAX_Q_SIZE || !is_pow2(queue_size)) { @@ -1920,6 +1939,18 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, } - if (queue_size != *new_size) { - *new_size = queue_size; + if (lim) { + /* Check for device limits. */ + if (lim->nb_align) { + queue_size = ROUND_UP(queue_size, lim->nb_align); + } + queue_size = MIN(queue_size, lim->nb_max); + queue_size = MAX(queue_size, lim->nb_min); + } + + if (*requested_size != queue_size) { + VLOG_INFO("Interface %s cannot set %s descriptor size to %d. " + "Adjusted to %d.", dev->up.name, is_rx ? "rx": "tx" , + *requested_size, queue_size); + *requested_size = queue_size; netdev_request_reconfigure(netdev); } @@ -1938,7 +1969,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, {RTE_ETH_FC_RX_PAUSE, RTE_ETH_FC_FULL } }; + struct rte_eth_dev_info info; const char *new_devargs; const char *vf_mac; int err = 0; + int ret; ovs_mutex_lock(&dpdk_mutex); @@ -1947,11 +1980,4 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, dpdk_set_rxq_config(dev, args); - dpdk_process_queue_size(netdev, args, "n_rxq_desc", - NIC_PORT_DEFAULT_RXQ_SIZE, - &dev->requested_rxq_size); - dpdk_process_queue_size(netdev, args, "n_txq_desc", - NIC_PORT_DEFAULT_TXQ_SIZE, - &dev->requested_txq_size); - new_devargs = smap_get(args, "dpdk-devargs"); @@ -2073,4 +2099,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, } + ret = rte_eth_dev_info_get(dev->port_id, &info); + + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, true); + dpdk_process_queue_size(netdev, args, !ret ? &info : NULL, false); + out: ovs_mutex_unlock(&dev->mutex);
By default OVS configures 2048 descriptors for tx and rx queues on DPDK devices. It also allows the user to configure those values. If the values used are not acceptable to the device then queue setup would fail. The device exposes it's max/min/alignment requirements and OVS applies some limits also. Use these to ensure an acceptable value is used for the number of descriptors on a device tx/rx. If the default or user value is not acceptable, adjust to a suitable value and log. Reported-at: https://bugzilla.redhat.com/2119876 Signed-off-by: Kevin Traynor <ktraynor@redhat.com> --- lib/netdev-dpdk.c | 53 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 11 deletions(-)