Re: [PATCH net v2] net: openvswitch: fix race on port output

[Eric Dumazet]

On Tue, Apr 4, 2023 at 9:33 AM Felix Huettner
<felix.huettner@mail.schwarz> wrote:
>
> assume the following setup on a single machine:
> 1. An openvswitch instance with one bridge and default flows
> 2. two network namespaces "server" and "client"
> 3. two ovs interfaces "server" and "client" on the bridge
> 4. for each ovs interface a veth pair with a matching name and 32 rx and
>    tx queues
> 5. move the ends of the veth pairs to the respective network namespaces
> 6. assign ip addresses to each of the veth ends in the namespaces (needs
>    to be the same subnet)
> 7. start some http server on the server network namespace
> 8. test if a client in the client namespace can reach the http server
>
> when following the actions below the host has a chance of getting a cpu
> stuck in a infinite loop:
> 1. send a large amount of parallel requests to the http server (around
>    3000 curls should work)
> 2. in parallel delete the network namespace (do not delete interfaces or
>    stop the server, just kill the namespace)
>

> Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
> Co-developed-by: Luca Czesla <luca.czesla@mail.schwarz>
> Signed-off-by: Luca Czesla <luca.czesla@mail.schwarz>
> Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz>
> ---
> v2:
>   - replace BUG_ON with DEBUG_NET_WARN_ON_ONCE
>   - use netif_carrier_ok() instead of checking for NETREG_REGISTERED
> v1: https://lore.kernel.org/netdev/ZCaXfZTwS9MVk8yZ@kernel-bug-kernel-bug/
>
>  net/core/dev.c            | 1 +
>  net/openvswitch/actions.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 253584777101..37b26017f458 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3199,6 +3199,7 @@ static u16 skb_tx_hash(const struct net_device *dev,
>         }
>
>         if (skb_rx_queue_recorded(skb)) {
> +               DEBUG_NET_WARN_ON_ONCE(unlikely(qcount == 0));

No need for unlikely(), it is already done in DEBUG_NET_WARN_ON_ONCE()

Thanks.