Re: [PATCH net-next] net: skbuff: Skip early return in skb_unref when debugging

[Breno Leitao <leitao@debian.org>]

Hello Paolo,

On Tue, Jul 30, 2024 at 11:38:38AM +0200, Paolo Abeni wrote:
> On 7/29/24 12:47, Breno Leitao wrote:

> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -1225,7 +1225,7 @@ static inline bool skb_unref(struct sk_buff *skb)
> >   {
> >   	if (unlikely(!skb))
> >   		return false;
> > -	if (likely(refcount_read(&skb->users) == 1))
> > +	if (!IS_ENABLED(CONFIG_DEBUG_NET) && likely(refcount_read(&skb->users) == 1))
> >   		smp_rmb();
> >   	else if (likely(!refcount_dec_and_test(&skb->users)))
> >   		return false;
 
> I think one assumption behind CONFIG_DEBUG_NET is that enabling such config
> should not have any measurable impact on performances.
> 
> I suspect the above could indeed cause some measurable impact, e.g. under
> UDP flood, when the user-space receiver and the BH runs on different cores,
> as this will increase pressure on the CPU cache. Could you please benchmark
> such scenario before and after this patch?

Sure, I am more than happy to do so. I will be back with it soon.

Assuming there is some performance overhead, isn't it a worthwhile
trade-off for those who are debugging the network? In other words,
wouldn't it be better to prioritize correctness over optimization in
this the CONFIG_DEBUG_NET case, even if it means sacrificing some
performance?

Thanks for reviewing,
--breno