Only a few style suggestions here, nothing critical and you can ignore them if you don't like them. :)
On Fri, May 24, 2013 at 10:02:28AM +0200, Linus Lüssing wrote:
With this patch a multicast packet is not always simply flooded anymore, the bevahiour for the following cases is changed to reduce unnecessary overhead:
If all nodes within the horizon of a certain node have signalized multicast listener announcement capability (BATADV_MCAST_LISTENER_ANNOUNCEMENT) then an IPv6 multicast packet with a destination of IPv6 link-local scope coming from the upstream of this node...
- ...is dropped if there is no according multicast listener in the translation table,
- ...is forwarded via unicast if there is a single node with interested multicast listeners
- ...and otherwise still gets flooded.
Signed-off-by: Linus Lüssing linus.luessing@web.de
multicast.c | 43 +++++++++++++++++++++++++++++++++ multicast.h | 8 +++++++ soft-interface.c | 10 ++++++++ translation-table.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ translation-table.h | 1 + 5 files changed, 128 insertions(+)
diff --git a/multicast.c b/multicast.c index 36e4c59..bd55c8f 100644 --- a/multicast.c +++ b/multicast.c @@ -213,6 +213,49 @@ out: }
/**
- batadv_mcast_flood - check on how to forward a multicast packet
- @skb: The multicast packet to check
- @bat_priv: the bat priv with all the soft interface information
- Return 1 if the packet should be flooded, 0 if it should be forwarded
- via unicast or -1 if it should be drooped.
- */
+int batadv_mcast_flood(struct sk_buff *skb, struct batadv_priv *bat_priv) +{
- struct ethhdr *ethhdr = (struct ethhdr *)(skb->data);
- struct ipv6hdr *ip6hdr;
- int count, ret = 1;
- if (atomic_read(&bat_priv->mcast_group_awareness) &&
!atomic_read(&bat_priv->mcast.num_non_aware) &&ntohs(ethhdr->h_proto) == ETH_P_IPV6) {
You can safe an indendation below if you return -1 immediately here if the statement above is false. Also multiple statements might be better for readability and later changes, e.g.
if (!atomic_read(&bat_priv->mcast_group_awareness)) return 1;
if (atomic_read(&bat_priv->mcast.num_non_aware)) return 1;
if (ntohs(ethhdr->h_proto) != ETH_P_IPV6) return 1;
if (!pskb_may_pull(skb, sizeof(*ethhdr) + sizeof(*ip6hdr))) {ret = -1;goto out;}
You could directly return -1 here, the out label is not needed (as we don't unlock/free anything here).
I don't quite understand why you return -1, maybe the packet could still be forwarded even if it could not be pulled?
ip6hdr = ipv6_hdr(skb);/* TODO: Implement Multicast Router Discovery, then add* scope >= IPV6_ADDR_SCOPE_LINKLOCAL, too*/if (IPV6_ADDR_MC_SCOPE(&ip6hdr->daddr) !=IPV6_ADDR_SCOPE_LINKLOCAL)goto out;count = batadv_tt_global_hash_count(bat_priv, ethhdr->h_dest);if (!count)ret = -1;else if (count == 1)ret = 0;- }
You could use a switch statement here instead for readability, e.g.:
switch (count) { case 0: return -1; case 1: return 0; default: return 1; }
+out:
- return ret;
+}
+/**
- batadv_mcast_tvlv_ogm_handler_v1 - process incoming multicast tvlv container
- @bat_priv: the bat priv with all the soft interface information
- @orig: the orig_node of the ogm
diff --git a/soft-interface.c b/soft-interface.c index 8bdd649..83e4679 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -36,6 +36,7 @@ #include <linux/if_vlan.h> #include <linux/if_ether.h> #include "unicast.h" +#include "multicast.h" #include "bridge_loop_avoidance.h" #include "network-coding.h"
@@ -222,6 +223,15 @@ static int batadv_interface_tx(struct sk_buff *skb, } }
- if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
I'd suggest to put this inside the "is_multicast_etheraddr()" above to make more clear that this handles multicast packets. I was a little confused by the do_bcast && !is_broadcast_ether_addr() first, but that might just be me.
Cheers, Simon