Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <ZCvSskSPwFv6kYrD@kernel-bug-kernel-bug>
In-Reply-To: <ZCvSskSPwFv6kYrD@kernel-bug-kernel-bug>
From:   Eric Dumazet <edumazet@google.com>
Date:   Tue, 4 Apr 2023 10:03:21 +0200
Message-ID: <CANn89iLpqh3vZ2qEYhhL12qDgVH1rgSJWqHf42cfX0qQfCvQ_w@mail.gmail.com>
Subject: Re: [PATCH net v2] net: openvswitch: fix race on port output
To:     Felix Huettner <felix.huettner@mail.schwarz>
Cc:     netdev@vger.kernel.org, dev@openvswitch.org,
        linux-kernel@vger.kernel.org, pabeni@redhat.com, kuba@kernel.org,
        pshelar@ovn.org, davem@davemloft.net, luca.czesla@mail.schwarz
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Apr 4, 2023 at 9:33=E2=80=AFAM 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 =3D=3D 0));

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

Thanks.