Re: [PATCH net-next 02/10] net: bridge: BROPT_FDB_LOCAL_VLAN_0: Look up FDB on VLAN 0 on miss

[Petr Machata <petrm@nvidia.com>]

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 4 Sep 2025 19:07:19 +0200 Petr Machata wrote:
>>  		dst = br_fdb_find_rcu(br, eth_hdr(skb)->h_dest, vid);
>> +		if (unlikely(!dst && vid &&
>> +			     br_opt_get(br, BROPT_FDB_LOCAL_VLAN_0))) {
>
> What does the assembly look like for this? I wonder if we're not better
> off with:
>
> 	unlikely(!dst) && vid && br_opt..

Well, I don't see much there. A couple basic blocks end up reordered is
all I can glean out. I looked at GCC tree dumps which are more
transparent to me, and it's the same exact code in both cases, except
for variable naming, basic block numbering and branch prediction
annotations -- kinda obviously.

> Checking dst will be very fast here, and if we missed we're already 
> on the slow path. So maybe we're better off moving the rest of the
> condition out of the fast path, too?

GCC appears to compile unlikely(A && B && C) as unlikely(A) &&
unlikely(B) && unlikely(C). So it's really much of a muchness -- when we
annotate just !dst, it's going to assume 50/50 on those latter branches,
but we don't really care at that point anymore, because the first 90/10
is going to send us to the slow path. There's a bunch of 50/50 branches
because of __builtin_constant_p's, but those are optimized away in
middle-end. In the end it's the same code, just different basic block
layout decisions.

So we can do either.