Re: [RFC v4 00/19] batman-adv: netlink restructuring, part 2

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:
[...]

quoted

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.

quoted

* 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 [off-list ref]
  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 ?
* ...

quoted

* 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 [off-list ref]
   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