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

From: Jakub Kicinski
Date: Mon Apr 03 2023 - 14:50:17 EST


On Mon, 3 Apr 2023 11:18:46 +0000 Felix Hüttner wrote:
> On Sat, 1 Apr 2023 6:25:00 +0000 Jakub Kicinski wrote:
> > On Fri, 31 Mar 2023 06:25:13 +0000 Felix Hüttner wrote:
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 253584777101..6628323b7bea 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)) {
> > > + BUG_ON(unlikely(qcount == 0));
> >
> > DEBUG_NET_WARN_ON()
> >
>
> However if this condition triggers we will be permanently stuck in the loop below.
> From my understading this also means that future calls to `synchronize_net` will never finish (as the packet never finishes processing).
> So the user will quite probably need to restart his system.
> I find DEBUG_NET_WARN_ON_ONCE to offer too little visiblity as CONFIG_DEBUG_NET is not necessarily enabled per default.
> I as the user would see it as helpful to have this information available without additional config flags.
> I would propose to use WARN_ON_ONCE

skb_tx_hash() may get called a lot, we shouldn't slow it down on
production systems just to catch buggy drivers, IMO.