Re: [RFC PATCH net] net-gro: restore check for NULL skb in napi_gro_frags

[Tyler Stachecki <stachecki.tyler@gmail.com>]

On Fri, Aug 04, 2023 at 04:45:13PM +0200, Eric Dumazet wrote:
> This is already done, a compiler should already infer this from this code:
> 
> if (unlikely(skb_gro_header_hard(skb, hlen))) {
>     eth = skb_gro_header_slow(skb, hlen, 0);
>     if (unlikely(!eth)) {
>         net_warn_ratelimited("%s: dropping impossible skb from %s\n",
>                                            __func__, napi->dev->name);
>         napi_reuse_skb(napi, skb);
>         return NULL;
>     }

It is a good point - though, FWIW, at least with clang-16 I am observing that
it does not leverage this fact (fully?). Mostly for my own curiosity, I took a
look...

In both cases, the generated code for the NULL check is emitted as a a forward
branch, meaning that (at least on x86), the branch direction hint is rendered
as desired.

However, without unlikely(...), the code for the taken branch is clumped
roughly after the inlined code for napi_frags_finish and before the (hot paths
of) napi_frags_skb.

With unlikely added, the code for the taken NULL check is seated right next to
the code generated for the unlikely paths in your comment. So, it does seem to
have an effect, however minor!

---

Anyways: perhaps the more important note here is that, at least with clang-16,
I can confirm that everything is still inlined with this change.