Maybe I am missing the whole point of WANT_ALL but why do we maintain a list of WANT_ALL nodes to only send the packet to the first valid entry in the list?
Hm, the thing is, there can be multiple nodes with that flag. But most of the time we only end up in this function when there's just a single node in this list. (when there's more than one, we'd usually end up in the broadcast instead of unicasting path)
As far as I see, send_skb_via_mcast() is only called if forw_mode returned BATADV_FORW_SINGLE, that is there is only one recipient in the list, and there is no "regular" listener in TT.
(I don't see any good reason for calling that "most of the time" and "usually")
We could remove the list entirely but then we'd have to loop over all orig_nodes and check their mcast_flags for every packet - which is too costly on the fast path.
We could replace these two lists by two variables holding a single originator address or orig_node each. But then we'd still have to loop over all originators when the numbers of nodes with such a flag decreases to one to find this one node to update the variable with.
But yes, this pointer pointer to a list head is not really nice either... what do you think about returning a pointer pointer to an orig_node with batadv_mcast_want_all_count() already, instead? That way we'd spare checking the IP address family twice, too. Or the list-to-orig_node-variable substitution approach?
I think that would be good to do (returning an orig_node, that is). And also please return an orig_node if a TT match was found for the single case.
With this we would unify the multicast sending in batadv_interface_tx() as well which would make it more readable. Right now mcast packets are sent via batadv_send_skb_via_mcast() if the want_all_list is present or via batadv_send_skb_via_tt() if want_all_list is not present, which isn't exactly easy to understand. :)
something like
forw_mode = batadv_mcast_forw_mode(bat_priv, skb, &mcast_single_orig);
and then in the unicast branch below
} else if (mcast_single_orig) { ret = batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0, mcast_single_orig, vid); }
should do the trick, no?
Thanks, Simon