batman-adv: introduce "noflood" broadcast flood prevention option

Message ID 20190426171231.18156-1-linus.luessing@c0d3.blue (mailing list archive)
State Rejected, archived
Delegated to: Simon Wunderlich
Headers
Series batman-adv: introduce "noflood" broadcast flood prevention option |

Commit Message

Linus Lüssing April 26, 2019, 5:12 p.m. UTC
  With DAT DHCP snooping, the gateway feature and multicast optimizations
in place in some scenarios broadcast flooding might not be strictly
necessary anymore to be able to establish IPv4/IPv6 communication.
Therefore this patch adds an option to disable broadcast flooding.

Larger mesh networks typically filter a variety of multicast packets via
ebtables/netfilter to clamp on overhead. With this option such firewall
rules can be relaxed so that such multicast packets are only dropped
if they cannot be handled by multicast-to-unicast, for instance.

"noflood" comes in two flavours: If set to 1 then flood prevention is
enabled for all multicast/broadcast packets except ICMPv6 and IGMP
(cautious mode). Or, if set to 2 then flood prevention is enabled for
all multicast/broadcast packets (aggressive mode). If set to 0 then
flood prevention is disabled.

"noflood" is disabled by default as there are still some things to take
care of to avoid breaking things (especially for the "aggressive mode").

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

---
The initial approach was a "noflood"-mark which worked similar to the
isolation mark. Which allowed more flexibility so that the user could
mark specific packets to be excluded from broadcast flooding via
ebtables/netfilter. However, in practice skb-marking is not that easy to
configure and if misconfigured will break a network horribly. Which was
also a downside noted and discussed at BattleMesh v11.

This version now is a less flexible but therefore also simpler to use
variant.

[0]: https://git.open-mesh.org/batman-adv.git/shortlog/refs/heads/linus/noflood-mark
[1]: https://github.com/freifunk-gluon/gluon/pull/1357
---
 include/uapi/linux/batman_adv.h | 10 ++++
 net/batman-adv/netlink.c        | 10 ++++
 net/batman-adv/soft-interface.c | 85 +++++++++++++++++++++++++++++++++
 net/batman-adv/types.h          |  6 +++
 4 files changed, 111 insertions(+)
  

Comments

Marek Lindner April 26, 2019, 9:56 p.m. UTC | #1
On Saturday, 27 April 2019 01:12:31 HKT Linus Lüssing wrote:
> With DAT DHCP snooping, the gateway feature and multicast optimizations
> in place in some scenarios broadcast flooding might not be strictly
> necessary anymore to be able to establish IPv4/IPv6 communication.
> Therefore this patch adds an option to disable broadcast flooding.
> 
> Larger mesh networks typically filter a variety of multicast packets via
> ebtables/netfilter to clamp on overhead. With this option such firewall
> rules can be relaxed so that such multicast packets are only dropped
> if they cannot be handled by multicast-to-unicast, for instance.

Could you outline the use-case for this specific noflood option in more detail ?
The description above is not entirely clear to me. Especially, the 'might not 
be strictly necessary anymore' to 'firewall rules can be relaxed'. How are 
these things connected ? Is this option implemented only, so that some firewall 
rules don't need to be set anymore ?
What happens if a user enables 'noflood' but does not fall into the 'might not 
be strictly necessary anymore' category ?

Thanks,
Marek
  
Linus Lüssing April 27, 2019, 2:38 a.m. UTC | #2
On Sat, Apr 27, 2019 at 05:56:03AM +0800, Marek Lindner wrote:
> On Saturday, 27 April 2019 01:12:31 HKT Linus Lüssing wrote:
> > With DAT DHCP snooping, the gateway feature and multicast optimizations
> > in place in some scenarios broadcast flooding might not be strictly
> > necessary anymore to be able to establish IPv4/IPv6 communication.
> > Therefore this patch adds an option to disable broadcast flooding.
> > 
> > Larger mesh networks typically filter a variety of multicast packets via
> > ebtables/netfilter to clamp on overhead. With this option such firewall
> > rules can be relaxed so that such multicast packets are only dropped
> > if they cannot be handled by multicast-to-unicast, for instance.
> 
> Could you outline the use-case for this specific noflood option in more detail ?
> The description above is not entirely clear to me. Especially, the 'might not 
> be strictly necessary anymore' to 'firewall rules can be relaxed'. How are 
> these things connected ? Is this option implemented only, so that some firewall 
> rules don't need to be set anymore ?

The main use-case I currently have in mind is safely enabling multicast in
larger, public mesh networks:

Currently we have firewall rules in Gluon to drop most multicast.
With multicast-to-multi-unicast we could in theory use multicast
without creating broadcast overhead for the whole mesh. However
only until we hit the multicast_fanout threshold. Then things
would get flooded again.

The desired behaviour in this case would be to let multicast packets pass
unless they would be flooded. A firewall does not know which
mechanism batman-adv would choose. Hence this option within
batman-adv to create this desired behaviour.


With "might not be strictly necessary anymore" I ment that if
certain requirements are met that address assignments and address
resolution can now be achieved without needing broadcast flooding.


> What happens if a user enables 'noflood' but does not fall into the 'might not 
> be strictly necessary anymore' category ?

Well, broken connectivity. Typing "ip link set dev eth0 multicast off"
in a setup which still needs multicast to function would be an
analogy then :).

Regards, Linus
  
Linus Lüssing April 27, 2019, 2:53 a.m. UTC | #3
On Sat, Apr 27, 2019 at 04:38:49AM +0200, Linus Lüssing wrote:
> The desired behaviour in this case would be to let multicast packets pass
> unless they would be flooded. A firewall does not know which
> mechanism batman-adv would choose. Hence this option within
> batman-adv to create this desired behaviour.

Or in other words: I want to allow people to experiment with and use
multicast in a variety of undefined applications. But I don't want
them to break things for everyone else (which could happen with no
firewall rules, too many listeners and a broadcast flooding
fallback).
  
Sven Eckelmann April 28, 2019, 5:04 p.m. UTC | #4
On Friday, 26 April 2019 19:12:31 CEST Linus Lüssing wrote:
> With DAT DHCP snooping, the gateway feature and multicast optimizations
> in place in some scenarios broadcast flooding might not be strictly
> necessary anymore to be able to establish IPv4/IPv6 communication.
> Therefore this patch adds an option to disable broadcast flooding.
> 
> Larger mesh networks typically filter a variety of multicast packets via
> ebtables/netfilter to clamp on overhead. With this option such firewall
> rules can be relaxed so that such multicast packets are only dropped
> if they cannot be handled by multicast-to-unicast, for instance.
> 
> "noflood" comes in two flavours: If set to 1 then flood prevention is
> enabled for all multicast/broadcast packets except ICMPv6 and IGMP
> (cautious mode). Or, if set to 2 then flood prevention is enabled for
> all multicast/broadcast packets (aggressive mode). If set to 0 then
> flood prevention is disabled.
> 
> "noflood" is disabled by default as there are still some things to take
> care of to avoid breaking things (especially for the "aggressive mode").
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

@Martin, I think you've started to experiment with this noflood feature. Any 
experiences which you can share with us?

Kind regards,
	Sven
  
Martin Weinelt April 28, 2019, 7:04 p.m. UTC | #5
Hi everyone,

Please see my reply below.

On 4/28/19 7:04 PM, Sven Eckelmann wrote:
> On Friday, 26 April 2019 19:12:31 CEST Linus Lüssing wrote:
>> With DAT DHCP snooping, the gateway feature and multicast optimizations
>> in place in some scenarios broadcast flooding might not be strictly
>> necessary anymore to be able to establish IPv4/IPv6 communication.
>> Therefore this patch adds an option to disable broadcast flooding.
>>
>> Larger mesh networks typically filter a variety of multicast packets via
>> ebtables/netfilter to clamp on overhead. With this option such firewall
>> rules can be relaxed so that such multicast packets are only dropped
>> if they cannot be handled by multicast-to-unicast, for instance.
>>
>> "noflood" comes in two flavours: If set to 1 then flood prevention is
>> enabled for all multicast/broadcast packets except ICMPv6 and IGMP
>> (cautious mode). Or, if set to 2 then flood prevention is enabled for
>> all multicast/broadcast packets (aggressive mode). If set to 0 then
>> flood prevention is disabled.
>>
>> "noflood" is disabled by default as there are still some things to take
>> care of to avoid breaking things (especially for the "aggressive mode").
>>
>> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> 
> @Martin, I think you've started to experiment with this noflood feature. Any 
> experiences which you can share with us?
> 
> Kind regards,
> 	Sven
> 

We have been using the early noflood and DHCP snooping patches from
Linus since roughly 2018/04. They were based on sockmarking packets that
should be handled by noflood. This resulted in quite some amount of
ebtables rules on our gateways, that marked addresses within DHCP ranges
for noflood, since they were very likely already snooped. The result can
be seen in graphs I provided to Linus back then, that are now visible at
https://www.open-mesh.org/projects/batman-adv/wiki/DAT_DHCP_Snooping#Result.

We did not experience any usability issues that could be traced back to
these patches. I've dropped the patches when the DHCP snooping landed
upstream, so we're currently (since 2019/04) running
v2019.1-14-g28573050 without without noflood.

I have no knowledge of this versions two new "flavours", I was pretty
happy with the aggressiveness of the earlier patches even though it's
manual setup, where we created those ebtables rules from scratch. I
would be happy to also test these new changes, but I'm currently lacking
the wirerrd (https://github.com/T-X/wirerrd) setup that created those
fancy graphs.


Best regards,
Martin
  
Linus Lüssing April 30, 2019, 4:01 p.m. UTC | #6
On Sun, Apr 28, 2019 at 09:04:19PM +0200, Martin Weinelt wrote:
> We have been using the early noflood and DHCP snooping patches from
> Linus since roughly 2018/04. They were based on sockmarking packets that
> should be handled by noflood. This resulted in quite some amount of
> ebtables rules on our gateways, that marked addresses within DHCP ranges
> for noflood, since they were very likely already snooped. The result can
> be seen in graphs I provided to Linus back then, that are now visible at
> https://www.open-mesh.org/projects/batman-adv/wiki/DAT_DHCP_Snooping#Result.

Though to be fair, I'm expecting similar results with the DAT DHCP
snooping + pending DAT cache/DHT split patches. At least the
Total->unanswered->ok->last-reply->0...30min. and
Total->answered->"via DAT BCAST" in the link should go
away then (90.09% of all broadcast flooded ARP Replies in this link).

So while that was the main use-case for me before the discussions
we had last year on how to improve DAT, it's now more a minor one
for me. Though it would still be nice to have / work towards a zero-broadcast
mesh as RFC7772 for instance explains that frequent broadcasts
have a quite significant impact on battery power for small, mobile
devices. And the noflood option would be a quick and easy option
for us for now to achieve this for ARP and other packet types.

[0]: https://tools.ietf.org/html/rfc7772
  
Linus Lüssing April 30, 2019, 4:07 p.m. UTC | #7
On Tue, Apr 30, 2019 at 06:01:21PM +0200, Linus Lüssing wrote:
> So while that was the main use-case for me before the discussions
> we had last year on how to improve DAT, it's now more a minor one
> for me. Though it would still be nice to have / work towards a zero-broadcast
> mesh as RFC7772 for instance explains that frequent broadcasts
> have a quite significant impact on battery power for small, mobile
> devices. And the noflood option would be a quick and easy option
> for us for now to achieve this for ARP and other packet types.
> 
> [0]: https://tools.ietf.org/html/rfc7772

More precisely, RFC7772 section 4 has some estimates regarding power
cost for having frequent broadcast packets.
  
Sven Eckelmann May 2, 2019, 6:40 a.m. UTC | #8
On Friday, 26 April 2019 19:12:31 CEST Linus Lüssing wrote:
> With DAT DHCP snooping, the gateway feature and multicast optimizations
> in place in some scenarios broadcast flooding might not be strictly
> necessary anymore to be able to establish IPv4/IPv6 communication.
> Therefore this patch adds an option to disable broadcast flooding.
> 
> Larger mesh networks typically filter a variety of multicast packets via
> ebtables/netfilter to clamp on overhead. With this option such firewall
> rules can be relaxed so that such multicast packets are only dropped
> if they cannot be handled by multicast-to-unicast, for instance.
> 
> "noflood" comes in two flavours: If set to 1 then flood prevention is
> enabled for all multicast/broadcast packets except ICMPv6 and IGMP
> (cautious mode). Or, if set to 2 then flood prevention is enabled for
> all multicast/broadcast packets (aggressive mode). If set to 0 then
> flood prevention is disabled.
> 
> "noflood" is disabled by default as there are still some things to take
> care of to avoid breaking things (especially for the "aggressive mode").
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> 
> ---
> The initial approach was a "noflood"-mark which worked similar to the
> isolation mark. Which allowed more flexibility so that the user could
> mark specific packets to be excluded from broadcast flooding via
> ebtables/netfilter. However, in practice skb-marking is not that easy to
> configure and if misconfigured will break a network horribly. Which was
> also a downside noted and discussed at BattleMesh v11.
> 
> This version now is a less flexible but therefore also simpler to use
> variant.

It looks like this is an option which can break the mesh rather easily. Here 
some quotes from 2019-04-30/2019-05-01:

<marec> even the 'cautious' option drops everything except for IGMP and ICMPv6
<marec> nobody has a clear picture what that $everything is
<marec> how does it help you if IGMP and ICMPv6 go through if all other multicast / broadcast is dropped ?
<marec> (because of too many mesh participants with multicast optimizations disabled)
<T_X> with IGMP/ICMPv6 to always go through I hope we have the most basic things for IP-IP communication covered and unsucceptible to DoS cases you have already outlined
<marec> T_X: how so ?
<marec> T_X: even considering your gluon case - how many nodes would you need to update and have to enable mcast optimization to safely turn on noflood ?
<marec> as I recall, your meshes are all bridged together over vpns
<marec> with only 16 nodes not upgraded or mcast optimization disabled noflood will be permanently active
<marec> (or whatever your fanout value is)

<T_X> so even if no node were having the multicast optimizations enabled, then communication both on the intranet and to the internet would still work
<marec> that was not my point
<marec> I want to allow people to experiment with and use multicast in a variety of undefined applications. <<< how do you want to get there ?
<marec> while noflood drops everything
<marec> (except for maybe IGMP and ICMPv6)
<T_X> ah. yes. in that case, such applications would not / would stop working. but that's our current default case with basically filtering all/most multicast traffic except ICMPv6 via ebtables in Gluon

<marec> even standard DAT falls back to flooding, isn't it ?
<T_X> marec: yep
<marec> so, ARP does not work with noflood ?
<T_X> if no ARP response in x milliseconds (250ms I think?) then it will flood the ARP request
<T_X> marec: right


<marec> sounds to me the noflood will be pretty difficult to understand and debug
<marec> how does a normal user understand when ARP works sometimes .. or even normal mcast apps work sometimes ?
<marec> how will that seeming randomness be distinguishable from normal packet loss ?
<T_X> yes. would at least need some thorough wiki page, I guess. *cough* :)
<marec> I am talking about user outside the gluon garden
<marec> sorry for pointing it out again but this noflood feature still feels like a gluon tailored solution - assuming auto-updaters, mcast enabled, no IPv4 or other things other than IPv6 with router advertisements


<ordex> my opinion is that I agree with T_X when he says "just add this patch to the Gluon repository for now"
<ordex> honestly, skb-marking is much better imho. way less prone to mesh-explosions and people (that have a thorough understanding of ARP, ICMPv6, IGMP, MLD and understand what batman-adv would do or not) can decide how to treat such packets *externally* to batman-adv
<ordex> this is what how we use the client-isolation with skb-marking - the dropping logic is implemented outside
<ordex> with a match on the skb-mark
<T_X> ok. and both the client-isolation and noflood skb->markings would fullfil a kind of similar purpose - that is preventing specific packets from reaching $destination, which could not be fullfilled with netfilter alone, right?
[...]
<ordex> the reason for the skb-marking is to have something to match packets against. In the client isolation case the knowledge about what to drop was inside batman-adv, thus we needed a way to "export" this knowledge



I would guess that you can also do something like this using ebtables + tc 
filter. Even with (tc) eBPF... since eBPF seems to be the miracle solution 
for everything these days.

Kind regards,
	Sven
  

Patch

diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batman_adv.h
index 67f46367..d68f8868 100644
--- a/include/uapi/linux/batman_adv.h
+++ b/include/uapi/linux/batman_adv.h
@@ -480,6 +480,16 @@  enum batadv_nl_attrs {
 	 */
 	BATADV_ATTR_MULTICAST_FANOUT,
 
+	/**
+	 * @BATADV_ATTR_NOFLOOD: defines if and how strictly flooding prevention
+	 * is configured. If the value is 0 then flood prevention is disabled.
+	 * If the value is 1 then flood prevention is enabled for all multicast
+	 * /broadcast packets excluding ICMPv6 and IGMP (cautious mode). If set
+	 * to 2 then flood prevention is enabled for all multicast/broadcast
+	 * packets (aggressive mode).
+	 */
+	BATADV_ATTR_NOFLOOD,
+
 	/* 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 e7907308..940a9c37 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -358,6 +358,10 @@  static int batadv_netlink_mesh_fill(struct sk_buff *msg,
 			atomic_read(&bat_priv->orig_interval)))
 		goto nla_put_failure;
 
+	if (nla_put_u8(msg, BATADV_ATTR_NOFLOOD,
+		       atomic_read(&bat_priv->noflood)))
+		goto nla_put_failure;
+
 	if (primary_if)
 		batadv_hardif_put(primary_if);
 
@@ -614,6 +618,12 @@  static int batadv_netlink_set_mesh(struct sk_buff *skb, struct genl_info *info)
 		atomic_set(&bat_priv->orig_interval, orig_interval);
 	}
 
+	if (info->attrs[BATADV_ATTR_NOFLOOD]) {
+		attr = info->attrs[BATADV_ATTR_NOFLOOD];
+
+		atomic_set(&bat_priv->noflood, nla_get_u8(attr));
+	}
+
 	batadv_netlink_notify_mesh(bat_priv);
 
 	return 0;
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index a7677e1d..680e3b03 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -18,6 +18,11 @@ 
 #include <linux/gfp.h>
 #include <linux/if_ether.h>
 #include <linux/if_vlan.h>
+#include <linux/igmp.h>
+#include <linux/in.h>
+#include <linux/in6.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
@@ -37,6 +42,7 @@ 
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <net/addrconf.h>
 #include <uapi/linux/batadv_packet.h>
 #include <uapi/linux/batman_adv.h>
 
@@ -176,6 +182,81 @@  static void batadv_interface_set_rx_mode(struct net_device *dev)
 {
 }
 
+/**
+ * batadv_softif_noflood_is_igmp() - check if a packet is IGMP
+ * @bat_priv: the bat priv with all the soft interface information
+ * @skb: the multicast/broadcast packet to check
+ *
+ * Return: True if the given skb is an IGMP packet, false otherwise.
+ */
+static bool batadv_softif_noflood_is_igmp(struct sk_buff *skb)
+{
+	int ret = ip_mc_check_igmp(skb);
+
+	if (ret == -ENOMEM || ret == -EINVAL)
+		return false;
+
+	/* ret == -ENOMSG => valid IPv6 header */
+	if (ret == -ENOMSG && ip_hdr(skb)->protocol != IPPROTO_IGMP)
+		return false;
+
+	/* it's IGMP */
+	return true;
+}
+
+/**
+ * batadv_softif_noflood_is_icmpv6() - check if a packet is ICMPv6
+ * @bat_priv: the bat priv with all the soft interface information
+ * @skb: the multicast/broadcast packet to check
+ *
+ * Return: True if the given skb is an ICMPv6 packet, false otherwise.
+ */
+static bool batadv_softif_noflood_is_icmpv6(struct sk_buff *skb)
+{
+	int ret = ipv6_mc_check_mld(skb);
+
+	if (ret == -ENOMEM || ret == -EINVAL)
+		return false;
+
+	/* ret == -ENOMSG => valid IPv6 header */
+	if (ret == -ENOMSG && ipv6_hdr(skb)->nexthdr != IPPROTO_ICMPV6)
+		return false;
+
+	/* it's ICMPv6 */
+	return true;
+}
+
+/**
+ * batadv_softif_check_noflood() - check if flood-prevention is enabled
+ * @bat_priv: the bat priv with all the soft interface information
+ * @skb: the multicast/broadcast packet to check
+ *
+ * Return: True if flood prevention is enabled for this packet type, false
+ * otherwise.
+ */
+static bool
+batadv_softif_check_noflood(struct batadv_priv *bat_priv, struct sk_buff *skb)
+{
+	int ret = atomic_read(&bat_priv->noflood);
+
+	/* disabled */
+	if (!ret)
+		return false;
+	/* aggressive mode */
+	else if (ret == 2)
+		return true;
+
+	/* exclude ICMPv6 and IGMP from "noflood" */
+	switch (ntohs(eth_hdr(skb)->h_proto)) {
+	case ETH_P_IP:
+		return !batadv_softif_noflood_is_igmp(skb);
+	case ETH_P_IPV6:
+		return !batadv_softif_noflood_is_icmpv6(skb);
+	default:
+		return true;
+	}
+}
+
 static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 				       struct net_device *soft_iface)
 {
@@ -326,6 +407,9 @@  static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 		if (batadv_dat_snoop_outgoing_arp_request(bat_priv, skb))
 			brd_delay = msecs_to_jiffies(ARP_REQ_DELAY);
 
+		if (batadv_softif_check_noflood(bat_priv, skb))
+			goto dropped;
+
 		if (batadv_skb_head_push(skb, sizeof(*bcast_packet)) < 0)
 			goto dropped;
 
@@ -823,6 +907,7 @@  static int batadv_softif_init_late(struct net_device *dev)
 	atomic_set(&bat_priv->log_level, 0);
 #endif
 	atomic_set(&bat_priv->fragmentation, 1);
+	atomic_set(&bat_priv->noflood, 0);
 	atomic_set(&bat_priv->packet_size_max, ETH_DATA_LEN);
 	atomic_set(&bat_priv->bcast_queue_left, BATADV_BCAST_QUEUE_LEN);
 	atomic_set(&bat_priv->batman_queue_left, BATADV_BATMAN_QUEUE_LEN);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 357ca119..f3f96f02 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1575,6 +1575,12 @@  struct batadv_priv {
 	atomic_t log_level;
 #endif
 
+	/**
+	 * @noflood: option indicating whether to drop packets that would
+	 *  be flooded through the mesh otherwise
+	 */
+	atomic_t noflood;
+
 	/**
 	 * @isolation_mark: the skb->mark value used to match packets for AP
 	 *  isolation