On Thursday 12 June 2014 02:00:27 Linus Lüssing wrote:
@@ -30,17 +53,21 @@
- Collect multicast addresses of the local multicast listeners
- on the given soft interface, dev, in the given mcast_list.
- If there is a bridge interface on top of dev, collect from that one
- instead.
*/
- Returns -ENOMEM on memory allocation error or the number of
- items added to the mcast_list otherwise.
static int batadv_mcast_mla_softif_get(struct net_device *dev, struct hlist_head *mcast_list) {
- struct net_device *bridge = batadv_mcast_get_bridge(dev); struct netdev_hw_addr *mc_list_entry; struct batadv_hw_addr *new; int ret = 0;
- netif_addr_lock_bh(dev);
- netif_addr_lock_bh(bridge ? bridge : dev); netdev_for_each_mc_addr(mc_list_entry, dev) { new = kmalloc(sizeof(*new), GFP_ATOMIC); if (!new) {
Maybe I am missing something but shouldn't we fetch the entries from the bridge interface instead of dev ?
/**
- batadv_mcast_mla_br_addr_cpy - copy a bridge multicast address
- @dst: destination to write to - a multicast MAC address
- @src: source to read from - a multicast IP address
- Converts a given multicast IPv4/IPv6 address from a bridge
- to its matching multicast MAC address and copies it into the given
- destination buffer.
- Caller needs to make sure the destination buffer can hold
- at least ETH_ALEN bytes.
- */
+static void batadv_mcast_mla_br_addr_cpy(char *dst, const struct br_ip *src) +{
- if (src->proto == htons(ETH_P_IP)) {
/* RFC 1112 */memcpy(dst, "\x01\x00\x5e", 3);memcpy(dst + 3, ((char *)&src->u.ip4) + 1, ETH_ALEN - 3);dst[3] &= 0x7F;- }
+#if IS_ENABLED(CONFIG_IPV6)
- else if (src->proto == htons(ETH_P_IPV6)) {
/* RFC 2464 */memcpy(dst, "\x33\x33", 2);memcpy(dst + 2, &src->u.ip6.s6_addr32[3],sizeof(src->u.ip6.s6_addr32[3]));- }
+#endif
- else
memset(dst, 0, ETH_ALEN);+}
Is there no cleaner way to do this ? "\x01\x00\x5e" and "\x33\x33" looks pretty hackish. Are there no defines for mcast prefixes somewhere ?
+/**
- batadv_mcast_mla_bridge_get - get bridged-in multicast listeners
- @dev: a bridge slave whose bridge to collect multicast addresses from
- @mcast_list: a list to put found addresses into
- Collects multicast addresses of the bridged-in multicast listeners
- from the bridge on top of the given soft interface, dev, in the
- given mcast_list.
- Returns -ENOMEM on memory allocation error or the number of
- items added to the mcast_list otherwise.
- */
+static int batadv_mcast_mla_bridge_get(struct net_device *dev,
struct hlist_head *mcast_list)+{
That is probably where my confusion is coming from. Why do we query the bridge interface again ?
@@ -195,19 +306,18 @@ static bool batadv_mcast_mla_tvlv_update(struct batadv_priv *bat_priv) mcast_data.flags = BATADV_NO_FLAGS; memset(mcast_data.reserved, 0, sizeof(mcast_data.reserved));
- /* Avoid attaching MLAs, if there is a bridge on top of our soft
* interface, we don't support that yet (TODO)*/- if (batadv_mcast_has_bridge(bat_priv)) {
if (bat_priv->mcast.enabled) {batadv_tvlv_container_unregister(bat_priv,BATADV_TVLV_MCAST, 1);bat_priv->mcast.enabled = false;}
- if (!batadv_mcast_has_bridge(bat_priv))
goto skip;
return false;- }
mcast_data.flags |= BATADV_MCAST_WANT_ALL_UNSNOOPABLES;
if (br_multicast_has_querier_adjacent(bat_priv->soft_iface, ETH_P_IP))
mcast_data.flags |= BATADV_MCAST_WANT_ALL_IPV4;if (br_multicast_has_querier_adjacent(bat_priv->soft_iface, ETH_P_IPV6))
mcast_data.flags |= BATADV_MCAST_WANT_ALL_IPV6;+skip: if (!bat_priv->mcast.enabled || mcast_data.flags != bat_priv->mcast.flags) { batadv_tvlv_container_register(bat_priv, BATADV_TVLV_MCAST, 1,
Please find a more intuitive goto label ('skip' is quite generic).
@@ -233,13 +344,17 @@ void batadv_mcast_mla_update(struct batadv_priv *bat_priv) int ret;
if (!batadv_mcast_mla_tvlv_update(bat_priv))
goto update;
goto skip;ret = batadv_mcast_mla_softif_get(soft_iface, &mcast_list); if (ret < 0) goto out;
-update:
- ret = batadv_mcast_mla_bridge_get(soft_iface, &mcast_list);
- if (ret < 0)
goto out;+skip: batadv_mcast_mla_tt_retract(bat_priv, &mcast_list); batadv_mcast_mla_tt_add(bat_priv, &mcast_list);
Again, 'skip' isn't very self-explanatory.
Cheers, Marek