On 24/03/2020 23:50, Ilya Maximets wrote: > In case number of polling threads goes from exact number of Tx queues > in port to higher value while set_tx_multiq() not implemented or not > requesting reconfiguration, port will not be reconfigured and datapath > will continue using static Tx queue ids leading to crash. > > Ex.: > Assuming that port p0 supports up to 4 Tx queues and doesn't support > set_tx_multiq() method. For example, netdev-afxdp could be the case, > because it could have multiple Tx queues, but doesn't have > set_tx_multiq() implementation because number of Tx queues always > equals to number of Rx queues. > > 1. Configuring pmd-cpu-mask to have 3 pmd threads. > > 2. Adding port p0 to OVS. > At this point wanted_txqs = 4 (3 for pmd threads + 1 for non-pmd). > Port reconfigured to have 4 Tx queues successfully. > dynamic_txqs = (4 < 4) = false; > > 3. Configuring pmd-cpu-mask to have 10 pmd threads. > At this point wanted_txqs = 11 (10 for pmd threads + 1 for non-pmd). > Since set_tx_multiq() is not implemented, netdev doesn't request > reconfiguration and 'dynamic_txqs' remains in 'false' state. > > 4. Since 'dynamic_txqs == false', dpif-netdev uses static Tx queue > ids that are in range [0, 10] while device only supports 4 leading > to unwanted behavior and crashes. > > Fix that by marking for reconfiguration all the ports that will likely > change their 'dynamic_txqs' value. > > It looks like the issue could be reproduced only with afxdp ports, > because all other non-dpdk ports ignores Tx queue ids and dpdk ports > requests for reconfiguration on set_tx_multiq(). > > Reported-by: William Tu <[email protected]> > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-dev/2020-March/368364.html > Fixes: e32971b8ddb4 ("dpif-netdev: Centralized threads and queues handling > code.") > Signed-off-by: Ilya Maximets <[email protected]> > --- > > Since the issue could be reproduced only with afxdp ports, this should > be backported only down to 2.12. > > lib/dpif-netdev.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index a798db45d..e456cc9be 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -4941,9 +4941,17 @@ reconfigure_datapath(struct dp_netdev *dp) > > /* Check for all the ports that need reconfiguration. We cache this in > * 'port->need_reconfigure', because netdev_is_reconf_required() can > - * change at any time. */ > + * change at 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) < wanted_txqs))) { > port->need_reconfigure = true; > } > } >
LGTM Acked-by: Kevin Traynor <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
