Re: [PATCH maint v2 4/4] batman-adv: mcast: fix duplicate mcast packets from BLA backbone to mesh

[Simon Wunderlich <sw@simonwunderlich.de>]

[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]

On Wednesday, September 9, 2020 5:27:56 PM CEST Linus Lüssing wrote:
> On Wed, Sep 09, 2020 at 02:15:51PM +0200, Simon Wunderlich wrote:
> > On Friday, September 4, 2020 8:28:03 PM CEST Linus Lüssing wrote:
> > > @@ -1626,7 +1626,8 @@ bool batadv_bla_check_bcast_duplist(struct
> > > batadv_priv *bat_priv, if (entry->crc != crc)
> > > 
> > >                         continue;
> > > 
> > > -               if (batadv_compare_eth(entry->orig, bcast_packet->orig))
> > > +               if (!is_zero_ether_addr(entry->orig) &&
> > > +                   batadv_compare_eth(entry->orig, orig))
> > > 
> > >                         continue;
> > >                 
> > >                 /* this entry seems to match: same crc, not too old,
> > 
> > Shouldn't this check also be skipped if the orig parameter is a zero mac
> > address? i.e.:
> > 
> > if (!is_zero_ether_addr(orig)) {
> > 
> > 	if (!is_zero_ether_addr(entry->orig) && batadv_compare_eth(entry->orig,
> > 
> > orig))
> > 
> > 		continue;
> > 
> > }
> 
> Would be redundant. If entry->orig is non-zero and
> the compare_eth() says they are equal, then orig must also be
> non-zero.

OK good point, that's not really obvious (at least to me).

> 
> I initially wanted to leave the code as unchanged as possible for
> net / maint. Should I do the restructuring to enhance readability, with
> the bool in this patch or in additional patch for net-next?

Personally, I would prefer having a bit more readability or verbose comments 
in front of those kind of logic if statements. Or avoid those logic 
connections and have multiple "ifs" in a row where possible to enhance 
readbility.

This patch is pretty heavy already as is, adding a bool doesn't make a big 
difference IMHO.

Cheers,
       Simon


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]