Re: [B.A.T.M.A.N.] [RFC v4 00/19] batman-adv: netlink restructuring, part 2

[Jiri Pirko <jiri@resnulli.us>]

Sat, Jan 26, 2019 at 11:47:20AM CET, sven@narfation.org wrote:
>On Saturday, 19 January 2019 16.56.07 CET Sven Eckelmann wrote:
>[...]
>> There were also two topics which were not yet really discussed and thus
>> these requests (from Linus) were not yet implemented:
>
>@Jiri, @Linus maybe you can discuss these topics further and select the 
>correct solution.
>
>> * convert BATADV_ATTR_MULTICAST_MODE_ENABLED to an u32 and let don't handle
>>   it like a boolean. Instead use it to select how multicast traffic has to
>>   be handled:
>>   
>>   - 0: ignore multicast optimization and just flood it like broadcast
>>     traffic
>>   - 1: enabled multicast optimization
>>   - 2: undefined but also some kind of multicast optimization
>>   - 3: undefined but also some kind of multicast of optimization
>>   - ...
>
>Multicast mode is currently defined.
>
>* according to batctl manpage:
>
>    multicast_mode|mm [0|1]
>           If no parameter is given the current multicast mode set‐
>           ting is displayed. Otherwise the parameter is used to en‐
>           able or disable multicast optimizations (i.e. disabling
>           means always sending own multicast frames via classic
>           flooding).
>
>* according to sysfs ABI:
>
>   What:           /sys/class/net/<mesh_iface>/mesh/multicast_mode
>   Date:           Feb 2014
>   Contact:        Linus Lüssing <linus.luessing@web.de>
>   Description:
>                   Indicates whether multicast optimizations are enabled
>                   or disabled. If set to zero then all nodes in the
>                   mesh are going to use classic flooding for any
>                   multicast packet with no optimizations.
>
>Both define it as boolean value and therefore it was converted to a boolean 
>value (via u8) in netlink.
>
>But Linus now suggested that it is actually an u32. Most likely 0 == to 
>something like BATADV_MULTICAST_MODE_FLOODING. But I have no idea what 1 is or 
>what 2, 3, 4, .. would be. So I need some input here.
>
>And Jiri said that it should be renamed to BATADV_ATTR_MULTICAST_ENABLED -
>which seems to suggest that he doesn't like the idea of a u32 for some reason
>and prefers to use a boolean value.
>
>And now Linus even said that it should be a bit field - which makes it even 
>more vague to me and I have now absolutely no idea what should be implemented.

If it is bool, it should be bool. If it is a bitfield (future more bits
than one needed), it should be a bitfield.


>
>* BIT 0 for flooding vs ?
>* BIT 1 for ?
>* ...
>
>> * convert BATADV_ATTR_AGGREGATION_OGM_ENABLED to u32 and use it
>>   to mark which type of traffic should be aggregated:
>> 
>>   - bit 0: enable aggregation of OGM(2)s
>>   - bit 1: yet undefined packet type which allows some kind of aggregation
>>   - bit 2: yet undefined packet type which allows some kind of aggregation
>>   - ...
>
>Aggregated OGM is currently defined as:
>
>
>* according to batctl manpage:
>
>    aggregation|ag [0|1]
>           If no parameter is given the current aggregation setting
>           is displayed. Otherwise the parameter is used to enable or
>           disable OGM packet aggregation.
>
>* according to sysfs ABI:
>
>    What:           /sys/class/net/<mesh_iface>/mesh/aggregated_ogms
>    Date:           May 2010
>    Contact:        Marek Lindner <mareklindner@neomailbox.ch>
>    Description:
>                    Indicates whether the batman protocol messages of the
>                    mesh <mesh_iface> shall be aggregated or not.
>
>So sysfs is only one possible backend for the batctl command. There is 
>currently nothing which I would assume to be aggregatable beside OGMs but let 
>us assume for now that there is now something and some way to aggregate things 
>beside OGMs in a save and backward compatible way. Let's call this FOO - so we 
>have BATADV_ATTR_AGGREGATION_OGM_ENABLED and 
>BATADV_ATTR_AGGREGATION_FOO_ENABLED. Or we have BATADV_ATTR_AGGREGATION as an 
>u32 and just use the second bit as marker for FOO (and of course the first bit 
>as marker for OGM).
>
>Would it now be more preferable to use BATADV_ATTR_AGGREGATION_OGM_ENABLED as 
>u8 (boolean) or to to switch to BATADV_ATTR_AGGREGATION (u32) & assign single 
>bits to packet types.
>
>Kind regards,
>	Sven