Re: [B.A.T.M.A.N.] [PATCHv6 1/3] batman-adv: Multicast Listener Announcements via Translation Table

[Antonio Quartulli <ordex@autistici.org>]

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

On Tue, Jun 25, 2013 at 02:55:08PM +0200, Linus Lüssing wrote:
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -27,6 +27,8 @@ export CONFIG_BATMAN_ADV_BLA=y
> > >  export CONFIG_BATMAN_ADV_DAT=y
> > >  # B.A.T.M.A.N network coding (catwoman):
> > >  export CONFIG_BATMAN_ADV_NC=n
> > > +# B.A.T.M.A.N. multicast optimizations:
> > > +export CONFIG_BATMAN_ADV_MCAST_OPTIMIZATIONS=y
> > 
> > Can we please find a shorter define ? How about "CONFIG_BATMAN_ADV_MCAST" ? 
> > That would be in line with the rest.
> 
> Hm, yes I also don't like the length of that so much. But I had
> dismissed "CONFIG_BATMAN_ADV_MCAST" so far because I thought it
> might seem as if batman-adv were not able to handle multicast at
> all without that option.
> 
> What about CONFIG_BATMAN_ADV_MCAST_OPT (hm, that could suggest
> 'optional', too...) or CONFIG_BATMAN_ADV_MCO?
> 
> .oO(also don't sound as nice as the other CONFIG_ exports...)
> 
> Ok, maybe CONFIG_BATMAN_ADV_MCAST is still the best option. I'll
> change it to that.

I agree with using the CONFIG_BATMAN_ADV_MCAST too.


> 
> > 
> > 
> > > +struct batadv_hw_addr {
> > > +	struct list_head	list;
> > > +	unsigned char addr[ETH_ALEN];
> > > +};
> > 
> > All struct defines should go into types.h or packet.h if they are sent over 
> > the wire.
> 
> This struct isn't sent over the wire, it's just for local book
> keeping. I was thinking about naming it 'struct batadv_mcast_hw_addr'
> but didn't do that because it is so generic and thought maybe
> because of that it might be reusable in the future.
> 
> Hm, maybe it's better to rename it to 'struct
> batadv_mcast_hw_addr' anyways and leave it in that place to avoid
> bloating types.h/packet.h with "unimportant" structs and defines?

I think this struct should go into type.h too. Many structs in there are used by
a single component only, but it is better to keep them into the same file.
And yes, renaming to batadv_mcast_hw_addr is probably better (given that it is
used by the mcast component only).

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]