Netdev List
 help / color / mirror / Atom feed
* [PATCH] sh_eth: fix napi_{en|dis}able() calls racing against interrupts
From: Sergei Shtylyov @ 2013-09-03 22:41 UTC (permalink / raw)
  To: netdev; +Cc: nobuhiro.iwamatsu.yj, linux-sh

While implementing NAPI for the driver, I overlooked the race conditions where
interrupt  handler might have called napi_schedule_prep() before napi_enable()
was called or after napi_disable() was called. If RX interrupt happens, this
would cause the endless interrupts and messages like:

sh-eth eth0: ignoring interrupt, status 0x00040000, mask 0x01ff009f.

The interrupt wouldn't even be masked by the kernel eventually since the handler
would return IRQ_HANDLED all the time.

As a fix, move napi_enable() call before request_irq() call and napi_disable()
call after free_irq() call.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against Dave's 'net.git' repo.  It should be applied to 3.11.y too.

 drivers/net/ethernet/renesas/sh_eth.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Index: net/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net/drivers/net/ethernet/renesas/sh_eth.c
@@ -1857,11 +1857,13 @@ static int sh_eth_open(struct net_device
 
 	pm_runtime_get_sync(&mdp->pdev->dev);
 
+	napi_enable(&mdp->napi);
+
 	ret = request_irq(ndev->irq, sh_eth_interrupt,
 			  mdp->cd->irq_flags, ndev->name, ndev);
 	if (ret) {
 		dev_err(&ndev->dev, "Can not assign IRQ number\n");
-		return ret;
+		goto out_napi_off;
 	}
 
 	/* Descriptor set */
@@ -1879,12 +1881,12 @@ static int sh_eth_open(struct net_device
 	if (ret)
 		goto out_free_irq;
 
-	napi_enable(&mdp->napi);
-
 	return ret;
 
 out_free_irq:
 	free_irq(ndev->irq, ndev);
+out_napi_off:
+	napi_disable(&mdp->napi);
 	pm_runtime_put_sync(&mdp->pdev->dev);
 	return ret;
 }
@@ -1976,8 +1978,6 @@ static int sh_eth_close(struct net_devic
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
 
-	napi_disable(&mdp->napi);
-
 	netif_stop_queue(ndev);
 
 	/* Disable interrupts by clearing the interrupt mask. */
@@ -1995,6 +1995,8 @@ static int sh_eth_close(struct net_devic
 
 	free_irq(ndev->irq, ndev);
 
+	napi_disable(&mdp->napi);
+
 	/* Free all the skbuffs in the Rx queue. */
 	sh_eth_ring_free(ndev);
 

^ permalink raw reply

* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Eric Dumazet @ 2013-09-03 22:29 UTC (permalink / raw)
  To: Jean Sacren; +Cc: Daniel Borkmann, davem, netdev, yoshfuji, Jiri Benc
In-Reply-To: <20130903215915.GF8262@mail.gmail.com>

On Tue, 2013-09-03 at 15:59 -0600, Jean Sacren wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 03 Sep 2013 13:35:17 -0700
> >
> > How did you get your conclusion ?
> 
> If one changes one line of code, doesn't one own that line?
> 

'Owning' like he is the guy who can sell it ? OK I understand now
so many people send 'cleanups' ;)

Someone adding a space is not responsible for the real bug.

You'll have to be very careful to do proper attributions.

This is _critical_ for proper stable submissions.

In this case we want the fix to reach linux-3.4 and further versions.

The commit you pointed out was for linux-3.9, and added no bug.

^ permalink raw reply

* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Daniel Borkmann @ 2013-09-03 22:25 UTC (permalink / raw)
  To: Jean Sacren; +Cc: Eric Dumazet, davem, netdev, yoshfuji, Jiri Benc
In-Reply-To: <20130903215915.GF8262@mail.gmail.com>

On 09/03/2013 11:59 PM, Jean Sacren wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 03 Sep 2013 13:35:17 -0700
>>
>> How did you get your conclusion ?
>
> If one changes one line of code, doesn't one own that line?

The question is not about who currently "owns" the line, but who
first *introduced* an error.

^ permalink raw reply

* [PATCH net-next v2 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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>
---
 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 3be4fe0..ffae869 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)
  */
@@ -1234,15 +1254,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);
 
@@ -2434,14 +2448,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

* [PATCH net-next v2 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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

* [PATCH net-next v2 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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>
---
 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 d7e7c58..3be4fe0 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1218,6 +1218,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)
 {
@@ -1229,7 +1278,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;
@@ -1255,47 +1304,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

* [PATCH net-next v2 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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>
---
 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 24fdfbb..d7e7c58 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);
@@ -1258,12 +1255,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

* [PATCH net-next v2 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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  or
  echo 2 > /proc/sys/net/ipv6/conf/all/force_mld_version

Note that <all> device has a higher precedence as it was previously
also the case in the macro MLD_V1_SEEN() that would "short-circuit"
if condition on <all> case.

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

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 005b22f..24fdfbb 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1112,11 +1112,34 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 	return true;
 }
 
+static int mld_force_mld_version(const struct inet6_dev *idev)
+{
+	/* Normally, both are 0 here. If enforcement to a particular is
+	 * being used, individual device enforcement will have a lower
+	 * precedence over 'all' device (.../conf/all/force_mld_version).
+	 */
+
+	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
+		return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
+	else
+		return idev->cnf.force_mld_version;
+}
+
+static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
+{
+	return mld_force_mld_version(idev) == 2;
+}
+
+static bool mld_in_v1_mode_only(const struct inet6_dev *idev)
+{
+	return mld_force_mld_version(idev) == 1;
+}
+
 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)
+	if (mld_in_v2_mode_only(idev))
+		return false;
+	if (mld_in_v1_mode_only(idev))
 		return true;
 	if (idev->mc_v1_seen && time_before(jiffies, idev->mc_v1_seen))
 		return true;
@@ -1223,7 +1246,6 @@ int igmp6_event_query(struct sk_buff *skb)
 		return -EINVAL;
 
 	idev = __in6_dev_get(skb->dev);
-
 	if (idev == NULL)
 		return 0;
 
@@ -1236,6 +1258,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

* [PATCH net-next v2 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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

* [PATCH net-next v2 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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

* [PATCH net-next v2 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
From: Daniel Borkmann @ 2013-09-03 22:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, David Stevens, Hannes Frederic Sowa
In-Reply-To: <1378246784-21067-1-git-send-email-dborkman@redhat.com>

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

* [PATCH net-next v2 0/8] IPv6 MLD updates
From: Daniel Borkmann @ 2013-09-03 22:19 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.

Changes from v1 to v2:

Minor update in "net: ipv6: mld: implement RFC3810 MLDv2 mode only" in MLD
version selection. Rest stays the same, added Acked-bys from Hannes in
unchanged patches.

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                       | 248 +++++++++++++++++++++++++++------
 5 files changed, 259 insertions(+), 57 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* Re: [PATCH 2/2] sh_eth: add device tree support
From: Kumar Gala @ 2013-09-03 22:08 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev, rob.herring, pawel.moll, mark.rutland, swarren,
	ian.campbell, grant.likely, devicetree, nobuhiro.iwamatsu.yj,
	linux-sh, rob, linux-doc
In-Reply-To: <201308310429.34856.sergei.shtylyov@cogentembedded.com>


On Aug 30, 2013, at 7:29 PM, Sergei Shtylyov wrote:

> Add support of the device tree probing for the Renesas SH-Mobile SoCs.
> 
> This work is loosely based on an original patch by Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com>.
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against Dave's 'net-next.git' repo.
> 
> Documentation/devicetree/bindings/net/sh_eth.txt |   40 +++++++++++++
> drivers/net/ethernet/renesas/sh_eth.c            |   66 ++++++++++++++++++++++-
> 2 files changed, 105 insertions(+), 1 deletion(-)
> 
> Index: net-next/Documentation/devicetree/bindings/net/sh_eth.txt
> ===================================================================
> --- /dev/null
> +++ net-next/Documentation/devicetree/bindings/net/sh_eth.txt
> @@ -0,0 +1,40 @@
> +* Renesas Electronics SH EtherMAC
> +
> +This file provides information on what the device node for the SH EtherMAC
> +interface contains.
> +
> +Required properties:
> +- compatible: "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
> +	      "renesas,ether-r8a7779" if the device is a part of R8A7778/9 SoCs.
> +	      "renesas,ether-r8a7790" if the device is a part of R8A7790/1 SoCs.

Curious, 7779 (vs 7778) but 7790 (vs 7791).

> +- reg: offset and length of the register set for the device; if the device has
> +       TSU registers, you need to specify two register sets here.
> +- interrupt-parent: the phandle for the interrupt controller that services
> +		    interrupts for this device.
> +- interrupts: interrupt mapping for the interrupt source.
> +- phy-mode: string, operation mode of the PHY interface (a string that
> +	    of_get_phy_mode() can understand).
> +- phy-handle: phandle of the PHY device.
> +

Should add something about requiring a phy subnode

> +Optional properties:
> +- local-mac-address: 6 bytes, MAC address.

What's the assumption if local-mac-address is not specified?

> +- renesas,no-ether-link: specify when a board does not provide a proper LINK
> +			 signal.

How is this different from the fixed link concept we have?

> +- renesas,ether-link-active-low: specify when the LINK signal is active-low.
> +
> +Example (Armadillo800EVA board):
> +
> +	ethernet@e9a00000 {
> +		compatible = "renesas,gether-r8a7740";
> +		reg = <0xe9a00000 0x800>, <0xe9a01800 0x800>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 142 0x4>;
> +		phy-mode = "mii";
> +		phy-handle = <&phy0>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		phy0: ethernet-phy@0 {
> +			reg = <0>;
> +		};
> +	};

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCH] openvswitch: fix sw_flow_key alignment
From: Jesse Gross @ 2013-09-03 22:06 UTC (permalink / raw)
  To: David Miller; +Cc: Andy Zhou, dev@openvswitch.org, netdev
In-Reply-To: <20130831.001650.130549489746201651.davem@davemloft.net>

On Fri, Aug 30, 2013 at 9:16 PM, David Miller <davem@davemloft.net> wrote:
> From: Jesse Gross <jesse@nicira.com>
> Date: Fri, 30 Aug 2013 16:20:25 -0700
>
>> I looked through the struct definition and I think that the idea of
>> manually padding as Geert did in his patch will be difficult to
>> maintain over time (and actually there are a few that he missed) since
>> there are a number of different structs/unions contained in there.
>
> You have to be mindful of the gaps and wasted space for performance
> reasons anyways.

Yes, although the approaches for performance and correctness are not
necessarily the same. For example, we're taking about potentially
packing the struct, in which case we would still have the same
alignment needs even without any gaps. If the correctness is clearly
handled (through an explicit align and build assert) then it's easier
to pack/rearrange/whatever for performance since the whole thing isn't
fragile.

^ permalink raw reply

* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Jean Sacren @ 2013-09-03 21:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Daniel Borkmann, davem, netdev, yoshfuji, Jiri Benc
In-Reply-To: <1378240517.7360.41.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 03 Sep 2013 13:35:17 -0700
>
> How did you get your conclusion ?

If one changes one line of code, doesn't one own that line?

-- 
Jean Sacren

^ permalink raw reply

* Re: [PATCH] xen-netback: count number required slots for an skb more carefully
From: Wei Liu @ 2013-09-03 21:53 UTC (permalink / raw)
  To: David Vrabel
  Cc: xen-devel, Konrad Rzeszutek Wilk, Boris Ostrovsky, Ian Campbell,
	netdev, wei.liu2, msw, annie.li
In-Reply-To: <1378229390-12919-1-git-send-email-david.vrabel@citrix.com>

On Tue, Sep 03, 2013 at 06:29:50PM +0100, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> When a VM is providing an iSCSI target and the LUN is used by the
> backend domain, the generated skbs for direct I/O writes to the disk
> have large, multi-page skb->data but no frags.
> 
> With some lengths and starting offsets, xen_netbk_count_skb_slots()
> would be one short because the simple calculation of
> DIV_ROUND_UP(skb_headlen(), PAGE_SIZE) was not accounting for the
> decisions made by start_new_rx_buffer() which does not guarantee
> responses are fully packed.
> 
> For example, a skb with length < 2 pages but which spans 3 pages would
> be counted as requiring 2 slots but would actually use 3 slots.
> 
> skb->data:
> 
>     |        1111|222222222222|3333        |
> 
> Fully packed, this would need 2 slots:
> 
>     |111122222222|22223333    |
> 
> But because the 2nd page wholy fits into a slot it is not split across
> slots and goes into a slot of its own:
> 
>     |1111        |222222222222|3333        |
> 
> Miscounting the number of slots means netback may push more responses
> than the number of available requests.  This will cause the frontend
> to get very confused and report "Too many frags/slots".  The frontend
> never recovers and will eventually BUG.
> 
> Fix this by counting the number of required slots more carefully.  In
> xen_netbk_count_skb_slots(), more closely follow the algorithm used by
> xen_netbk_gop_skb() by introducing xen_netbk_count_frag_slots() which
> is the dry-run equivalent of netbk_gop_frag_copy().
> 

Phew! So this is backend miscounting bug. I thought it was a frontend
bug so it didn't ring a bell when we had our face-to-face discussion,
sorry. :-(

This bug was discussed back in July among Annie, Matt, Ian and I. We
finally agreed to take Matt's solution. Matt agreed to post final
version within a week but obviously he's too busy to do so. I was away
so I didn't follow closely. Eventually it fell through the crack. :-(

Matt, do you fancy sending the final version? IIRC the commit message
needs to be re-written. I personally still prefer Matt's solution as
it a) make efficient use of the ring, b) uses ring pointers to
calculate slots which is most accurate, c) removes the dependence on
MAX_SKB_FRAGS in guest RX path.

Anyway, we should get this fixed ASAP.

Thanks
Wei.

REF:
<1373409659-22383-1-git-send-email-msw@amazon.com>
<1373350520-19985-1-git-send-email-annie.li@oracle.com>


> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> [Resend to actually Cc netdev, sorry.]
> ---
>  drivers/net/xen-netback/netback.c |   94 +++++++++++++++++++++++++------------
>  1 files changed, 64 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 64828de..f149ce5 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -361,6 +361,49 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
>  	return false;
>  }
>  
> +struct xen_netbk_count_slot_state {
> +	unsigned long copy_off;
> +	bool head;
> +};
> +
> +unsigned int xen_netbk_count_frag_slots(struct xenvif *vif,
> +					unsigned long offset, unsigned long size,
> +					struct xen_netbk_count_slot_state *state)
> +{
> +	unsigned count = 0;
> +
> +	offset &= ~PAGE_MASK;
> +
> +	while (size > 0) {
> +		unsigned long bytes;
> +
> +		bytes = PAGE_SIZE - offset;
> +
> +		if (bytes > size)
> +			bytes = size;
> +
> +		if (start_new_rx_buffer(state->copy_off, bytes, state->head)) {
> +			count++;
> +			state->copy_off = 0;
> +		}
> +
> +		if (state->copy_off + bytes > MAX_BUFFER_OFFSET)
> +			bytes = MAX_BUFFER_OFFSET - state->copy_off;
> +
> +		state->copy_off += bytes;
> +
> +		offset += bytes;
> +		size -= bytes;
> +
> +		if (offset == PAGE_SIZE)
> +			offset = 0;
> +
> +		state->head = false;
> +	}
> +
> +	return count;
> +}
> +
>  /*
>   * Figure out how many ring slots we're going to need to send @skb to
>   * the guest. This function is essentially a dry run of
> @@ -368,48 +411,39 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
>   */
>  unsigned int xen_netbk_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
>  {
> +	struct xen_netbk_count_slot_state state;
>  	unsigned int count;
> -	int i, copy_off;
> +	unsigned char *data;
> +	unsigned i;
>  
> -	count = DIV_ROUND_UP(skb_headlen(skb), PAGE_SIZE);
> +	state.head = true;
> +	state.copy_off = 0;
>  
> -	copy_off = skb_headlen(skb) % PAGE_SIZE;
> +	/* Slot for the first (partial) page of data. */
> +	count = 1;
>  
> +	/* Need a slot for the GSO prefix for GSO extra data? */
>  	if (skb_shinfo(skb)->gso_size)
>  		count++;
>  
> -	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> -		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> -		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> -		unsigned long bytes;
> -
> -		offset &= ~PAGE_MASK;
> -
> -		while (size > 0) {
> -			BUG_ON(offset >= PAGE_SIZE);
> -			BUG_ON(copy_off > MAX_BUFFER_OFFSET);
> -
> -			bytes = PAGE_SIZE - offset;
> -
> -			if (bytes > size)
> -				bytes = size;
> +	data = skb->data;
> +	while (data < skb_tail_pointer(skb)) {
> +		unsigned long offset = offset_in_page(data);
> +		unsigned long size = PAGE_SIZE - offset;
>  
> -			if (start_new_rx_buffer(copy_off, bytes, 0)) {
> -				count++;
> -				copy_off = 0;
> -			}
> +		if (data + size > skb_tail_pointer(skb))
> +			size = skb_tail_pointer(skb) - data;
>  
> -			if (copy_off + bytes > MAX_BUFFER_OFFSET)
> -				bytes = MAX_BUFFER_OFFSET - copy_off;
> +		count += xen_netbk_count_frag_slots(vif, offset, size, &state);
>  
> -			copy_off += bytes;
> +		data += size;
> +	}
>  
> -			offset += bytes;
> -			size -= bytes;
> +	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> +		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> +		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
>  
> -			if (offset == PAGE_SIZE)
> -				offset = 0;
> -		}
> +		count += xen_netbk_count_frag_slots(vif, offset, size, &state);
>  	}
>  	return count;
>  }
> -- 
> 1.7.2.5
> 
> --
> 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

^ permalink raw reply

* Re: [-next] openvswitch BUILD_BUG_ON failed
From: Jesse Gross @ 2013-09-03 21:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: David Miller, Andy Zhou, dev@openvswitch.org, netdev,
	Linux Kernel Mailing List, Linux-Next
In-Reply-To: <CAMuHMdU3M9uUFKd3EQ0HbcAvf5e62PA4NJG_mQHhpvZwFJnUWQ@mail.gmail.com>

On Sat, Aug 31, 2013 at 5:11 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Fri, Aug 30, 2013 at 3:11 AM, Jesse Gross <jesse@nicira.com> wrote:
>> On Thu, Aug 29, 2013 at 3:10 PM, David Miller <davem@davemloft.net> wrote:
>>> From: Jesse Gross <jesse@nicira.com>
>>> Date: Thu, 29 Aug 2013 14:42:22 -0700
>>>
>>>> On Thu, Aug 29, 2013 at 2:21 PM, Geert Uytterhoeven
>>>> <geert@linux-m68k.org> wrote:
>>>>> However, I have some doubts about other alignment "enforcements":
>>>>>
>>>>> "__aligned(__alignof__(long))" makes the whole struct aligned to the
>>>>> alignment rule for "long":
>>>>>    1. This is only 2 bytes on m68k, i.e. != sizeof(long).
>>>>>    2. This is 4 bytes on many 32-bit platforms, which may be less than the
>>>>>       default alignment for "__be64" (cfr. some members of struct
>>>>>       ovs_key_ipv4_tunnel), so this may make those 64-bit members unaligned.
>>>>
>>>> Do any of those 32-bit architectures actually care about alignment of
>>>> 64 bit values? On 32-bit x86, a long is 32 bits but the alignment
>>>> requirement of __be64 is also 32 bit.
>>>
>>> All except x86-32 do, it is in fact the odd man out with respect to this
>>> issue.
>>
>> Thanks, good to know.
>>
>> Andy, do you want to modify your patch to just drop the alignment
>> specification as Geert suggested (but definitely keep the new build
>> assert that you added)? It's probably better to just send the patch to
>> netdev (against net-next) as well since you'll likely get better
>> comments there and we can fix this faster if you cut out the
>> middleman.
>
> Why do you want to keep the build asserts?
> Is this in-memory structure also transfered as-is over the network?
> If yes, you definitely want the padding.

Well they caught this bug and really don't cost anything.

> Nevertheless, as the struct contains u32 and even __be64 members, the
> size of the struct will always be a multiple of the alignment unit for
> 64-bit quantities (and thus also for long), as per the C standard.
> Hence the check
>
>     BUILD_BUG_ON(sizeof(struct sw_flow_key) % __alignof__(long));
>
> will only catch bad compiler bugs or people adding __packed to the struct.

It's possible that we might want to pack the structure in the future.
More generally though, the contents of the struct is really
independent of the alignment requirements here because we're accessing
it as an array of bytes in long-sized chunks so implicitly depending
on the size of the members is not that great.

^ permalink raw reply

* Re: [PATCH net-next] tcp: better comments for RTO initiallization
From: Eric Dumazet @ 2013-09-03 21:33 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: davem, ncardwell, edumazet, netdev
In-Reply-To: <1378242875-5172-1-git-send-email-ycheng@google.com>

On Tue, 2013-09-03 at 14:14 -0700, Yuchung Cheng wrote:
> Commit 1b7fdd2ab585("tcp: do not use cached RTT for RTT estimation")
> removes important comments on how RTO is initialized and updated.
> Hopefully this patch puts those information back.
> 
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH net-next] tcp: better comments for RTO initiallization
From: Neal Cardwell @ 2013-09-03 21:31 UTC (permalink / raw)
  To: Yuchung Cheng; +Cc: David Miller, Eric Dumazet, Netdev
In-Reply-To: <1378242875-5172-1-git-send-email-ycheng@google.com>

On Tue, Sep 3, 2013 at 5:14 PM, Yuchung Cheng <ycheng@google.com> wrote:
> Commit 1b7fdd2ab585("tcp: do not use cached RTT for RTT estimation")
> removes important comments on how RTO is initialized and updated.
> Hopefully this patch puts those information back.
>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>
> ---
>  net/ipv4/tcp_metrics.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)

Acked-by: Neal Cardwell <ncardwell@google.com>

Thanks, Yuchung!

neal

^ permalink raw reply

* Possible to add netfilter hooks to IFB driver?
From: Brad Johnson @ 2013-09-03 21:26 UTC (permalink / raw)
  To: netdev

I would like to add netfilter hooks to the IFB driver so I can do 
iptables -j IFB (just like we can with IMQ). But I would like to know 
first if there are any reasons this can not work. Please advise if this 
is possible, and if so any tips would be appreciated.

Cheers,
Brad

^ permalink raw reply

* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Daniel Borkmann @ 2013-09-03 21:16 UTC (permalink / raw)
  To: davem, netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <20130903210011.GB28889@order.stressinduktion.org>

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

* [PATCH net-next] tcp: better comments for RTO initiallization
From: Yuchung Cheng @ 2013-09-03 21:14 UTC (permalink / raw)
  To: davem, ncardwell, edumazet; +Cc: netdev, Yuchung Cheng

Commit 1b7fdd2ab585("tcp: do not use cached RTT for RTT estimation")
removes important comments on how RTO is initialized and updated.
Hopefully this patch puts those information back.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_metrics.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 273ed73..4a22f3e 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -481,13 +481,27 @@ void tcp_init_metrics(struct sock *sk)
 	crtt = tcp_metric_get_jiffies(tm, TCP_METRIC_RTT);
 	rcu_read_unlock();
 reset:
+	/* The initial RTT measurement from the SYN/SYN-ACK is not ideal
+	 * to seed the RTO for later data packets because SYN packets are
+	 * small. Use the per-dst cached values to seed the RTO but keep
+	 * the RTT estimator variables intact (e.g., srtt, mdev, rttvar).
+	 * Later the RTO will be updated immediately upon obtaining the first
+	 * data RTT sample (tcp_rtt_estimator()). Hence the cached RTT only
+	 * influences the first RTO but not later RTT estimation.
+	 *
+	 * But if RTT is not available from the SYN (due to retransmits or
+	 * syn cookies) or the cache, force a conservative 3secs timeout.
+	 *
+	 * A bit of theory. RTT is time passed after "normal" sized packet
+	 * is sent until it is ACKed. In normal circumstances sending small
+	 * packets force peer to delay ACKs and calculation is correct too.
+	 * The algorithm is adaptive and, provided we follow specs, it
+	 * NEVER underestimate RTT. BUT! If peer tries to make some clever
+	 * tricks sort of "quick acks" for time long enough to decrease RTT
+	 * to low value, and then abruptly stops to do it and starts to delay
+	 * ACKs, wait for troubles.
+	 */
 	if (crtt > tp->srtt) {
-		/* Initial RTT (tp->srtt) from SYN usually don't measure
-		 * serialization delay on low BW links well so RTO may be
-		 * under-estimated. Stay conservative and seed RTO with
-		 * the RTTs from past data exchanges, using the same seeding
-		 * formula in tcp_rtt_estimator().
-		 */
 		inet_csk(sk)->icsk_rto = crtt + max(crtt >> 2, tcp_rto_min(sk));
 	} else if (tp->srtt == 0) {
 		/* RFC6298: 5.7 We've failed to get a valid RTT sample from
-- 
1.8.4

^ permalink raw reply related

* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Hannes Frederic Sowa @ 2013-09-03 21:00 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-5-git-send-email-dborkman@redhat.com>

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

* Re: [PATCH v2 0/8] Drop support for Renesas H8/300 architecture
From: Guenter Roeck @ 2013-09-03 20:59 UTC (permalink / raw)
  To: Al Viro
  Cc: Chen Gang F T, linux-kernel, linux-ide, netdev, linux-watchdog,
	Wim Van Sebroeck, David S. Miller, Yoshinori Sato,
	Geert Uytterhoeven, Eric Paris, Greg Kroah-Hartman, Jiang Liu,
	David Howells, Thomas Gleixner, Stephen Rothwell, Linus Torvalds,
	Andrew Morton, Rusty Russell, Linus Walleij
In-Reply-To: <20130903193938.GB13318@ZenIV.linux.org.uk>

On Tue, Sep 03, 2013 at 08:39:38PM +0100, Al Viro wrote:
> On Tue, Sep 03, 2013 at 11:52:17AM +0800, Chen Gang F T wrote:
> 
> >   extreme sample: let 'kernel code style' and 'gcc code style' in one file, that will make the code very ugly.
> 
> gcc style will make any code very ugly, no matter what (if anything) else is
> in the same file...
> 
> [digs out the ports history table]
> x86:		0.01			[alive]
> 	i386:		0.01..2.6.24-rc1 [folded into x86]
> 	x86_64:		2.5.5-pre1..2.6.24-rc1 [folded into x86]
> 	x86:		2.6.24-rc1		[alive]
> alpha:		1.1.67			[alive]
> sparc:		1.1.77			[alive]
> 	sparc64:	2.1.19..2.6.28 [folded into sparc]
> mips:		1.1.82			[alive]
> 	mips64:		2.3.48-pre2..2.6.0-test2 [folded into mips]
> powerpc:	1.3.45			[alive]
> 	ppc:    	1.3.45..2.6.26 [folded into powerpc]
> 	ppc64:		2.5.5..2.6.15-rc1 [folded into powerpc]
> 	powerpc:	2.6.15-rc1		[alive]
> m68k:		1.3.94			[alive]
> 	m68knommu:	2.5.46..2.6.38 [folded into m68k]
> arm:		2.1.80			[alive]
> 	arm26:		2.5.71..2.6.23-rc2 [gone]
> 	arm64:		3.7-rc1			[alive][might eventually fold]
> sh:		2.3.16			[alive]
> 	sh64:		2.6.8-rc1..2.6.24 [folded into sh, nearly dead there]
> ia64:		2.3.43-pre1		[alive]
> s390:		2.3.99pre8		[alive]
> 	s390x:		2.5.0..2.5.67 [folded into s390]
> parisc:		2.4.0-test12		[alive]
> cris:		2.5.0			[alive]
> um:		2.5.35			[alive]
> v850:		2.5.46..2.6.26 [gone]
> h8300:		2.5.68			[moderately responsive]
> m32r:		2.6.9-rc3		[alive]
> frv:		2.6.11-rc1		[alive]
> xtensa:		2.6.13-rc1		[alive]
> avr32:		2.6.19-rc1		[alive]
> blackfin:	2.6.22-rc1		[alive]
> mn10300:	2.6.25-rc1		[alive]
> microblaze:	2.6.30-rc2		[alive]
> score:		2.6.32-rc1		[abandoned][cloned off mips]
> tile:		2.6.36-rc1		[alive]
> unicore32:	2.6.39-rc1		[alive][cloned off arm]
> openrisc:	3.1-rc1			[alive]
> hexagon:	3.2-rc1			[alive]
> c6x:		3.3-rc1			[alive]
> arc:		3.9-rc1			[alive]
> metag:		3.9-rc1			[alive]
> 
> Frankly, I would've expected score and lefotvers of sh64 (aka sh5) to be
> the first against the wall - h8300 was a bit surprising...
> 

Great summary.

There seemed to be a consensus to remove h8300, at least so far and sufficiently
enough for me to ask Stephen to add the removal branch to linux-next.
We'll see if that triggers any further responses.

With score, I am not entirely sure. I got one Ack for the removal, but
on the other side the score maintainers came back and claimed they would
still support it. We'll see if anything changes in practice. I am still
not sure if I should ask for the removal branch to be added to linux-next.
Frankly I thought I might jump the gun here more than with h8300.

Either case, what to ultimately do with those two architectures will be
up to the community to decide.

Guenter

^ permalink raw reply

* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
From: Ben Hutchings @ 2013-09-03 20:53 UTC (permalink / raw)
  To: David Daney; +Cc: David Miller, David Daney, David Daney, netdev
In-Reply-To: <52263872.40209@caviumnetworks.com>

On Tue, Sep 03, 2013 at 12:28:50PM -0700, David Daney wrote:
> On 09/03/2013 12:13 PM, David Daney wrote:
> >On 09/03/2013 11:53 AM, Ben Hutchings wrote:
> >>On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
> >>>On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> >>>>bcm87xx currently isn't auto-loaded if built as a module.
> >>>>
> >>>>Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> >>>>---
> >>>>Compile-tested only.
> >>>
> >>>Then how do you know that it does anything sensible?
> >>
> >>This is generally required in modular PHY drivers.  I was hoping you'd
> >>be able to say whether that it's useful or necessary for this one.
> >
> >
> >OK. I just tested the patch, and it is not sufficient to get the module
> >loaded automatically.
> >
> 
> The problem is that get_phy_c45_ids() sets the phy_id that is passed
> to request_module() to zero, so it will never match anything.

Right.  Now I see that phy_driver::phy_id{,_mask} are actually not
used for this driver because phy_driver::match_phy_device overrides
those.

> We need to think about how this should work for  802.3-c45 PHYs.
> 
> Most c54 PHYs are conceptually composed of several pieces each with
> its own set of identifiers.

Yes, I know that but I wasn't sure how libphy deals with them.

> Which of these should be used?  Will
> something break if we start supplying a C22 phy_id for these
> devices?

But which one?  Maybe it would make sense to generate a modalias for
the device with all MMDs listed:

    mdio:<mmd><id>...

where <mmd> is the MMD number converted to a letter (a-z, A-F) and
<id> is the device ID as a bitstring, and they're repeated for all
present MMDs.  A module would then have modaliases that match on
any particular MMD's ID, e.g. this module would have the aliases:

    mdio:*e00000001010000111011110111000001*
    mdio:*e00000001010000111011111111110000*

(I'm assuming that a driver wouldn't need to match on multiple
MMD IDs, which would make the device table structure and
modpost logic more complicated.)

Maybe there's a simpler way to do it, but I don't know what.

Ben.

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox