2023-04-04 08:11:53

by Eric Dumazet

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

On Tue, Apr 4, 2023 at 9:33 AM Felix Huettner
<[email protected]> 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 <[email protected]>
> Signed-off-by: Luca Czesla <[email protected]>
> Signed-off-by: Felix Huettner <[email protected]>
> ---
> 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.


2023-04-04 08:26:22

by Felix Huettner

[permalink] [raw]
Subject: RE: [PATCH net v2] net: openvswitch: fix race on port output

On Tue, Apr 4, 2023 at 10:03 AM Eric Dumazet wrote:
> On Tue, Apr 4, 2023 at 9:33 AM Felix Huettner
> <[email protected]> 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 <[email protected]>
> > Signed-off-by: Luca Czesla <[email protected]>
> > Signed-off-by: Felix Huettner <[email protected]>
> > ---
> > 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.

Thanks for the feedback. Will include that in v3.
Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die Verwertung durch den vorgesehenen Empfänger bestimmt. Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender bitte unverzüglich in Kenntnis und löschen diese E Mail. Hinweise zum Datenschutz finden Sie hier<https://www.datenschutz.schwarz>.