[ovs-dev] [PATCH] dpif-netdev: Force port reconfiguration to change dynamic_txqs.

[William Tu]

On Wed, Mar 25, 2020 at 12:50:45AM +0100, 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.

I always wonder why those static analysis tools such as Coverity can't
detect crash like this.

> 
> 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 <u9012063 at gmail.com>
> 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 <i.maximets at ovn.org>
> ---
Thanks a lot! I tested it and it works without crash.

Acked-by: William Tu <u9012063 at gmail.com>