> Reconfiguration of HW NICs may lead to packet drops. > In current model all physical ports will be reconfigured each time number > of PMD threads changed. Since we not stopping threads on pmd-cpu-mask > changes, this patch will help to further decrease port's downtime by > setting the maximum possible number of wanted tx queues to avoid > unnecessary reconfigurations.
Hi Ilya, Thanks for the patch, in testing I can confirm it resolves the issue. A few observations and comments below. > > Signed-off-by: Ilya Maximets <[email protected]> > --- > lib/dpif-netdev.c | 26 +++++++++++++++++++++----- > 1 file changed, 21 insertions(+), 5 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 596d133..79db770 > 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -3453,7 +3453,7 @@ reconfigure_datapath(struct dp_netdev *dp) { > struct dp_netdev_pmd_thread *pmd; > struct dp_netdev_port *port; > - int wanted_txqs; > + int needed_txqs, wanted_txqs; > > dp->last_reconfigure_seq = seq_read(dp->reconfigure_seq); > > @@ -3461,7 +3461,15 @@ reconfigure_datapath(struct dp_netdev *dp) > * on the system and the user configuration. */ > reconfigure_pmd_threads(dp); > > - wanted_txqs = cmap_count(&dp->poll_threads); > + /* We need 1 Tx queue for each thread to avoid locking, but we will > try > + * to allocate the maximum possible value to minimize the number of > port > + * reconfigurations. */ > + needed_txqs = cmap_count(&dp->poll_threads); > + /* (n_cores + 1) is the maximum that we might need to have. > + * Additional queue is for non-PMD threads. */ > + wanted_txqs = ovs_numa_get_n_cores(); > + ovs_assert(wanted_txqs != OVS_CORE_UNSPEC); > + wanted_txqs++; So just after this line there is a call HMAP_FOR_EACH (port, node, &dp->ports) { netdev_set_tx_multiq(port->netdev, wanted_txqs); } My initial concern with this patch was twofold: (i) What would happen if the number of cores was greater than the number of supported queues. That's not an issue from what I can see as the requested_txqs will be compared to what's supported by the device and the smaller of the two will be chosen in the eventual call to dpdk_eth_dev_init(): n_txq = MIN(info.max_tx_queues, dev->up.n_txq); (ii) What happens when a device reports the incorrect number of max tx queues? (can occur depending on the mode of the device). I think this case is ok as well as it's handled by the existing txq retry logic in dpdk_eth_dev_queue_setup(). > > /* The number of pmd threads might have changed, or a port can be > new: > * adjust the txqs. */ > @@ -3474,9 +3482,17 @@ reconfigure_datapath(struct dp_netdev *dp) > > /* Check for all the ports that need reconfiguration. We cache this > in > * 'port->reconfigure', because netdev_is_reconf_required() can > change at > - * any time. */ > + * any time. > + * Also mark for reconfiguration all ports which will likely change > their > + * 'dynamic_txqs' parameter. It's required to stop using them before > + * changing this setting and it's simpler to mark ports here and > allow > + * 'pmd_remove_stale_ports' to remove them from threads. There will > be > + * no actual reconfiguration in 'port_reconfigure' because it's > + * unnecessary. */ > HMAP_FOR_EACH (port, node, &dp->ports) { > - if (netdev_is_reconf_required(port->netdev)) { > + if (netdev_is_reconf_required(port->netdev) > + || (port->dynamic_txqs > + != netdev_n_txq(port->netdev) < needed_txqs)) { GCC caught the following lib/dpif-netdev.c:3448:48: error: suggest parentheses around comparison in operand of '!=' [-Werror=parentheses] != netdev_n_txq(port->netdev) < needed_txqs)) { > port->need_reconfigure = true; > } > } > @@ -3510,7 +3526,7 @@ reconfigure_datapath(struct dp_netdev *dp) > seq_change(dp->port_seq); > port_destroy(port); > } else { > - port->dynamic_txqs = netdev_n_txq(port->netdev) < > wanted_txqs; > + port->dynamic_txqs = netdev_n_txq(port->netdev) < > + needed_txqs; > } > } > > -- > 2.7.4 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
