[B.A.T.M.A.N.] [RFC PATCH maint 1/2] batman-adv: fix packet loss for broadcasted DHCP packets to a server

[B.A.T.M.A.N.] [RFC PATCH maint 1/2] batman-adv: fix packet loss for broadcasted DHCP packets to a server

[Linus Lüssing]

DHCP connectivity issues can currently occur if the following conditions
are met:

1) A DHCP packet from a client to a server
2) This packet has a multicast destination
3) This destination has a matching entry in the translation table
   (FF:FF:FF:FF:FF:FF for IPv4, 33:33:00:01:00:02/33:33:00:01:00:03
    for IPv6)
4) The orig-node determined by TT for the multicast destination
   does not match the orig-node determined by best-gateway-selection

In this case the DHCP packet will be dropped.

The "gateway-out-of-range" check is supposed to only be applied to
unicasted DHCP packets to a specific DHCP server.

In that case dropping the the unicasted frame forces the client to
retry via a broadcasted one, but now directed to the new best
gateway.

A DHCP packet with broadcast/multicast destination is already ensured to
always be delivered to the best gateway. Dropping a multicasted
DHCP packet here will only prevent completing DHCP as there is no
other fallback.

So far, it seems the unicast check was implicitly performed by
expecting the batadv_transtable_search() to return NULL for multicast
destinations. However, a multicast address could have always ended up in
the translation table and in fact is now common.

To fix this potential loss of a DHCP client-to-server packet to a
multicast address this patch adds an explicit multicast destination
check to reliably bail out of the gateway-out-of-range check for such
destinations.

Fixes: afae4e42aae6 ("batman-adv: refactoring gateway handling code")
Not-yet-signed-off-because-untested
---
 net/batman-adv/gateway_client.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index c294f6fd..e7a245bd 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -757,6 +757,9 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
 
 	vid = batadv_get_vid(skb, 0);
 
+	if (is_multicast_ether_addr(ethhdr->h_dest))
+		goto out;
+
 	orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source,
 						 ethhdr->h_dest, vid);
 	if (!orig_dst_node)
-- 
2.11.0

[B.A.T.M.A.N.] [RFC PATCH maint 2/2] batman-adv: fix gateway-out-of-range check

[Linus Lüssing]

The gateway feature is supposed to drop DHCP packet from a client to a
server which was transmitted via unicast if the client's selected gateway
is not the best one anymore and the difference in TQ dropped below a
certain threshold.

This will trigger the DHCP client to resend its DHCP packet via
broadcast/multicast allowing us to forward it to the new, best gateway.

Unfortunately, the gateway-out-of-range check was actually never applied
to unicasted packets so far. Resulting in unhealthily clingy DHCP
clients.

Fixing this by parsing DHCP packets not only for multicasted but also
unicasted ones.

Fixes: afae4e42aae6 ("batman-adv: refactoring gateway handling code")
Not-yet-signed-off-because-untested
---
 net/batman-adv/gateway_client.c | 10 ++++++++--
 net/batman-adv/gateway_client.h |  4 ++--
 net/batman-adv/soft-interface.c | 31 ++++++++++++++-----------------
 3 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index e7a245bd..67dc5642 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -608,6 +608,7 @@ int batadv_gw_dump(struct sk_buff *msg, struct netlink_callback *cb)
 
 /**
  * batadv_gw_dhcp_recipient_get() - check if a packet is a DHCP message
+ * @bat_priv: the bat priv with all the soft interface information
  * @skb: the packet to check
  * @header_len: a pointer to the batman-adv header size
  * @chaddr: buffer where the client address will be stored. Valid
@@ -622,8 +623,8 @@ int batadv_gw_dump(struct sk_buff *msg, struct netlink_callback *cb)
  * - BATADV_DHCP_TO_CLIENT if this is a message going to a DHCP client
  */
 enum batadv_dhcp_recipient
-batadv_gw_dhcp_recipient_get(struct sk_buff *skb, unsigned int *header_len,
-			     u8 *chaddr)
+batadv_gw_dhcp_recipient_get(struct batadv_priv *bat_priv, struct sk_buff *skb,
+			     unsigned int *header_len, u8 *chaddr)
 {
 	enum batadv_dhcp_recipient ret = BATADV_DHCP_NO;
 	struct ethhdr *ethhdr;
@@ -633,8 +634,13 @@ batadv_gw_dhcp_recipient_get(struct sk_buff *skb, unsigned int *header_len,
 	struct vlan_ethhdr *vhdr;
 	int chaddr_offset;
 	__be16 proto;
+	int gw_mode;
 	u8 *p;
 
+	gw_mode = atomic_read(&bat_priv->gw.mode);
+	if (gw_mode == BATADV_GW_MODE_OFF)
+		return BATADV_DHCP_NO;
+
 	/* check for ethernet header */
 	if (!pskb_may_pull(skb, *header_len + ETH_HLEN))
 		return BATADV_DHCP_NO;
diff --git a/net/batman-adv/gateway_client.h b/net/batman-adv/gateway_client.h
index f0b86fcb..2451e1fe 100644
--- a/net/batman-adv/gateway_client.h
+++ b/net/batman-adv/gateway_client.h
@@ -48,8 +48,8 @@ int batadv_gw_client_seq_print_text(struct seq_file *seq, void *offset);
 int batadv_gw_dump(struct sk_buff *msg, struct netlink_callback *cb);
 bool batadv_gw_out_of_range(struct batadv_priv *bat_priv, struct sk_buff *skb);
 enum batadv_dhcp_recipient
-batadv_gw_dhcp_recipient_get(struct sk_buff *skb, unsigned int *header_len,
-			     u8 *chaddr);
+batadv_gw_dhcp_recipient_get(struct batadv_priv *bat_priv, struct sk_buff *skb,
+			     unsigned int *header_len, u8 *chaddr);
 struct batadv_gw_node *batadv_gw_node_get(struct batadv_priv *bat_priv,
 					  struct batadv_orig_node *orig_node);
 
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index edeffcb9..4f9e1440 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -268,20 +268,14 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	if (batadv_compare_eth(ethhdr->h_dest, ectp_addr))
 		goto dropped;
 
-	gw_mode = atomic_read(&bat_priv->gw.mode);
-	if (is_multicast_ether_addr(ethhdr->h_dest)) {
-		/* if gw mode is off, broadcast every packet */
-		if (gw_mode == BATADV_GW_MODE_OFF) {
-			do_bcast = true;
-			goto send;
-		}
+	dhcp_rcp = batadv_gw_dhcp_recipient_get(bat_priv, skb, &header_len,
+						chaddr);
+	/* skb->data may have been modified by
+	 * batadv_gw_dhcp_recipient_get()
+	 */
+	ethhdr = eth_hdr(skb);
 
-		dhcp_rcp = batadv_gw_dhcp_recipient_get(skb, &header_len,
-							chaddr);
-		/* skb->data may have been modified by
-		 * batadv_gw_dhcp_recipient_get()
-		 */
-		ethhdr = eth_hdr(skb);
+	if (is_multicast_ether_addr(ethhdr->h_dest)) {
 		/* if gw_mode is on, broadcast any non-DHCP message.
 		 * All the DHCP packets are going to be sent as unicast
 		 */
@@ -290,14 +284,17 @@ static int batadv_interface_tx(struct sk_buff *skb,
 			goto send;
 		}
 
-		if (dhcp_rcp == BATADV_DHCP_TO_CLIENT)
+		if (dhcp_rcp == BATADV_DHCP_TO_CLIENT) {
 			dst_hint = chaddr;
-		else if ((gw_mode == BATADV_GW_MODE_SERVER) &&
-			 (dhcp_rcp == BATADV_DHCP_TO_SERVER))
+		} else if (dhcp_rcp == BATADV_DHCP_TO_SERVER) {
 			/* gateways should not forward any DHCP message if
 			 * directed to a DHCP server
 			 */
-			goto dropped;
+			gw_mode = atomic_read(&bat_priv->gw.mode);
+
+			if (gw_mode == BATADV_GW_MODE_SERVER)
+				goto dropped;
+		}
 
 send:
 		if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
-- 
2.11.0