IGMP Unsolicited Report Interval too long for IGMPv3?

IGMP Unsolicited Report Interval too long for IGMPv3?

[William Manley]

If an IGMP join packet is lost you will not receive data sent to the 
multicast group so if no data arrives from that multicast group in a 
period of time after the IGMP join a second IGMP join will be sent.  The 
delay between joins is the "IGMP Unsolicited Report Interval".

In the kernel this seems to be hard coded to be chosen randomly between 
0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
change to be slow in the presence of packet loss.

I would guess that this 10s has come from IGMPv2 RFC2236, which was 
reduced to 1s in IGMPv3 RFC3376.

There was a thread about this on linux-rdma in 2010 in the context of IP 
over Infiniband but it seems no patches got applied as a result of the 
discussion:

http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg05740.html

Would the right patch reducing the unsolicited report interval for 
IGMPv3 be acceptable now?

Thanks

Will

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Ben Hutchings]

On Mon, 2013-07-22 at 21:43 +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the 
> multicast group so if no data arrives from that multicast group in a 
> period of time after the IGMP join a second IGMP join will be sent.  The 
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> In the kernel this seems to be hard coded to be chosen randomly between 
> 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
> change to be slow in the presence of packet loss.
> 
> I would guess that this 10s has come from IGMPv2 RFC2236, which was 
> reduced to 1s in IGMPv3 RFC3376.
> 
> There was a thread about this on linux-rdma in 2010 in the context of IP 
> over Infiniband but it seems no patches got applied as a result of the 
> discussion:
> 
> http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg05740.html
> 
> Would the right patch reducing the unsolicited report interval for 
> IGMPv3 be acceptable now?

I suggest you post a patch and find out what people think of it.  That
usually gets more response than asking in the abstract.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Benjamin LaHaise]

On Mon, Jul 22, 2013 at 09:43:57PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the 
> multicast group so if no data arrives from that multicast group in a 
> period of time after the IGMP join a second IGMP join will be sent.  The 
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> In the kernel this seems to be hard coded to be chosen randomly between 
> 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
> change to be slow in the presence of packet loss.
> 
> I would guess that this 10s has come from IGMPv2 RFC2236, which was 
> reduced to 1s in IGMPv3 RFC3376.

Reducing the timeout does not solve the problem you are encountering, as 
any packet loss will still result in a 1 second delay.  I've encountered 
similar issues dealing with LCP Echo request/replies for keepalive 
messages on PPP sessions.  The correct approach is to queue the IGMP 
multicast join with a higher priority than other traffic in the system 
so that the requests are not lost due to congestion of a single queue.  
Sending packets with an 802.1p header might be appropriate in your 
use-case, or perhaps using higher priority internal queues.

		-ben

> There was a thread about this on linux-rdma in 2010 in the context of IP 
> over Infiniband but it seems no patches got applied as a result of the 
> discussion:
> 
> http://www.mail-archive.com/linux-rdma@vger.kernel.org/msg05740.html
> 
> Would the right patch reducing the unsolicited report interval for 
> IGMPv3 be acceptable now?
> 
> Thanks
> 
> Will
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
"Thought is the essence of where you are now."

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Hannes Frederic Sowa]

On Mon, Jul 22, 2013 at 05:18:55PM -0400, Benjamin LaHaise wrote:
> On Mon, Jul 22, 2013 at 09:43:57PM +0100, William Manley wrote:
> > If an IGMP join packet is lost you will not receive data sent to the 
> > multicast group so if no data arrives from that multicast group in a 
> > period of time after the IGMP join a second IGMP join will be sent.  The 
> > delay between joins is the "IGMP Unsolicited Report Interval".
> > 
> > In the kernel this seems to be hard coded to be chosen randomly between 
> > 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
> > change to be slow in the presence of packet loss.
> > 
> > I would guess that this 10s has come from IGMPv2 RFC2236, which was 
> > reduced to 1s in IGMPv3 RFC3376.
> 
> Reducing the timeout does not solve the problem you are encountering, as 
> any packet loss will still result in a 1 second delay.  I've encountered 
> similar issues dealing with LCP Echo request/replies for keepalive 
> messages on PPP sessions.  The correct approach is to queue the IGMP 
> multicast join with a higher priority than other traffic in the system 
> so that the requests are not lost due to congestion of a single queue.  
> Sending packets with an 802.1p header might be appropriate in your 
> use-case, or perhaps using higher priority internal queues.

Yes, we can do a little bit better. What do you think?

[PATCH net-next] ipv6: send igmpv3/mld packets with TC_PRIO_CONTROL

Reported-by: William Manley <william.manley@youview.com>
Suggested-by: Benjamin LaHaise <bcrl@kvack.org>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 99cd65c..db25b8e 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -44,6 +44,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/pkt_sched.h>
 #include <net/mld.h>
 
 #include <linux/netfilter.h>
@@ -1376,6 +1377,7 @@ static struct sk_buff *mld_newpack(struct inet6_dev *idev, int size)
 	if (!skb)
 		return NULL;
 
+	skb->priority = TC_PRIO_CONTROL;
 	skb_reserve(skb, hlen);
 
 	if (__ipv6_get_lladdr(idev, &addr_buf, IFA_F_TENTATIVE)) {
@@ -1769,7 +1771,7 @@ static void igmp6_send(struct in6_addr *addr, struct net_device *dev, int type)
 		rcu_read_unlock();
 		return;
 	}
-
+	skb->priority = TC_PRIO_CONTROL;
 	skb_reserve(skb, hlen);
 
 	if (ipv6_get_lladdr(dev, &addr_buf, IFA_F_TENTATIVE)) {
-- 
1.8.3.1

RE: IGMP Unsolicited Report Interval too long for IGMPv3?

[Lukas Tribus]

> Date: Mon, 22 Jul 2013 17:18:55 -0400
>> I would guess that this 10s has come from IGMPv2 RFC2236, which was
>> reduced to 1s in IGMPv3 RFC3376.
>
> Reducing the timeout does not solve the problem you are encountering, as
> any packet loss will still result in a 1 second delay.

Packet loss will always result in a delay and I think William is well aware
of that.

off-topic: 1 second is not a problem in IPTV, 10 seconds are ([1]).



> The correct approach is to queue the IGMP multicast join with a higher
> priority than other traffic in the system so that the requests are not
> lost due to congestion of a single queue.

While this certainly makes sense, congestion is not the only reason for
packet loss. There is no way to fix packet loss in lower network layers,
like ADSL, satellite links or IPoAC.

Improving retransmission by making it more predictable, bringing it closer
to RFC proposals and real life problems makes a lot of sense, imho. This
includes setting TC_PRIO_CONTROL, but I'm not sure it will fix Williams use
case.


lukas

[1] http://en.wikipedia.org/wiki/Zap_time 		 	   		  

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Hannes Frederic Sowa]

On Tue, Jul 23, 2013 at 12:06:31AM +0200, Lukas Tribus wrote:
> > Date: Mon, 22 Jul 2013 17:18:55 -0400
> >> I would guess that this 10s has come from IGMPv2 RFC2236, which was
> >> reduced to 1s in IGMPv3 RFC3376.
> >
> > Reducing the timeout does not solve the problem you are encountering, as
> > any packet loss will still result in a 1 second delay.
> 
> Packet loss will always result in a delay and I think William is well aware
> of that.
> 
> off-topic: 1 second is not a problem in IPTV, 10 seconds are ([1]).
> 
> 
> 
> > The correct approach is to queue the IGMP multicast join with a higher
> > priority than other traffic in the system so that the requests are not
> > lost due to congestion of a single queue.
> 
> While this certainly makes sense, congestion is not the only reason for
> packet loss. There is no way to fix packet loss in lower network layers,
> like ADSL, satellite links or IPoAC.
> 
> Improving retransmission by making it more predictable, bringing it closer
> to RFC proposals and real life problems makes a lot of sense, imho. This
> includes setting TC_PRIO_CONTROL, but I'm not sure it will fix Williams use
> case.

Yes, it was merely meant as an RFC without the appropriate changes for IPv4
(I somehow missed this in the patch marker :| ).

Regarding the value of the IGMP unsolicited report interval, I would strictly
stick to the RFCs. So a change for IGMPv3 would be appropriate IMHO.

I would also suggest that posting patches would be favourable. I don't think
it should be too much work.

Thanks,

  Hannes

[PATCH] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Previously this value was hard coded to be chosen randomly between 0-10s.
This can be too long for some use-cases, such as IPTV as it can cause
channel change to be slow in the presence of packet loss.

The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
unaffected.

Tested with Wireshark and a simple program to join a (non-existant)
multicast group.  The distribution of timings for the second join differ
based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
---
 net/ipv4/igmp.c |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d8c2327..f8142df 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -113,7 +113,8 @@
 
 #define IGMP_V1_Router_Present_Timeout		(400*HZ)
 #define IGMP_V2_Router_Present_Timeout		(400*HZ)
-#define IGMP_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V3_Unsolicited_Report_Interval	( 1*HZ)
 #define IGMP_Query_Response_Interval		(10*HZ)
 #define IGMP_Unsolicited_Report_Count		2
 
@@ -138,6 +139,15 @@
 	 ((in_dev)->mr_v2_seen && \
 	  time_before(jiffies, (in_dev)->mr_v2_seen)))
 
+static int unsolicited_report_interval(struct in_device *in_dev)
+{
+	if (IGMP_V2_SEEN(in_dev)) {
+		return IGMP_V2_Unsolicited_Report_Interval;
+	} else { /* v3 */
+		return IGMP_V3_Unsolicited_Report_Interval;
+	}
+}
+
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
 static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
 static void igmpv3_clear_delrec(struct in_device *in_dev);
@@ -719,7 +729,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
 	igmpv3_send_cr(in_dev);
 	if (in_dev->mr_ifc_count) {
 		in_dev->mr_ifc_count--;
-		igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
+		igmp_ifc_start_timer(in_dev,
+				     unsolicited_report_interval(in_dev));
 	}
 	__in_dev_put(in_dev);
 }
@@ -744,7 +755,7 @@ static void igmp_timer_expire(unsigned long data)
 
 	if (im->unsolicit_count) {
 		im->unsolicit_count--;
-		igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
+		igmp_start_timer(im, unsolicited_report_interval(in_dev));
 	}
 	im->reporter = 1;
 	spin_unlock(&im->lock);
-- 
1.7.10.4

Re: [PATCH] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

On 24/07/13 14:38, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
>
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
>
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.
>
> Tested with Wireshark and a simple program to join a (non-existant)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> ---
>   net/ipv4/igmp.c |   17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)

I'm also working on a patch which builds on this adding a procfs knob 
similar to force_igmp_version but called 
"force_igmp_unsolicited_report_interval".  This will allow the interval 
to be overridden from user-space independent of the version of IGMP in 
use as for our use-case we want the interval to be short even for IGMPv2.

Thanks

Will

Re: [PATCH] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Sergei Shtylyov]

Hello.

On 24-07-2013 17:38, William Manley wrote:

> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".

> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.

> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.

> Tested with Wireshark and a simple program to join a (non-existant)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> ---
>   net/ipv4/igmp.c |   17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)

> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index d8c2327..f8142df 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
[...]
> @@ -138,6 +139,15 @@
>   	 ((in_dev)->mr_v2_seen && \
>   	  time_before(jiffies, (in_dev)->mr_v2_seen)))
>
> +static int unsolicited_report_interval(struct in_device *in_dev)
> +{
> +	if (IGMP_V2_SEEN(in_dev)) {
> +		return IGMP_V2_Unsolicited_Report_Interval;
> +	} else { /* v3 */
> +		return IGMP_V3_Unsolicited_Report_Interval;
> +	}

    {} not needed on either arm of *if*. I guess scripts/checkpatch.pl should 
have warned you...

WBR, Sergei

[PATCH 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Previously this value was hard coded to be chosen randomly between 0-10s.
This can be too long for some use-cases, such as IPTV as it can cause
channel change to be slow in the presence of packet loss.

The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
unaffected.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.

Signed-off-by: William Manley <william.manley@youview.com>
---
 net/ipv4/igmp.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d8c2327..12ffc2b 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -113,7 +113,8 @@
 
 #define IGMP_V1_Router_Present_Timeout		(400*HZ)
 #define IGMP_V2_Router_Present_Timeout		(400*HZ)
-#define IGMP_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
 #define IGMP_Query_Response_Interval		(10*HZ)
 #define IGMP_Unsolicited_Report_Count		2
 
@@ -138,6 +139,14 @@
 	 ((in_dev)->mr_v2_seen && \
 	  time_before(jiffies, (in_dev)->mr_v2_seen)))
 
+static int unsolicited_report_interval(struct in_device *in_dev)
+{
+	if (IGMP_V2_SEEN(in_dev))
+		return IGMP_V2_Unsolicited_Report_Interval;
+	else /* v3 */
+		return IGMP_V3_Unsolicited_Report_Interval;
+}
+
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
 static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
 static void igmpv3_clear_delrec(struct in_device *in_dev);
@@ -719,7 +728,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
 	igmpv3_send_cr(in_dev);
 	if (in_dev->mr_ifc_count) {
 		in_dev->mr_ifc_count--;
-		igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
+		igmp_ifc_start_timer(in_dev,
+				     unsolicited_report_interval(in_dev));
 	}
 	__in_dev_put(in_dev);
 }
@@ -744,7 +754,7 @@ static void igmp_timer_expire(unsigned long data)
 
 	if (im->unsolicit_count) {
 		im->unsolicit_count--;
-		igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
+		igmp_start_timer(im, unsolicited_report_interval(in_dev));
 	}
 	im->reporter = 1;
 	spin_unlock(&im->lock);
-- 
1.7.10.4

[PATCH 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[William Manley]

Adds a new procfs knob:

    /proc/sys/net/ipv4/conf/*/force_igmp_unsolicited_report_interval

Which will allow userspace configuration of the IGMP unsolicited report
interval (see below) in milliseconds.  The default is -1 which means use
default behaviour (10s for IGMPv3 and 1s for IGMPv2 in accordance with
RFC2236 and RFC3376).

Background:

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Prior to this patch this value was hard coded in the kernel to 10s for
IGMPv2 and 1s for IGMPv3.  10s is unsuitable for some use-cases, such as
IPTV as it can cause channel change to be slow in the presence of packet
loss.

This patch allows the value to be overridden from userspace changed for
both IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting
/proc/sys/net/ipv4/conf/eth0/force_igmp_unsolicited_report_interval.

force_igmp_unsolicited_report_interval is intended to follow the pattern
established by force_igmp_version, and while a procfs entry has been added
a corresponding sysctl knob has not as it is my understanding that sysctl
is deprecated[1].

[1]: http://lwn.net/Articles/247243/

Signed-off-by: William Manley <william.manley@youview.com>
---
 include/linux/inetdevice.h |    2 ++
 net/ipv4/devinet.c         |    4 ++++
 net/ipv4/igmp.c            |   13 ++++++++++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ea1e3b8..d8019f5 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -30,6 +30,7 @@ enum
 	IPV4_DEVCONF_NOXFRM,
 	IPV4_DEVCONF_NOPOLICY,
 	IPV4_DEVCONF_FORCE_IGMP_VERSION,
+	IPV4_DEVCONF_FORCE_IGMP_UNSOLICITED_REPORT_INTERVAL,
 	IPV4_DEVCONF_ARP_ANNOUNCE,
 	IPV4_DEVCONF_ARP_IGNORE,
 	IPV4_DEVCONF_PROMOTE_SECONDARIES,
@@ -62,6 +63,7 @@ struct in_device {
 	unsigned long		mr_v1_seen;
 	unsigned long		mr_v2_seen;
 	unsigned long		mr_maxdelay;
+	unsigned long		mr_unsolicited_report_interval;
 	unsigned char		mr_qrv;
 	unsigned char		mr_gq_running;
 	unsigned char		mr_ifc_count;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index dfc39d4..cd9b0f1 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -73,6 +73,7 @@ static struct ipv4_devconf ipv4_devconf = {
 		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
+		[IPV4_DEVCONF_FORCE_IGMP_UNSOLICITED_REPORT_INTERVAL - 1] = -1,
 	},
 };
 
@@ -83,6 +84,7 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
 		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
+		[IPV4_DEVCONF_FORCE_IGMP_UNSOLICITED_REPORT_INTERVAL - 1] = -1,
 	},
 };
 
@@ -2099,6 +2101,8 @@ static struct devinet_sysctl_table {
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
 					      "force_igmp_version"),
+		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_UNSOLICITED_REPORT_INTERVAL,
+					      "force_igmp_unsolicited_report_interval"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
 					      "promote_secondaries"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 12ffc2b..4f681ff 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -141,7 +141,18 @@
 
 static int unsolicited_report_interval(struct in_device *in_dev)
 {
-	if (IGMP_V2_SEEN(in_dev))
+	int interval_ms = IPV4_DEVCONF_ALL(
+		dev_net(in_dev->dev),
+		FORCE_IGMP_UNSOLICITED_REPORT_INTERVAL);
+	if (interval_ms >= 0) {
+		/* _timer functions can't handle a delay of 0 jiffies so ensure
+		 *  we always return a positive value.
+		 */
+		int interval_jiffies = msecs_to_jiffies(interval_ms);
+		if (interval_jiffies == 0)
+			interval_jiffies = 1;
+		return interval_jiffies;
+	} else if (IGMP_V2_SEEN(in_dev))
 		return IGMP_V2_Unsolicited_Report_Interval;
 	else /* v3 */
 		return IGMP_V3_Unsolicited_Report_Interval;
-- 
1.7.10.4

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Mon, 22 Jul 2013 23:51:08 +0200

> On Mon, Jul 22, 2013 at 05:18:55PM -0400, Benjamin LaHaise wrote:
>> On Mon, Jul 22, 2013 at 09:43:57PM +0100, William Manley wrote:
>> > If an IGMP join packet is lost you will not receive data sent to the 
>> > multicast group so if no data arrives from that multicast group in a 
>> > period of time after the IGMP join a second IGMP join will be sent.  The 
>> > delay between joins is the "IGMP Unsolicited Report Interval".
>> > 
>> > In the kernel this seems to be hard coded to be chosen randomly between 
>> > 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
>> > change to be slow in the presence of packet loss.
>> > 
>> > I would guess that this 10s has come from IGMPv2 RFC2236, which was 
>> > reduced to 1s in IGMPv3 RFC3376.
>> 
>> Reducing the timeout does not solve the problem you are encountering, as 
>> any packet loss will still result in a 1 second delay.  I've encountered 
>> similar issues dealing with LCP Echo request/replies for keepalive 
>> messages on PPP sessions.  The correct approach is to queue the IGMP 
>> multicast join with a higher priority than other traffic in the system 
>> so that the requests are not lost due to congestion of a single queue.  
>> Sending packets with an 802.1p header might be appropriate in your 
>> use-case, or perhaps using higher priority internal queues.
> 
> Yes, we can do a little bit better. What do you think?
> 
> [PATCH net-next] ipv6: send igmpv3/mld packets with TC_PRIO_CONTROL
> 
> Reported-by: William Manley <william.manley@youview.com>
> Suggested-by: Benjamin LaHaise <bcrl@kvack.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Ben, please give Hannes the feedback he is asking for.

Thanks.

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Benjamin LaHaise]

On Thu, Jul 25, 2013 at 04:42:53PM -0700, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Mon, 22 Jul 2013 23:51:08 +0200
> 
> > On Mon, Jul 22, 2013 at 05:18:55PM -0400, Benjamin LaHaise wrote:
> >> On Mon, Jul 22, 2013 at 09:43:57PM +0100, William Manley wrote:
> >> > If an IGMP join packet is lost you will not receive data sent to the 
> >> > multicast group so if no data arrives from that multicast group in a 
> >> > period of time after the IGMP join a second IGMP join will be sent.  The 
> >> > delay between joins is the "IGMP Unsolicited Report Interval".
> >> > 
> >> > In the kernel this seems to be hard coded to be chosen randomly between 
> >> > 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
> >> > change to be slow in the presence of packet loss.
> >> > 
> >> > I would guess that this 10s has come from IGMPv2 RFC2236, which was 
> >> > reduced to 1s in IGMPv3 RFC3376.
> >> 
> >> Reducing the timeout does not solve the problem you are encountering, as 
> >> any packet loss will still result in a 1 second delay.  I've encountered 
> >> similar issues dealing with LCP Echo request/replies for keepalive 
> >> messages on PPP sessions.  The correct approach is to queue the IGMP 
> >> multicast join with a higher priority than other traffic in the system 
> >> so that the requests are not lost due to congestion of a single queue.  
> >> Sending packets with an 802.1p header might be appropriate in your 
> >> use-case, or perhaps using higher priority internal queues.
> > 
> > Yes, we can do a little bit better. What do you think?
> > 
> > [PATCH net-next] ipv6: send igmpv3/mld packets with TC_PRIO_CONTROL
> > 
> > Reported-by: William Manley <william.manley@youview.com>
> > Suggested-by: Benjamin LaHaise <bcrl@kvack.org>
> > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> 
> Ben, please give Hannes the feedback he is asking for.
> 
> Thanks.

I think Hannes' patch is good step in the right direction, so please add:
Acked-by: Benjamin LaHaise <bcrl@kvack.org>

		-ben
-- 
"Thought is the essence of where you are now."

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Hannes Frederic Sowa]

On Fri, Jul 26, 2013 at 09:11:23AM -0400, Benjamin LaHaise wrote:
> On Thu, Jul 25, 2013 at 04:42:53PM -0700, David Miller wrote:
> > From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Date: Mon, 22 Jul 2013 23:51:08 +0200
> > 
> > > On Mon, Jul 22, 2013 at 05:18:55PM -0400, Benjamin LaHaise wrote:
> > >> On Mon, Jul 22, 2013 at 09:43:57PM +0100, William Manley wrote:
> > >> > If an IGMP join packet is lost you will not receive data sent to the 
> > >> > multicast group so if no data arrives from that multicast group in a 
> > >> > period of time after the IGMP join a second IGMP join will be sent.  The 
> > >> > delay between joins is the "IGMP Unsolicited Report Interval".
> > >> > 
> > >> > In the kernel this seems to be hard coded to be chosen randomly between 
> > >> > 0-10s.  In our use-case (IPTV) this is too long as it can cause channel 
> > >> > change to be slow in the presence of packet loss.
> > >> > 
> > >> > I would guess that this 10s has come from IGMPv2 RFC2236, which was 
> > >> > reduced to 1s in IGMPv3 RFC3376.
> > >> 
> > >> Reducing the timeout does not solve the problem you are encountering, as 
> > >> any packet loss will still result in a 1 second delay.  I've encountered 
> > >> similar issues dealing with LCP Echo request/replies for keepalive 
> > >> messages on PPP sessions.  The correct approach is to queue the IGMP 
> > >> multicast join with a higher priority than other traffic in the system 
> > >> so that the requests are not lost due to congestion of a single queue.  
> > >> Sending packets with an 802.1p header might be appropriate in your 
> > >> use-case, or perhaps using higher priority internal queues.
> > > 
> > > Yes, we can do a little bit better. What do you think?
> > > 
> > > [PATCH net-next] ipv6: send igmpv3/mld packets with TC_PRIO_CONTROL
> > > 
> > > Reported-by: William Manley <william.manley@youview.com>
> > > Suggested-by: Benjamin LaHaise <bcrl@kvack.org>
> > > Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > 
> > Ben, please give Hannes the feedback he is asking for.
> > 
> > Thanks.
> 
> I think Hannes' patch is good step in the right direction, so please add:
> Acked-by: Benjamin LaHaise <bcrl@kvack.org>

I just send a new patch with the priority changes for ipv4 included. I copied
your Acked-by, I hope this is ok.

Thanks,

  Hannes

Re: IGMP Unsolicited Report Interval too long for IGMPv3?

[Benjamin LaHaise]

On Fri, Jul 26, 2013 at 05:06:51PM +0200, Hannes Frederic Sowa wrote:
> On Fri, Jul 26, 2013 at 09:11:23AM -0400, Benjamin LaHaise wrote:
> > I think Hannes' patch is good step in the right direction, so please add:
> > Acked-by: Benjamin LaHaise <bcrl@kvack.org>
> 
> I just send a new patch with the priority changes for ipv4 included. I copied
> your Acked-by, I hope this is ok.

Sure, no problem.  Thanks for fixing this.

		-ben
-- 
"Thought is the essence of where you are now."

Re: [PATCH 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Hannes Frederic Sowa]

On Thu, Jul 25, 2013 at 01:14:04PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
> 
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> 
> Signed-off-by: William Manley <william.manley@youview.com>
> ---
>  net/ipv4/igmp.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index d8c2327..12ffc2b 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -113,7 +113,8 @@
>  
>  #define IGMP_V1_Router_Present_Timeout		(400*HZ)
>  #define IGMP_V2_Router_Present_Timeout		(400*HZ)
> -#define IGMP_Unsolicited_Report_Interval	(10*HZ)
> +#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
> +#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
>  #define IGMP_Query_Response_Interval		(10*HZ)
>  #define IGMP_Unsolicited_Report_Count		2
>  
> @@ -138,6 +139,14 @@
>  	 ((in_dev)->mr_v2_seen && \
>  	  time_before(jiffies, (in_dev)->mr_v2_seen)))
>  
> +static int unsolicited_report_interval(struct in_device *in_dev)
> +{
> +	if (IGMP_V2_SEEN(in_dev))
> +		return IGMP_V2_Unsolicited_Report_Interval;
> +	else /* v3 */
> +		return IGMP_V3_Unsolicited_Report_Interval;
> +}

if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))

Otherwise I am fine with it.

Thanks,

  Hannes

Re: [PATCH 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Hannes Frederic Sowa]

On Thu, Jul 25, 2013 at 01:14:05PM +0100, William Manley wrote:
> Adds a new procfs knob:
> 
>     /proc/sys/net/ipv4/conf/*/force_igmp_unsolicited_report_interval
> 
> Which will allow userspace configuration of the IGMP unsolicited report
> interval (see below) in milliseconds.  The default is -1 which means use
> default behaviour (10s for IGMPv3 and 1s for IGMPv2 in accordance with
> RFC2236 and RFC3376).

Hm, would it be easy to split the timers per protocol? It seems a bit
hackish to me.

Thanks,

  Hannes

Re: [PATCH 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Hannes Frederic Sowa]

On Fri, Jul 26, 2013 at 06:32:39PM +0200, Hannes Frederic Sowa wrote:
> On Thu, Jul 25, 2013 at 01:14:04PM +0100, William Manley wrote:
> > If an IGMP join packet is lost you will not receive data sent to the
> > multicast group so if no data arrives from that multicast group in a
> > period of time after the IGMP join a second IGMP join will be sent.  The
> > delay between joins is the "IGMP Unsolicited Report Interval".
> > 
> > Previously this value was hard coded to be chosen randomly between 0-10s.
> > This can be too long for some use-cases, such as IPTV as it can cause
> > channel change to be slow in the presence of packet loss.
> > 
> > The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> > IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> > later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> > unaffected.
> > 
> > Tested with Wireshark and a simple program to join a (non-existent)
> > multicast group.  The distribution of timings for the second join differ
> > based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> >
> > [...]
> >
>
> [...]
> 
> Otherwise I am fine with it.

Also, could you have a look at IPv6, too? We currently use a hardcoded
IGMP6_UNSOLICITED_IVAL = 10*HZ there.

Thanks,

  Hannes

[PATCH v3 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Previously this value was hard coded to be chosen randomly between 0-10s.
This can be too long for some use-cases, such as IPTV as it can cause
channel change to be slow in the presence of packet loss.

The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
unaffected.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.

Signed-off-by: William Manley <william.manley@youview.com>
---
 net/ipv4/igmp.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d8c2327..9f0aaea 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -113,7 +113,8 @@
 
 #define IGMP_V1_Router_Present_Timeout		(400*HZ)
 #define IGMP_V2_Router_Present_Timeout		(400*HZ)
-#define IGMP_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
 #define IGMP_Query_Response_Interval		(10*HZ)
 #define IGMP_Unsolicited_Report_Count		2
 
@@ -138,6 +139,14 @@
 	 ((in_dev)->mr_v2_seen && \
 	  time_before(jiffies, (in_dev)->mr_v2_seen)))
 
+static int unsolicited_report_interval(struct in_device *in_dev)
+{
+	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
+		return IGMP_V2_Unsolicited_Report_Interval;
+	else /* v3 */
+		return IGMP_V3_Unsolicited_Report_Interval;
+}
+
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
 static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
 static void igmpv3_clear_delrec(struct in_device *in_dev);
@@ -719,7 +728,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
 	igmpv3_send_cr(in_dev);
 	if (in_dev->mr_ifc_count) {
 		in_dev->mr_ifc_count--;
-		igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
+		igmp_ifc_start_timer(in_dev,
+				     unsolicited_report_interval(in_dev));
 	}
 	__in_dev_put(in_dev);
 }
@@ -744,7 +754,7 @@ static void igmp_timer_expire(unsigned long data)
 
 	if (im->unsolicit_count) {
 		im->unsolicit_count--;
-		igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
+		igmp_start_timer(im, unsolicited_report_interval(in_dev));
 	}
 	im->reporter = 1;
 	spin_unlock(&im->lock);
-- 
1.7.10.4

[PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[William Manley]

Adds the new procfs knobs:

    /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
    /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval

Which will allow userspace configuration of the IGMP unsolicited report
interval (see below) in milliseconds.  The default 1000ms for IGMPv2 and
10000 for IGMPv3 in accordance with RFC2236 and RFC3376.

Background:

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Prior to this patch this value was hard coded in the kernel to 10s for
IGMPv2 and 1s for IGMPv3.  10s is unsuitable for some use-cases, such as
IPTV as it can cause channel change to be slow in the presence of packet
loss.

This patch allows the value to be overridden from userspace changed for
both IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting
/proc/sys/net/ipv4/conf/eth0/force_igmp_unsolicited_report_interval.

igmpvX_unsolicited_report_interval is intended to follow the pattern
established by force_igmp_version, and while a procfs entry has been added
a corresponding sysctl knob has not as it is my understanding that sysctl
is deprecated[1].

[1]: http://lwn.net/Articles/247243/

Signed-off-by: William Manley <william.manley@youview.com>
---
 include/linux/inetdevice.h |    4 ++++
 net/ipv4/devinet.c         |    8 ++++++++
 net/ipv4/igmp.c            |   19 +++++++++++++++++--
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ea1e3b8..d100e4c 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -30,6 +30,8 @@ enum
 	IPV4_DEVCONF_NOXFRM,
 	IPV4_DEVCONF_NOPOLICY,
 	IPV4_DEVCONF_FORCE_IGMP_VERSION,
+	IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
+	IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
 	IPV4_DEVCONF_ARP_ANNOUNCE,
 	IPV4_DEVCONF_ARP_IGNORE,
 	IPV4_DEVCONF_PROMOTE_SECONDARIES,
@@ -62,6 +64,8 @@ struct in_device {
 	unsigned long		mr_v1_seen;
 	unsigned long		mr_v2_seen;
 	unsigned long		mr_maxdelay;
+	unsigned long		mr_v2_unsolicited_report_interval;
+	unsigned long		mr_v3_unsolicited_report_interval;
 	unsigned char		mr_qrv;
 	unsigned char		mr_gq_running;
 	unsigned char		mr_ifc_count;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index dfc39d4..f681518 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -73,6 +73,8 @@ static struct ipv4_devconf ipv4_devconf = {
 		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
+		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
+		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
 	},
 };
 
@@ -83,6 +85,8 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
 		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
+		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
+		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
 	},
 };
 
@@ -2099,6 +2103,10 @@ static struct devinet_sysctl_table {
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
 					      "force_igmp_version"),
+		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
+					      "igmpv2_unsolicited_report_interval"),
+		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
+					      "igmpv3_unsolicited_report_interval"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
 					      "promote_secondaries"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 9f0aaea..035ee77 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -141,10 +141,25 @@
 
 static int unsolicited_report_interval(struct in_device *in_dev)
 {
+	int interval_ms, interval_jiffies;
+
 	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
-		return IGMP_V2_Unsolicited_Report_Interval;
+		interval_ms = IPV4_DEVCONF_ALL(
+			dev_net(in_dev->dev),
+			IGMPV2_UNSOLICITED_REPORT_INTERVAL);
 	else /* v3 */
-		return IGMP_V3_Unsolicited_Report_Interval;
+		interval_ms = IPV4_DEVCONF_ALL(
+			dev_net(in_dev->dev),
+			IGMPV3_UNSOLICITED_REPORT_INTERVAL);
+
+	interval_jiffies = msecs_to_jiffies(interval_ms);
+
+	/* _timer functions can't handle a delay of 0 jiffies so ensure
+	 *  we always return a positive value.
+	 */
+	if (interval_jiffies <= 0)
+		interval_jiffies = 1;
+	return interval_jiffies;
 }
 
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
-- 
1.7.10.4

Re: [PATCH 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

On 26/07/13 17:39, Hannes Frederic Sowa wrote:
> On Fri, Jul 26, 2013 at 06:32:39PM +0200, Hannes Frederic Sowa wrote:
>> On Thu, Jul 25, 2013 at 01:14:04PM +0100, William Manley wrote:
>>> If an IGMP join packet is lost you will not receive data sent to the
>>> multicast group so if no data arrives from that multicast group in a
>>> period of time after the IGMP join a second IGMP join will be sent.  The
>>> delay between joins is the "IGMP Unsolicited Report Interval".
>>>
>>> Previously this value was hard coded to be chosen randomly between 0-10s.
>>> This can be too long for some use-cases, such as IPTV as it can cause
>>> channel change to be slow in the presence of packet loss.
>>>
>>> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
>>> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
>>> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
>>> unaffected.
>>>
>>> Tested with Wireshark and a simple program to join a (non-existent)
>>> multicast group.  The distribution of timings for the second join differ
>>> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
>>>
>>> [...]
>>>
>>
>> [...]
>>
>> Otherwise I am fine with it.
>
> Also, could you have a look at IPv6, too? We currently use a hardcoded
> IGMP6_UNSOLICITED_IVAL = 10*HZ there.

I'll have a look, but I have to admit my ignorance on how multicast 
works with IPv6 and I may have difficulty setting up a test environment.

It seems that IGMP has been replaced with "Multicast Listener Discovery" 
on top of ICMPv6 and much like IGMP there is more than one version:

1. RFC2710 - MLD - Unsolicited Report Interval = 10s
2. RFC3810 - MLDv2 - Unsolicited Report Interval = 1s

There also seems to be a /proc/sys/net/ipv6/conf/all/force_mld_version. 
  Ok, so maybe it's not so difficult.  I'll see if I can come up with 
some additional patches but I'd prefer to keep them separate from the 
IPv4 ones which I hope are now good-to-go.

Thanks

Will

Re: [PATCH 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Hannes Frederic Sowa]

On Mon, Jul 29, 2013 at 03:39:04PM +0100, William Manley wrote:
> I'll have a look, but I have to admit my ignorance on how multicast 
> works with IPv6 and I may have difficulty setting up a test environment.
> 
> It seems that IGMP has been replaced with "Multicast Listener Discovery" 
> on top of ICMPv6 and much like IGMP there is more than one version:
> 
> 1. RFC2710 - MLD - Unsolicited Report Interval = 10s
> 2. RFC3810 - MLDv2 - Unsolicited Report Interval = 1s

Correct.

> There also seems to be a /proc/sys/net/ipv6/conf/all/force_mld_version. 
>  Ok, so maybe it's not so difficult.  I'll see if I can come up with 
> some additional patches but I'd prefer to keep them separate from the 
> IPv4 ones which I hope are now good-to-go.

Of course, separate patches for ipv6 are favorable. The code is actually
pretty similar to IPv4. Let me know if you need help.

I'll review your IPv4 patches now.

Thanks,

  Hannes

Re: [PATCH v3 1/2] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Hannes Frederic Sowa]

On Mon, Jul 29, 2013 at 03:21:50PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
> 
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> 
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks,

  Hannes

Re: [PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Hannes Frederic Sowa]

On Mon, Jul 29, 2013 at 03:21:51PM +0100, William Manley wrote:
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -30,6 +30,8 @@ enum
>  	IPV4_DEVCONF_NOXFRM,
>  	IPV4_DEVCONF_NOPOLICY,
>  	IPV4_DEVCONF_FORCE_IGMP_VERSION,
> +	IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> +	IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
>  	IPV4_DEVCONF_ARP_ANNOUNCE,
>  	IPV4_DEVCONF_ARP_IGNORE,
>  	IPV4_DEVCONF_PROMOTE_SECONDARIES,
> @@ -62,6 +64,8 @@ struct in_device {
>  	unsigned long		mr_v1_seen;
>  	unsigned long		mr_v2_seen;
>  	unsigned long		mr_maxdelay;
> +	unsigned long		mr_v2_unsolicited_report_interval;
> +	unsigned long		mr_v3_unsolicited_report_interval;
>  	unsigned char		mr_qrv;
>  	unsigned char		mr_gq_running;
>  	unsigned char		mr_ifc_count;
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index dfc39d4..f681518 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -73,6 +73,8 @@ static struct ipv4_devconf ipv4_devconf = {
>  		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
> +		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> +		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
>  	},
>  };
>  
> @@ -83,6 +85,8 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
>  		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
>  		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
> +		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> +		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
>  	},
>  };
>  
> @@ -2099,6 +2103,10 @@ static struct devinet_sysctl_table {
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
>  					      "force_igmp_version"),
> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> +					      "igmpv2_unsolicited_report_interval"),
> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
> +					      "igmpv3_unsolicited_report_interval"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
>  					      "promote_secondaries"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,

Why did you use DEVINET_SYSCTL_FLUSHING_ENTRY here? Wouldn't
DEVINET_SYSCTL_RW_ENTRY be a better choice?

> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 9f0aaea..035ee77 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -141,10 +141,25 @@
>  
>  static int unsolicited_report_interval(struct in_device *in_dev)
>  {
> +	int interval_ms, interval_jiffies;
> +
>  	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> -		return IGMP_V2_Unsolicited_Report_Interval;
> +		interval_ms = IPV4_DEVCONF_ALL(
> +			dev_net(in_dev->dev),
> +			IGMPV2_UNSOLICITED_REPORT_INTERVAL);

IN_DEV_CONF_GET()?

Or should the igmpv{2,3}_unsolicited_report_interval only be a global setting?

>  	else /* v3 */
> -		return IGMP_V3_Unsolicited_Report_Interval;
> +		interval_ms = IPV4_DEVCONF_ALL(
> +			dev_net(in_dev->dev),
> +			IGMPV3_UNSOLICITED_REPORT_INTERVAL);

Here as well.

> +
> +	interval_jiffies = msecs_to_jiffies(interval_ms);
> +
> +	/* _timer functions can't handle a delay of 0 jiffies so ensure
> +	 *  we always return a positive value.
> +	 */

I am not sure here, but have seen mod_timer with 0 work before. But it is a
good idea anyway.

> +	if (interval_jiffies <= 0)
> +		interval_jiffies = 1;
> +	return interval_jiffies;
>  }

Re: [PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[David Miller]

From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Tue, 30 Jul 2013 08:14:26 +0200

> On Mon, Jul 29, 2013 at 03:21:51PM +0100, William Manley wrote:
>> @@ -2099,6 +2103,10 @@ static struct devinet_sysctl_table {
>>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
>>  		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
>>  					      "force_igmp_version"),
>> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
>> +					      "igmpv2_unsolicited_report_interval"),
>> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
>> +					      "igmpv3_unsolicited_report_interval"),
>>  		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
>>  					      "promote_secondaries"),
>>  		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
> 
> Why did you use DEVINET_SYSCTL_FLUSHING_ENTRY here? Wouldn't
> DEVINET_SYSCTL_RW_ENTRY be a better choice?

Agreed, there is no reason to flush the routing cache just because
the igmp unsolicited report interval changed.

Re: [PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Bill Fink]

On Mon, 29 Jul 2013, William Manley wrote:

> Adds the new procfs knobs:
> 
>     /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
>     /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval
> 
> Which will allow userspace configuration of the IGMP unsolicited report
> interval (see below) in milliseconds.  The default 1000ms for IGMPv2 and
> 10000 for IGMPv3 in accordance with RFC2236 and RFC3376.

You have the 1000/10000 ms for IGMPv2/IGMPv3 flipped in your
commit message above.

					-Bill

Re: [PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Hannes Frederic Sowa]

On Tue, Jul 30, 2013 at 04:55:57PM -0700, David Miller wrote:
> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Tue, 30 Jul 2013 08:14:26 +0200
> 
> > On Mon, Jul 29, 2013 at 03:21:51PM +0100, William Manley wrote:
> >> @@ -2099,6 +2103,10 @@ static struct devinet_sysctl_table {
> >>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
> >>  		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
> >>  					      "force_igmp_version"),
> >> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> >> +					      "igmpv2_unsolicited_report_interval"),
> >> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
> >> +					      "igmpv3_unsolicited_report_interval"),
> >>  		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
> >>  					      "promote_secondaries"),
> >>  		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
> > 
> > Why did you use DEVINET_SYSCTL_FLUSHING_ENTRY here? Wouldn't
> > DEVINET_SYSCTL_RW_ENTRY be a better choice?
> 
> Agreed, there is no reason to flush the routing cache just because
> the igmp unsolicited report interval changed.

William, could you convert force_igmp_version to DEVINET_SYSCTL_RW_ENTRY as
well when you send a new patch?

Thanks,

  Hannes

Re: [PATCH v3 2/2] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[William Manley]

On 31/07/13 07:34, Hannes Frederic Sowa wrote:
> On Tue, Jul 30, 2013 at 04:55:57PM -0700, David Miller wrote:
>> From: Hannes Frederic Sowa<hannes@stressinduktion.org>
>> Date: Tue, 30 Jul 2013 08:14:26 +0200
>>
>>> On Mon, Jul 29, 2013 at 03:21:51PM +0100, William Manley wrote:
>>>> @@ -2099,6 +2103,10 @@ static struct devinet_sysctl_table {
>>>>   		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
>>>>   		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
>>>>   					      "force_igmp_version"),
>>>> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
>>>> +					      "igmpv2_unsolicited_report_interval"),
>>>> +		DEVINET_SYSCTL_FLUSHING_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
>>>> +					      "igmpv3_unsolicited_report_interval"),
>>>>   		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
>>>>   					      "promote_secondaries"),
>>>>   		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
>>>
>>> Why did you use DEVINET_SYSCTL_FLUSHING_ENTRY here? Wouldn't
>>> DEVINET_SYSCTL_RW_ENTRY be a better choice?
>>
>> Agreed, there is no reason to flush the routing cache just because
>> the igmp unsolicited report interval changed.
>
> William, could you convert force_igmp_version to DEVINET_SYSCTL_RW_ENTRY as
> well when you send a new patch?

Will do.  I have to confess I only made it flushing because I was 
copying force_igmp_version without understanding.

Will take a couple of days as I have a bunch of other stuff on ATM.

Thanks

Will

IGMP Unsolicited report interval patches

[William Manley]

4th version of the patches.

The significant changes since last review are: 

1. there is a new patch (2/3) as requested by Hannes.
2. the third patch now uses IN_DEV_CONF_GET in place of
   IPV4_DEVCONF_ALL.  This means that the unsolicited report interval can
   now be configured on an interface-by-interface basis as I'd originally
   intended but messed up in the implementation.  One concern I have now
   is that with this latest patch-set is that while
   /proc/sys/net/ipv4/conf/eth0/igmp... will now have an effect 
   /proc/sys/net/ipv4/conf/all/igmp... will not.  I'm not sure how to
   resolve this.

   One option would be to have a special value of -1 to mean use the
   default so I could implement fall-back semantics.  A down-side of this
   approach is that it makes the meaning of the knobs less clear for
   someone browsing through the filesystem.  Another option would be to
   remove the knob from all/ entirely, although I'm not sure how to do
   this.  Suggestions are very much welcome :)

Thanks 

Will

[PATCH v4 1/3] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[William Manley]

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Previously this value was hard coded to be chosen randomly between 0-10s.
This can be too long for some use-cases, such as IPTV as it can cause
channel change to be slow in the presence of packet loss.

The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
unaffected.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.

Signed-off-by: William Manley <william.manley@youview.com>
---
 net/ipv4/igmp.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d8c2327..9f0aaea 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -113,7 +113,8 @@
 
 #define IGMP_V1_Router_Present_Timeout		(400*HZ)
 #define IGMP_V2_Router_Present_Timeout		(400*HZ)
-#define IGMP_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
+#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
 #define IGMP_Query_Response_Interval		(10*HZ)
 #define IGMP_Unsolicited_Report_Count		2
 
@@ -138,6 +139,14 @@
 	 ((in_dev)->mr_v2_seen && \
 	  time_before(jiffies, (in_dev)->mr_v2_seen)))
 
+static int unsolicited_report_interval(struct in_device *in_dev)
+{
+	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
+		return IGMP_V2_Unsolicited_Report_Interval;
+	else /* v3 */
+		return IGMP_V3_Unsolicited_Report_Interval;
+}
+
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
 static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
 static void igmpv3_clear_delrec(struct in_device *in_dev);
@@ -719,7 +728,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
 	igmpv3_send_cr(in_dev);
 	if (in_dev->mr_ifc_count) {
 		in_dev->mr_ifc_count--;
-		igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
+		igmp_ifc_start_timer(in_dev,
+				     unsolicited_report_interval(in_dev));
 	}
 	__in_dev_put(in_dev);
 }
@@ -744,7 +754,7 @@ static void igmp_timer_expire(unsigned long data)
 
 	if (im->unsolicit_count) {
 		im->unsolicit_count--;
-		igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
+		igmp_start_timer(im, unsolicited_report_interval(in_dev));
 	}
 	im->reporter = 1;
 	spin_unlock(&im->lock);
-- 
1.7.10.4

[PATCH v4 2/3] net: igmp: Don't flush routing cache when force_igmp_version is modified

[William Manley]

The procfs knob /proc/sys/net/ipv4/conf/*/force_igmp_version allows the
IGMP protocol version to use to be explicitly set.  As a side effect this
caused the routing cache to be flushed as it was declared as a
DEVINET_SYSCTL_FLUSHING_ENTRY.  Flushing is unnecessary and this patch
makes it so flushing does not occur.

Requested by Hannes Frederic Sowa as he was reviewing other patches
adding procfs entries.

Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: William Manley <william.manley@youview.com>
---
 include/linux/inetdevice.h |    2 +-
 net/ipv4/devinet.c         |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index ea1e3b8..d4c56fc 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -27,9 +27,9 @@ enum
 	IPV4_DEVCONF_TAG,
 	IPV4_DEVCONF_ARPFILTER,
 	IPV4_DEVCONF_MEDIUM_ID,
+	IPV4_DEVCONF_FORCE_IGMP_VERSION,
 	IPV4_DEVCONF_NOXFRM,
 	IPV4_DEVCONF_NOPOLICY,
-	IPV4_DEVCONF_FORCE_IGMP_VERSION,
 	IPV4_DEVCONF_ARP_ANNOUNCE,
 	IPV4_DEVCONF_ARP_IGNORE,
 	IPV4_DEVCONF_PROMOTE_SECONDARIES,
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index dfc39d4..a9561c4 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -2094,11 +2094,11 @@ static struct devinet_sysctl_table {
 		DEVINET_SYSCTL_RW_ENTRY(ARP_ACCEPT, "arp_accept"),
 		DEVINET_SYSCTL_RW_ENTRY(ARP_NOTIFY, "arp_notify"),
 		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
+		DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
+					"force_igmp_version"),
 
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
-		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
-					      "force_igmp_version"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
 					      "promote_secondaries"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
-- 
1.7.10.4

[PATCH v4 3/3] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[William Manley]

Adds the new procfs knobs:

    /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
    /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval

Which will allow userspace configuration of the IGMP unsolicited report
interval (see below) in milliseconds.  The defaults are 10000ms for IGMPv2
and 1000ms for IGMPv3 in accordance with RFC2236 and RFC3376.

Background:

If an IGMP join packet is lost you will not receive data sent to the
multicast group so if no data arrives from that multicast group in a
period of time after the IGMP join a second IGMP join will be sent.  The
delay between joins is the "IGMP Unsolicited Report Interval".

Prior to this patch this value was hard coded in the kernel to 10s for
IGMPv2 and 1s for IGMPv3.  10s is unsuitable for some use-cases, such as
IPTV as it can cause channel change to be slow in the presence of packet
loss.

This patch allows the value to be overridden from userspace for both
IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.

Tested with Wireshark and a simple program to join a (non-existent)
multicast group.  The distribution of timings for the second join differ
based upon setting the procfs knobs.

igmpvX_unsolicited_report_interval is intended to follow the pattern
established by force_igmp_version, and while a procfs entry has been added
a corresponding sysctl knob has not as it is my understanding that sysctl
is deprecated[1].

[1]: http://lwn.net/Articles/247243/

Signed-off-by: William Manley <william.manley@youview.com>
---
 include/linux/inetdevice.h |    2 ++
 net/ipv4/devinet.c         |    8 ++++++++
 net/ipv4/igmp.c            |   19 +++++++++++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index d4c56fc..43b3c72 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -28,6 +28,8 @@ enum
 	IPV4_DEVCONF_ARPFILTER,
 	IPV4_DEVCONF_MEDIUM_ID,
 	IPV4_DEVCONF_FORCE_IGMP_VERSION,
+	IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
+	IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
 	IPV4_DEVCONF_NOXFRM,
 	IPV4_DEVCONF_NOPOLICY,
 	IPV4_DEVCONF_ARP_ANNOUNCE,
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a9561c4..56ecb1b 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -73,6 +73,8 @@ static struct ipv4_devconf ipv4_devconf = {
 		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
+		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
+		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
 	},
 };
 
@@ -83,6 +85,8 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
 		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
 		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
 		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
+		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
+		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
 	},
 };
 
@@ -2096,6 +2100,10 @@ static struct devinet_sysctl_table {
 		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
 		DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
 					"force_igmp_version"),
+		DEVINET_SYSCTL_RW_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
+					"igmpv2_unsolicited_report_interval"),
+		DEVINET_SYSCTL_RW_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
+					"igmpv3_unsolicited_report_interval"),
 
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
 		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 9f0aaea..c5541da 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -141,10 +141,25 @@
 
 static int unsolicited_report_interval(struct in_device *in_dev)
 {
+	int interval_ms, interval_jiffies;
+
 	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
-		return IGMP_V2_Unsolicited_Report_Interval;
+		interval_ms = IN_DEV_CONF_GET(
+			in_dev,
+			IGMPV2_UNSOLICITED_REPORT_INTERVAL);
 	else /* v3 */
-		return IGMP_V3_Unsolicited_Report_Interval;
+		interval_ms = IN_DEV_CONF_GET(
+			in_dev,
+			IGMPV3_UNSOLICITED_REPORT_INTERVAL);
+
+	interval_jiffies = msecs_to_jiffies(interval_ms);
+
+	/* _timer functions can't handle a delay of 0 jiffies so ensure
+	 *  we always return a positive value.
+	 */
+	if (interval_jiffies <= 0)
+		interval_jiffies = 1;
+	return interval_jiffies;
 }
 
 static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
-- 
1.7.10.4

Re: [PATCH v4 1/3] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Hannes Frederic Sowa]

On Tue, Aug 06, 2013 at 07:03:13PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
> 
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> 
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Re: [PATCH v4 2/3] net: igmp: Don't flush routing cache when force_igmp_version is modified

[Hannes Frederic Sowa]

On Tue, Aug 06, 2013 at 07:03:14PM +0100, William Manley wrote:
> The procfs knob /proc/sys/net/ipv4/conf/*/force_igmp_version allows the
> IGMP protocol version to use to be explicitly set.  As a side effect this
> caused the routing cache to be flushed as it was declared as a
> DEVINET_SYSCTL_FLUSHING_ENTRY.  Flushing is unnecessary and this patch
> makes it so flushing does not occur.
> 
> Requested by Hannes Frederic Sowa as he was reviewing other patches
> adding procfs entries.
> 
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Re: [PATCH v4 3/3] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Hannes Frederic Sowa]

On Tue, Aug 06, 2013 at 07:03:15PM +0100, William Manley wrote:
> Adds the new procfs knobs:
> 
>     /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
>     /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval
> 
> Which will allow userspace configuration of the IGMP unsolicited report
> interval (see below) in milliseconds.  The defaults are 10000ms for IGMPv2
> and 1000ms for IGMPv3 in accordance with RFC2236 and RFC3376.
> 
> Background:
> 
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Prior to this patch this value was hard coded in the kernel to 10s for
> IGMPv2 and 1s for IGMPv3.  10s is unsuitable for some use-cases, such as
> IPTV as it can cause channel change to be slow in the presence of packet
> loss.
> 
> This patch allows the value to be overridden from userspace for both
> IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting the procfs knobs.
> 
> igmpvX_unsolicited_report_interval is intended to follow the pattern
> established by force_igmp_version, and while a procfs entry has been added
> a corresponding sysctl knob has not as it is my understanding that sysctl
> is deprecated[1].
> 
> [1]: http://lwn.net/Articles/247243/
> 
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Re: IGMP Unsolicited report interval patches

[Hannes Frederic Sowa]

On Tue, Aug 06, 2013 at 07:03:12PM +0100, William Manley wrote:
> 4th version of the patches.
> 
> The significant changes since last review are: 
> 
> 1. there is a new patch (2/3) as requested by Hannes.

Thanks!

> 2. the third patch now uses IN_DEV_CONF_GET in place of
>    IPV4_DEVCONF_ALL.  This means that the unsolicited report interval can
>    now be configured on an interface-by-interface basis as I'd originally
>    intended but messed up in the implementation.  One concern I have now
>    is that with this latest patch-set is that while
>    /proc/sys/net/ipv4/conf/eth0/igmp... will now have an effect 
>    /proc/sys/net/ipv4/conf/all/igmp... will not.  I'm not sure how to
>    resolve this.

Hm, it seems to be come more difficult dealing with ranges.

One way would be, to check the state bit for the devinet entry and chose the
all value always but when the state bit for the interface for this entry is
set. I'll have a look on how to do this.


>    One option would be to have a special value of -1 to mean use the
>    default so I could implement fall-back semantics.  A down-side of this
>    approach is that it makes the meaning of the knobs less clear for
>    someone browsing through the filesystem.  Another option would be to
>    remove the knob from all/ entirely, although I'm not sure how to do
>    this.  Suggestions are very much welcome :)

This seems to be confusing, at least or me. ;)

Thanks for the series! The patches are find IMHO.

Do you plan to make the corresponding changes for ipv6 or should I put that on
my todo list?

Thanks,

  Hannes

Re: [PATCH v4 2/3] net: igmp: Don't flush routing cache when force_igmp_version is modified

[Benjamin LaHaise]

On Tue, Aug 06, 2013 at 07:03:14PM +0100, William Manley wrote:
> The procfs knob /proc/sys/net/ipv4/conf/*/force_igmp_version allows the
> IGMP protocol version to use to be explicitly set.  As a side effect this
> caused the routing cache to be flushed as it was declared as a
> DEVINET_SYSCTL_FLUSHING_ENTRY.  Flushing is unnecessary and this patch
> makes it so flushing does not occur.
> 
> Requested by Hannes Frederic Sowa as he was reviewing other patches
> adding procfs entries.
> 
> Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Benjamin LaHaise <bcrl@kvack.org>

Looks good.

> ---
>  include/linux/inetdevice.h |    2 +-
>  net/ipv4/devinet.c         |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index ea1e3b8..d4c56fc 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -27,9 +27,9 @@ enum
>  	IPV4_DEVCONF_TAG,
>  	IPV4_DEVCONF_ARPFILTER,
>  	IPV4_DEVCONF_MEDIUM_ID,
> +	IPV4_DEVCONF_FORCE_IGMP_VERSION,
>  	IPV4_DEVCONF_NOXFRM,
>  	IPV4_DEVCONF_NOPOLICY,
> -	IPV4_DEVCONF_FORCE_IGMP_VERSION,
>  	IPV4_DEVCONF_ARP_ANNOUNCE,
>  	IPV4_DEVCONF_ARP_IGNORE,
>  	IPV4_DEVCONF_PROMOTE_SECONDARIES,
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index dfc39d4..a9561c4 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -2094,11 +2094,11 @@ static struct devinet_sysctl_table {
>  		DEVINET_SYSCTL_RW_ENTRY(ARP_ACCEPT, "arp_accept"),
>  		DEVINET_SYSCTL_RW_ENTRY(ARP_NOTIFY, "arp_notify"),
>  		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
> +		DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
> +					"force_igmp_version"),
>  
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
> -		DEVINET_SYSCTL_FLUSHING_ENTRY(FORCE_IGMP_VERSION,
> -					      "force_igmp_version"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(PROMOTE_SECONDARIES,
>  					      "promote_secondaries"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(ROUTE_LOCALNET,
> -- 
> 1.7.10.4

-- 
"Thought is the essence of where you are now."

Re: [PATCH v4 1/3] net: igmp: Reduce Unsolicited report interval to 1s when using IGMPv3

[Benjamin LaHaise]

On Tue, Aug 06, 2013 at 07:03:13PM +0100, William Manley wrote:
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Previously this value was hard coded to be chosen randomly between 0-10s.
> This can be too long for some use-cases, such as IPTV as it can cause
> channel change to be slow in the presence of packet loss.
> 
> The value 10s has come from IGMPv2 RFC2236, which was reduced to 1s in
> IGMPv3 RFC3376.  This patch makes the kernel use the 1s value from the
> later RFC if we are operating in IGMPv3 mode.  IGMPv2 behaviour is
> unaffected.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting /proc/sys/net/ipv4/conf/eth0/force_igmp_version.
> 
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Benjamin LaHaise <bcrl@kvack.org>

		-ben

> ---
>  net/ipv4/igmp.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index d8c2327..9f0aaea 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -113,7 +113,8 @@
>  
>  #define IGMP_V1_Router_Present_Timeout		(400*HZ)
>  #define IGMP_V2_Router_Present_Timeout		(400*HZ)
> -#define IGMP_Unsolicited_Report_Interval	(10*HZ)
> +#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
> +#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
>  #define IGMP_Query_Response_Interval		(10*HZ)
>  #define IGMP_Unsolicited_Report_Count		2
>  
> @@ -138,6 +139,14 @@
>  	 ((in_dev)->mr_v2_seen && \
>  	  time_before(jiffies, (in_dev)->mr_v2_seen)))
>  
> +static int unsolicited_report_interval(struct in_device *in_dev)
> +{
> +	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> +		return IGMP_V2_Unsolicited_Report_Interval;
> +	else /* v3 */
> +		return IGMP_V3_Unsolicited_Report_Interval;
> +}
> +
>  static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
>  static void igmpv3_del_delrec(struct in_device *in_dev, __be32 multiaddr);
>  static void igmpv3_clear_delrec(struct in_device *in_dev);
> @@ -719,7 +728,8 @@ static void igmp_ifc_timer_expire(unsigned long data)
>  	igmpv3_send_cr(in_dev);
>  	if (in_dev->mr_ifc_count) {
>  		in_dev->mr_ifc_count--;
> -		igmp_ifc_start_timer(in_dev, IGMP_Unsolicited_Report_Interval);
> +		igmp_ifc_start_timer(in_dev,
> +				     unsolicited_report_interval(in_dev));
>  	}
>  	__in_dev_put(in_dev);
>  }
> @@ -744,7 +754,7 @@ static void igmp_timer_expire(unsigned long data)
>  
>  	if (im->unsolicit_count) {
>  		im->unsolicit_count--;
> -		igmp_start_timer(im, IGMP_Unsolicited_Report_Interval);
> +		igmp_start_timer(im, unsolicited_report_interval(in_dev));
>  	}
>  	im->reporter = 1;
>  	spin_unlock(&im->lock);
> -- 
> 1.7.10.4

-- 
"Thought is the essence of where you are now."

Re: [PATCH v4 3/3] net: igmp: Allow user-space configuration of igmp unsolicited report interval

[Benjamin LaHaise]

On Tue, Aug 06, 2013 at 07:03:15PM +0100, William Manley wrote:
> Adds the new procfs knobs:
> 
>     /proc/sys/net/ipv4/conf/*/igmpv2_unsolicited_report_interval
>     /proc/sys/net/ipv4/conf/*/igmpv3_unsolicited_report_interval
> 
> Which will allow userspace configuration of the IGMP unsolicited report
> interval (see below) in milliseconds.  The defaults are 10000ms for IGMPv2
> and 1000ms for IGMPv3 in accordance with RFC2236 and RFC3376.
> 
> Background:
> 
> If an IGMP join packet is lost you will not receive data sent to the
> multicast group so if no data arrives from that multicast group in a
> period of time after the IGMP join a second IGMP join will be sent.  The
> delay between joins is the "IGMP Unsolicited Report Interval".
> 
> Prior to this patch this value was hard coded in the kernel to 10s for
> IGMPv2 and 1s for IGMPv3.  10s is unsuitable for some use-cases, such as
> IPTV as it can cause channel change to be slow in the presence of packet
> loss.
> 
> This patch allows the value to be overridden from userspace for both
> IGMPv2 and IGMPv3 such that it can be tuned accoding to the network.
> 
> Tested with Wireshark and a simple program to join a (non-existent)
> multicast group.  The distribution of timings for the second join differ
> based upon setting the procfs knobs.
> 
> igmpvX_unsolicited_report_interval is intended to follow the pattern
> established by force_igmp_version, and while a procfs entry has been added
> a corresponding sysctl knob has not as it is my understanding that sysctl
> is deprecated[1].
> 
> [1]: http://lwn.net/Articles/247243/
> 
> Signed-off-by: William Manley <william.manley@youview.com>

Acked-by: Benjamin LaHaise <bcrl@kvack.org>

Also good.  Thanks!

		-ben

> ---
>  include/linux/inetdevice.h |    2 ++
>  net/ipv4/devinet.c         |    8 ++++++++
>  net/ipv4/igmp.c            |   19 +++++++++++++++++--
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index d4c56fc..43b3c72 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -28,6 +28,8 @@ enum
>  	IPV4_DEVCONF_ARPFILTER,
>  	IPV4_DEVCONF_MEDIUM_ID,
>  	IPV4_DEVCONF_FORCE_IGMP_VERSION,
> +	IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> +	IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL,
>  	IPV4_DEVCONF_NOXFRM,
>  	IPV4_DEVCONF_NOPOLICY,
>  	IPV4_DEVCONF_ARP_ANNOUNCE,
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index a9561c4..56ecb1b 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -73,6 +73,8 @@ static struct ipv4_devconf ipv4_devconf = {
>  		[IPV4_DEVCONF_SEND_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
> +		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> +		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
>  	},
>  };
>  
> @@ -83,6 +85,8 @@ static struct ipv4_devconf ipv4_devconf_dflt = {
>  		[IPV4_DEVCONF_SECURE_REDIRECTS - 1] = 1,
>  		[IPV4_DEVCONF_SHARED_MEDIA - 1] = 1,
>  		[IPV4_DEVCONF_ACCEPT_SOURCE_ROUTE - 1] = 1,
> +		[IPV4_DEVCONF_IGMPV2_UNSOLICITED_REPORT_INTERVAL - 1] = 10000 /*ms*/,
> +		[IPV4_DEVCONF_IGMPV3_UNSOLICITED_REPORT_INTERVAL - 1] =  1000 /*ms*/,
>  	},
>  };
>  
> @@ -2096,6 +2100,10 @@ static struct devinet_sysctl_table {
>  		DEVINET_SYSCTL_RW_ENTRY(PROXY_ARP_PVLAN, "proxy_arp_pvlan"),
>  		DEVINET_SYSCTL_RW_ENTRY(FORCE_IGMP_VERSION,
>  					"force_igmp_version"),
> +		DEVINET_SYSCTL_RW_ENTRY(IGMPV2_UNSOLICITED_REPORT_INTERVAL,
> +					"igmpv2_unsolicited_report_interval"),
> +		DEVINET_SYSCTL_RW_ENTRY(IGMPV3_UNSOLICITED_REPORT_INTERVAL,
> +					"igmpv3_unsolicited_report_interval"),
>  
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOXFRM, "disable_xfrm"),
>  		DEVINET_SYSCTL_FLUSHING_ENTRY(NOPOLICY, "disable_policy"),
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 9f0aaea..c5541da 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -141,10 +141,25 @@
>  
>  static int unsolicited_report_interval(struct in_device *in_dev)
>  {
> +	int interval_ms, interval_jiffies;
> +
>  	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> -		return IGMP_V2_Unsolicited_Report_Interval;
> +		interval_ms = IN_DEV_CONF_GET(
> +			in_dev,
> +			IGMPV2_UNSOLICITED_REPORT_INTERVAL);
>  	else /* v3 */
> -		return IGMP_V3_Unsolicited_Report_Interval;
> +		interval_ms = IN_DEV_CONF_GET(
> +			in_dev,
> +			IGMPV3_UNSOLICITED_REPORT_INTERVAL);
> +
> +	interval_jiffies = msecs_to_jiffies(interval_ms);
> +
> +	/* _timer functions can't handle a delay of 0 jiffies so ensure
> +	 *  we always return a positive value.
> +	 */
> +	if (interval_jiffies <= 0)
> +		interval_jiffies = 1;
> +	return interval_jiffies;
>  }
>  
>  static void igmpv3_add_delrec(struct in_device *in_dev, struct ip_mc_list *im);
> -- 
> 1.7.10.4

-- 
"Thought is the essence of where you are now."

Re: IGMP Unsolicited report interval patches

[Hannes Frederic Sowa]

On Wed, Aug 07, 2013 at 03:03:10AM +0200, Hannes Frederic Sowa wrote:
> On Tue, Aug 06, 2013 at 07:03:12PM +0100, William Manley wrote:
> > 4th version of the patches.
> > 
> > The significant changes since last review are: 
> > 
> > 1. there is a new patch (2/3) as requested by Hannes.
> 
> Thanks!
> 
> > 2. the third patch now uses IN_DEV_CONF_GET in place of
> >    IPV4_DEVCONF_ALL.  This means that the unsolicited report interval can
> >    now be configured on an interface-by-interface basis as I'd originally
> >    intended but messed up in the implementation.  One concern I have now
> >    is that with this latest patch-set is that while
> >    /proc/sys/net/ipv4/conf/eth0/igmp... will now have an effect 
> >    /proc/sys/net/ipv4/conf/all/igmp... will not.  I'm not sure how to
> >    resolve this.
> 
> Hm, it seems to be come more difficult dealing with ranges.
> 
> One way would be, to check the state bit for the devinet entry and chose the
> all value always but when the state bit for the interface for this entry is
> set. I'll have a look on how to do this.

We copy ipv4_devconf_dftl over to the per-interface ipv4_devconf as long no
address is assigned to the interface.

The copy-over is guarded by by the cnf.state bitmap. It is set to all ones in
ipv4_devconf_setall which gets called at the relevant places. Instead of
setting all bits we could just apply a bitmask which would still allow a
propagation of the default/*igmp* parameters.

The all namespace has special semantics and decision what to apply are done
per knob. I wouldn't fiddle with that.

Greetings,

  Hannes

Re: IGMP Unsolicited report interval patches

[David Miller]

From: William Manley <william.manley@youview.com>
Date: Tue,  6 Aug 2013 19:03:12 +0100

> 4th version of the patches.

Series applied, thanks.