RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-31 Thread Philip Downey


> -Original Message-
> From: Cong Wang [mailto:cw...@twopensource.com]
> Sent: Friday, August 28, 2015 10:20 PM
> To: Philip Downey
> Cc: David Miller; Alexey Kuznetsov; James Morris; Hideaki YOSHIFUJI; Patrick
> McHardy; linux-ker...@vger.kernel.org; netdev
> Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
> 
> On Thu, Aug 27, 2015 at 8:46 AM, Philip Downey <pdow...@brocade.com>
> wrote:
> > IGMP reports for local multicast groups can now be optionally
> > inhibited by means of a system control variable (by setting the value
> > to zero) e.g.:
> > echo 0 > /proc/sys/net/ipv4/igmp_link_local_mcast_reports
> >
> > To retain backwards compatibility the previous behaviour is retained
> > by default on system boot or reverted by setting the value back to
> > non-zero e.g.:
> > echo 1 >  /proc/sys/net/ipv4/igmp_link_local_mcast_reports
> >
> 
> Please document it in Documentation/networking/ip-sysctl.txt.
Thanks for the comment.
I have generated a new patch for the proposed documentation change.
Hope this is the correct thing to do.

Regards

Philip


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-28 Thread Cong Wang
On Thu, Aug 27, 2015 at 8:46 AM, Philip Downey pdow...@brocade.com wrote:
 IGMP reports for local multicast groups can now be optionally
 inhibited by means of a system control variable (by setting the value
 to zero) e.g.:
 echo 0  /proc/sys/net/ipv4/igmp_link_local_mcast_reports

 To retain backwards compatibility the previous behaviour is retained
 by default on system boot or reverted by setting the value back to
 non-zero e.g.:
 echo 1   /proc/sys/net/ipv4/igmp_link_local_mcast_reports


Please document it in Documentation/networking/ip-sysctl.txt.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-28 Thread David Miller
From: Philip Downey pdow...@brocade.com
Date: Thu, 27 Aug 2015 16:46:26 +0100

 The range of addresses between 224.0.0.0 and 224.0.0.255 inclusive, is
 reserved for the use of routing protocols and other low-level topology
 discovery or maintenance protocols, such as gateway discovery and
 group membership reporting.  Multicast routers should not forward any
 multicast datagram with destination addresses in this range,
 regardless of its TTL.
 
 Currently, IGMP reports are generated for this reserved range of
 addresses even though a router will ignore this information since it
 has no purpose.  However, the presence of reserved group addresses in
 an IGMP membership report uses up network bandwidth and can also
 obscure addresses of interest when inspecting membership reports using
 packet inspection or debug messages.
 
 Although the RFCs for the various version of IGMP (e.g.RFC 3376 for
 v3) do not specify that the reserved addresses be excluded from
 membership reports, it should do no harm in doing so.  In particular
 there should be no adverse effect in any IGMP snooping functionality
 since 224.0.0.x is specifically excluded as per RFC 4541 (IGMP and MLD
 Snooping Switches Considerations) section 2.1.2. Data Forwarding
 Rules:
 
 2) Packets with a destination IP (DIP) address in the 224.0.0.X
range which are not IGMP must be forwarded on all ports.
 
 IGMP reports for local multicast groups can now be optionally
 inhibited by means of a system control variable (by setting the value
 to zero) e.g.:
 echo 0  /proc/sys/net/ipv4/igmp_link_local_mcast_reports
 
 To retain backwards compatibility the previous behaviour is retained
 by default on system boot or reverted by setting the value back to
 non-zero e.g.:
 echo 1   /proc/sys/net/ipv4/igmp_link_local_mcast_reports
 
 Signed-off-by: Philip Downey pdow...@brocade.com

Applied to net-next, thanks.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-27 Thread Philip Downey
The range of addresses between 224.0.0.0 and 224.0.0.255 inclusive, is
reserved for the use of routing protocols and other low-level topology
discovery or maintenance protocols, such as gateway discovery and
group membership reporting.  Multicast routers should not forward any
multicast datagram with destination addresses in this range,
regardless of its TTL.

Currently, IGMP reports are generated for this reserved range of
addresses even though a router will ignore this information since it
has no purpose.  However, the presence of reserved group addresses in
an IGMP membership report uses up network bandwidth and can also
obscure addresses of interest when inspecting membership reports using
packet inspection or debug messages.

Although the RFCs for the various version of IGMP (e.g.RFC 3376 for
v3) do not specify that the reserved addresses be excluded from
membership reports, it should do no harm in doing so.  In particular
there should be no adverse effect in any IGMP snooping functionality
since 224.0.0.x is specifically excluded as per RFC 4541 (IGMP and MLD
Snooping Switches Considerations) section 2.1.2. Data Forwarding
Rules:

2) Packets with a destination IP (DIP) address in the 224.0.0.X
   range which are not IGMP must be forwarded on all ports.

IGMP reports for local multicast groups can now be optionally
inhibited by means of a system control variable (by setting the value
to zero) e.g.:
echo 0  /proc/sys/net/ipv4/igmp_link_local_mcast_reports

To retain backwards compatibility the previous behaviour is retained
by default on system boot or reverted by setting the value back to
non-zero e.g.:
echo 1   /proc/sys/net/ipv4/igmp_link_local_mcast_reports

Signed-off-by: Philip Downey pdow...@brocade.com
---
 include/linux/igmp.h   |1 +
 net/ipv4/igmp.c|   26 +-
 net/ipv4/sysctl_net_ipv4.c |7 +++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index 193ad48..9084292 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -37,6 +37,7 @@ static inline struct igmpv3_query *
return (struct igmpv3_query *)skb_transport_header(skb);
 }
 
+extern int sysctl_igmp_llm_reports;
 extern int sysctl_igmp_max_memberships;
 extern int sysctl_igmp_max_msf;
 extern int sysctl_igmp_qrv;
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 9fdfd9d..d38b8b6 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -110,6 +110,9 @@
 #define IP_MAX_MEMBERSHIPS 20
 #define IP_MAX_MSF 10
 
+/* IGMP reports for link-local multicast groups are enabled by default */
+int sysctl_igmp_llm_reports __read_mostly = 1;
+
 #ifdef CONFIG_IP_MULTICAST
 /* Parameter names and values are taken from igmp-v2-06 draft */
 
@@ -437,6 +440,8 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct 
ip_mc_list *pmc,
 
if (pmc-multiaddr == IGMP_ALL_HOSTS)
return skb;
+   if (ipv4_is_local_multicast(pmc-multiaddr)  !sysctl_igmp_llm_reports)
+   return skb;
 
isquery = type == IGMPV3_MODE_IS_INCLUDE ||
  type == IGMPV3_MODE_IS_EXCLUDE;
@@ -545,6 +550,9 @@ static int igmpv3_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc)
for_each_pmc_rcu(in_dev, pmc) {
if (pmc-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (ipv4_is_local_multicast(pmc-multiaddr) 
+!sysctl_igmp_llm_reports)
+   continue;
spin_lock_bh(pmc-lock);
if (pmc-sfcount[MCAST_EXCLUDE])
type = IGMPV3_MODE_IS_EXCLUDE;
@@ -678,7 +686,11 @@ static int igmp_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc,
 
if (type == IGMPV3_HOST_MEMBERSHIP_REPORT)
return igmpv3_send_report(in_dev, pmc);
-   else if (type == IGMP_HOST_LEAVE_MESSAGE)
+
+   if (ipv4_is_local_multicast(group)  !sysctl_igmp_llm_reports)
+   return 0;
+
+   if (type == IGMP_HOST_LEAVE_MESSAGE)
dst = IGMP_ALL_ROUTER;
else
dst = group;
@@ -851,6 +863,8 @@ static bool igmp_heard_report(struct in_device *in_dev, 
__be32 group)
 
if (group == IGMP_ALL_HOSTS)
return false;
+   if (ipv4_is_local_multicast(group)  !sysctl_igmp_llm_reports)
+   return false;
 
rcu_read_lock();
for_each_pmc_rcu(in_dev, im) {
@@ -957,6 +971,9 @@ static bool igmp_heard_query(struct in_device *in_dev, 
struct sk_buff *skb,
continue;
if (im-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (ipv4_is_local_multicast(im-multiaddr) 
+   !sysctl_igmp_llm_reports)
+   continue;
spin_lock_bh(im-lock);
if 

RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-26 Thread Philip Downey


 -Original Message-
 From: David Miller [mailto:da...@davemloft.net]
 Sent: Tuesday, August 25, 2015 10:20 PM
 To: Philip Downey
 Cc: kuz...@ms2.inr.ac.ru; jmor...@namei.org; yoshf...@linux-ipv6.org;
 ka...@trash.net; linux-ker...@vger.kernel.org; netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 From: Philip Downey pdow...@brocade.com
 Date: Mon, 24 Aug 2015 12:39:17 +0100
 
  +extern int sysctl_igmp_link_local_reports;
  ...
  +/* IGMP reports for link-local multicast groups are enabled by default */
  +#define IGMP_ENABLE_LLM 1
  +
  +int sysctl_igmp_link_local_reports __read_mostly = IGMP_ENABLE_LLM;
  +
  +#define IGMP_INHIBIT_LINK_LOCAL_REPORTS(_ipaddr) \
  +   (ipv4_is_local_multicast(_ipaddr)  \
  +(sysctl_igmp_link_local_reports == 0))
  +
 
 People know that 1 and 0 means enable and disable respectively, so this
 macros is pretty excessive.  Just remove it.
 
 Also, simplify the name of the sysctl to something like
 sysctl_igmp_llm_reports or similar, and simplify the test against 0 to be in
 the canonical !x format.  Then the test can fit on one
 line:
 
   (ipv4_is_local_multicast(_ipaddr)  !sysctl_igmp_llm_reports).

Thanks for reviewing David.
I will make the requested changes  (fitting the test on a single line was my 
main reason for introducing the macro - that and making it patently obvious 
what the test was doing.  Your suggestion would seem to meet that aim).

Will amend and resubmit.

Regards

Philip
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-25 Thread David Miller
From: Philip Downey pdow...@brocade.com
Date: Mon, 24 Aug 2015 12:39:17 +0100

 +extern int sysctl_igmp_link_local_reports;
 ...
 +/* IGMP reports for link-local multicast groups are enabled by default */
 +#define IGMP_ENABLE_LLM 1
 +
 +int sysctl_igmp_link_local_reports __read_mostly = IGMP_ENABLE_LLM;
 +
 +#define IGMP_INHIBIT_LINK_LOCAL_REPORTS(_ipaddr) \
 + (ipv4_is_local_multicast(_ipaddr)  \
 +  (sysctl_igmp_link_local_reports == 0))
 +

People know that 1 and 0 means enable and disable respectively, so this
macros is pretty excessive.  Just remove it.

Also, simplify the name of the sysctl to something like
sysctl_igmp_llm_reports or similar, and simplify the test against 0
to be in the canonical !x format.  Then the test can fit on one
line:

(ipv4_is_local_multicast(_ipaddr)  !sysctl_igmp_llm_reports)

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-24 Thread Philip Downey
The range of addresses between 224.0.0.0 and 224.0.0.255 inclusive, is
reserved for the use of routing protocols and other low-level topology
discovery or maintenance protocols, such as gateway discovery and
group membership reporting.  Multicast routers should not forward any
multicast datagram with destination addresses in this range,
regardless of its TTL.

Currently, IGMP reports are generated for this reserved range of
addresses even though a router will ignore this information since it
has no purpose.  However, the presence of reserved group addresses in
an IGMP membership report uses up network bandwidth and can also
obscure addresses of interest when inspecting membership reports using
packet inspection or debug messages.

Although the RFCs for the various version of IGMP (e.g.RFC 3376 for
v3) do not specify that the reserved addresses be excluded from
membership reports, it should do no harm in doing so.  In particular
there should be no adverse effect in any IGMP snooping functionality
since 224.0.0.x is specifically excluded as per RFC 4541 (IGMP and MLD
Snooping Switches Considerations) section 2.1.2. Data Forwarding
Rules:

2) Packets with a destination IP (DIP) address in the 224.0.0.X
   range which are not IGMP must be forwarded on all ports.

IGMP reports for local multicast groups can now be optionally
inhibited by means of a system control variable (by setting the value
to zero) e.g.:
echo 0  /proc/sys/net/ipv4/igmp_link_local_reports

To retain backwards compatibility the previous behaviour is retained
by default on system boot or reverted by setting the value back to
non-zero e.g.:
echo 1   /proc/sys/net/ipv4/igmp_link_local_reports

Signed-off-by: Philip Downey pdow...@brocade.com
---
 include/linux/igmp.h   |1 +
 net/ipv4/igmp.c|   29 -
 net/ipv4/sysctl_net_ipv4.c |7 +++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index 193ad48..e3e0dae 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -37,6 +37,7 @@ static inline struct igmpv3_query *
return (struct igmpv3_query *)skb_transport_header(skb);
 }
 
+extern int sysctl_igmp_link_local_reports;
 extern int sysctl_igmp_max_memberships;
 extern int sysctl_igmp_max_msf;
 extern int sysctl_igmp_qrv;
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 9fdfd9d..a3df89d 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -110,6 +110,15 @@
 #define IP_MAX_MEMBERSHIPS 20
 #define IP_MAX_MSF 10
 
+/* IGMP reports for link-local multicast groups are enabled by default */
+#define IGMP_ENABLE_LLM 1
+
+int sysctl_igmp_link_local_reports __read_mostly = IGMP_ENABLE_LLM;
+
+#define IGMP_INHIBIT_LINK_LOCAL_REPORTS(_ipaddr) \
+   (ipv4_is_local_multicast(_ipaddr)  \
+(sysctl_igmp_link_local_reports == 0))
+
 #ifdef CONFIG_IP_MULTICAST
 /* Parameter names and values are taken from igmp-v2-06 draft */
 
@@ -437,6 +446,8 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct 
ip_mc_list *pmc,
 
if (pmc-multiaddr == IGMP_ALL_HOSTS)
return skb;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(pmc-multiaddr))
+   return skb;
 
isquery = type == IGMPV3_MODE_IS_INCLUDE ||
  type == IGMPV3_MODE_IS_EXCLUDE;
@@ -545,6 +556,8 @@ static int igmpv3_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc)
for_each_pmc_rcu(in_dev, pmc) {
if (pmc-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(pmc-multiaddr))
+   continue;
spin_lock_bh(pmc-lock);
if (pmc-sfcount[MCAST_EXCLUDE])
type = IGMPV3_MODE_IS_EXCLUDE;
@@ -678,7 +691,11 @@ static int igmp_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc,
 
if (type == IGMPV3_HOST_MEMBERSHIP_REPORT)
return igmpv3_send_report(in_dev, pmc);
-   else if (type == IGMP_HOST_LEAVE_MESSAGE)
+
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(group))
+   return 0;
+
+   if (type == IGMP_HOST_LEAVE_MESSAGE)
dst = IGMP_ALL_ROUTER;
else
dst = group;
@@ -851,6 +868,8 @@ static bool igmp_heard_report(struct in_device *in_dev, 
__be32 group)
 
if (group == IGMP_ALL_HOSTS)
return false;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(group))
+   return false;
 
rcu_read_lock();
for_each_pmc_rcu(in_dev, im) {
@@ -957,6 +976,8 @@ static bool igmp_heard_query(struct in_device *in_dev, 
struct sk_buff *skb,
continue;
if (im-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(im-multiaddr))
+   continue;

Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-14 Thread Andrew Lunn
Hi Philip

So with a bit of poking and prodding, we have a much better
understanding as to why this is O.K. Maybe your next patch can quote
the relevant RFCs and have a much fuller commit message?

Thanks
Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-14 Thread Philip Downey
Sorry for the duplication - I responded in a similar manner before seeing this.

Thanks

Philip

 -Original Message-
 From: Thadeu Lima de Souza Cascardo [mailto:casca...@redhat.com]
 Sent: Thursday, August 13, 2015 7:08 PM
 To: Andrew Lunn
 Cc: Philip Downey; David Miller; kuz...@ms2.inr.ac.ru; jmor...@namei.org;
 yoshf...@linux-ipv6.org; ka...@trash.net; linux-ker...@vger.kernel.org;
 netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 On Thu, Aug 13, 2015 at 07:01:37PM +0200, Andrew Lunn wrote:
  On Thu, Aug 13, 2015 at 04:52:32PM +, Philip Downey wrote:
   Hi Andrew
   IGMP snooping is designed to prevent hosts on a local network from
 receiving traffic for a multicast group they have not explicitly joined.   
 Link-
 Local multicast traffic should not have an IGMP client since it is reserved 
 for
 routing protocols.  One would expect that IGMP snooping needs to ignore
 local multicast traffic in the reserved range intended for routers since there
 should be no IGMP client to make join requests.
 
  The point of this patch is that Linux is sending out group membership
  for these addresses, it is acting as a client. What happens with a
  switch which is applying IGMP snooping to link-local multicast groups?
  You turn on this feature, and you no longer get your routing protocol
  messages.
 
  I had a quick look at RFC 3376. The only mention i spotted for not
  sending IGMP messages is:
 
 The all-systems multicast address, 224.0.0.1, is handled as a special
 case.  On all systems -- that is all hosts and routers, including
 multicast routers -- reception of packets destined to the all-systems
 multicast address, from all sources, is permanently enabled on all
 interfaces on which multicast reception is supported.  No IGMP
 messages are ever sent regarding the all-systems multicast address.
 
  IGMP v2 has something similar:
 
 The all-systems group (address 224.0.0.1) is handled as a special
 case.  The host starts in Idle Member state for that group on every
 interface, never transitions to another state, and never sends a
 report for that group.
 
  But i did not find anything which says all other link-local addresses
  don't need member reports. Did i miss something?
 
Andrew
 
 From RFC 4541 (Considerations for Internet Group Management Protocol
 (IGMP) and Multicast Listener Discovery (MLD) Snooping Switches):
 
  2) Packets with a destination IP (DIP) address in the 224.0.0.X range
   which are not IGMP must be forwarded on all ports.
 
   This recommendation is based on the fact that many host systems do
   not send Join IP multicast addresses in this range before sending
   or listening to IP multicast packets.  Furthermore, since the
   224.0.0.X address range is defined as link-local (not to be
   routed), it seems unnecessary to keep the state for each address
   in this range.  Additionally, some routers operate in the
   224.0.0.X address range without issuing IGMP Joins, and these
   applications would break if the switch were to prune them due to
   not having seen a Join Group message from the router.
 
 So, it looks like some hosts and routers out there in the field do not send
 joins for those local addresses. In fact, IPv4 local multicast addresses are
 ignored when Linux bridge multicast snooping adds a new group.
 
 static int br_ip4_multicast_add_group(struct net_bridge *br, ...
   if (ipv4_is_local_multicast(group))
   return 0;
 
 Cascardo.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-14 Thread Philip Downey
Hi Andrew
Answers inline...

 -Original Message-
 From: Andrew Lunn [mailto:and...@lunn.ch]
 Sent: Thursday, August 13, 2015 6:02 PM
 To: Philip Downey
 Cc: David Miller; kuz...@ms2.inr.ac.ru; jmor...@namei.org; yoshfuji@linux-
 ipv6.org; ka...@trash.net; linux-ker...@vger.kernel.org;
 netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 On Thu, Aug 13, 2015 at 04:52:32PM +, Philip Downey wrote:
  Hi Andrew
  IGMP snooping is designed to prevent hosts on a local network from
 receiving traffic for a multicast group they have not explicitly joined.   
 Link-
 Local multicast traffic should not have an IGMP client since it is reserved 
 for
 routing protocols.  One would expect that IGMP snooping needs to ignore
 local multicast traffic in the reserved range intended for routers since there
 should be no IGMP client to make join requests.
 
 The point of this patch is that Linux is sending out group membership for
 these addresses, it is acting as a client. What happens with a switch which is
 applying IGMP snooping to link-local multicast groups?
 You turn on this feature, and you no longer get your routing protocol
 messages.

It is expected that link-local multicast is always forwarded by switches 
otherwise routers may not function correctly.

From the relevant RFC:

RFC 4541 IGMP and MLD Snooping Switches Considerations  May 2006


2.1.2. Data Forwarding Rules


   1) Packets with a destination IP address outside 224.0.0.X which are
  not IGMP should be forwarded according to group-based port
  membership tables and must also be forwarded on router ports.

  This is the main IGMP snooping functionality for the data path.
  One approach that an implementation could take would be to
  maintain separate membership and multicast router tables in
  software and then merge these tables into a forwarding cache.

   2) Packets with a destination IP (DIP) address in the 224.0.0.X range
  which are not IGMP must be forwarded on all ports.

  This recommendation is based on the fact that many host systems do
  not send Join IP multicast addresses in this range before sending
  or listening to IP multicast packets.  Furthermore, since the
  224.0.0.X address range is defined as link-local (not to be
  routed), it seems unnecessary to keep the state for each address
  in this range.  Additionally, some routers operate in the
  224.0.0.X address range without issuing IGMP Joins, and these
  applications would break if the switch were to prune them due to
  not having seen a Join Group message from the router.

 
 I had a quick look at RFC 3376. The only mention i spotted for not sending
 IGMP messages is:
 
The all-systems multicast address, 224.0.0.1, is handled as a special
case.  On all systems -- that is all hosts and routers, including
multicast routers -- reception of packets destined to the all-systems
multicast address, from all sources, is permanently enabled on all
interfaces on which multicast reception is supported.  No IGMP
messages are ever sent regarding the all-systems multicast address.
 
 IGMP v2 has something similar:
 
The all-systems group (address 224.0.0.1) is handled as a special
case.  The host starts in Idle Member state for that group on every
interface, never transitions to another state, and never sends a
report for that group.
 
 But i did not find anything which says all other link-local addresses don't
 need member reports. Did i miss something?

No you did not miss anything - that is correct.
However, the RFCs don't really cover the behavior of routers well in some 
areas.   Routing protocols which use the 224.0.0.x address space do not need 
IGMP therefore it makes no sense to distribute membership reports for these 
groups.  A router which receives an IGMP membership report which includes 
groups from this reserved address range will ignore it -and probably generate 
debug messages highlighting an invalid address.

Regards

Philip
 
   Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-14 Thread Philip Downey
Hi Andrew
Will resubmit with the information you suggest.
There might be a slight delay for this as I am on holiday now for a week.

Rest regards

Philip

 -Original Message-
 From: Andrew Lunn [mailto:and...@lunn.ch]
 Sent: Friday, August 14, 2015 2:35 PM
 To: Philip Downey
 Cc: David Miller; kuz...@ms2.inr.ac.ru; jmor...@namei.org; yoshfuji@linux-
 ipv6.org; ka...@trash.net; linux-ker...@vger.kernel.org;
 netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 Hi Philip
 
 So with a bit of poking and prodding, we have a much better understanding
 as to why this is O.K. Maybe your next patch can quote the relevant RFCs
 and have a much fuller commit message?
 
 Thanks
   Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-13 Thread Philip Downey
Hi David
Thanks for taking the time to review and comment.
This is my first upstream request so please forgive any ignorance on my part.   
I have added a new proposed commit wording below with a view to agreeing the 
content before resubmitting the patch.
I hope it is sufficient to address your concerns.

   IGMP: Inhibit reports for local multicast groups

The range of addresses between 224.0.0.0 and 224.0.0.255
inclusive, is reserved for the use of routing protocols and other
low-level topology discovery or maintenance protocols, such as
gateway discovery and group membership reporting.  Multicast
routers should not forward any multicast datagram with   destination
addresses in this range, regardless of its TTL.

Currently, IGMP reports are generated for this reserved range of
addresses even though a router will ignore this information since
it has no purpose.  However, the presence of reserved group
addresses in an IGMP membership report uses up network bandwidth
and can also obscure addresses of interest when inspecting
membership reports using packet inspection or debug messages.

IGMP reports for local multicast groups can now be inhibited by means
of a system control variable (setting the value to zero).

To retain backwards compatibility the previous behaviour is retained by
default on system boot.

Signed-off-by: Philip Downey pdow...@brocade.com


Regards

Philip

 -Original Message-
 From: David Miller [mailto:da...@davemloft.net]
 Sent: Thursday, August 13, 2015 12:45 AM
 To: Philip Downey
 Cc: kuz...@ms2.inr.ac.ru; jmor...@namei.org; yoshf...@linux-ipv6.org;
 ka...@trash.net; linux-ker...@vger.kernel.org; netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 From: Philip Downey pdow...@brocade.com
 Date: Wed, 12 Aug 2015 17:13:53 +0100
 
  IGMP reports are generated for link local multicast groups (224.0.0.1
  - 224.0.0.255) used by the routing protocols such as RIP, OSPF etc.
  In general routers do not generate reports for local multicast groups.
 
  IGMP reports for local multicast groups can now be inhibited by means
  of a system control variable (setting the value to zero).
 
  To retain backwards compatibility the previous behaviour is retained
  by default on system boot.
 
  Signed-off-by: Philip Downey pdow...@brocade.com
 
 I'm always hesitent to apply patches like this.
 
 I can't even understand from your explanation:
 
 1) what about local reporting behavior is so bad
 
 2) why you want to inhibit them at all
 
 For example, this:
 
  In general routers do not generate reports for local multicast groups.
 
 Doesn't tell me anything.  You need to go into more detail about this, and
 explain the situation sufficiently.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-13 Thread Philip Downey
Hi Andrew
IGMP snooping is designed to prevent hosts on a local network from receiving 
traffic for a multicast group they have not explicitly joined.   Link-Local 
multicast traffic should not have an IGMP client since it is reserved for 
routing protocols.  One would expect that IGMP snooping needs to ignore local 
multicast traffic in the reserved range intended for routers since there should 
be no IGMP client to make join requests.

Regards

Philip

 -Original Message-
 From: Andrew Lunn [mailto:and...@lunn.ch]
 Sent: Thursday, August 13, 2015 5:06 PM
 To: Philip Downey
 Cc: David Miller; kuz...@ms2.inr.ac.ru; jmor...@namei.org; yoshfuji@linux-
 ipv6.org; ka...@trash.net; linux-ker...@vger.kernel.org;
 netdev@vger.kernel.org
 Subject: Re: [PATCH] IGMP: Inhibit reports for local multicast groups
 
 On Thu, Aug 13, 2015 at 02:48:23PM +, Philip Downey wrote:
  Hi David
  Thanks for taking the time to review and comment.
  This is my first upstream request so please forgive any ignorance on my
 part.   I have added a new proposed commit wording below with a view to
 agreeing the content before resubmitting the patch.
  I hope it is sufficient to address your concerns.
 
 IGMP: Inhibit reports for local multicast groups
 
  The range of addresses between 224.0.0.0 and 224.0.0.255
  inclusive, is reserved for the use of routing protocols and other
  low-level topology discovery or maintenance protocols, such as
  gateway discovery and group membership reporting.  Multicast
  routers should not forward any multicast datagram with   destination
  addresses in this range, regardless of its TTL.
 
  Currently, IGMP reports are generated for this reserved range of
  addresses even though a router will ignore this information since
  it has no purpose.
 
 Hi Philip
 
 What about switches which are doing IGMP snooping?
 
  Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-13 Thread Andrew Lunn
On Thu, Aug 13, 2015 at 04:52:32PM +, Philip Downey wrote:
 Hi Andrew
 IGMP snooping is designed to prevent hosts on a local network from receiving 
 traffic for a multicast group they have not explicitly joined.   Link-Local 
 multicast traffic should not have an IGMP client since it is reserved for 
 routing protocols.  One would expect that IGMP snooping needs to ignore local 
 multicast traffic in the reserved range intended for routers since there 
 should be no IGMP client to make join requests.

The point of this patch is that Linux is sending out group membership
for these addresses, it is acting as a client. What happens with a
switch which is applying IGMP snooping to link-local multicast groups?
You turn on this feature, and you no longer get your routing protocol
messages.

I had a quick look at RFC 3376. The only mention i spotted for not
sending IGMP messages is:

   The all-systems multicast address, 224.0.0.1, is handled as a special
   case.  On all systems -- that is all hosts and routers, including
   multicast routers -- reception of packets destined to the all-systems
   multicast address, from all sources, is permanently enabled on all
   interfaces on which multicast reception is supported.  No IGMP
   messages are ever sent regarding the all-systems multicast address.

IGMP v2 has something similar:

   The all-systems group (address 224.0.0.1) is handled as a special
   case.  The host starts in Idle Member state for that group on every
   interface, never transitions to another state, and never sends a
   report for that group.

But i did not find anything which says all other link-local addresses
don't need member reports. Did i miss something?

  Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-13 Thread Thadeu Lima de Souza Cascardo
On Thu, Aug 13, 2015 at 07:01:37PM +0200, Andrew Lunn wrote:
 On Thu, Aug 13, 2015 at 04:52:32PM +, Philip Downey wrote:
  Hi Andrew
  IGMP snooping is designed to prevent hosts on a local network from 
  receiving traffic for a multicast group they have not explicitly joined.   
  Link-Local multicast traffic should not have an IGMP client since it is 
  reserved for routing protocols.  One would expect that IGMP snooping needs 
  to ignore local multicast traffic in the reserved range intended for 
  routers since there should be no IGMP client to make join requests.
 
 The point of this patch is that Linux is sending out group membership
 for these addresses, it is acting as a client. What happens with a
 switch which is applying IGMP snooping to link-local multicast groups?
 You turn on this feature, and you no longer get your routing protocol
 messages.
 
 I had a quick look at RFC 3376. The only mention i spotted for not
 sending IGMP messages is:
 
The all-systems multicast address, 224.0.0.1, is handled as a special
case.  On all systems -- that is all hosts and routers, including
multicast routers -- reception of packets destined to the all-systems
multicast address, from all sources, is permanently enabled on all
interfaces on which multicast reception is supported.  No IGMP
messages are ever sent regarding the all-systems multicast address.
 
 IGMP v2 has something similar:
 
The all-systems group (address 224.0.0.1) is handled as a special
case.  The host starts in Idle Member state for that group on every
interface, never transitions to another state, and never sends a
report for that group.
 
 But i did not find anything which says all other link-local addresses
 don't need member reports. Did i miss something?
 
   Andrew

From RFC 4541 (Considerations for Internet Group Management Protocol (IGMP) and
Multicast Listener Discovery (MLD) Snooping Switches):

 2) Packets with a destination IP (DIP) address in the 224.0.0.X range
  which are not IGMP must be forwarded on all ports.

  This recommendation is based on the fact that many host systems do
  not send Join IP multicast addresses in this range before sending
  or listening to IP multicast packets.  Furthermore, since the
  224.0.0.X address range is defined as link-local (not to be
  routed), it seems unnecessary to keep the state for each address
  in this range.  Additionally, some routers operate in the
  224.0.0.X address range without issuing IGMP Joins, and these
  applications would break if the switch were to prune them due to
  not having seen a Join Group message from the router.

So, it looks like some hosts and routers out there in the field do not send
joins for those local addresses. In fact, IPv4 local multicast addresses are
ignored when Linux bridge multicast snooping adds a new group.

static int br_ip4_multicast_add_group(struct net_bridge *br,
...
if (ipv4_is_local_multicast(group))
return 0;

Cascardo.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-13 Thread Andrew Lunn
On Thu, Aug 13, 2015 at 02:48:23PM +, Philip Downey wrote:
 Hi David
 Thanks for taking the time to review and comment.
 This is my first upstream request so please forgive any ignorance on my part. 
   I have added a new proposed commit wording below with a view to agreeing 
 the content before resubmitting the patch.
 I hope it is sufficient to address your concerns.
 
IGMP: Inhibit reports for local multicast groups
 
 The range of addresses between 224.0.0.0 and 224.0.0.255
 inclusive, is reserved for the use of routing protocols and other
 low-level topology discovery or maintenance protocols, such as
 gateway discovery and group membership reporting.  Multicast
 routers should not forward any multicast datagram with   destination
 addresses in this range, regardless of its TTL.
 
 Currently, IGMP reports are generated for this reserved range of
 addresses even though a router will ignore this information since
 it has no purpose.

Hi Philip

What about switches which are doing IGMP snooping?

 Andrew
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-12 Thread David Miller
From: Philip Downey pdow...@brocade.com
Date: Wed, 12 Aug 2015 17:13:53 +0100

 IGMP reports are generated for link local multicast groups (224.0.0.1
 - 224.0.0.255) used by the routing protocols such as RIP, OSPF etc.
 In general routers do not generate reports for local multicast groups.
 
 IGMP reports for local multicast groups can now be inhibited by means
 of a system control variable (setting the value to zero).
 
 To retain backwards compatibility the previous behaviour is retained by
 default on system boot.
 
 Signed-off-by: Philip Downey pdow...@brocade.com

I'm always hesitent to apply patches like this.

I can't even understand from your explanation:

1) what about local reporting behavior is so bad

2) why you want to inhibit them at all

For example, this:

 In general routers do not generate reports for local multicast groups.

Doesn't tell me anything.  You need to go into more detail about
this, and explain the situation sufficiently.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] IGMP: Inhibit reports for local multicast groups

2015-08-12 Thread Philip Downey
IGMP reports are generated for link local multicast groups (224.0.0.1
- 224.0.0.255) used by the routing protocols such as RIP, OSPF etc.
In general routers do not generate reports for local multicast groups.

IGMP reports for local multicast groups can now be inhibited by means
of a system control variable (setting the value to zero).

To retain backwards compatibility the previous behaviour is retained by
default on system boot.

Signed-off-by: Philip Downey pdow...@brocade.com
---
 include/linux/igmp.h   |1 +
 net/ipv4/igmp.c|   29 -
 net/ipv4/sysctl_net_ipv4.c |7 +++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/linux/igmp.h b/include/linux/igmp.h
index 193ad48..e3e0dae 100644
--- a/include/linux/igmp.h
+++ b/include/linux/igmp.h
@@ -37,6 +37,7 @@ static inline struct igmpv3_query *
return (struct igmpv3_query *)skb_transport_header(skb);
 }
 
+extern int sysctl_igmp_link_local_reports;
 extern int sysctl_igmp_max_memberships;
 extern int sysctl_igmp_max_msf;
 extern int sysctl_igmp_qrv;
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 651cdf6..2e4d5b8 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -110,6 +110,15 @@
 #define IP_MAX_MEMBERSHIPS 20
 #define IP_MAX_MSF 10
 
+/* IGMP reports for link-local multicast groups are enabled by default */
+#define IGMP_ENABLE_LLM 1
+
+int sysctl_igmp_link_local_reports __read_mostly = IGMP_ENABLE_LLM;
+
+#define IGMP_INHIBIT_LINK_LOCAL_REPORTS(_ipaddr) \
+   (ipv4_is_local_multicast(_ipaddr)  \
+(sysctl_igmp_link_local_reports == 0))
+
 #ifdef CONFIG_IP_MULTICAST
 /* Parameter names and values are taken from igmp-v2-06 draft */
 
@@ -437,6 +446,8 @@ static struct sk_buff *add_grec(struct sk_buff *skb, struct 
ip_mc_list *pmc,
 
if (pmc-multiaddr == IGMP_ALL_HOSTS)
return skb;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(pmc-multiaddr))
+   return skb;
 
isquery = type == IGMPV3_MODE_IS_INCLUDE ||
  type == IGMPV3_MODE_IS_EXCLUDE;
@@ -545,6 +556,8 @@ static int igmpv3_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc)
for_each_pmc_rcu(in_dev, pmc) {
if (pmc-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(pmc-multiaddr))
+   continue;
spin_lock_bh(pmc-lock);
if (pmc-sfcount[MCAST_EXCLUDE])
type = IGMPV3_MODE_IS_EXCLUDE;
@@ -678,7 +691,11 @@ static int igmp_send_report(struct in_device *in_dev, 
struct ip_mc_list *pmc,
 
if (type == IGMPV3_HOST_MEMBERSHIP_REPORT)
return igmpv3_send_report(in_dev, pmc);
-   else if (type == IGMP_HOST_LEAVE_MESSAGE)
+
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(group))
+   return 0;
+
+   if (type == IGMP_HOST_LEAVE_MESSAGE)
dst = IGMP_ALL_ROUTER;
else
dst = group;
@@ -851,6 +868,8 @@ static bool igmp_heard_report(struct in_device *in_dev, 
__be32 group)
 
if (group == IGMP_ALL_HOSTS)
return false;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(group))
+   return false;
 
rcu_read_lock();
for_each_pmc_rcu(in_dev, im) {
@@ -957,6 +976,8 @@ static bool igmp_heard_query(struct in_device *in_dev, 
struct sk_buff *skb,
continue;
if (im-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(im-multiaddr))
+   continue;
spin_lock_bh(im-lock);
if (im-tm_running)
im-gsquery = im-gsquery  mark;
@@ -1181,6 +1202,8 @@ static void igmp_group_dropped(struct ip_mc_list *im)
 #ifdef CONFIG_IP_MULTICAST
if (im-multiaddr == IGMP_ALL_HOSTS)
return;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(im-multiaddr))
+   return;
 
reporter = im-reporter;
igmp_stop_timer(im);
@@ -1213,6 +1236,8 @@ static void igmp_group_added(struct ip_mc_list *im)
 #ifdef CONFIG_IP_MULTICAST
if (im-multiaddr == IGMP_ALL_HOSTS)
return;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(im-multiaddr))
+   return;
 
if (in_dev-dead)
return;
@@ -1515,6 +1540,8 @@ static void ip_mc_rejoin_groups(struct in_device *in_dev)
for_each_pmc_rtnl(in_dev, im) {
if (im-multiaddr == IGMP_ALL_HOSTS)
continue;
+   if (IGMP_INHIBIT_LINK_LOCAL_REPORTS(im-multiaddr))
+   continue;
 
/* a failover is happening and switches
 * must be notified immediately
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 433231c..e7fd509 100644
---