All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/8] IPv6 MLD updates
@ 2013-09-03  7:59 Daniel Borkmann
  2013-09-03  7:59 ` [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12 Daniel Borkmann
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev

This set contains the non-RFC version of the previous submission entitled
"[RFC PATCH net-next 0/7] IPv6 MLD updates". Most importantly it contains
a fix for MLDv1/v2 switchback timeout where hosts currently are switching
back from v1 compat mode to normal v2 too early (i.e. switchback time was
<= 30secs instead of >= 260secs on default), and the set also contains a
patch that allows for v2-only mode as per RFC recommendation. The rest is
related to cleanups that make the code more readable resp. maintainable.

Changes from RFC to non-RFC:

We ignore v2 messages now when in v1 compat mode, otherwise report timers
are reset and triggered, also stop current v2 report timer in case it is
currently running and we received a v1 query; use WARN_ON instead of BUG_ON
for RV of 0 (patch1).

Two more patches have been added that makes to code more readable, that is
"net: ipv6: mld: refactor query processing into v1/v2 functions" and
"net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions", the
sysctl patch "net: ipv6: mld: restrict min/max of sysctl force_mld_version"
has been dropped as extra1 and extra2 vars are overwritten with idev and net
anyway when addrconf sysctl is registered, hence dropped for now. The rest is
unchanged, only adapted to take changes into account.

Thanks!

Daniel Borkmann (8):
  net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
  net: ipv6: mld: clean up MLD_V1_SEEN macro
  net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
  net: ipv6: mld: implement RFC3810 MLDv2 mode only
  net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
  net: ipv6: mld: refactor query processing into v1/v2 functions
  net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
  net: ipv6: mld: document force_mld_version in ip-sysctl.txt

 Documentation/networking/ip-sysctl.txt |   5 +
 include/net/if_inet6.h                 |   9 +-
 include/net/mld.h                      |  51 +++++--
 net/bridge/br_multicast.c              |   3 +-
 net/ipv6/mcast.c                       | 237 ++++++++++++++++++++++++++-------
 5 files changed, 248 insertions(+), 57 deletions(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 17:52   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro Daniel Borkmann
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, David Stevens, Hannes Frederic Sowa

i) RFC3810, 9.2. Query Interval [QI] says:

   The Query Interval variable denotes the interval between General
   Queries sent by the Querier. Default value: 125 seconds. [...]

ii) RFC3810, 9.3. Query Response Interval [QRI] says:

  The Maximum Response Delay used to calculate the Maximum Response
  Code inserted into the periodic General Queries. Default value:
  10000 (10 seconds) [...] The number of seconds represented by the
  [Query Response Interval] must be less than the [Query Interval].

iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:

  The Older Version Querier Present Timeout is the time-out for
  transitioning a host back to MLDv2 Host Compatibility Mode. When an
  MLDv1 query is received, MLDv2 hosts set their Older Version Querier
  Present Timer to [Older Version Querier Present Timeout].

  This value MUST be ([Robustness Variable] times (the [Query Interval]
  in the last Query received)) plus ([Query Response Interval]).

Hence, on *default* the timeout results in:

  [RV] = 2, [QI] = 125sec, [QRI] = 10sec
  [OVQPT] = [RV] * [QI] + [QRI] = 260sec

Having that said, we currently calculate [OVQPT] (here given as 'switchback'
variable) as ...

  switchback = (idev->mc_qrv + 1) * max_delay

RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
section "9.14. Configuring timers", it is said:

  This section is meant to provide advice to network administrators on
  how to tune these settings to their network. Ambitious router
  implementations might tune these settings dynamically based upon
  changing characteristics of the network. [...]

iv) RFC38010, 9.14.2. Query Interval:

  The overall level of periodic MLD traffic is inversely proportional
  to the Query Interval. A longer Query Interval results in a lower
  overall level of MLD traffic. The value of the Query Interval MUST
  be equal to or greater than the Maximum Response Delay used to
  calculate the Maximum Response Code inserted in General Query
  messages.

I assume that was why switchback is calculated as is (3 * max_delay), although
this setting seems to be meant for routers only to configure their [QI]
interval for non-default intervals. So usage here like this is clearly wrong.

Concluding, the current behaviour in IPv6's multicast code is not conform
to the RFC as switch back is calculated wrongly. That is, it has a too small
value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
instead of ~260secs on default.

Hence, introduce necessary helper functions and fix this up properly as it
should be.

Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
who did initial testing.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: David Stevens <dlstevens@us.ibm.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 06da92283 is linux-history tree.

 include/net/if_inet6.h |   9 +++-
 include/net/mld.h      |  27 +++++++++++-
 net/ipv6/mcast.c       | 116 ++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 143 insertions(+), 9 deletions(-)

diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 736b5fb..02ef772 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -171,12 +171,17 @@ struct inet6_dev {
 	struct ifmcaddr6	*mc_list;
 	struct ifmcaddr6	*mc_tomb;
 	spinlock_t		mc_lock;
-	unsigned char		mc_qrv;
+
+	unsigned char		mc_qrv;		/* Query Robustness Variable */
 	unsigned char		mc_gq_running;
 	unsigned char		mc_ifc_count;
 	unsigned char		mc_dad_count;
-	unsigned long		mc_v1_seen;
+
+	unsigned long		mc_v1_seen;	/* Max time we stay in MLDv1 mode */
+	unsigned long		mc_qi;		/* Query Interval */
+	unsigned long		mc_qri;		/* Query Response Interval */
 	unsigned long		mc_maxdelay;
+
 	struct timer_list	mc_gq_timer;	/* general query timer */
 	struct timer_list	mc_ifc_timer;	/* interface change timer */
 	struct timer_list	mc_dad_timer;	/* dad complete mc timer */
diff --git a/include/net/mld.h b/include/net/mld.h
index 467143c..2b5421f 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,7 +63,7 @@ struct mld2_query {
 #define mld2q_mrc		mld2q_hdr.icmp6_maxdelay
 #define mld2q_resv1		mld2q_hdr.icmp6_dataun.un_data16[1]
 
-/* Max Response Code */
+/* Max Response Code, TODO: transform this to use the below */
 #define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
 #define MLDV2_EXP(thresh, nbmant, nbexp, value) \
 	((value) < (thresh) ? (value) : \
@@ -72,4 +72,29 @@ struct mld2_query {
 
 #define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
 
+/* RFC3810, 5.1.3. Maximum Response Code:
+ *
+ * If Maximum Response Code >= 32768, Maximum Response Code represents a
+ * floating-point value as follows:
+ *
+ *  0 1 2 3 4 5 6 7 8 9 A B C D E F
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |1| exp |          mant         |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_MRC_EXP(value)	(((value) >> 12) & 0x0007)
+#define MLDV2_MRC_MAN(value)	((value) & 0x0fff)
+
+/* RFC3810, 5.1.9. QQIC (Querier's Query Interval Code):
+ *
+ * If QQIC >= 128, QQIC represents a floating-point value as follows:
+ *
+ *  0 1 2 3 4 5 6 7
+ * +-+-+-+-+-+-+-+-+
+ * |1| exp | mant  |
+ * +-+-+-+-+-+-+-+-+
+ */
+#define MLDV2_QQIC_EXP(value)	(((value) >> 4) & 0x07)
+#define MLDV2_QQIC_MAN(value)	((value) & 0x0f)
+
 #endif
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 98ead2b..8992ff2 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -108,6 +108,10 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 			    struct inet6_dev *idev);
 
 #define MLD_QRV_DEFAULT		2
+/* RFC3810, 9.2. Query Interval */
+#define MLD_QI_DEFAULT		(125 * HZ)
+/* RFC3810, 9.3. Query Response Interval */
+#define MLD_QRI_DEFAULT		(10 * HZ)
 
 /* RFC3810, 8.1 Query Version Distinctions */
 #define MLD_V1_QUERY_LEN	24
@@ -1112,6 +1116,93 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static void mld_set_v1_mode(struct inet6_dev *idev)
+{
+	/* RFC3810, relevant sections:
+	 *  - 9.1. Robustness Variable
+	 *  - 9.2. Query Interval
+	 *  - 9.3. Query Response Interval
+	 *  - 9.12. Older Version Querier Present Timeout
+	 */
+	unsigned long switchback;
+
+	switchback = (idev->mc_qrv * idev->mc_qi) + idev->mc_qri;
+
+	idev->mc_v1_seen = jiffies + switchback;
+}
+
+static void mld_update_qrv(struct inet6_dev *idev,
+			   const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.8. QRV (Querier's Robustness Variable)
+	 *  - 9.1. Robustness Variable
+	 */
+
+	/* The value of the Robustness Variable MUST NOT be zero,
+	 * and SHOULD NOT be one. Catch this here if we ever run
+	 * into such a case in future.
+	 */
+	WARN_ON(idev->mc_qrv == 0);
+
+	if (mlh2->mld2q_qrv > 0)
+		idev->mc_qrv = mlh2->mld2q_qrv;
+
+	if (unlikely(idev->mc_qrv < 2)) {
+		net_warn_ratelimited("IPv6: MLD: clamping QRV from %u to %u!\n",
+				     idev->mc_qrv, MLD_QRV_DEFAULT);
+		idev->mc_qrv = MLD_QRV_DEFAULT;
+	}
+}
+
+static void mld_update_qi(struct inet6_dev *idev,
+			  const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.9. QQIC (Querier's Query Interval Code)
+	 *  - 9.2. Query Interval
+	 *  - 9.12. Older Version Querier Present Timeout
+	 *    (the [Query Interval] in the last Query received)
+	 */
+	unsigned long mc_qqi;
+
+	if (mlh2->mld2q_qqic < 128) {
+		mc_qqi = mlh2->mld2q_qqic;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_QQIC_EXP(mlh2->mld2q_qqic);
+		mc_man = MLDV2_QQIC_MAN(mlh2->mld2q_qqic);
+
+		mc_qqi = (mc_man | 0x10) << (mc_exp + 3);
+	}
+
+	idev->mc_qi = mc_qqi * HZ;
+}
+
+static void mld_update_qri(struct inet6_dev *idev,
+			   const struct mld2_query *mlh2)
+{
+	/* RFC3810, relevant sections:
+	 *  - 5.1.3. Maximum Response Code
+	 *  - 9.3. Query Response Interval
+	 */
+	unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+	if (mc_mrc < 32768) {
+		mc_qri = mc_mrc;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_MRC_EXP(mc_mrc);
+		mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+		mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
+	}
+
+	idev->mc_qri = msecs_to_jiffies(mc_qri);
+}
+
 /* called with rcu_read_lock() */
 int igmp6_event_query(struct sk_buff *skb)
 {
@@ -1150,12 +1241,15 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
-		int switchback;
 		/* MLDv1 router present */
-
 		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
-		switchback = (idev->mc_qrv + 1) * max_delay;
-		idev->mc_v1_seen = jiffies + switchback;
+
+		mld_set_v1_mode(idev);
+
+		/* cancel MLDv2 report timer */
+		idev->mc_gq_running = 0;
+		if (del_timer(&idev->mc_gq_timer))
+			__in6_dev_put(idev);
 
 		/* cancel the interface change timer */
 		idev->mc_ifc_count = 0;
@@ -1166,6 +1260,10 @@ int igmp6_event_query(struct sk_buff *skb)
 	} else if (len >= MLD_V2_QUERY_LEN_MIN) {
 		int srcs_offset = sizeof(struct mld2_query) -
 				  sizeof(struct icmp6hdr);
+
+		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+		if (MLD_V1_SEEN(idev))
+			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
 
@@ -1175,8 +1273,10 @@ int igmp6_event_query(struct sk_buff *skb)
 
 		idev->mc_maxdelay = max_delay;
 
-		if (mlh2->mld2q_qrv)
-			idev->mc_qrv = mlh2->mld2q_qrv;
+		mld_update_qrv(idev, mlh2);
+		mld_update_qi(idev, mlh2);
+		mld_update_qri(idev, mlh2);
+
 		if (group_type == IPV6_ADDR_ANY) { /* general query */
 			if (mlh2->mld2q_nsrcs)
 				return -EINVAL; /* no sources allowed */
@@ -2337,7 +2437,11 @@ void ipv6_mc_init_dev(struct inet6_dev *idev)
 			(unsigned long)idev);
 	setup_timer(&idev->mc_dad_timer, mld_dad_timer_expire,
 		    (unsigned long)idev);
+
 	idev->mc_qrv = MLD_QRV_DEFAULT;
+	idev->mc_qi = MLD_QI_DEFAULT;
+	idev->mc_qri = MLD_QRI_DEFAULT;
+
 	idev->mc_maxdelay = unsolicited_report_interval(idev);
 	idev->mc_v1_seen = 0;
 	write_unlock_bh(&idev->lock);
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
  2013-09-03  7:59 ` [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12 Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 17:55   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation Daniel Borkmann
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

Replace the macro with a function to make it more readable. GCC will
eventually decide whether to inline this or not (also, that's not
fast-path anyway).

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 8992ff2..f86e26b 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -95,6 +95,7 @@ static void mld_ifc_event(struct inet6_dev *idev);
 static void mld_add_delrec(struct inet6_dev *idev, struct ifmcaddr6 *pmc);
 static void mld_del_delrec(struct inet6_dev *idev, const struct in6_addr *addr);
 static void mld_clear_delrec(struct inet6_dev *idev);
+static bool mld_in_v1_mode(const struct inet6_dev *idev);
 static int sf_setstate(struct ifmcaddr6 *pmc);
 static void sf_markstate(struct ifmcaddr6 *pmc);
 static void ip6_mc_clear_src(struct ifmcaddr6 *pmc);
@@ -117,11 +118,6 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 #define MLD_V1_QUERY_LEN	24
 #define MLD_V2_QUERY_LEN_MIN	28
 
-#define MLD_V1_SEEN(idev) (dev_net((idev)->dev)->ipv6.devconf_all->force_mld_version == 1 || \
-		(idev)->cnf.force_mld_version == 1 || \
-		((idev)->mc_v1_seen && \
-		time_before(jiffies, (idev)->mc_v1_seen)))
-
 #define IPV6_MLD_MAX_MSF	64
 
 int sysctl_mld_max_msf __read_mostly = IPV6_MLD_MAX_MSF;
@@ -139,7 +135,7 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
 {
 	int iv;
 
-	if (MLD_V1_SEEN(idev))
+	if (mld_in_v1_mode(idev))
 		iv = idev->cnf.mldv1_unsolicited_report_interval;
 	else
 		iv = idev->cnf.mldv2_unsolicited_report_interval;
@@ -695,7 +691,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc)
 	if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT))
 		return;
 
-	if (MLD_V1_SEEN(mc->idev)) {
+	if (mld_in_v1_mode(mc->idev)) {
 		igmp6_join_group(mc);
 		return;
 	}
@@ -1116,6 +1112,18 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static bool mld_in_v1_mode(const struct inet6_dev *idev)
+{
+	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
+		return true;
+	if (idev->cnf.force_mld_version == 1)
+		return true;
+	if (idev->mc_v1_seen && time_before(jiffies, idev->mc_v1_seen))
+		return true;
+
+	return false;
+}
+
 static void mld_set_v1_mode(struct inet6_dev *idev)
 {
 	/* RFC3810, relevant sections:
@@ -1262,7 +1270,7 @@ int igmp6_event_query(struct sk_buff *skb)
 				  sizeof(struct icmp6hdr);
 
 		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
-		if (MLD_V1_SEEN(idev))
+		if (mld_in_v1_mode(idev))
 			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
@@ -1942,7 +1950,7 @@ err_out:
 
 static void mld_resend_report(struct inet6_dev *idev)
 {
-	if (MLD_V1_SEEN(idev)) {
+	if (mld_in_v1_mode(idev)) {
 		struct ifmcaddr6 *mcaddr;
 		read_lock_bh(&idev->lock);
 		for (mcaddr = idev->mc_list; mcaddr; mcaddr = mcaddr->next) {
@@ -2006,7 +2014,7 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *pmc, int sfmode,
 		else
 			pmc->mca_sources = psf->sf_next;
 		if (psf->sf_oldin && !(pmc->mca_flags & MAF_NOREPORT) &&
-		    !MLD_V1_SEEN(idev)) {
+		    !mld_in_v1_mode(idev)) {
 			psf->sf_crcount = idev->mc_qrv;
 			psf->sf_next = pmc->mca_tomb;
 			pmc->mca_tomb = psf;
@@ -2306,7 +2314,7 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 
 static void igmp6_leave_group(struct ifmcaddr6 *ma)
 {
-	if (MLD_V1_SEEN(ma->idev)) {
+	if (mld_in_v1_mode(ma->idev)) {
 		if (ma->mca_flags & MAF_LAST_REPORTER)
 			igmp6_send(&ma->mca_addr, ma->idev->dev,
 				ICMPV6_MGM_REDUCTION);
@@ -2340,7 +2348,7 @@ static void mld_ifc_timer_expire(unsigned long data)
 
 static void mld_ifc_event(struct inet6_dev *idev)
 {
-	if (MLD_V1_SEEN(idev))
+	if (mld_in_v1_mode(idev))
 		return;
 	idev->mc_ifc_count = idev->mc_qrv;
 	mld_ifc_start_timer(idev, 1);
@@ -2351,7 +2359,7 @@ static void igmp6_timer_handler(unsigned long data)
 {
 	struct ifmcaddr6 *ma = (struct ifmcaddr6 *) data;
 
-	if (MLD_V1_SEEN(ma->idev))
+	if (mld_in_v1_mode(ma->idev))
 		igmp6_send(&ma->mca_addr, ma->idev->dev, ICMPV6_MGM_REPORT);
 	else
 		mld_send_report(ma->idev, ma);
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
  2013-09-03  7:59 ` [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12 Daniel Borkmann
  2013-09-03  7:59 ` [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:09   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only Daniel Borkmann
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

Get rid of MLDV2_MRC and use our new macros for mantisse and
exponent to calculate Maximum Response Delay out of the Maximum
Response Code.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 This will give a merge conflict in net/bridge/br_multicast.c with net tree. You can
 resolve it by simply taking this version as we already do msecs_to_jiffies() here.

 include/net/mld.h         | 28 +++++++++++++++++++---------
 net/bridge/br_multicast.c |  3 ++-
 net/ipv6/mcast.c          | 18 ++----------------
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/include/net/mld.h b/include/net/mld.h
index 2b5421f..faa1d16 100644
--- a/include/net/mld.h
+++ b/include/net/mld.h
@@ -63,15 +63,6 @@ struct mld2_query {
 #define mld2q_mrc		mld2q_hdr.icmp6_maxdelay
 #define mld2q_resv1		mld2q_hdr.icmp6_dataun.un_data16[1]
 
-/* Max Response Code, TODO: transform this to use the below */
-#define MLDV2_MASK(value, nb) ((nb)>=32 ? (value) : ((1<<(nb))-1) & (value))
-#define MLDV2_EXP(thresh, nbmant, nbexp, value) \
-	((value) < (thresh) ? (value) : \
-	((MLDV2_MASK(value, nbmant) | (1<<(nbmant))) << \
-	(MLDV2_MASK((value) >> (nbmant), nbexp) + (nbexp))))
-
-#define MLDV2_MRC(value) MLDV2_EXP(0x8000, 12, 3, value)
-
 /* RFC3810, 5.1.3. Maximum Response Code:
  *
  * If Maximum Response Code >= 32768, Maximum Response Code represents a
@@ -97,4 +88,23 @@ struct mld2_query {
 #define MLDV2_QQIC_EXP(value)	(((value) >> 4) & 0x07)
 #define MLDV2_QQIC_MAN(value)	((value) & 0x0f)
 
+static inline unsigned long mldv2_mrc(const struct mld2_query *mlh2)
+{
+	/* RFC3810, 5.1.3. Maximum Response Code */
+	unsigned long ret, mc_mrc = ntohs(mlh2->mld2q_mrc);
+
+	if (mc_mrc < 32768) {
+		ret = mc_mrc;
+	} else {
+		unsigned long mc_man, mc_exp;
+
+		mc_exp = MLDV2_MRC_EXP(mc_mrc);
+		mc_man = MLDV2_MRC_MAN(mc_mrc);
+
+		ret = (mc_man | 0x1000) << (mc_exp + 3);
+	}
+
+	return ret;
+}
+
 #endif
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 08e576a..4accd0d 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1203,7 +1203,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 		mld2q = (struct mld2_query *)icmp6_hdr(skb);
 		if (!mld2q->mld2q_nsrcs)
 			group = &mld2q->mld2q_mca;
-		max_delay = mld2q->mld2q_mrc ? MLDV2_MRC(ntohs(mld2q->mld2q_mrc)) : 1;
+
+		max_delay = max(msecs_to_jiffies(mldv2_mrc(mld2q)), 1UL);
 	}
 
 	br_multicast_query_received(br, port, !ipv6_addr_any(&ip6h->saddr),
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index f86e26b..005b22f 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1195,20 +1195,7 @@ static void mld_update_qri(struct inet6_dev *idev,
 	 *  - 5.1.3. Maximum Response Code
 	 *  - 9.3. Query Response Interval
 	 */
-	unsigned long mc_qri, mc_mrc = ntohs(mlh2->mld2q_mrc);
-
-	if (mc_mrc < 32768) {
-		mc_qri = mc_mrc;
-	} else {
-		unsigned long mc_man, mc_exp;
-
-		mc_exp = MLDV2_MRC_EXP(mc_mrc);
-		mc_man = MLDV2_MRC_MAN(mc_mrc);
-
-		mc_qri = (mc_man | 0x1000) << (mc_exp + 3);
-	}
-
-	idev->mc_qri = msecs_to_jiffies(mc_qri);
+	idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
 }
 
 /* called with rcu_read_lock() */
@@ -1277,8 +1264,7 @@ int igmp6_event_query(struct sk_buff *skb)
 
 		mlh2 = (struct mld2_query *)skb_transport_header(skb);
 
-		max_delay = max(msecs_to_jiffies(MLDV2_MRC(ntohs(mlh2->mld2q_mrc))), 1UL);
-
+		max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
 		idev->mc_maxdelay = max_delay;
 
 		mld_update_qrv(idev, mlh2);
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
                   ` (2 preceding siblings ...)
  2013-09-03  7:59 ` [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:12   ` Hannes Frederic Sowa
  2013-09-03 21:00   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1 Daniel Borkmann
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

RFC3810, 10. Security Considerations says under subsection 10.1.
Query Message:

  A forged Version 1 Query message will put MLDv2 listeners on that
  link in MLDv1 Host Compatibility Mode. This scenario can be avoided
  by providing MLDv2 hosts with a configuration option to ignore
  Version 1 messages completely.

Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:

  echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 005b22f..02cd0c5 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
+{
+	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;
+}
+
+static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
+{
+	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;
+}
+
 static bool mld_in_v1_mode(const struct inet6_dev *idev)
 {
-	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
+	if (mld_in_v2_mode_only(idev))
+		return false;
+	if (mld_in_v1_mode_only(idev))
 		return true;
 	if (idev->cnf.force_mld_version == 1)
 		return true;
@@ -1223,7 +1235,6 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	idev = __in6_dev_get(skb->dev);
-
 	if (idev == NULL)
 		return 0;
 
@@ -1236,6 +1247,10 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
+		/* Ignore v1 queries */
+		if (mld_in_v2_mode_only(idev))
+			return 0;
+
 		/* MLDv1 router present */
 		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
                   ` (3 preceding siblings ...)
  2013-09-03  7:59 ` [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:38   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions Daniel Borkmann
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

Similarly as we do in MLDv2 queries, set a forged MLDv1 query with
0 ms mld_maxdelay to minimum timer shot time of 1 jiffies. This is
eventually done in igmp6_group_queried() anyway, so we can simplify
a check there.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 02cd0c5..75a4324 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1040,12 +1040,9 @@ static void igmp6_group_queried(struct ifmcaddr6 *ma, unsigned long resptime)
 		delay = ma->mca_timer.expires - jiffies;
 	}
 
-	if (delay >= resptime) {
-		if (resptime)
-			delay = net_random() % resptime;
-		else
-			delay = 1;
-	}
+	if (delay >= resptime)
+		delay = net_random() % resptime;
+
 	ma->mca_timer.expires = jiffies + delay;
 	if (!mod_timer(&ma->mca_timer, jiffies + delay))
 		atomic_inc(&ma->mca_refcnt);
@@ -1247,12 +1244,15 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
+		unsigned long mldv1_md;
+
 		/* Ignore v1 queries */
 		if (mld_in_v2_mode_only(idev))
 			return 0;
 
 		/* MLDv1 router present */
-		max_delay = msecs_to_jiffies(ntohs(mld->mld_maxdelay));
+		mldv1_md = ntohs(mld->mld_maxdelay);
+		max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
 
 		mld_set_v1_mode(idev);
 
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
                   ` (4 preceding siblings ...)
  2013-09-03  7:59 ` [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1 Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:49   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions Daniel Borkmann
  2013-09-03  7:59 ` [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt Daniel Borkmann
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

Make igmp6_event_query() a bit easier to read by refactoring code
parts into mld_process_v1() and mld_process_v2().

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 89 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 33 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 75a4324..b01aa32 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1207,6 +1207,55 @@ static void mld_update_qri(struct inet6_dev *idev,
 	idev->mc_qri = msecs_to_jiffies(mldv2_mrc(mlh2));
 }
 
+static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
+			  unsigned long *max_delay)
+{
+	unsigned long mldv1_md;
+
+	/* Ignore v1 queries */
+	if (mld_in_v2_mode_only(idev))
+		return -EINVAL;
+
+	/* MLDv1 router present */
+	mldv1_md = ntohs(mld->mld_maxdelay);
+	*max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
+
+	mld_set_v1_mode(idev);
+
+	/* cancel MLDv2 report timer */
+	idev->mc_gq_running = 0;
+	if (del_timer(&idev->mc_gq_timer))
+		__in6_dev_put(idev);
+
+	/* cancel the interface change timer */
+	idev->mc_ifc_count = 0;
+	if (del_timer(&idev->mc_ifc_timer))
+		__in6_dev_put(idev);
+
+	/* clear deleted report items */
+	mld_clear_delrec(idev);
+
+	return 0;
+}
+
+static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
+			  unsigned long *max_delay)
+{
+	/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
+	if (mld_in_v1_mode(idev))
+		return -EINVAL;
+
+	*max_delay = max(msecs_to_jiffies(mldv2_mrc(mld)), 1UL);
+
+	mld_update_qrv(idev, mld);
+	mld_update_qi(idev, mld);
+	mld_update_qri(idev, mld);
+
+	idev->mc_maxdelay = *max_delay;
+
+	return 0;
+}
+
 /* called with rcu_read_lock() */
 int igmp6_event_query(struct sk_buff *skb)
 {
@@ -1218,7 +1267,7 @@ int igmp6_event_query(struct sk_buff *skb)
 	struct mld_msg *mld;
 	int group_type;
 	int mark = 0;
-	int len;
+	int len, err;
 
 	if (!pskb_may_pull(skb, sizeof(struct in6_addr)))
 		return -EINVAL;
@@ -1244,47 +1293,21 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	if (len == MLD_V1_QUERY_LEN) {
-		unsigned long mldv1_md;
-
-		/* Ignore v1 queries */
-		if (mld_in_v2_mode_only(idev))
-			return 0;
-
-		/* MLDv1 router present */
-		mldv1_md = ntohs(mld->mld_maxdelay);
-		max_delay = max(msecs_to_jiffies(mldv1_md), 1UL);
-
-		mld_set_v1_mode(idev);
-
-		/* cancel MLDv2 report timer */
-		idev->mc_gq_running = 0;
-		if (del_timer(&idev->mc_gq_timer))
-			__in6_dev_put(idev);
-
-		/* cancel the interface change timer */
-		idev->mc_ifc_count = 0;
-		if (del_timer(&idev->mc_ifc_timer))
-			__in6_dev_put(idev);
-		/* clear deleted report items */
-		mld_clear_delrec(idev);
+		err = mld_process_v1(idev, mld, &max_delay);
+		if (err < 0)
+			return err;
 	} else if (len >= MLD_V2_QUERY_LEN_MIN) {
 		int srcs_offset = sizeof(struct mld2_query) -
 				  sizeof(struct icmp6hdr);
 
-		/* hosts need to stay in MLDv1 mode, discard MLDv2 queries */
-		if (mld_in_v1_mode(idev))
-			return 0;
 		if (!pskb_may_pull(skb, srcs_offset))
 			return -EINVAL;
 
 		mlh2 = (struct mld2_query *)skb_transport_header(skb);
 
-		max_delay = max(msecs_to_jiffies(mldv2_mrc(mlh2)), 1UL);
-		idev->mc_maxdelay = max_delay;
-
-		mld_update_qrv(idev, mlh2);
-		mld_update_qi(idev, mlh2);
-		mld_update_qri(idev, mlh2);
+		err = mld_process_v2(idev, mlh2, &max_delay);
+		if (err < 0)
+			return err;
 
 		if (group_type == IPV6_ADDR_ANY) { /* general query */
 			if (mlh2->mld2q_nsrcs)
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
                   ` (5 preceding siblings ...)
  2013-09-03  7:59 ` [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:51   ` Hannes Frederic Sowa
  2013-09-03  7:59 ` [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt Daniel Borkmann
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

We already have mld_{gq,ifc,dad}_start_timer() functions, so introduce
mld_{gq,ifc,dad}_stop_timer() functions to reduce code size and make it
more readable.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 net/ipv6/mcast.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index b01aa32..a43028d 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1006,6 +1006,13 @@ static void mld_gq_start_timer(struct inet6_dev *idev)
 		in6_dev_hold(idev);
 }
 
+static void mld_gq_stop_timer(struct inet6_dev *idev)
+{
+	idev->mc_gq_running = 0;
+	if (del_timer(&idev->mc_gq_timer))
+		__in6_dev_put(idev);
+}
+
 static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
 	unsigned long tv = net_random() % delay;
@@ -1014,6 +1021,13 @@ static void mld_ifc_start_timer(struct inet6_dev *idev, unsigned long delay)
 		in6_dev_hold(idev);
 }
 
+static void mld_ifc_stop_timer(struct inet6_dev *idev)
+{
+	idev->mc_ifc_count = 0;
+	if (del_timer(&idev->mc_ifc_timer))
+		__in6_dev_put(idev);
+}
+
 static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
 {
 	unsigned long tv = net_random() % delay;
@@ -1022,6 +1036,12 @@ static void mld_dad_start_timer(struct inet6_dev *idev, unsigned long delay)
 		in6_dev_hold(idev);
 }
 
+static void mld_dad_stop_timer(struct inet6_dev *idev)
+{
+	if (del_timer(&idev->mc_dad_timer))
+		__in6_dev_put(idev);
+}
+
 /*
  *	IGMP handling (alias multicast ICMPv6 messages)
  */
@@ -1223,15 +1243,9 @@ static int mld_process_v1(struct inet6_dev *idev, struct mld_msg *mld,
 	mld_set_v1_mode(idev);
 
 	/* cancel MLDv2 report timer */
-	idev->mc_gq_running = 0;
-	if (del_timer(&idev->mc_gq_timer))
-		__in6_dev_put(idev);
-
+	mld_gq_stop_timer(idev);
 	/* cancel the interface change timer */
-	idev->mc_ifc_count = 0;
-	if (del_timer(&idev->mc_ifc_timer))
-		__in6_dev_put(idev);
-
+	mld_ifc_stop_timer(idev);
 	/* clear deleted report items */
 	mld_clear_delrec(idev);
 
@@ -2423,14 +2437,9 @@ void ipv6_mc_down(struct inet6_dev *idev)
 	/* Withdraw multicast list */
 
 	read_lock_bh(&idev->lock);
-	idev->mc_ifc_count = 0;
-	if (del_timer(&idev->mc_ifc_timer))
-		__in6_dev_put(idev);
-	idev->mc_gq_running = 0;
-	if (del_timer(&idev->mc_gq_timer))
-		__in6_dev_put(idev);
-	if (del_timer(&idev->mc_dad_timer))
-		__in6_dev_put(idev);
+	mld_ifc_stop_timer(idev);
+	mld_gq_stop_timer(idev);
+	mld_dad_stop_timer(idev);
 
 	for (i = idev->mc_list; i; i=i->next)
 		igmp6_group_dropped(i);
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
  2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
                   ` (6 preceding siblings ...)
  2013-09-03  7:59 ` [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions Daniel Borkmann
@ 2013-09-03  7:59 ` Daniel Borkmann
  2013-09-03 18:52   ` Hannes Frederic Sowa
  7 siblings, 1 reply; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03  7:59 UTC (permalink / raw
  To: davem; +Cc: netdev, Hannes Frederic Sowa

Document force_mld_version parameter in ip-sysctl.txt.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
 Documentation/networking/ip-sysctl.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 1cb3aeb..a46d785 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1358,6 +1358,11 @@ mldv2_unsolicited_report_interval - INTEGER
 	MLDv2 report retransmit will take place.
 	Default: 1000 (1 second)
 
+force_mld_version - INTEGER
+	0 - (default) No enforcement of a MLD version, MLDv1 fallback allowed
+	1 - Enforce to use MLD version 1
+	2 - Enforce to use MLD version 2
+
 suppress_frag_ndisc - INTEGER
 	Control RFC 6980 (Security Implications of IPv6 Fragmentation
 	with IPv6 Neighbor Discovery) behavior:
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
  2013-09-03  7:59 ` [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12 Daniel Borkmann
@ 2013-09-03 17:52   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 17:52 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev, David Stevens

On Tue, Sep 03, 2013 at 09:59:31AM +0200, Daniel Borkmann wrote:
> i) RFC3810, 9.2. Query Interval [QI] says:
> 
>    The Query Interval variable denotes the interval between General
>    Queries sent by the Querier. Default value: 125 seconds. [...]
> 
> ii) RFC3810, 9.3. Query Response Interval [QRI] says:
> 
>   The Maximum Response Delay used to calculate the Maximum Response
>   Code inserted into the periodic General Queries. Default value:
>   10000 (10 seconds) [...] The number of seconds represented by the
>   [Query Response Interval] must be less than the [Query Interval].
> 
> iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:
> 
>   The Older Version Querier Present Timeout is the time-out for
>   transitioning a host back to MLDv2 Host Compatibility Mode. When an
>   MLDv1 query is received, MLDv2 hosts set their Older Version Querier
>   Present Timer to [Older Version Querier Present Timeout].
> 
>   This value MUST be ([Robustness Variable] times (the [Query Interval]
>   in the last Query received)) plus ([Query Response Interval]).
> 
> Hence, on *default* the timeout results in:
> 
>   [RV] = 2, [QI] = 125sec, [QRI] = 10sec
>   [OVQPT] = [RV] * [QI] + [QRI] = 260sec
> 
> Having that said, we currently calculate [OVQPT] (here given as 'switchback'
> variable) as ...
> 
>   switchback = (idev->mc_qrv + 1) * max_delay
> 
> RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
> section "9.14. Configuring timers", it is said:
> 
>   This section is meant to provide advice to network administrators on
>   how to tune these settings to their network. Ambitious router
>   implementations might tune these settings dynamically based upon
>   changing characteristics of the network. [...]
> 
> iv) RFC38010, 9.14.2. Query Interval:
> 
>   The overall level of periodic MLD traffic is inversely proportional
>   to the Query Interval. A longer Query Interval results in a lower
>   overall level of MLD traffic. The value of the Query Interval MUST
>   be equal to or greater than the Maximum Response Delay used to
>   calculate the Maximum Response Code inserted in General Query
>   messages.
> 
> I assume that was why switchback is calculated as is (3 * max_delay), although
> this setting seems to be meant for routers only to configure their [QI]
> interval for non-default intervals. So usage here like this is clearly wrong.
> 
> Concluding, the current behaviour in IPv6's multicast code is not conform
> to the RFC as switch back is calculated wrongly. That is, it has a too small
> value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
> instead of ~260secs on default.
> 
> Hence, introduce necessary helper functions and fix this up properly as it
> should be.
> 
> Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
> Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
> who did initial testing.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: David Stevens <dlstevens@us.ibm.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
  2013-09-03  7:59 ` [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro Daniel Borkmann
@ 2013-09-03 17:55   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 17:55 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:32AM +0200, Daniel Borkmann wrote:
> Replace the macro with a function to make it more readable. GCC will
> eventually decide whether to inline this or not (also, that's not
> fast-path anyway).
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
  2013-09-03  7:59 ` [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation Daniel Borkmann
@ 2013-09-03 18:09   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:09 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:33AM +0200, Daniel Borkmann wrote:
> Get rid of MLDV2_MRC and use our new macros for mantisse and
> exponent to calculate Maximum Response Delay out of the Maximum
> Response Code.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
  2013-09-03  7:59 ` [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only Daniel Borkmann
@ 2013-09-03 18:12   ` Hannes Frederic Sowa
  2013-09-03 21:00   ` Hannes Frederic Sowa
  1 sibling, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:12 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:34AM +0200, Daniel Borkmann wrote:
> RFC3810, 10. Security Considerations says under subsection 10.1.
> Query Message:
> 
>   A forged Version 1 Query message will put MLDv2 listeners on that
>   link in MLDv1 Host Compatibility Mode. This scenario can be avoided
>   by providing MLDv2 hosts with a configuration option to ignore
>   Version 1 messages completely.
> 
> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
> 
>   echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
  2013-09-03  7:59 ` [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1 Daniel Borkmann
@ 2013-09-03 18:38   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:38 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:35AM +0200, Daniel Borkmann wrote:
> Similarly as we do in MLDv2 queries, set a forged MLDv1 query with
> 0 ms mld_maxdelay to minimum timer shot time of 1 jiffies. This is
> eventually done in igmp6_group_queried() anyway, so we can simplify
> a check there.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
  2013-09-03  7:59 ` [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions Daniel Borkmann
@ 2013-09-03 18:49   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:49 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:36AM +0200, Daniel Borkmann wrote:
> Make igmp6_event_query() a bit easier to read by refactoring code
> parts into mld_process_v1() and mld_process_v2().
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
  2013-09-03  7:59 ` [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions Daniel Borkmann
@ 2013-09-03 18:51   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:51 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:37AM +0200, Daniel Borkmann wrote:
> We already have mld_{gq,ifc,dad}_start_timer() functions, so introduce
> mld_{gq,ifc,dad}_stop_timer() functions to reduce code size and make it
> more readable.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
  2013-09-03  7:59 ` [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt Daniel Borkmann
@ 2013-09-03 18:52   ` Hannes Frederic Sowa
  0 siblings, 0 replies; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 18:52 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

On Tue, Sep 03, 2013 at 09:59:38AM +0200, Daniel Borkmann wrote:
> Document force_mld_version parameter in ip-sysctl.txt.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>

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

Daniel, great series. It is now easier to follow the rfc and check the code
for correctness!

Thanks a lot,

  Hannes

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
  2013-09-03  7:59 ` [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only Daniel Borkmann
  2013-09-03 18:12   ` Hannes Frederic Sowa
@ 2013-09-03 21:00   ` Hannes Frederic Sowa
  2013-09-03 21:16     ` Daniel Borkmann
  1 sibling, 1 reply; 19+ messages in thread
From: Hannes Frederic Sowa @ 2013-09-03 21:00 UTC (permalink / raw
  To: Daniel Borkmann; +Cc: davem, netdev

Hi Daniel!

On Tue, Sep 03, 2013 at 09:59:34AM +0200, Daniel Borkmann wrote:
> RFC3810, 10. Security Considerations says under subsection 10.1.
> Query Message:
> 
>   A forged Version 1 Query message will put MLDv2 listeners on that
>   link in MLDv1 Host Compatibility Mode. This scenario can be avoided
>   by providing MLDv2 hosts with a configuration option to ignore
>   Version 1 messages completely.
> 
> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
> 
>   echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version

I just played around with MLDv2-only mode and noticed that the commit message
diverges from the code:

> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index 005b22f..02cd0c5 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
>  	return true;
>  }
>  
> +static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
> +{
> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;

Maybe something like

int val = idev->cnf.force_mld_version ?: dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
return val == 2;

> +}
> +
> +static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
> +{
> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;

Likewise.

> +}
> +
>  static bool mld_in_v1_mode(const struct inet6_dev *idev)
>  {
> -	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
> +	if (mld_in_v2_mode_only(idev))
> +		return false;
> +	if (mld_in_v1_mode_only(idev))
>  		return true;
>  	if (idev->cnf.force_mld_version == 1)
>  		return true;

This last if statement could be dropped then.

Thanks,

  Hannes

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
  2013-09-03 21:00   ` Hannes Frederic Sowa
@ 2013-09-03 21:16     ` Daniel Borkmann
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Borkmann @ 2013-09-03 21:16 UTC (permalink / raw
  To: davem, netdev; +Cc: Hannes Frederic Sowa

On 09/03/2013 11:00 PM, Hannes Frederic Sowa wrote:
> Hi Daniel!
>
> On Tue, Sep 03, 2013 at 09:59:34AM +0200, Daniel Borkmann wrote:
>> RFC3810, 10. Security Considerations says under subsection 10.1.
>> Query Message:
>>
>>    A forged Version 1 Query message will put MLDv2 listeners on that
>>    link in MLDv1 Host Compatibility Mode. This scenario can be avoided
>>    by providing MLDv2 hosts with a configuration option to ignore
>>    Version 1 messages completely.
>>
>> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
>>
>>    echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version
>
> I just played around with MLDv2-only mode and noticed that the commit message
> diverges from the code:
>
>> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
>> index 005b22f..02cd0c5 100644
>> --- a/net/ipv6/mcast.c
>> +++ b/net/ipv6/mcast.c
>> @@ -1112,9 +1112,21 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
>>   	return true;
>>   }
>>
>> +static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
>> +{
>> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 2;
>
> Maybe something like
>
> int val = idev->cnf.force_mld_version ?: dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
> return val == 2;

Hm, true, thanks for spotting. I think it makes sense, first check for individual idev
setting, then for whole namespace. I will update the series and send v2.

>> +}
>> +
>> +static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
>> +{
>> +	return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1;
>
> Likewise.
>
>> +}
>> +
>>   static bool mld_in_v1_mode(const struct inet6_dev *idev)
>>   {
>> -	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version == 1)
>> +	if (mld_in_v2_mode_only(idev))
>> +		return false;
>> +	if (mld_in_v1_mode_only(idev))
>>   		return true;
>>   	if (idev->cnf.force_mld_version == 1)
>>   		return true;
>
> This last if statement could be dropped then.
>
> Thanks,
>
>    Hannes
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-09-03 21:16 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-03  7:59 [PATCH net-next 0/8] IPv6 MLD updates Daniel Borkmann
2013-09-03  7:59 ` [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12 Daniel Borkmann
2013-09-03 17:52   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro Daniel Borkmann
2013-09-03 17:55   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation Daniel Borkmann
2013-09-03 18:09   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only Daniel Borkmann
2013-09-03 18:12   ` Hannes Frederic Sowa
2013-09-03 21:00   ` Hannes Frederic Sowa
2013-09-03 21:16     ` Daniel Borkmann
2013-09-03  7:59 ` [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1 Daniel Borkmann
2013-09-03 18:38   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions Daniel Borkmann
2013-09-03 18:49   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions Daniel Borkmann
2013-09-03 18:51   ` Hannes Frederic Sowa
2013-09-03  7:59 ` [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt Daniel Borkmann
2013-09-03 18:52   ` Hannes Frederic Sowa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.