[B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Add wrapper for ARP reply creation

[B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Add wrapper for ARP reply creation

[Linus Lüssing]

Removing duplicate code.

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

Changes in v2:
* Rebase to master
* Moved skb_reset_mac_header to earlier position (to avoid calling it on NULL)

Changes in RFC -> non-RFC:
 * Added kerneldoc
 * Added Signed-off-by

 net/batman-adv/distributed-arp-table.c | 67 ++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 27 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index b1cc8bf..081b1dc 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -948,6 +948,41 @@ static unsigned short batadv_dat_get_vid(struct sk_buff *skb, int *hdr_size)
 }
 
 /**
+ * batadv_dat_arp_create_reply - create an ARP Reply
+ * @bat_priv: the bat priv with all the soft interface information
+ * @ip_src: ARP sender IP
+ * @ip_dst: ARP target IP
+ * @hw_src: Ethernet source and ARP sender MAC
+ * @hw_dst: Ethernet destination and ARP target MAC
+ * @vid: VLAN identifier (optional, set to zero otherwise)
+ *
+ * Creates an ARP Reply from the given values, optionally encapsulated in a
+ * VLAN header.
+ *
+ * Return: An skb containing an ARP Reply.
+ */
+static struct sk_buff *
+batadv_dat_arp_create_reply(struct batadv_priv *bat_priv, __be32 ip_src,
+			    __be32 ip_dst, u8 *hw_src, u8 *hw_dst,
+			    unsigned short vid)
+{
+	struct sk_buff *skb;
+
+	skb = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_dst, bat_priv->soft_iface,
+			 ip_src, hw_dst, hw_src, hw_dst);
+	if (!skb)
+		return NULL;
+
+	skb_reset_mac_header(skb);
+
+	if (vid & BATADV_VLAN_HAS_TAG)
+		skb = vlan_insert_tag(skb, htons(ETH_P_8021Q),
+				      vid & VLAN_VID_MASK);
+
+	return skb;
+}
+
+/**
  * batadv_dat_snoop_outgoing_arp_request - snoop the ARP request and try to
  * answer using DAT
  * @bat_priv: the bat priv with all the soft interface information
@@ -1005,20 +1040,12 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 			goto out;
 		}
 
-		skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src,
-				     bat_priv->soft_iface, ip_dst, hw_src,
-				     dat_entry->mac_addr, hw_src);
+		skb_new = batadv_dat_arp_create_reply(bat_priv, ip_dst, ip_src,
+						      dat_entry->mac_addr,
+						      hw_src, vid);
 		if (!skb_new)
 			goto out;
 
-		if (vid & BATADV_VLAN_HAS_TAG) {
-			skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q),
-						  vid & VLAN_VID_MASK);
-			if (!skb_new)
-				goto out;
-		}
-
-		skb_reset_mac_header(skb_new);
 		skb_new->protocol = eth_type_trans(skb_new,
 						   bat_priv->soft_iface);
 		bat_priv->stats.rx_packets++;
@@ -1082,25 +1109,11 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 	if (!dat_entry)
 		goto out;
 
-	skb_new = arp_create(ARPOP_REPLY, ETH_P_ARP, ip_src,
-			     bat_priv->soft_iface, ip_dst, hw_src,
-			     dat_entry->mac_addr, hw_src);
-
+	skb_new = batadv_dat_arp_create_reply(bat_priv, ip_dst, ip_src,
+					      dat_entry->mac_addr, hw_src, vid);
 	if (!skb_new)
 		goto out;
 
-	/* the rest of the TX path assumes that the mac_header offset pointing
-	 * to the inner Ethernet header has been set, therefore reset it now.
-	 */
-	skb_reset_mac_header(skb_new);
-
-	if (vid & BATADV_VLAN_HAS_TAG) {
-		skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q),
-					  vid & VLAN_VID_MASK);
-		if (!skb_new)
-			goto out;
-	}
-
 	/* To preserve backwards compatibility, the node has choose the outgoing
 	 * format based on the incoming request packet type. The assumption is
 	 * that a node not using the 4addr packet format doesn't support it.
-- 
2.1.4

[B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Linus Lüssing]

In a typical mesh network, when a new client connects then it will
usually first try to grab an IPv4 address via DHCP. Afterwards in
public mesh networks a client will try to contact the internet over
the server.

While the IPv4 address of the DHCP-Server is usually well propagated
in the DHT, the IPv4 address of a newly joining client is not.

This can lead to a considerable amount of ARP broadcasts not caught
by DAT from the servers.

In a 1000 nodes mesh network (Freifunk Hamburg) we can still see
30KBit/s of ARP traffic (equalling about 25% of all layer two
specific overhead, remaining after some filtering) flooded through
the mesh. These 30KBit/s are mainly ARP Requests from the
gateways / DHCP servers.

Through snooping DHCPACKs we can actually learn about MAC/IP address
pairs without the need of any flooded ARP messages in advance. This
allows servers to fill their local DAT cache with according entries
before any communciation with a client can possibly have taken place.

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

Changes in v2:
* Rebase to master
* Fix compilation with CONFIG_BATMAN_ADV=n (added stubs)

Changes in RFC -> non-RFC

 * Added kerneldoc
 * Added Signed-off-by
 * More IP Header checks
   (iph->hlen considered, ip version checked, ...)
 * Parsing & checking DHCP Message Type Option,
   only snooping DHCPACKs now
 * Moved ethernet protocol check from batadv_dat_check_dhcp to
   batadv_dat_check_dhcp_ipudp
 * Removed buffer-length parameter from
   batadv_dat_dhcp_get_{yiaddr,chaddr}()
 * Renamed batadv_dat_put() to batadv_dat_put_pairs()

 net/batman-adv/distributed-arp-table.c | 296 +++++++++++++++++++++++++++++++++
 net/batman-adv/distributed-arp-table.h |  11 ++
 net/batman-adv/packet.h                |  48 ++++++
 net/batman-adv/soft-interface.c        |  11 +-
 4 files changed, 364 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 081b1dc..b6b94e2 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -28,6 +28,7 @@
 #include <linux/if_ether.h>
 #include <linux/if_vlan.h>
 #include <linux/in.h>
+#include <linux/ip.h>
 #include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
@@ -40,8 +41,10 @@
 #include <linux/spinlock.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
+#include <linux/udp.h>
 #include <linux/workqueue.h>
 #include <net/arp.h>
+#include <net/ip.h>
 
 #include "hard-interface.h"
 #include "hash.h"
@@ -974,6 +977,7 @@ batadv_dat_arp_create_reply(struct batadv_priv *bat_priv, __be32 ip_src,
 		return NULL;
 
 	skb_reset_mac_header(skb);
+	skb_set_network_header(skb, ETH_HLEN);
 
 	if (vid & BATADV_VLAN_HAS_TAG)
 		skb = vlan_insert_tag(skb, htons(ETH_P_8021Q),
@@ -1237,6 +1241,298 @@ out:
 }
 
 /**
+ * batadv_dat_check_dhcp_ipudp - check skb for IP+UDP headers valid for DHCP
+ * @skb: the packet to check
+ * @proto: ethernet protocol hint (behind a potential vlan)
+ *
+ * Checks whether the given skb has an IP and UDP header valid for a DHCP
+ * message from a DHCP server.
+ *
+ * Return: True if valid, false otherwise.
+ */
+static bool batadv_dat_check_dhcp_ipudp(struct sk_buff *skb)
+{
+	struct iphdr *iphdr, _iphdr;
+	struct udphdr *udphdr, _udphdr;
+	unsigned int offset = skb_network_offset(skb);
+
+	iphdr = skb_header_pointer(skb, offset, sizeof(_iphdr), &_iphdr);
+	if (!iphdr || iphdr->version != 4 || ip_hdrlen(skb) < sizeof(_iphdr))
+		return false;
+
+	if (iphdr->protocol != IPPROTO_UDP)
+		return false;
+
+	offset += ip_hdrlen(skb);
+	skb_set_transport_header(skb, offset);
+
+	udphdr = skb_header_pointer(skb, offset, sizeof(_udphdr), &_udphdr);
+	if (!udphdr || udphdr->source != htons(67))
+		return false;
+
+	return true;
+}
+
+/**
+ * batadv_dat_check_dhcp - examine packet for valid DHCP message
+ * @skb: the packet to check
+ * @proto: ethernet protocol hint (behind a potential vlan)
+ *
+ * Checks whether the given skb is a valid DHCP packet.
+ *
+ * Caller needs to ensure that the skb network header is set correctly.
+ *
+ * Return: If skb is a valid DHCP packet, then returns its op code
+ * (e.g. BOOTREPLY vs. BOOTREQUEST). Otherwise returns -EINVAL.
+ */
+static int batadv_dat_check_dhcp(struct sk_buff *skb, __be16 proto)
+{
+	u8 *op, _op;
+	u8 *htype, _htype;
+	u8 *hlen, _hlen;
+	__be32 *magic, _magic;
+	unsigned int dhcp_offset;
+	unsigned int offset;
+
+	if (proto != htons(ETH_P_IP))
+		return -EINVAL;
+
+	if (!batadv_dat_check_dhcp_ipudp(skb))
+		return -EINVAL;
+
+	dhcp_offset = skb_transport_offset(skb) + sizeof(struct udphdr);
+	if (skb->len < dhcp_offset + sizeof(struct batadv_dhcp_packet))
+		return -EINVAL;
+
+	offset = dhcp_offset + offsetof(struct batadv_dhcp_packet, op);
+
+	op = skb_header_pointer(skb, offset, sizeof(_op), &_op);
+	if (!op)
+		return -EINVAL;
+
+	offset = dhcp_offset + offsetof(struct batadv_dhcp_packet, htype);
+
+	htype = skb_header_pointer(skb, offset, sizeof(_htype), &_htype);
+	if (!htype || *htype != BATADV_HTYPE_ETHERNET)
+		return -EINVAL;
+
+	offset = dhcp_offset + offsetof(struct batadv_dhcp_packet, hlen);
+
+	hlen = skb_header_pointer(skb, offset, sizeof(_hlen), &_hlen);
+	if (!hlen || *hlen != ETH_ALEN)
+		return -EINVAL;
+
+	offset = dhcp_offset + offsetof(struct batadv_dhcp_packet, magic);
+
+	magic = skb_header_pointer(skb, offset, sizeof(_magic), &_magic);
+	if (!magic || *magic != htonl(BATADV_DHCP_MAGIC))
+		return -EINVAL;
+
+	return *op;
+}
+
+/**
+ * batadv_dat_get_dhcp_message_type - get message type of a DHCP packet
+ * @skb: the DHCP packet to parse
+ *
+ * Iterates over the DHCP options of the given DHCP packet to find a
+ * DHCP Message Type option and parse it.
+ *
+ * Caller needs to ensure that the given skb is a valid DHCP packet and
+ * that the skb transport header is set correctly.
+ *
+ * Return: The found DHCP message type value, if found. -EINVAL otherwise.
+ */
+static int batadv_dat_get_dhcp_message_type(struct sk_buff *skb)
+{
+	unsigned int offset = skb_transport_offset(skb) + sizeof(struct udphdr);
+	u8 *type, _type;
+	struct {
+		u8 type;
+		u8 len;
+	} *tl, _tl;
+
+	offset += sizeof(struct batadv_dhcp_packet);
+
+	while ((tl = skb_header_pointer(skb, offset, sizeof(_tl), &_tl))) {
+		if (tl->type == BATADV_DHCP_OPT_MSG_TYPE)
+			break;
+
+		if (tl->type == BATADV_DHCP_OPT_END)
+			break;
+
+		if (tl->type == BATADV_DHCP_OPT_PAD)
+			offset++;
+		else
+			offset += tl->len + sizeof(_tl);
+	}
+
+	/* Option Overload Code not supported */
+	if (!tl || tl->type != BATADV_DHCP_OPT_MSG_TYPE ||
+	    tl->len != sizeof(_type))
+		return -EINVAL;
+
+	offset += sizeof(_tl);
+
+	type = skb_header_pointer(skb, offset, sizeof(_type), &_type);
+	if (!type)
+		return -EINVAL;
+
+	return *type;
+}
+
+/**
+ * batadv_dat_get_dhcp_yiaddr - get yiaddr from a DHCP packet
+ * @skb: the DHCP packet to parse
+ * @buffer: a buffer to store the yiaddr in (if necessary / skb is non-linear)
+ *
+ * Caller needs to ensure that the given skb is a valid DHCP packet and
+ * that the skb transport header is set correctly.
+ *
+ * Return: A safely accessible "Your IP Address" field from the provided DHCP
+ * packet.
+ */
+static __be32 *batadv_dat_dhcp_get_yiaddr(struct sk_buff *skb, __be32 *buffer)
+{
+	unsigned int offset = skb_transport_offset(skb) + sizeof(struct udphdr);
+	unsigned int len = sizeof(((struct batadv_dhcp_packet *)0)->yiaddr);
+
+	offset += offsetof(struct batadv_dhcp_packet, yiaddr);
+
+	return skb_header_pointer(skb, offset, len, buffer);
+}
+
+/**
+ * batadv_dat_get_dhcp_chaddr - get chaddr from a DHCP packet
+ * @skb: the DHCP packet to parse
+ * @buffer: a buffer to store the chaddr in (if necessary / skb is non-linear)
+ *
+ * Caller needs to ensure that the given skb is a valid DHCP packet and
+ * that the skb transport header is set correctly.
+ *
+ * Return: A safely accessible "Client Hardware Address" field from the provided
+ * DHCP packet.
+ */
+static u8 *batadv_dat_get_dhcp_chaddr(struct sk_buff *skb, u8 *buffer)
+{
+	unsigned int offset = skb_transport_offset(skb) + sizeof(struct udphdr);
+	unsigned int len = sizeof(((struct batadv_dhcp_packet *)0)->chaddr);
+
+	offset += offsetof(struct batadv_dhcp_packet, chaddr);
+
+	return skb_header_pointer(skb, offset, len, buffer);
+}
+
+/**
+ * batadv_dat_put_pairs - puts two MAC/IP pairs into the DHT and DAT cache
+ * @bat_priv: the bat priv with all the soft interface information
+ * @hw_src: first value of DHT and ARP sender MAC
+ * @ip_src: first key of DHT and ARP sender IP
+ * @hw_dst: second value of DHT and ARP target MAC
+ * @ip_dst: second key of DHT and ARP target IP
+ * @vid: VLAN identifier
+ *
+ * First checks whether the given MAC/IP pairs are suitable for DAT. If so, adds
+ * them to the local DAT cache and propagates them further into the DHT.
+ *
+ * For the DHT propagation, hw_src/ip_src will appear as the ARP Reply
+ * transmitter (and hw_dst/ip_dst as the target).
+ *
+ * Return: True on success, false otherwise.
+ */
+static bool batadv_dat_put_pairs(struct batadv_priv *bat_priv, u8 *hw_src,
+				 __be32 ip_src, u8 *hw_dst, __be32 ip_dst,
+				 unsigned short vid)
+{
+	struct sk_buff *skb;
+	int hdr_size;
+	u16 type;
+	int ret = false;
+
+	skb = batadv_dat_arp_create_reply(bat_priv, ip_src, ip_dst, hw_src,
+					  hw_dst, vid);
+	if (!skb)
+		return false;
+
+	/* Check for validity of provided addresses */
+	hdr_size = skb_network_offset(skb) - ETH_HLEN;
+	type = batadv_arp_get_type(bat_priv, skb, hdr_size);
+	if (type != ARPOP_REPLY)
+		goto err_skip_commit;
+
+	batadv_dat_entry_add(bat_priv, ip_src, hw_src, vid);
+	batadv_dat_entry_add(bat_priv, ip_dst, hw_dst, vid);
+
+	batadv_dat_send_data(bat_priv, skb, ip_src, vid, BATADV_P_DAT_DHT_PUT);
+	batadv_dat_send_data(bat_priv, skb, ip_dst, vid, BATADV_P_DAT_DHT_PUT);
+
+	ret = true;
+
+err_skip_commit:
+	dev_kfree_skb(skb);
+	return ret;
+}
+
+/**
+ * batadv_dat_snoop_outgoing_dhcp_ack - snoop DHCPACK and fill DAT with it
+ * @bat_priv: the bat priv with all the soft interface information
+ * @skb: the packet to snoop
+ * @proto: ethernet protocol hint (behind a potential vlan)
+ * @vid: VLAN identifier
+ *
+ * This function first checks whether the given skb is a valid DHCPACK. If
+ * so then its source MAC and IP as well as its DHCP Client Hardware Address
+ * field and DHCP Your IP Address field are added to the local DAT cache and
+ * propagated into the DHT.
+ *
+ * Caller needs to ensure that the skb mac and network headers are set
+ * correctly.
+ */
+void batadv_dat_snoop_outgoing_dhcp_ack(struct batadv_priv *bat_priv,
+					struct sk_buff *skb,
+					__be16 proto,
+					unsigned short vid)
+{
+	int type;
+	u8 *chaddr, _chaddr[ETH_ALEN];
+	__be32 *yiaddr, _yiaddr;
+
+	if (!atomic_read(&bat_priv->distributed_arp_table))
+		return;
+
+	if (batadv_dat_check_dhcp(skb, proto) != BATADV_BOOTREPLY)
+		return;
+
+	type = batadv_dat_get_dhcp_message_type(skb);
+	if (type != BATADV_DHCPACK)
+		return;
+
+	yiaddr = batadv_dat_dhcp_get_yiaddr(skb, &_yiaddr);
+	if (!yiaddr)
+		return;
+
+	chaddr = batadv_dat_get_dhcp_chaddr(skb, _chaddr);
+	if (!chaddr)
+		return;
+
+	/* ARP sender MAC + IP -> DHCP Client (chaddr+yiaddr),
+	 * ARP target MAC + IP -> DHCP Server (ethhdr/iphdr sources)
+	 */
+	if (!batadv_dat_put_pairs(bat_priv, chaddr, *yiaddr,
+				  eth_hdr(skb)->h_source, ip_hdr(skb)->saddr,
+				  vid))
+		return;
+
+	batadv_dbg(BATADV_DBG_DAT, bat_priv,
+		   "Snooped from DHCPACK (server-side): %pI4, %pM (vid: %i)\n",
+		   &ip_hdr(skb)->saddr, eth_hdr(skb)->h_source,
+		   BATADV_PRINT_VID(vid));
+	batadv_dbg(BATADV_DBG_DAT, bat_priv,
+		   "Snooped from DHCPACK (client-side): %pI4, %pM (vid: %i)\n",
+		   yiaddr, chaddr, BATADV_PRINT_VID(vid));
+}
+
+/**
  * batadv_dat_drop_broadcast_packet - check if an ARP request has to be dropped
  * (because the node has already obtained the reply via DAT) or not
  * @bat_priv: the bat priv with all the soft interface information
diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h
index 813ecea..cab1172 100644
--- a/net/batman-adv/distributed-arp-table.h
+++ b/net/batman-adv/distributed-arp-table.h
@@ -44,6 +44,10 @@ void batadv_dat_snoop_outgoing_arp_reply(struct batadv_priv *bat_priv,
 					 struct sk_buff *skb);
 bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
 					 struct sk_buff *skb, int hdr_size);
+void batadv_dat_snoop_outgoing_dhcp_ack(struct batadv_priv *bat_priv,
+					struct sk_buff *skb,
+					__be16 proto,
+					unsigned short vid);
 bool batadv_dat_drop_broadcast_packet(struct batadv_priv *bat_priv,
 				      struct batadv_forw_packet *forw_packet);
 
@@ -137,6 +141,13 @@ batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
 	return false;
 }
 
+static inline void
+batadv_dat_snoop_outgoing_dhcp_ack(struct batadv_priv *bat_priv,
+				   struct sk_buff *skb, __be16 proto,
+				   unsigned short vid)
+{
+}
+
 static inline bool
 batadv_dat_drop_broadcast_packet(struct batadv_priv *bat_priv,
 				 struct batadv_forw_packet *forw_packet)
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 6b011ff..d8afbfa 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -664,4 +664,52 @@ struct batadv_tvlv_mcast_data {
 	u8 reserved[3];
 };
 
+enum batadv_bootpop {
+	BATADV_BOOTREQUEST	= 1,
+	BATADV_BOOTREPLY	= 2,
+};
+
+enum batadv_boothtype {
+	BATADV_HTYPE_ETHERNET	= 1,
+};
+
+enum batadv_dhcpoptioncode {
+	BATADV_DHCP_OPT_PAD		= 0,
+	BATADV_DHCP_OPT_MSG_TYPE	= 53,
+	BATADV_DHCP_OPT_END		= 255,
+};
+
+enum batadv_dhcptype {
+	BATADV_DHCPDISCOVER	= 1,
+	BATADV_DHCPOFFER	= 2,
+	BATADV_DHCPREQUEST	= 3,
+	BATADV_DHCPDECLINE	= 4,
+	BATADV_DHCPACK		= 5,
+	BATADV_DHCPNAK		= 6,
+	BATADV_DHCPRELEASE	= 7,
+	BATADV_DHCPINFORM	= 8,
+};
+
+/* { 99, 130, 83, 99 } */
+#define BATADV_DHCP_MAGIC 1669485411
+
+struct batadv_dhcp_packet {
+	u8 op;
+	u8 htype;
+	u8 hlen;
+	u8 hops;
+	__be32 xid;
+	__be16 secs;
+	__be16 flags;
+	__be32 ciaddr;
+	__be32 yiaddr;
+	__be32 siaddr;
+	__be32 giaddr;
+	u8 chaddr[16];
+	u8 sname[64];
+	u8 file[128];
+	__be32 magic;
+	u8 options[0];
+};
+
 #endif /* _NET_BATMAN_ADV_PACKET_H_ */
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 216ac03..12bc41b 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -204,6 +204,7 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	enum batadv_forw_mode forw_mode;
 	struct batadv_orig_node *mcast_single_orig = NULL;
 	int network_offset = ETH_HLEN;
+	__be16 proto;
 
 	if (atomic_read(&bat_priv->mesh_state) != BATADV_MESH_ACTIVE)
 		goto dropped;
@@ -212,12 +213,15 @@ static int batadv_interface_tx(struct sk_buff *skb,
 	vid = batadv_get_vid(skb, 0);
 	ethhdr = eth_hdr(skb);
 
-	switch (ntohs(ethhdr->h_proto)) {
+	proto = ethhdr->h_proto;
+
+	switch (ntohs(proto)) {
 	case ETH_P_8021Q:
 		vhdr = vlan_eth_hdr(skb);
+		proto = vhdr->h_vlan_encapsulated_proto;
 
 		/* drop batman-in-batman packets to prevent loops */
-		if (vhdr->h_vlan_encapsulated_proto != htons(ETH_P_BATMAN)) {
+		if (proto != htons(ETH_P_BATMAN)) {
 			network_offset += VLAN_HLEN;
 			break;
 		}
@@ -244,6 +248,9 @@ static int batadv_interface_tx(struct sk_buff *skb,
 			goto dropped;
 	}
 
+	/* Snoop address candidates from DHCPACKs for early DAT filling */
+	batadv_dat_snoop_outgoing_dhcp_ack(bat_priv, skb, proto, vid);
+
 	/* don't accept stp packets. STP does not help in meshes.
 	 * better use the bridge loop avoidance ...
 	 *
-- 
2.1.4

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]

On Tue, Jul 05, 2016 at 08:01:37PM +0200, Linus Lüssing wrote:
> In a typical mesh network, when a new client connects then it will
> usually first try to grab an IPv4 address via DHCP. Afterwards in
> public mesh networks a client will try to contact the internet over
> the server.
> 
> While the IPv4 address of the DHCP-Server is usually well propagated
> in the DHT, the IPv4 address of a newly joining client is not.
> 
> This can lead to a considerable amount of ARP broadcasts not caught
> by DAT from the servers.
> 
> In a 1000 nodes mesh network (Freifunk Hamburg) we can still see
> 30KBit/s of ARP traffic (equalling about 25% of all layer two
> specific overhead, remaining after some filtering) flooded through
> the mesh. These 30KBit/s are mainly ARP Requests from the
> gateways / DHCP servers.
> 
> Through snooping DHCPACKs we can actually learn about MAC/IP address
> pairs without the need of any flooded ARP messages in advance. This
> allows servers to fill their local DAT cache with according entries
> before any communciation with a client can possibly have taken place.

Linus,

have you tried applying this patch on one of your servers and measure the local
effect? (i.e. if the number of BRD ARP req is reduced or not?)

I think that a DHCP ACK packet should already refresh the local ARP cache (or
not?), thus the need for an ARP request should not be triggered by the newly
joined client. (I might be wrong, but that's why I ask measuring the effect)

Cheers,

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

On Thursday 07 July 2016 17:24:36 Antonio Quartulli wrote:
[...]
> have you tried applying this patch on one of your servers and measure the local
> effect? (i.e. if the number of BRD ARP req is reduced or not?)

He is currently testing with the help of Freifunk Darmstadt. The pretests are
already done but he is waiting for the patches to be deployed on all nodes
running DHCPd. He is also waiting for some extra feedback regarding their
actual setup.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Linus Lüssing]

Hi Antonio,

> have you tried applying this patch on one of your servers and measure the local
> effect? (i.e. if the number of BRD ARP req is reduced or not?)

I had queried Martin Weinelt just yesterday, they will test it on
a 500 nodes compat-v15 setup soon :).

> 
> I think that a DHCP ACK packet should already refresh the local ARP cache (or
> not?), thus the need for an ARP request should not be triggered by the newly
> joined client. (I might be wrong, but that's why I ask measuring the effect)

Hm, I don't think that the operating system will snoop DHCPACKs
itself to fill its ARP cache. I didn't see that in the VMs either.

But now that you are asking, re-read my thoughts regarding ARP+DAT
on the Gluon issue tracker. Which made me wonder again, why
the issue occures in the first place (e.g. client issues an ARP
Request first - why does that one not fill the DAT appropriately,
why does it need to fallback from the unicast DAT query to an ARP
Request flood?).

Looking at the code again now, I have a hunch. Question: Why 
does batadv_dat_snoop_outgoing_arp_request() add the MAC/IP source
pair of the client to the local DAT cache only, why isn't it
propagated to the DHT?

Regards, Linus

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Linus Lüssing]

On Thu, Jul 07, 2016 at 12:14:42PM +0200, Linus Lüssing wrote:
> Hi Antonio,
> 
> > have you tried applying this patch on one of your servers and measure the local
> > effect? (i.e. if the number of BRD ARP req is reduced or not?)
> 
> I had queried Martin Weinelt just yesterday, they will test it on
> a 500 nodes compat-v15 setup soon :).

Unfortunately, looks like this patch has no effect regarding the
ARP Request overhead. According to batctl log, DHCPACKs are
snooped, but still 84.3% of all ARP Requests come from the three
DHCP servers / gateways.

These ARP Requests come encapsulated in a batman-adv broadcast
packet type.

Hm, anyone having another idea what might still be causing these
ARP Requests?

Cheers, Linus

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

On Thu, Jul 07, 2016 at 03:40:52PM +0200, Linus Lüssing wrote:
> On Thu, Jul 07, 2016 at 12:14:42PM +0200, Linus Lüssing wrote:
> > Hi Antonio,
> > 
> > > have you tried applying this patch on one of your servers and measure the local
> > > effect? (i.e. if the number of BRD ARP req is reduced or not?)
> > 
> > I had queried Martin Weinelt just yesterday, they will test it on
> > a 500 nodes compat-v15 setup soon :).
> 
> Unfortunately, looks like this patch has no effect regarding the
> ARP Request overhead. According to batctl log, DHCPACKs are
> snooped, but still 84.3% of all ARP Requests come from the three
> DHCP servers / gateways.
> 
> These ARP Requests come encapsulated in a batman-adv broadcast
> packet type.
> 
> Hm, anyone having another idea what might still be causing these
> ARP Requests?

have you tried enabling DAT debug and observing step by step what happens ? You
could try to study a simple case: i.e. an unknown client that comes and
connects for the first time..you can save the full log somewhere and then we can
all have a look (knowing the MAC address of this client)

Cheers,

-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Add wrapper for ARP reply creation

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Dienstag, 5. Juli 2016 20:01:36 CEST Linus Lüssing wrote:
> Removing duplicate code.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
> 
> Changes in v2:
> * Rebase to master
> * Moved skb_reset_mac_header to earlier position (to avoid calling it on
> NULL)

Reviewed-by: Sven Eckelmann <sven@narfation.org>

Small request for the next submissions: Could you please add a space between 
"PATCH" and "v2" (like `git format-patch -v2` does). This will make sure that 
patchwork will remove the unnecessary information "PATCH" from the title.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

On Dienstag, 5. Juli 2016 20:01:37 CEST Linus Lüssing wrote:
>  /**
> + * batadv_dat_check_dhcp_ipudp - check skb for IP+UDP headers valid for
> DHCP + * @skb: the packet to check
> + * @proto: ethernet protocol hint (behind a potential vlan)
> + *
> + * Checks whether the given skb has an IP and UDP header valid for a DHCP
> + * message from a DHCP server.
> + *
> + * Return: True if valid, false otherwise.
> + */
> +static bool batadv_dat_check_dhcp_ipudp(struct sk_buff *skb)
> +{


Looks like this function doesn't have the parameter proto which you've 
described in the kerneldoc.

It sounds like the patch didn't help. So I will postpone my review.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [B.A.T.M.A.N.] [PATCHv2 2/2] batman-adv: Snoop DHCPACKs for DAT

[Linus Lüssing]

On Sun, Jul 10, 2016 at 01:42:32PM +0200, Sven Eckelmann wrote:
> It sounds like the patch didn't help. So I will postpone my review.

Right. As discussed with Antonio on IRC, we might go with
Andreas's original patch to snoop IP traffic - but only to update
already existing, local DAT entries. So some changes still needed
in that patch there.

The approach is similar to what the Linux IP stack does for it's own
neighbor tables already.

Then we will see how much ARP traffic is still left afterwards.

I'm marking this patch as "Defered" in patchwork.


Further revelations, test results, ideas and progress will be
probably be tracked here:

https://www.open-mesh.org/issues/291

Re: [B.A.T.M.A.N.] [v2, 1/2] batman-adv: Add wrapper for ARP reply creation

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

On Dienstag, 5. Juli 2016 20:01:36 CEST Linus Lüssing wrote:
> Removing duplicate code.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> Reviewed-by: Sven Eckelmann <sven@narfation.org>
> ---
> 
> Changes in v2:
> * Rebase to master
> * Moved skb_reset_mac_header to earlier position (to avoid calling it on
> NULL)
> 
> Changes in RFC -> non-RFC:
>  * Added kerneldoc
>  * Added Signed-off-by
> 
>  net/batman-adv/distributed-arp-table.c | 67
> ++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 27
> deletions(-)

Applied in 6be651ac911060ef7e0364650275b56a6af28cfd [1].

Kind regards,
	Sven

[1] https://git.open-mesh.org/batman-adv.git/commit/6be651ac911060ef7e0364650275b56a6af28cfd

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]