IGMP Unsolicited Report Interval too long for IGMPv3?

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/[email protected]/msg05740.html

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

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(-)

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(-)

net/ipv4/igmp.c +0 -0

index d8c2327..f8142df 100644

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).

include/linux/inetdevice.h +4 -0

@@ -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;

net/ipv4/devinet.c +4 -0

index dfc39d4..f681518 100644
@@ -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*/,
 	},
 };
 

net/ipv4/igmp.c +0 -0

index 9f0aaea..035ee77 100644

net/ipv4/igmp.c +3 -0

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

Quoted 6 lines

net/ipv4/devinet.c +2 -0

@@ -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"),

net/ipv4/devinet.c static struct devinet_sysctl_table { · line 2112

Quoted 5 lines

net/ipv4/devinet.c +2 -0

+		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?

From: Hannes Frederic Sowa <[email protected]>
Date: Tue, 30 Jul 2013 08:14:26 +0200

Quoted 7 lines · William Manley

Quoted 6 lines

net/ipv4/devinet.c +2 -0

@@ -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"),

Quoted 8 lines

net/ipv4/devinet.c static struct devinet_sysctl_table { · line 2112

Quoted 5 lines

net/ipv4/devinet.c +2 -0

+		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.

Quoted 19 lines · David Miller

From: Hannes Frederic Sowa<[email protected]>
Date: Tue, 30 Jul 2013 08:14:26 +0200

Quoted 15 lines · William Manley

net/ipv4/devinet.c static struct devinet_sysctl_table { · line 2112

Quoted 11 lines

net/ipv4/devinet.c +4 -0

@@ -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?

Quoted 2 lines

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?

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 <[email protected]>

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 <[email protected]>

---
 net/ipv4/igmp.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

net/ipv4/igmp.c +13 -3

index d8c2327..9f0aaea 100644
@@ -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);

Signature · 1 line

1.7.10.4

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 <[email protected]>

Signed-off-by William Manley <[email protected]>

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 <[email protected]>

Signed-off-by William Manley <[email protected]>

---
 include/linux/inetdevice.h |    2 +-
 net/ipv4/devinet.c         |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

include/linux/inetdevice.h +1 -1

index ea1e3b8..d4c56fc 100644
@@ -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,

net/ipv4/devinet.c +2 -2

index dfc39d4..a9561c4 100644
@@ -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,

Signature · 1 line

1.7.10.4

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 <[email protected]>

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 <[email protected]>

---
 include/linux/inetdevice.h |    2 ++
 net/ipv4/devinet.c         |    8 ++++++++
 net/ipv4/igmp.c            |   19 +++++++++++++++++--
 3 files changed, 27 insertions(+), 2 deletions(-)

include/linux/inetdevice.h +2 -0

index d4c56fc..43b3c72 100644
@@ -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,

net/ipv4/devinet.c +8 -0

index a9561c4..56ecb1b 100644
@@ -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"),

net/ipv4/igmp.c +17 -2

index 9f0aaea..c5541da 100644
@@ -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);

Signature · 1 line

1.7.10.4

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 :)

Quoted 5 lines · 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.

Thanks!

Quoted 8 lines

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.

4th version of the patches.

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.

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 <[email protected]>

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 <[email protected]>

---
 net/ipv4/igmp.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

net/ipv4/igmp.c +2 -1

index d8c2327..12ffc2b 100644
@@ -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
 

Quoted 18 lines · 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.

Quoted 2 lines

[...]

[...]

Otherwise I am fine with it.

Quoted 22 lines · Hannes Frederic Sowa

Quoted 20 lines · 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.

[...]

Quoted 3 lines

[...]

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.

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/[email protected]/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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 11 lines · 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.

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.

Quoted 13 lines · William Manley

Quoted 11 lines

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.

Quoted 8 lines

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 <[email protected]>

Suggested-by Benjamin LaHaise <[email protected]>

Signed-off-by Hannes Frederic Sowa <[email protected]>

From: Hannes Frederic Sowa <[email protected]>
Date: Mon, 22 Jul 2013 23:51:08 +0200

Quoted 27 lines · Benjamin LaHaise

Quoted 13 lines · William Manley

Quoted 11 lines

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.

Quoted 8 lines

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

Quoted 3 lines

Reported-by William Manley <[email protected]>

Suggested-by Benjamin LaHaise <[email protected]>

Signed-off-by Hannes Frederic Sowa <[email protected]>

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

Thanks.

Quoted 34 lines · David Miller

From: Hannes Frederic Sowa <[email protected]>
Date: Mon, 22 Jul 2013 23:51:08 +0200

Quoted 30 lines · Benjamin LaHaise

Quoted 13 lines · William Manley

Quoted 11 lines

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.

Quoted 8 lines

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 <[email protected]>

Suggested-by Benjamin LaHaise <[email protected]>

Signed-off-by Hannes Frederic Sowa <[email protected]>

Quoted 3 lines

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 <[email protected]>

Quoted 2 lines · Benjamin LaHaise

I think Hannes' patch is good step in the right direction, so please add:

Acked-by Benjamin LaHaise <[email protected]>

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

Date: Mon, 22 Jul 2013 17:18:55 -0400

Quoted 2 lines

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.

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.

Quoted 6 lines

Date: Mon, 22 Jul 2013 17:18:55 -0400

Quoted 2 lines

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]).

Quoted 3 lines

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.