[v2,0/2] batman-adv: netlink support for DAT and MCAST

  This patchset adds netlink support for dumping DAT cache and multicast
flags tables.

Changes in v2:
==============

- Added missing includes, "#include <linux/netlink.h>" and
  "#include <uapi/linux/batman_adv.h>", to distributed-arp-table.c
- Added missing forward declaration for "struct netlink_callback" to
  distributed-arp-table.h
- Changed nla_put_u32() to nla_put_in_addr() for BATADV_ATTR_DC_ADDRESS,
  removed the then obsolete ntohl() conversion
- Added missing include, "#include <linux/netlink.h>", to multicast.c
- Changed nla_put_u8() to nla_put_u32() for BATADV_ATTR_MCAST_FLAGS and
  BATADV_ATTR_MCAST_FLAGS_PRIV to enhance extensibility
  

On Sonntag, 11. März 2018 00:53:19 CET Linus Lüssing wrote:
> This patchset adds netlink support for dumping DAT cache and multicast
> flags tables.
> 
> Changes in v2:
> ==============
> 
> - Added missing includes, "#include <linux/netlink.h>" and
>   "#include <uapi/linux/batman_adv.h>", to distributed-arp-table.c
> - Added missing forward declaration for "struct netlink_callback" to
>   distributed-arp-table.h
> - Changed nla_put_u32() to nla_put_in_addr() for BATADV_ATTR_DC_ADDRESS,
>   removed the then obsolete ntohl() conversion
> - Added missing include, "#include <linux/netlink.h>", to multicast.c
> - Changed nla_put_u8() to nla_put_u32() for BATADV_ATTR_MCAST_FLAGS and
>   BATADV_ATTR_MCAST_FLAGS_PRIV to enhance extensibility

Thanks for the doing the requested changes.

@Antonio: Can you please check whether you are ok with the new netlink names 
and types.

> +       [BATADV_ATTR_DC_ADDRESS]        = { .type = NLA_U32 },
> +       [BATADV_ATTR_DC_HWADDRESS]      = { .len = ETH_ALEN },
> +       [BATADV_ATTR_DC_VID]            = { .type = NLA_U16 },

Kind regards,
	Sven
  

On 11/03/18 18:54, Sven Eckelmann wrote:
> On Sonntag, 11. März 2018 00:53:19 CET Linus Lüssing wrote:
>> This patchset adds netlink support for dumping DAT cache and multicast
>> flags tables.
>>
>> Changes in v2:
>> ==============
>>
>> - Added missing includes, "#include <linux/netlink.h>" and
>>   "#include <uapi/linux/batman_adv.h>", to distributed-arp-table.c
>> - Added missing forward declaration for "struct netlink_callback" to
>>   distributed-arp-table.h
>> - Changed nla_put_u32() to nla_put_in_addr() for BATADV_ATTR_DC_ADDRESS,
>>   removed the then obsolete ntohl() conversion
>> - Added missing include, "#include <linux/netlink.h>", to multicast.c
>> - Changed nla_put_u8() to nla_put_u32() for BATADV_ATTR_MCAST_FLAGS and
>>   BATADV_ATTR_MCAST_FLAGS_PRIV to enhance extensibility
> 
> Thanks for the doing the requested changes.
> 
> @Antonio: Can you please check whether you are ok with the new netlink names 
> and types.
> 
>> +       [BATADV_ATTR_DC_ADDRESS]        = { .type = NLA_U32 },
>> +       [BATADV_ATTR_DC_HWADDRESS]      = { .len = ETH_ALEN },
>> +       [BATADV_ATTR_DC_VID]            = { .type = NLA_U16 },
> 

Personally I'd find "DAT_CACHE" easier to parse than "DC".
I know we have the "batctl dc" command, but that's more to keep the CLI
compact.

This said, DAT was meant to be extended in the future and not store only
IPv4 addresses. This said, shouldn't we use a more specific name for the
address?

Another observation: do these attributes need to be DAT specific? or can
we just define (or reuse) a generic HWADDRESS attribute that we might
already be using somewhere else (i.e. for TT)?


Regards,
  

On Dienstag, 13. März 2018 16:02:34 CET Antonio Quartulli wrote:
[...]
> Another observation: do these attributes need to be DAT specific? or can
> we just define (or reuse) a generic HWADDRESS attribute that we might
> already be using somewhere else (i.e. for TT)?

I've asked myself the same but we have BLA_- and TT_ADDRESS and these are 
named in a way which connects them to these specific subsystems. Using one of 
them for DAT seems to be a little bit odd.

Kind regards,
	Sven
  

On 13/03/18 16:20, Sven Eckelmann wrote:
> On Dienstag, 13. März 2018 16:02:34 CET Antonio Quartulli wrote:
> [...]
>> Another observation: do these attributes need to be DAT specific? or can
>> we just define (or reuse) a generic HWADDRESS attribute that we might
>> already be using somewhere else (i.e. for TT)?
> 
> I've asked myself the same but we have BLA_- and TT_ADDRESS and these are 
> named in a way which connects them to these specific subsystems. Using one of 
> them for DAT seems to be a little bit odd.

Oh ok. In this case it makes sense to have DAT specific attrs.

(if we wanted to, we could still rename all the attrs pointing to the
very same data type and unify them, but I don't have a strong opinion
about it at this time).

Cheers,
  

On Dienstag, 13. März 2018 16:22:24 CET Antonio Quartulli wrote:
[...]
> (if we wanted to, we could still rename all the attrs pointing to the
> very same data type and unify them, but I don't have a strong opinion
> about it at this time).

This would break backwards compatibility.

Kind regards,
	Sven
  

On 13/03/18 16:52, Sven Eckelmann wrote:
> On Dienstag, 13. März 2018 16:22:24 CET Antonio Quartulli wrote:
> [...]
>> (if we wanted to, we could still rename all the attrs pointing to the
>> very same data type and unify them, but I don't have a strong opinion
>> about it at this time).
> 
> This would break backwards compatibility.

Ah right, those names are exported to user space too with the same
header file.

We'll live with that then.

Cheers,