Re: [PATCH v4 3/5] batman-adv: mcast: implement multicast packet reception and forwarding

[Sven Eckelmann <sven@narfation.org>]

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

On Tuesday, 27 December 2022 20:34:07 CET Linus Lüssing wrote:
> Implement functionality to receive and forward a new TVLV capable
> multicast packet type.
> 
> The new batman-adv multicast packet type allows to contain several
> originator destination addresses within a TVLV. Routers on the way will
> potentially split the batman-adv multicast packet and adjust its tracker
> TVLV contents.
> 
> Routing decisions are still based on the selected BATMAN IV or BATMAN V
> routing algorithm. So this new batman-adv multicast packet type retains
> the same loop-free properties.
> 
> Also a new OGM multicast TVLV flag is introduced to signal to other
> nodes that we are capable of handling a batman-adv multicast packet and
> multicast tracker TVLV. And that all of our hard interfaces have an MTU
> of at least 1280 bytes (IPv6 minimum MTU), as a simple solution for now
> to avoid MTU issues while forwarding.
> 
> Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
> ---
>  include/uapi/linux/batadv_packet.h |  48 ++++-
>  net/batman-adv/Makefile            |   1 +
>  net/batman-adv/main.c              |   2 +
>  net/batman-adv/multicast.c         |  48 ++++-
>  net/batman-adv/multicast.h         |   5 +
>  net/batman-adv/multicast_forw.c    | 274 +++++++++++++++++++++++++++++
>  net/batman-adv/originator.c        |   1 +
>  net/batman-adv/routing.c           |  68 +++++++
>  net/batman-adv/routing.h           |  11 ++
>  net/batman-adv/soft-interface.c    |  12 ++
>  net/batman-adv/types.h             |  64 +++++++
>  11 files changed, 523 insertions(+), 11 deletions(-)
>  create mode 100644 net/batman-adv/multicast_forw.c


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

But I want to have at least another maintainer checking the new packet format 
and acking it.

Kind regards,
	Sven

[...]
> +static void
> +batadv_mcast_forw_scrub_dests(struct batadv_priv *bat_priv,
> +			      struct batadv_neigh_node *comp_neigh, u8 *dest,
> +			      u8 *next_dest, u16 num_dests)
> +{
> +	struct batadv_neigh_node *next_neigh;
> +
> +	/* skip first entry, this is what we are comparing with */
> +	eth_zero_addr(dest);
> +	dest += ETH_ALEN;
> +	next_dest += ETH_ALEN;
> +	num_dests--;
> +
> +	batadv_mcast_forw_tracker_for_each_dest(next_dest, num_dests) {
> +		if (is_zero_ether_addr(next_dest))
> +			goto scrub_next;
> +
> +		if (is_multicast_ether_addr(next_dest)) {
> +			eth_zero_addr(dest);
> +			eth_zero_addr(next_dest);
> +			goto scrub_next;
> +		}
> +
> +		next_neigh = batadv_mcast_forw_orig_to_neigh(bat_priv,
> +							     next_dest);
> +		if (!next_neigh) {
> +			eth_zero_addr(next_dest);

Why is the original skb not touched in this case?

It might not be a problem because you are also doing the 
batadv_mcast_forw_orig_to_neigh check in batadv_mcast_forw_packet. But I was 
just wondering about it


Btw. it could happen that you send out a packet with zero destinations in the 
TVLV because the neighbor disappeared between the 
batadv_mcast_forw_orig_to_neigh in batadv_mcast_forw_packet and in 
batadv_mcast_forw_scrub_dests

Kind regards,
	Sven

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