On Fri, Apr 14, 2023 at 5:45 PM Kevin Traynor <[email protected]> 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 <[email protected]>
Thanks for the added log. Overall, this looks good to me. I have a comment on how the limits are taken into account (see below). But in any case this patch is ok as is for me: Reviewed-by: David Marchand <[email protected]> > --- > lib/netdev-dpdk.c | 76 +++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 61 insertions(+), 15 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index fb0dd43f7..cf91990e5 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -510,4 +510,9 @@ struct netdev_dpdk { > int requested_txq_size; > > + /* Adjusted sizes are requested sizes after any adjustments > + * for OVS or device limits. */ > + int adjusted_rxq_size; > + int adjusted_txq_size; > + > /* Number of rx/tx descriptors for physical devices */ > int rxq_size; > @@ -1917,8 +1922,29 @@ 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, *adjusted_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; > + adjusted_size = &dev->adjusted_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; > + adjusted_size = &dev->adjusted_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)) { > @@ -1926,6 +1952,15 @@ 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); OVS put some constraints on user input: - queue_size <= NIC_PORT_MAX_Q_SIZE (== 4096) which is probably linked to the mempool sizing, - queue_size is a power of two (here.. the reason eludes me), On the other hand, with this patch, the OVS constraints are not applied to what a DPDK driver requests. So a DPDK driver may force a queue_size > NIC_PORT_MAX_Q_SIZE (to be fair, I can't find any atm in the DPDK tree). Configuring such a queue size may later result in some allocation failures on the rx side if the associated mempool is too small. I don't think OVS can do anything else but accept this constraint from the DPDK driver, otherwise DPDK ethdev will refuse to initialize it. And this patch adds a log on the adjustment, so I think this is good enough as is. -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
