[v6,3/3] batman-adv: mcast: shrink tracker packet after scrubbing

Message ID 20230720043556.12163-4-linus.luessing@c0d3.blue
State Superseded, archived
Delegated to: Simon Wunderlich
Headers
Series Implementation of a Stateless Multicast Packet Type |

Commit Message

Linus Lüssing July 20, 2023, 4:35 a.m. UTC
  Remove all zero MAC address entries (00:00:00:00:00:00) from a multicast
packet's tracker TVLV before transmitting it and update all headers
accordingly. This way, instead of keeping the multicast packet at a
constant size throughout its journey through the mesh, it will become
more lightweight, smaller with every interested receiver on the way and
on each splitting intersection. Which can save some valuable bandwidth.

Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
---
 net/batman-adv/multicast_forw.c | 195 ++++++++++++++++++++++++++++++++
 1 file changed, 195 insertions(+)
  

Comments

Sven Eckelmann July 29, 2023, 11:11 a.m. UTC | #1
On Thursday, 20 July 2023 06:35:55 CEST Linus Lüssing wrote:
> Remove all zero MAC address entries (00:00:00:00:00:00) from a multicast
> packet's tracker TVLV before transmitting it and update all headers
> accordingly. This way, instead of keeping the multicast packet at a
> constant size throughout its journey through the mesh, it will become
> more lightweight, smaller with every interested receiver on the way and
> on each splitting intersection. Which can save some valuable bandwidth.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/batman-adv/multicast_forw.c | 195 ++++++++++++++++++++++++++++++++
>  1 file changed, 195 insertions(+)

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

Kind regards,
	Sven
  
Simon Wunderlich Aug. 14, 2023, 3:56 p.m. UTC | #2
Unaddressed
On Thursday, July 20, 2023 6:35:55 AM CEST Linus Lüssing wrote:
> Remove all zero MAC address entries (00:00:00:00:00:00) from a multicast
> packet's tracker TVLV before transmitting it and update all headers
> accordingly. This way, instead of keeping the multicast packet at a
> constant size throughout its journey through the mesh, it will become
> more lightweight, smaller with every interested receiver on the way and
> on each splitting intersection. Which can save some valuable bandwidth.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  net/batman-adv/multicast_forw.c | 195 ++++++++++++++++++++++++++++++++
>  1 file changed, 195 insertions(+)
> 
> diff --git a/net/batman-adv/multicast_forw.c
> b/net/batman-adv/multicast_forw.c index d0f75a63de2a..d7b1aabd4b72 100644
> --- a/net/batman-adv/multicast_forw.c
> +++ b/net/batman-adv/multicast_forw.c
> @@ -712,6 +712,200 @@ batadv_mcast_forw_scrub_dests(struct batadv_priv
> *bat_priv, }
>  }
> 
> +/**
> + * batadv_mcast_forw_shrink_pack_dests() - pack destinations of a tracker
> TVLV + * @skb: the batman-adv multicast packet to compact destinations in +
> *
> + * Compacts the originator destination MAC addresses in the multicast
> tracker + * TVLV of the given multicast packet. This is done by moving all
> non-zero + * MAC addresses in direction of the skb head and all zero MAC
> addresses in skb + * tail direction, within the multicast tracker TVLV.
> + *
> + * Return: The number of consecutive zero MAC address destinations which
> are + * now at the end of the multicast tracker TVLV.
> + */
> +static int batadv_mcast_forw_shrink_pack_dests(struct sk_buff *skb)
> +{
> +	struct batadv_tvlv_mcast_tracker *mcast_tracker;
> +	u16 num_dests_slot, num_dests_filler;
> +	unsigned char *skb_net_hdr;
> +	u8 *slot, *filler;
> +
> +	skb_net_hdr = skb_network_header(skb);
> +	mcast_tracker = (struct batadv_tvlv_mcast_tracker *)skb_net_hdr;
> +	num_dests_slot = ntohs(mcast_tracker->num_dests);
> +
> +	slot = (u8 *)mcast_tracker + sizeof(*mcast_tracker);
> +
> +	if (!num_dests_slot)
> +		return 0;
> +
> +	num_dests_filler = num_dests_slot - 1;
> +	filler = slot + ETH_ALEN;
> +
> +	batadv_mcast_forw_tracker_for_each_dest(slot, num_dests_slot) {
> +		/* find an empty slot */
> +		if (!is_zero_ether_addr(slot))
> +			continue;
> +
> +		/* keep filler ahead of slot */
> +		if (filler <= slot) {
> +			num_dests_filler = num_dests_slot - 1;
> +			filler = slot + ETH_ALEN;
> +		}
> +
> +		/* find a candidate to fill the empty slot */
> +		batadv_mcast_forw_tracker_for_each_dest(filler,
> +							num_dests_filler) {
> +			if (is_zero_ether_addr(filler))
> +				continue;
> +
> +			ether_addr_copy(slot, filler);
> +			eth_zero_addr(filler);
> +			goto cont_next_slot;

goto is redundant, just continue
> +		}
> +
> +		/* could not find a filler, we can stop
> +		 * - and must not advance the slot pointer!
> +		 */
> +		if (!num_dests_filler)
> +			break;
> +

This label can then be removed as well.

I'm wondering why we need to keep all those pointers and do the pointer magic in the first place? can't we just make two nested functions like this:

for (all entries)
    if zero-entry:
    for (all entries)
         if non-zero-entry
              swap()

(I find this current function very hard to read)

Cheers,
      Simon
  

Patch

diff --git a/net/batman-adv/multicast_forw.c b/net/batman-adv/multicast_forw.c
index d0f75a63de2a..d7b1aabd4b72 100644
--- a/net/batman-adv/multicast_forw.c
+++ b/net/batman-adv/multicast_forw.c
@@ -712,6 +712,200 @@  batadv_mcast_forw_scrub_dests(struct batadv_priv *bat_priv,
 	}
 }
 
+/**
+ * batadv_mcast_forw_shrink_pack_dests() - pack destinations of a tracker TVLV
+ * @skb: the batman-adv multicast packet to compact destinations in
+ *
+ * Compacts the originator destination MAC addresses in the multicast tracker
+ * TVLV of the given multicast packet. This is done by moving all non-zero
+ * MAC addresses in direction of the skb head and all zero MAC addresses in skb
+ * tail direction, within the multicast tracker TVLV.
+ *
+ * Return: The number of consecutive zero MAC address destinations which are
+ * now at the end of the multicast tracker TVLV.
+ */
+static int batadv_mcast_forw_shrink_pack_dests(struct sk_buff *skb)
+{
+	struct batadv_tvlv_mcast_tracker *mcast_tracker;
+	u16 num_dests_slot, num_dests_filler;
+	unsigned char *skb_net_hdr;
+	u8 *slot, *filler;
+
+	skb_net_hdr = skb_network_header(skb);
+	mcast_tracker = (struct batadv_tvlv_mcast_tracker *)skb_net_hdr;
+	num_dests_slot = ntohs(mcast_tracker->num_dests);
+
+	slot = (u8 *)mcast_tracker + sizeof(*mcast_tracker);
+
+	if (!num_dests_slot)
+		return 0;
+
+	num_dests_filler = num_dests_slot - 1;
+	filler = slot + ETH_ALEN;
+
+	batadv_mcast_forw_tracker_for_each_dest(slot, num_dests_slot) {
+		/* find an empty slot */
+		if (!is_zero_ether_addr(slot))
+			continue;
+
+		/* keep filler ahead of slot */
+		if (filler <= slot) {
+			num_dests_filler = num_dests_slot - 1;
+			filler = slot + ETH_ALEN;
+		}
+
+		/* find a candidate to fill the empty slot */
+		batadv_mcast_forw_tracker_for_each_dest(filler,
+							num_dests_filler) {
+			if (is_zero_ether_addr(filler))
+				continue;
+
+			ether_addr_copy(slot, filler);
+			eth_zero_addr(filler);
+			goto cont_next_slot;
+		}
+
+		/* could not find a filler, we can stop
+		 * - and must not advance the slot pointer!
+		 */
+		if (!num_dests_filler)
+			break;
+
+cont_next_slot:
+		continue;
+	}
+
+	/* num_dests_slot is the amount of reduced destinations */
+	return num_dests_slot;
+}
+
+/**
+ * batadv_mcast_forw_shrink_align_offset() - get new alignment offset
+ * @num_dests_old: the old, to be updated amount of destination nodes
+ * @num_dests_reduce: the number of destinations that were removed
+ *
+ * Calculates the amount of potential extra alignment offset that is needed to
+ * adjust the TVLV padding after the change in destination nodes.
+ *
+ * Return:
+ *	0: If no change to padding is needed.
+ *	2: If padding needs to be removed.
+ *	-2: If padding needs to be added.
+ */
+static short
+batadv_mcast_forw_shrink_align_offset(unsigned int num_dests_old,
+				      unsigned int num_dests_reduce)
+{
+	/* even amount of removed destinations -> no alignment change */
+	if (!(num_dests_reduce % 2))
+		return 0;
+
+	/* even to odd amount of destinations -> remove padding */
+	if (!(num_dests_old % 2))
+		return 2;
+
+	/* odd to even amount of destinations -> add padding */
+	return -2;
+}
+
+/**
+ * batadv_mcast_forw_shrink_update_headers() - update shrunk mc packet headers
+ * @skb: the batman-adv multicast packet to update headers of
+ * @num_dests_reduce: the number of destinations that were removed
+ *
+ * This updates any fields of a batman-adv multicast packet that are affected
+ * by the reduced number of destinations in the multicast tracket TVLV. In
+ * particular this updates:
+ *
+ * The num_dest field of the multicast tracker TVLV.
+ * The TVLV length field of the according generic TVLV header.
+ * The batman-adv multicast packet's total TVLV length field.
+ *
+ * Return: The offset in skb's tail direction at which the new batman-adv
+ * multicast packet header needs to start.
+ */
+static unsigned int
+batadv_mcast_forw_shrink_update_headers(struct sk_buff *skb,
+					unsigned int num_dests_reduce)
+{
+	struct batadv_tvlv_mcast_tracker *mcast_tracker;
+	struct batadv_mcast_packet *mcast_packet;
+	struct batadv_tvlv_hdr *tvlv_hdr;
+	unsigned char *skb_net_hdr;
+	unsigned int offset;
+	short align_offset;
+	u16 num_dests;
+
+	skb_net_hdr = skb_network_header(skb);
+	mcast_tracker = (struct batadv_tvlv_mcast_tracker *)skb_net_hdr;
+	num_dests = ntohs(mcast_tracker->num_dests);
+
+	align_offset = batadv_mcast_forw_shrink_align_offset(num_dests,
+							     num_dests_reduce);
+	offset = ETH_ALEN * num_dests_reduce + align_offset;
+	num_dests -= num_dests_reduce;
+
+	/* update tracker header */
+	mcast_tracker->num_dests = htons(num_dests);
+
+	/* update tracker's tvlv header's length field */
+	tvlv_hdr = (struct batadv_tvlv_hdr *)(skb_network_header(skb) -
+					      sizeof(*tvlv_hdr));
+	tvlv_hdr->len = htons(ntohs(tvlv_hdr->len) - offset);
+
+	/* update multicast packet header's tvlv length field */
+	mcast_packet = (struct batadv_mcast_packet *)skb->data;
+	mcast_packet->tvlv_len = htons(ntohs(mcast_packet->tvlv_len) - offset);
+
+	return offset;
+}
+
+/**
+ * batadv_mcast_forw_shrink_move_headers() - move multicast headers by offset
+ * @skb: the batman-adv multicast packet to move headers for
+ * @offset: a non-negative offset to move headers by, towards the skb tail
+ *
+ * Moves the batman-adv multicast packet header, its multicast tracker TVLV and
+ * any TVLVs in between by the given offset in direction towards the tail.
+ */
+static void
+batadv_mcast_forw_shrink_move_headers(struct sk_buff *skb, unsigned int offset)
+{
+	struct batadv_tvlv_mcast_tracker *mcast_tracker;
+	unsigned char *skb_net_hdr;
+	unsigned int len;
+	u16 num_dests;
+
+	skb_net_hdr = skb_network_header(skb);
+	mcast_tracker = (struct batadv_tvlv_mcast_tracker *)skb_net_hdr;
+	num_dests = ntohs(mcast_tracker->num_dests);
+	len = skb_network_offset(skb) + sizeof(*mcast_tracker);
+	len += num_dests * ETH_ALEN;
+
+	batadv_mcast_forw_scrape(skb, len, offset);
+}
+
+/**
+ * batadv_mcast_forw_shrink_tracker() - remove zero addresses in a tracker tvlv
+ * @skb: the batman-adv multicast packet to (potentially) shrink
+ *
+ * Removes all destinations with a zero MAC addresses (00:00:00:00:00:00) from
+ * the given batman-adv multicast packet's tracker TVLV and updates headers
+ * accordingly to maintain a valid batman-adv multicast packet.
+ */
+static void batadv_mcast_forw_shrink_tracker(struct sk_buff *skb)
+{
+	unsigned int offset;
+	u16 dests_reduced;
+
+	dests_reduced = batadv_mcast_forw_shrink_pack_dests(skb);
+	if (!dests_reduced)
+		return;
+
+	offset = batadv_mcast_forw_shrink_update_headers(skb, dests_reduced);
+	batadv_mcast_forw_shrink_move_headers(skb, offset);
+}
+
 /**
  * batadv_mcast_forw_packet() - forward a batman-adv multicast packet
  * @bat_priv: the bat priv with all the soft interface information
@@ -798,6 +992,7 @@  static int batadv_mcast_forw_packet(struct batadv_priv *bat_priv,
 
 		batadv_mcast_forw_scrub_dests(bat_priv, neigh_node, dest,
 					      next_dest, num_dests);
+		batadv_mcast_forw_shrink_tracker(nexthop_skb);
 
 		batadv_inc_counter(bat_priv, BATADV_CNT_MCAST_TX);
 		batadv_add_counter(bat_priv, BATADV_CNT_MCAST_TX_BYTES,