Linux Ethernet Bridging - Apr 2019 - [Bridge] [PATCH net-next] net: bridge: update multicast stats from maybe_deliver()

On 04/04/2019 14:56, Pablo Neira Ayuso wrote:
> Simplify this code by updating bridge multicast stats from
> maybe_deliver().
> 
> Note that commit 6db6f0eae605 ("bridge: multicast to unicast"),
in case
> the port flag BR_MULTICAST_TO_UNICAST is set, never updates the previous
> port pointer, therefore it is always going to be different from the
> existing port in this deduplicated list iteration.
> 
> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> ---
>  net/bridge/br_forward.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
This was intentional, the reason I didn't add that call to maybe_deliver()
is to avoid
these checks for the standard unicast fast-path.  We need to avoid touching the
mcast
cache lines (and checks) when using unicast.

On 04/04/2019 15:02, Nikolay Aleksandrov wrote:
> On 04/04/2019 14:56, Pablo Neira Ayuso wrote:
>> Simplify this code by updating bridge multicast stats from
>> maybe_deliver().
>>
>> Note that commit 6db6f0eae605 ("bridge: multicast to
unicast"), in case
>> the port flag BR_MULTICAST_TO_UNICAST is set, never updates the
previous
>> port pointer, therefore it is always going to be different from the
>> existing port in this deduplicated list iteration.
>>
>> Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
>> ---
>>  net/bridge/br_forward.c | 15 ++++-----------
>>  1 file changed, 4 insertions(+), 11 deletions(-)
>>
> 
> This was intentional, the reason I didn't add that call to
maybe_deliver() is to avoid
> these checks for the standard unicast fast-path.  We need to avoid touching
the mcast
> cache lines (and checks) when using unicast.
> 
To follow-up after my bridge boolean conversion to a flags field in the first
cache line
the hit is smaller but it still adds at least 2 new tests in the unicast
fast-path which
can easily be avoided.

On Thu, Apr 04, 2019 at 03:02:19PM +0300, Nikolay Aleksandrov
wrote:
> On 04/04/2019 14:56, Pablo Neira Ayuso wrote:
> > Simplify this code by updating bridge multicast stats from
> > maybe_deliver().
> > 
> > Note that commit 6db6f0eae605 ("bridge: multicast to
unicast"), in case
> > the port flag BR_MULTICAST_TO_UNICAST is set, never updates the
previous
> > port pointer, therefore it is always going to be different from the
> > existing port in this deduplicated list iteration.
> > 
> > Signed-off-by: Pablo Neira Ayuso <pablo at netfilter.org>
> > ---
> >  net/bridge/br_forward.c | 15 ++++-----------
> >  1 file changed, 4 insertions(+), 11 deletions(-)
This is removing two branches, it is less code to maintain and easier
to read than:

        if (prev == p)
> This was intentional, the reason I didn't add that call to
maybe_deliver() is to avoid
> these checks for the standard unicast fast-path.  We need to avoid touching
the mcast
> cache lines (and checks) when using unicast.
maybe_deliver() is called from br_flood() and br_multicast_flood(),
not the standard unicast fast-path.

Thanks.