diff mbox series

[RFC,v3,14/19] batman-adv: Add multicast_mode mesh genl configuration

Message ID 20181207135846.6152-15-sven@narfation.org (mailing list archive)
State RFC, archived
Delegated to: Simon Wunderlich
Headers
Series batman-adv: netlink restructuring, part 2 |

Commit Message

Sven Eckelmann Dec. 7, 2018, 1:58 p.m. UTC 
  The mesh interface can optimize the flooding of multicast packets based on
the content of the global translation tables.

The BATADV_CMD_SET_MESH/BATADV_CMD_GET_MESH commands allow to set/get the
configuration of this feature using the BATADV_ATTR_MULTICAST_MODE
attribute. Setting the u8 to zero will disable this feature and setting it
to something else is enabling this feature.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Cc: Linus Lüssing <linus.luessing@c0d3.blue>
---
 include/uapi/linux/batman_adv.h |  8 ++++++++
 net/batman-adv/netlink.c        | 15 +++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Linus Lüssing Jan. 4, 2019, 1:57 a.m. UTC | #1 
On Fri, Dec 07, 2018 at 02:58:41PM +0100, Sven Eckelmann wrote:
> diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
> index 8e3a1b6e..32762003 100644
> --- a/net/batman-adv/netlink.c
> +++ b/net/batman-adv/netlink.c
> @@ -155,6 +155,7 @@ static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
>  	[BATADV_ATTR_GW_SEL_CLASS]		= { .type = NLA_U32 },
>  	[BATADV_ATTR_HOP_PENALTY]		= { .type = NLA_U8 },
>  	[BATADV_ATTR_LOG_LEVEL]			= { .type = NLA_U32 },
> +	[BATADV_ATTR_MULTICAST_MODE]		= { .type = NLA_U8 },
>  };

Since the name is more generic than just a flag, would it make
sense to make this a U32, just to stay flexible for the future?

(I don't have any specific ideas or plans to use more
values for it right now, though)
Sven Eckelmann Jan. 4, 2019, 7:44 a.m. UTC | #2 
On Friday, 4 January 2019 02.57.26 CET Linus Lüssing wrote:
> On Fri, Dec 07, 2018 at 02:58:41PM +0100, Sven Eckelmann wrote:
> > diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
> > index 8e3a1b6e..32762003 100644
> > --- a/net/batman-adv/netlink.c
> > +++ b/net/batman-adv/netlink.c
> > @@ -155,6 +155,7 @@ static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
> >  	[BATADV_ATTR_GW_SEL_CLASS]		= { .type = NLA_U32 },
> >  	[BATADV_ATTR_HOP_PENALTY]		= { .type = NLA_U8 },
> >  	[BATADV_ATTR_LOG_LEVEL]			= { .type = NLA_U32 },
> > +	[BATADV_ATTR_MULTICAST_MODE]		= { .type = NLA_U8 },
> >  };
> 
> Since the name is more generic than just a flag, would it make
> sense to make this a U32, just to stay flexible for the future?
> 
> (I don't have any specific ideas or plans to use more
> values for it right now, though)

Please discuss this with Jiro because he told us to use u8.

Kind regards,
	Sven
Jiri Pirko Jan. 4, 2019, 8:51 a.m. UTC | #3 
Fri, Jan 04, 2019 at 08:44:38AM CET, sven@narfation.org wrote:
>On Friday, 4 January 2019 02.57.26 CET Linus Lüssing wrote:
>> On Fri, Dec 07, 2018 at 02:58:41PM +0100, Sven Eckelmann wrote:
>> > diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
>> > index 8e3a1b6e..32762003 100644
>> > --- a/net/batman-adv/netlink.c
>> > +++ b/net/batman-adv/netlink.c
>> > @@ -155,6 +155,7 @@ static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
>> >  	[BATADV_ATTR_GW_SEL_CLASS]		= { .type = NLA_U32 },
>> >  	[BATADV_ATTR_HOP_PENALTY]		= { .type = NLA_U8 },
>> >  	[BATADV_ATTR_LOG_LEVEL]			= { .type = NLA_U32 },
>> > +	[BATADV_ATTR_MULTICAST_MODE]		= { .type = NLA_U8 },
>> >  };
>> 
>> Since the name is more generic than just a flag, would it make
>> sense to make this a U32, just to stay flexible for the future?

If the attribute is supposed to be just a bool, the name would be
probably better to change to "BATADV_ATTR_MULTICAST_ENABLED" or
something.


>> 
>> (I don't have any specific ideas or plans to use more
>> values for it right now, though)
>
>Please discuss this with Jiro because he told us to use u8.
>
>Kind regards,
>	Sven
>
Linus Lüssing Jan. 5, 2019, 3:02 p.m. UTC | #4 
On Fri, Jan 04, 2019 at 08:51:56AM +0000, Jiri Pirko wrote:
> Fri, Jan 04, 2019 at 08:44:38AM CET, sven@narfation.org wrote:
> >On Friday, 4 January 2019 02.57.26 CET Linus Lüssing wrote:
> >> On Fri, Dec 07, 2018 at 02:58:41PM +0100, Sven Eckelmann wrote:
> >> > diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
> >> > index 8e3a1b6e..32762003 100644
> >> > --- a/net/batman-adv/netlink.c
> >> > +++ b/net/batman-adv/netlink.c
> >> > @@ -155,6 +155,7 @@ static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
> >> >  	[BATADV_ATTR_GW_SEL_CLASS]		= { .type = NLA_U32 },
> >> >  	[BATADV_ATTR_HOP_PENALTY]		= { .type = NLA_U8 },
> >> >  	[BATADV_ATTR_LOG_LEVEL]			= { .type = NLA_U32 },
> >> > +	[BATADV_ATTR_MULTICAST_MODE]		= { .type = NLA_U8 },
> >> >  };
> >> 
> >> Since the name is more generic than just a flag, would it make
> >> sense to make this a U32, just to stay flexible for the future?
> 
> If the attribute is supposed to be just a bool, the name would be
> probably better to change to "BATADV_ATTR_MULTICAST_ENABLED" or
> something.

Hm, BATADV_ATTR_MULTICAST_ENABLED sounds confusing to me. In a
layer 2 mesh network, multicast always needs to be enabled for
things like IP to work.

So setting this to 0 does not disable multicast.

What this option does is using "classic flooding" (0) for
multicast packets or dropping/unicasting if possible, if IGMP/MLD
snooping detected such potential (1).

And since there were many discussions regarding which additional
techniques and algorithms could be implemented I would expect more
options to follow.

And for complementary techniques, a bitfield of 32 bits size
could be useful, I guess?
diff mbox series

Patch

diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batman_adv.h
index efc06449..514acc06 100644
--- a/include/uapi/linux/batman_adv.h
+++ b/include/uapi/linux/batman_adv.h
@@ -454,6 +454,14 @@  enum batadv_nl_attrs {
 	 */
 	BATADV_ATTR_LOG_LEVEL,
 
+	/**
+	 * @BATADV_ATTR_MULTICAST_MODE: 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.
+	 */
+	BATADV_ATTR_MULTICAST_MODE,
+
 	/* add attributes above here, update the policy in netlink.c */
 
 	/**
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 8e3a1b6e..32762003 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -155,6 +155,7 @@  static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
 	[BATADV_ATTR_GW_SEL_CLASS]		= { .type = NLA_U32 },
 	[BATADV_ATTR_HOP_PENALTY]		= { .type = NLA_U8 },
 	[BATADV_ATTR_LOG_LEVEL]			= { .type = NLA_U32 },
+	[BATADV_ATTR_MULTICAST_MODE]		= { .type = NLA_U8 },
 };
 
 /**
@@ -342,6 +343,12 @@  static int batadv_netlink_mesh_put(struct sk_buff *msg,
 		goto nla_put_failure;
 #endif /* CONFIG_BATMAN_ADV_DEBUG */
 
+#ifdef CONFIG_BATMAN_ADV_MCAST
+	if (nla_put_u8(msg, BATADV_ATTR_MULTICAST_MODE,
+		       !!atomic_read(&bat_priv->multicast_mode)))
+		goto nla_put_failure;
+#endif /* CONFIG_BATMAN_ADV_MCAST */
+
 	batadv_hardif_put(primary_if);
 
 	genlmsg_end(msg, hdr);
@@ -562,6 +569,14 @@  static int batadv_netlink_set_mesh(struct sk_buff *skb, struct genl_info *info)
 	}
 #endif /* CONFIG_BATMAN_ADV_DEBUG */
 
+#ifdef CONFIG_BATMAN_ADV_MCAST
+	if (info->attrs[BATADV_ATTR_MULTICAST_MODE]) {
+		attr = info->attrs[BATADV_ATTR_MULTICAST_MODE];
+
+		atomic_set(&bat_priv->multicast_mode, !!nla_get_u8(attr));
+	}
+#endif /* CONFIG_BATMAN_ADV_MCAST */
+
 	batadv_netlink_notify_mesh(bat_priv);
 
 	return 0;