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 <[email protected]> > --- > 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
signature.asc
Description: Digital signature
