Re: [PATCH net-next 2/4] bridge: adhere to querier election mechanism specified by RFCs

2014-05-23 Thread David Miller
From: Linus Lüssing 
Date: Wed, 21 May 2014 10:59:00 +0200

> + if (saddr <= br->ip4_querier.addr.u.ip4)
> + goto update;

It is clear to me that you should be making these comparisons in cpu
endianness.

Otherwise the code will handle the same situations differently
on big-endian vs. little-endian machines.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH net-next 2/4] bridge: adhere to querier election mechanism specified by RFCs

2014-05-23 Thread David Miller
From: Linus Lüssing linus.luess...@web.de
Date: Wed, 21 May 2014 10:59:00 +0200

 + if (saddr = br-ip4_querier.addr.u.ip4)
 + goto update;

It is clear to me that you should be making these comparisons in cpu
endianness.

Otherwise the code will handle the same situations differently
on big-endian vs. little-endian machines.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH net-next 2/4] bridge: adhere to querier election mechanism specified by RFCs

2014-05-21 Thread Linus Lüssing
MLDv1 (RFC2710 section 6), MLDv2 (RFC3810 section 7.6.2), IGMPv2
(RFC2236 section 3) and IGMPv3 (RFC3376 section 6.6.2) specify that the
querier with lowest source address shall become the selected
querier.

So far the bridge stopped its querier as soon as it heard another
querier regardless of its source address. This results in the "wrong"
querier potentially becoming the active querier or a potential,
unnecessary querying delay.

With this patch the bridge memorizes the source address of the currently
selected querier and ignores queries from queriers with a higher source
address than the currently selected one. This slight optimization is
supposed to make it more RFC compliant (but is rather uncritical and
therefore probably not necessary to be queued for stable kernels).

Signed-off-by: Linus Lüssing 
---
 net/bridge/br_multicast.c |  101 +++--
 net/bridge/br_private.h   |7 
 2 files changed, 95 insertions(+), 13 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 6bc9cef..05c6593 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -789,6 +789,18 @@ static void br_ip6_multicast_querier_expired(unsigned long 
data)
 }
 #endif
 
+static void br_multicast_select_own_querier(struct net_bridge *br,
+   struct br_ip *ip,
+   struct sk_buff *skb)
+{
+   if (ip->proto == htons(ETH_P_IP))
+   br->ip4_querier.addr.u.ip4 = ip_hdr(skb)->saddr;
+#if IS_ENABLED(CONFIG_IPV6)
+   else
+   br->ip6_querier.addr.u.ip6 = ipv6_hdr(skb)->saddr;
+#endif
+}
+
 static void __br_multicast_send_query(struct net_bridge *br,
  struct net_bridge_port *port,
  struct br_ip *ip)
@@ -804,8 +816,10 @@ static void __br_multicast_send_query(struct net_bridge 
*br,
skb->dev = port->dev;
NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
dev_queue_xmit);
-   } else
+   } else {
+   br_multicast_select_own_querier(br, ip, skb);
netif_rx(skb);
+   }
 }
 
 static void br_multicast_send_query(struct net_bridge *br,
@@ -1065,6 +1079,62 @@ static int br_ip6_multicast_mld2_report(struct 
net_bridge *br,
 }
 #endif
 
+static bool br_ip4_multicast_select_querier(struct net_bridge *br,
+   __be32 saddr)
+{
+   if (!timer_pending(>ip4_own_query.timer) &&
+   !timer_pending(>ip4_other_query.timer))
+   goto update;
+
+   if (!br->ip4_querier.addr.u.ip4)
+   goto update;
+
+   if (saddr <= br->ip4_querier.addr.u.ip4)
+   goto update;
+
+   return false;
+
+update:
+   br->ip4_querier.addr.u.ip4 = saddr;
+
+   return true;
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static bool br_ip6_multicast_select_querier(struct net_bridge *br,
+   struct in6_addr *saddr)
+{
+   if (!timer_pending(>ip6_own_query.timer) &&
+   !timer_pending(>ip6_other_query.timer))
+   goto update;
+
+   if (ipv6_addr_cmp(saddr, >ip6_querier.addr.u.ip6) <= 0)
+   goto update;
+
+   return false;
+
+update:
+   br->ip6_querier.addr.u.ip6 = *saddr;
+
+   return true;
+}
+#endif
+
+static bool br_multicast_select_querier(struct net_bridge *br,
+   struct br_ip *saddr)
+{
+   switch (saddr->proto) {
+   case htons(ETH_P_IP):
+   return br_ip4_multicast_select_querier(br, saddr->u.ip4);
+#if IS_ENABLED(CONFIG_IPV6)
+   case htons(ETH_P_IPV6):
+   return br_ip6_multicast_select_querier(br, >u.ip6);
+#endif
+   }
+
+   return false;
+}
+
 static void
 br_multicast_update_query_timer(struct net_bridge *br,
struct bridge_mcast_other_query *query,
@@ -1127,15 +1197,13 @@ timer:
 static void br_multicast_query_received(struct net_bridge *br,
struct net_bridge_port *port,
struct bridge_mcast_other_query *query,
-   int saddr,
-   bool is_general_query,
+   struct br_ip *saddr,
unsigned long max_delay)
 {
-   if (saddr && is_general_query)
-   br_multicast_update_query_timer(br, query, max_delay);
-   else if (timer_pending(>timer))
+   if (!br_multicast_select_querier(br, saddr))
return;
 
+   br_multicast_update_query_timer(br, query, max_delay);
br_multicast_mark_router(br, port);
 }
 
@@ -1150,6 +1218,7 @@ static int br_ip4_multicast_query(struct net_bridge *br,
struct igmpv3_query *ih3;
struct 

[PATCH net-next 2/4] bridge: adhere to querier election mechanism specified by RFCs

2014-05-21 Thread Linus Lüssing
MLDv1 (RFC2710 section 6), MLDv2 (RFC3810 section 7.6.2), IGMPv2
(RFC2236 section 3) and IGMPv3 (RFC3376 section 6.6.2) specify that the
querier with lowest source address shall become the selected
querier.

So far the bridge stopped its querier as soon as it heard another
querier regardless of its source address. This results in the wrong
querier potentially becoming the active querier or a potential,
unnecessary querying delay.

With this patch the bridge memorizes the source address of the currently
selected querier and ignores queries from queriers with a higher source
address than the currently selected one. This slight optimization is
supposed to make it more RFC compliant (but is rather uncritical and
therefore probably not necessary to be queued for stable kernels).

Signed-off-by: Linus Lüssing linus.luess...@web.de
---
 net/bridge/br_multicast.c |  101 +++--
 net/bridge/br_private.h   |7 
 2 files changed, 95 insertions(+), 13 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 6bc9cef..05c6593 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -789,6 +789,18 @@ static void br_ip6_multicast_querier_expired(unsigned long 
data)
 }
 #endif
 
+static void br_multicast_select_own_querier(struct net_bridge *br,
+   struct br_ip *ip,
+   struct sk_buff *skb)
+{
+   if (ip-proto == htons(ETH_P_IP))
+   br-ip4_querier.addr.u.ip4 = ip_hdr(skb)-saddr;
+#if IS_ENABLED(CONFIG_IPV6)
+   else
+   br-ip6_querier.addr.u.ip6 = ipv6_hdr(skb)-saddr;
+#endif
+}
+
 static void __br_multicast_send_query(struct net_bridge *br,
  struct net_bridge_port *port,
  struct br_ip *ip)
@@ -804,8 +816,10 @@ static void __br_multicast_send_query(struct net_bridge 
*br,
skb-dev = port-dev;
NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb-dev,
dev_queue_xmit);
-   } else
+   } else {
+   br_multicast_select_own_querier(br, ip, skb);
netif_rx(skb);
+   }
 }
 
 static void br_multicast_send_query(struct net_bridge *br,
@@ -1065,6 +1079,62 @@ static int br_ip6_multicast_mld2_report(struct 
net_bridge *br,
 }
 #endif
 
+static bool br_ip4_multicast_select_querier(struct net_bridge *br,
+   __be32 saddr)
+{
+   if (!timer_pending(br-ip4_own_query.timer) 
+   !timer_pending(br-ip4_other_query.timer))
+   goto update;
+
+   if (!br-ip4_querier.addr.u.ip4)
+   goto update;
+
+   if (saddr = br-ip4_querier.addr.u.ip4)
+   goto update;
+
+   return false;
+
+update:
+   br-ip4_querier.addr.u.ip4 = saddr;
+
+   return true;
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static bool br_ip6_multicast_select_querier(struct net_bridge *br,
+   struct in6_addr *saddr)
+{
+   if (!timer_pending(br-ip6_own_query.timer) 
+   !timer_pending(br-ip6_other_query.timer))
+   goto update;
+
+   if (ipv6_addr_cmp(saddr, br-ip6_querier.addr.u.ip6) = 0)
+   goto update;
+
+   return false;
+
+update:
+   br-ip6_querier.addr.u.ip6 = *saddr;
+
+   return true;
+}
+#endif
+
+static bool br_multicast_select_querier(struct net_bridge *br,
+   struct br_ip *saddr)
+{
+   switch (saddr-proto) {
+   case htons(ETH_P_IP):
+   return br_ip4_multicast_select_querier(br, saddr-u.ip4);
+#if IS_ENABLED(CONFIG_IPV6)
+   case htons(ETH_P_IPV6):
+   return br_ip6_multicast_select_querier(br, saddr-u.ip6);
+#endif
+   }
+
+   return false;
+}
+
 static void
 br_multicast_update_query_timer(struct net_bridge *br,
struct bridge_mcast_other_query *query,
@@ -1127,15 +1197,13 @@ timer:
 static void br_multicast_query_received(struct net_bridge *br,
struct net_bridge_port *port,
struct bridge_mcast_other_query *query,
-   int saddr,
-   bool is_general_query,
+   struct br_ip *saddr,
unsigned long max_delay)
 {
-   if (saddr  is_general_query)
-   br_multicast_update_query_timer(br, query, max_delay);
-   else if (timer_pending(query-timer))
+   if (!br_multicast_select_querier(br, saddr))
return;
 
+   br_multicast_update_query_timer(br, query, max_delay);
br_multicast_mark_router(br, port);
 }
 
@@ -1150,6 +1218,7 @@ static int br_ip4_multicast_query(struct net_bridge *br,
struct igmpv3_query *ih3;
struct