b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
From: Simon Wunderlich <sw@simonwunderlich.de>
To: b.a.t.m.a.n@lists.open-mesh.org
Subject: Re: [PATCH v6 3/3] batman-adv: mcast: shrink tracker packet after scrubbing
Date: Mon, 14 Aug 2023 17:56:56 +0200	[thread overview]
Message-ID: <1930229.QkHrqEjB74@prime> (raw)
In-Reply-To: <20230720043556.12163-4-linus.luessing@c0d3.blue>

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

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

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

  parent reply	other threads:[~2023-08-14 15:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-20  4:35 [PATCH v6 0/3] Implementation of a Stateless Multicast Packet Type Linus Lüssing
2023-07-20  4:35 ` [PATCH v6 1/3] batman-adv: mcast: implement multicast packet reception and forwarding Linus Lüssing
2023-07-28 17:52   ` Sven Eckelmann
2023-08-14 15:35   ` Simon Wunderlich
2023-08-14 15:53     ` Simon Wunderlich
2023-07-20  4:35 ` [PATCH v6 2/3] batman-adv: mcast: implement multicast packet generation Linus Lüssing
2023-07-29 10:49   ` Sven Eckelmann
2023-07-20  4:35 ` [PATCH v6 3/3] batman-adv: mcast: shrink tracker packet after scrubbing Linus Lüssing
2023-07-29 11:11   ` Sven Eckelmann
2023-08-14 15:56   ` Simon Wunderlich [this message]
2023-07-29 11:11 ` [PATCH v6 0/3] Implementation of a Stateless Multicast Packet Type Sven Eckelmann
2023-08-14 16:05   ` Sven Eckelmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1930229.QkHrqEjB74@prime \
    --to=sw@simonwunderlich.de \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).