Netdev List
 help / color / mirror / Atom feed
* Re: igb + balance-rr + bridge + IPv6 = no go without promiscuous mode
From: Chris Boot @ 2011-12-27 21:53 UTC (permalink / raw)
  To: Nicolas de Pesloüan; +Cc: netdev
In-Reply-To: <4EF45E79.6020803@bootc.net>

On 23/12/2011 10:56, Chris Boot wrote:
> On 23/12/2011 10:48, Nicolas de Pesloüan wrote:
>> [ Forwarded to netdev, because two previous e-mail erroneously sent in
>> HTML ]
>>
>> Le 23/12/2011 11:15, Chris Boot a écrit :
>>> On 23/12/2011 09:52, Nicolas de Pesloüan wrote:
>>>>
>>>>
>>>> Le 23 déc. 2011 10:42, "Chris Boot" <bootc@bootc.net
>>>> <mailto:bootc@bootc.net>> a écrit :
>>>> >
>>>> > Hi folks,
>>>> >
>>>> > As per Eric Dumazet and Dave Miller, I'm opening up a separate
>>>> thread on this issue.
>>>> >
>>>> > I have two identical servers in a cluster for running KVM virtual
>>>> machines. They each have a
>>>> single connection to the Internet (irrelevant for this) and two
>>>> gigabit connections between each
>>>> other for cluster replication, etc... These two connections are in a
>>>> balance-rr bonded connection,
>>>> which is itself member of a bridge that the VMs attach to. I'm
>>>> running v3.2-rc6-140-gb9e26df on
>>>> Debian Wheezy.
>>>> >
>>>> > When the bridge is brought up, IPv4 works fine but IPv6 does not.
>>>> I can use neither the
>>>> automatic link-local on the brid ge nor the static global address I
>>>> assign. Neither machine can
>>>> perform neighbour discovery over the link until I put the bond
>>>> members (eth0 and eth1) into
>>>> promiscuous mode. I can do this either with tcpdump or 'ip link set
>>>> dev ethX promisc on' and this
>>>> is enough to make the link spring to life.
>>>>
>>>> For as far as I remember, setting bond0 to promisc should set the
>>>> bonding member to promisc too.
>>>> And inserting bond0 into br0 should set bond0 to promisc... So
>>>> everything should be in promisc
>>>> mode anyway... but you shoudn't have to do it by hand.
>>>>
>>>
>>> Sorry, I should have added that I tried this. Setting bond0 or br0 to
>>> promisc has no effect. I
>>> discovered this by running tcpdump on br0 first, then bond0, then
>>> eventually each bond member in
>>> turn. Only at the last stage did things jump to life.
>>>
>>>> >
>>>> > This cluster is not currently live so I can easily test patches
>>>> and various configurations.
>>>>
>>>> Can you try to remove the bonding part, connecting eth0 and eth1
>>>> directly to br0 and see if it
>>>> works better? (This is a test ony. I perfectly understand that you
>>>> would loose balance-rr in this
>>>> setup.)
>>>>
>>>
>>> Good call. Let's see.
>>>
>>> I took br0 and bond0 apart, took eth0 and eth1 out of enforced
>>> promisc mode, then manually built a
>>> br0 with eth0 in only so I didn't cause a network loop. Adding eth0
>>> to br0 did not make it go into
>>> promisc mode, but IPv6 does work over this setup. I also made sure ip
>>> -6 neigh was empty on both
>>> machines before I started.
>>>
>>> I then decided to try the test with just the bond0 in balance-rr
>>> mode. Once again I took everything
>>> down and ensured no promisc mode and no ip -6 neigh. I noticed bond0
>>> wasn't getting a link-local and
>>> I found out for some reason
>>> /proc/sys/net/ipv6/conf/bond0/disable_ipv6 was set on both servers so I
>>> set it to 0. That brought things to life.
>>>
>>> So then I put it all back together again and it didn't work. I once
>>> again noticed disable_ipv6 was
>>> set on the bond0 interfaces, now part of the bridge. Toggling this on
>>> the _bond_ interface made
>>> things work again.
>>>
>>> What's setting disable_ipv6? Should this be having an impact if the
>>> port is part of a bridge?
>
> Hmm, as a further update... I brought up my VMs on the bridge with
> disable_ipv6 turned off. The VMs on one host couldn't see what was on
> the other side of the bridge (on the other server) until I turned
> promisc back on manually. So it's not entirely disable_ipv6's fault.

Hi,

I don't want this to get lost around the Christmas break, so I'm just 
resending it. I'm still seeing the same behaviour as before.

 From above:

>>>> For as far as I remember, setting bond0 to promisc should set the
>>>> bonding member to promisc too.
>>>> And inserting bond0 into br0 should set bond0 to promisc... So
>>>> everything should be in promisc
>>>> mode anyway... but you shoudn't have to do it by hand.

This definitely doesn't happen, at least according to 'ip link show | 
grep PROMISC'.

Chris

-- 
Chris Boot
bootc@bootc.net

^ permalink raw reply

* Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor discovery
From: Joshua Kinard @ 2011-12-27 21:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Linux MIPS List
In-Reply-To: <20111227103408.01aad10e@nehalam.linuxnetplumber.net>

On 12/27/2011 13:34, Stephen Hemminger wrote:

> On Tue, 27 Dec 2011 00:06:15 -0500
> Joshua Kinard <kumba@gentoo.org> wrote:
> 
>> @@ -95,7 +95,7 @@ struct mace_video {
>>   * Ethernet interface
>>   */
>>  struct mace_ethernet {
>> -	volatile unsigned long mac_ctrl;
>> +	volatile u64 mac_ctrl;
>>  	volatile unsigned long int_stat;
>>  	volatile unsigned long dma_ctrl;
>>  	volatile unsigned long timer;
> 
> 
> This device driver writer needs to read:
>   Documentation/volatile-considered-harmful.txt

MIPS I/O registers are always memory-mapped, and to prevent the compiler
from trying to over-optimize, volatile is used to make sure we always read a
value from the hardware and not from some cached value.

See MIPS Run (2nd Ed), pp 307, section 10.5.2 highlights an example of this,
which is viewable here:
http://books.google.com/books?id=kk8G2gK4Tw8C&pg=PA307&lpg=PA308#v=onepage&q&f=false

But other than that, yeah, this driver needs to pretty much be stripped down
to the nuts and bolts and re-written.  Maybe something to tackle in the
future.  I still haven't gotten around to submitting the RTC driver for O2's
(that I re-wrote from a patch sent into LKML years ago) upstream yet.

-- 
Joshua Kinard
Gentoo/MIPS
kumba@gentoo.org
4096R/D25D95E3 2011-03-28

"The past tempts us, the present confuses us, the future frightens us.  And
our lives slip away, moment by moment, lost in that vast, terrible in-between."

--Emperor Turhan, Centauri Republic

^ permalink raw reply

* Re: bonding device in balance-alb mode shows packet loss in kernel 3.2-rc6
From: Nicolas de Pesloüan @ 2011-12-27 19:56 UTC (permalink / raw)
  To: Narendra_K; +Cc: netdev, fubar
In-Reply-To: <E31FB011129F30488D5861F38390491520C52BCC8B@BLRX7MCDC201.AMER.DELL.COM>

Le 27/12/2011 15:31, Narendra_K@Dell.com a écrit :
> Hello,
>
> On kernel version 3.2-rc6, when a bonding device is configured in 'balance-alb' mode,
> ping reported packet losses. By looking at protocol trance, it seemed like the lost
> packets had the destination MAC id of inactive slave.
>

[snip]

Can you provide the output of the following command?

grep . /sys/classs/net/bond0/bonding/*

Thanks,

	Nicolas.

^ permalink raw reply

* Re: The mystery of optimistic ipv6 DAD handling
From: David Miller @ 2011-12-27 19:53 UTC (permalink / raw)
  To: netdev; +Cc: nhorman
In-Reply-To: <20111227.140652.1729386263761283235.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 27 Dec 2011 14:06:52 -0500 (EST)

> However, two things strike me:
> 
> 1) even beforehand we'd only get a NULL neigh when the device pointer
>    is NULL, that can't happen here, rt->rt6i_dev will be non-NULL always
>    in these circumstances, it's forced to be net->loopback_dev by the
>    ip6_dst_alloc() call made by addrconf_dst_alloc() (the code back
>    in 2.6.21 does the same exact thing, just inline)
> 
> 2) on top of that it's looking up a neighbour using rt->rt6i_gateway
>    as the key, that's either bogus or pointless.  As far as I can tell
>    it's uninitialized at this point and therefore always all-zeros.
> 
> My quick hunch is that the neigh should be looked up based upon 'addr', and
> that ipv6_add_addr() should test if the neigh is resolved in order to determine
> if the optimistic flag should be cleared.  That matches the logic in the comment
> "do not yet know the link layer address of our nexthop router."
> 
> Neil, any chance you can help me unravel this?

So the fundamental thing is that these addrconf routes are mainly for
for input and looping back traffic on output.

We create the addrconf route "to us", for input.  That's why the lookup
key is the ipv6 address we are configuring on the interface.

For output, the neigh is "don't care".  It goes to the loopback device
and for loopback ->hard_header is NULL so
net/ipv6/ndisc.c:ndisc_constructor() directly hooks up
neigh->ops->queue_xmit as the neigh->output method, which is
dev_queue_xmit().  So it doesn't matter that we lookup the neigh in
addrconf_dst_alloc() using the all-zeros rt->rt6i_gateway.

So this route and it's neigh have nothing to do with link layer
address of any nexthop gatway(s) in this interface.

Therefore the test is completely wrong, and all I can determine is
that we should flat out remove it.  It never triggers anyways.

Perhaps we should find another appropriate spot to put this kind of
test, but in this spot and against this route object seems totally not
right.

Neil, what say you?

--------------------
ipv6: Remove optimistic DAD flag test in ipv6_add_addr()

The route we have here is for the address being added to the interface,
ie. for input packet processing.

Therefore using that route to determine whether an output nexthop gateway
is known and resolved doesn't make any sense.

So, simply remove this test, it never triggered anyways.

Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 59a9d0e..85421cc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -650,16 +650,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr, int pfxlen,
 
 	ifa->rt = rt;
 
-	/*
-	 * part one of RFC 4429, section 3.3
-	 * We should not configure an address as
-	 * optimistic if we do not yet know the link
-	 * layer address of our nexhop router
-	 */
-
-	if (dst_get_neighbour_noref_raw(&rt->dst) == NULL)
-		ifa->flags &= ~IFA_F_OPTIMISTIC;
-
 	ifa->idev = idev;
 	in6_dev_hold(idev);
 	/* For caller */

^ permalink raw reply related

* [PATCH -next] netrom: avoid overflows in nr_setsockopt()
From: Xi Wang @ 2011-12-27 19:44 UTC (permalink / raw)
  To: linux-hams; +Cc: David S. Miller, netdev, Ralf Baechle, Xi Wang

Check setsockopt arguments to avoid overflows and return -EINVAL for
too large arguments.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 net/netrom/af_netrom.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index c329b47..7dab229 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -306,26 +306,26 @@ static int nr_setsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	struct nr_sock *nr = nr_sk(sk);
-	int opt;
+	unsigned long opt;
 
 	if (level != SOL_NETROM)
 		return -ENOPROTOOPT;
 
-	if (optlen < sizeof(int))
+	if (optlen < sizeof(unsigned int))
 		return -EINVAL;
 
-	if (get_user(opt, (int __user *)optval))
+	if (get_user(opt, (unsigned int __user *)optval))
 		return -EFAULT;
 
 	switch (optname) {
 	case NETROM_T1:
-		if (opt < 1)
+		if (opt < 1 || opt > ULONG_MAX / HZ)
 			return -EINVAL;
 		nr->t1 = opt * HZ;
 		return 0;
 
 	case NETROM_T2:
-		if (opt < 1)
+		if (opt < 1 || opt > ULONG_MAX / HZ)
 			return -EINVAL;
 		nr->t2 = opt * HZ;
 		return 0;
@@ -337,13 +337,13 @@ static int nr_setsockopt(struct socket *sock, int level, int optname,
 		return 0;
 
 	case NETROM_T4:
-		if (opt < 1)
+		if (opt < 1 || opt > ULONG_MAX / HZ)
 			return -EINVAL;
 		nr->t4 = opt * HZ;
 		return 0;
 
 	case NETROM_IDLE:
-		if (opt < 0)
+		if (opt > ULONG_MAX / (60 * HZ))
 			return -EINVAL;
 		nr->idle = opt * 60 * HZ;
 		return 0;
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH -next] ax25: avoid overflows in ax25_setsockopt()
From: Xi Wang @ 2011-12-27 19:43 UTC (permalink / raw)
  To: linux-hams; +Cc: David S. Miller, netdev, Xi Wang, Ralf Baechle

Commit be639ac6 ("NET: AX.25: Check ioctl arguments to avoid overflows
further down the road") rejects very large arguments, but doesn't
completely fix overflows on 64-bit systems.  Consider the AX25_T2 case.

	int opt;
	...
	if (opt < 1 || opt > ULONG_MAX / HZ) {
		res = -EINVAL;
		break;
	}
	ax25->t2 = opt * HZ;

The 32-bit multiplication opt * HZ would overflow before being assigned
to 64-bit ax25->t2.  This patch changes "opt" to unsigned long.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
---
 net/ax25/af_ax25.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index b863c18..3cd0a0d 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -545,15 +545,16 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 	ax25_cb *ax25;
 	struct net_device *dev;
 	char devname[IFNAMSIZ];
-	int opt, res = 0;
+	unsigned long opt;
+	int res = 0;
 
 	if (level != SOL_AX25)
 		return -ENOPROTOOPT;
 
-	if (optlen < sizeof(int))
+	if (optlen < sizeof(unsigned int))
 		return -EINVAL;
 
-	if (get_user(opt, (int __user *)optval))
+	if (get_user(opt, (unsigned int __user *)optval))
 		return -EFAULT;
 
 	lock_sock(sk);
@@ -609,7 +610,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case AX25_IDLE:
-		if (opt < 0 || opt > ULONG_MAX / (60 * HZ)) {
+		if (opt > ULONG_MAX / (60 * HZ)) {
 			res = -EINVAL;
 			break;
 		}
@@ -617,7 +618,7 @@ static int ax25_setsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case AX25_BACKOFF:
-		if (opt < 0 || opt > 2) {
+		if (opt > 2) {
 			res = -EINVAL;
 			break;
 		}
-- 
1.7.5.4


^ permalink raw reply related

* Re: [PATCH v2] net: fec: Adjust ENET MDIO timeouts
From: David Miller @ 2011-12-27 19:07 UTC (permalink / raw)
  To: festevam; +Cc: netdev, shawn.guo, kernel, rogerio.pimentel, fabio.estevam
In-Reply-To: <1325012021-3134-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <festevam@gmail.com>
Date: Tue, 27 Dec 2011 16:53:41 -0200

> On extensive NFS boots on a mx6qsabrelite board it was noted that "FEC: MDIO read timeout" were occuring,
> which caused failure on loading the FEC driver.
> 
> The original FEC_MII_TIMEOUT was set to 1 ms, which is too low when passed to the usecs_to_jiffies macro.
> 
> On ARM one jiffy is 10ms, so use a timeout of 30ms, which corresponds to 3 jiffies.
> 
> After running extensive NFS boots, the MDIO timeouts do not occur anymore with this change.
> 
> Signed-off-by: Rogerio Pimentel <rogerio.pimentel@freescale.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied, thanks.

^ permalink raw reply

* The mystery of optimistic ipv6 DAD handling
From: David Miller @ 2011-12-27 19:06 UTC (permalink / raw)
  To: netdev; +Cc: nhorman


This afternoon I audited the neigh attachment logic for ipv6 routes
and found the following gem.

Since the beginning the optimistic DAD support has this code in
ipv6_add_addr():

	/*
	 * part one of RFC 4429, section 3.3
	 * We should not configure an address as
	 * optimistic if we do not yet know the link
	 * layer address of our nexhop router
	 */

	if (dst_get_neighbour_noref_raw(&rt->dst) == NULL)
		ifa->flags &= ~IFA_F_OPTIMISTIC;

(back at inclusion time the test was "rt->rt6i_nexthop == NULL" which
 is the same, we're just hiding it behind a function now so we can
 change the implementation)

But this is never, ever, true.

addrconf_dst_alloc() will never give you a route with a NULL neighbour.

If the neigh lookup fails, it puts the route and returns a pointer error:

	neigh = __neigh_lookup_errno(&nd_tbl, &rt->rt6i_gateway, rt->rt6i_dev);
	if (IS_ERR(neigh)) {
		dst_free(&rt->dst);

		return ERR_CAST(neigh);
	}
	dst_set_neighbour(&rt->dst, neigh);

I verified that back in 2.6.21 when the optimistic DAD code went in,
this same logic was present:

	rt->rt6i_nexthop = ndisc_get_neigh(rt->rt6i_dev, &rt->rt6i_gateway);
	if (rt->rt6i_nexthop == NULL) {
		dst_free(&rt->u.dst);
		return ERR_PTR(-ENOMEM);
	}

But there's a catch, ndisc_get_neigh() has changed over the years, it used to:

static inline struct neighbour * ndisc_get_neigh(struct net_device *dev, struct in6_addr *addr)                                   
{                                                                                                                                 
                                                                                                                                  
        if (dev)                                                                                                                  
                return __neigh_lookup(&nd_tbl, addr, dev, 1);                                                                     
                                                                                                                                  
        return NULL;                                                                                                              
}                                                                                                                                 

but then it was changed to say:

--------------------
 	if (dev)
-		return __neigh_lookup(&nd_tbl, addr, dev, 1);
+		return __neigh_lookup_errno(&nd_tbl, addr, dev);
 
-	return NULL;
+	return ERR_PTR(-ENODEV);
--------------------

in order to fix a DoS type scenerio where the neighbour cache could quickly fill up,
this is commit:

--------------------
commit 14deae41566b5cdd992c01d0069518ced5227c83
Author: David S. Miller <davem@davemloft.net>
Date:   Sun Jan 4 16:04:39 2009 -0800

    ipv6: Fix sporadic sendmsg -EINVAL when sending to multicast groups.
--------------------

Obviously, this could change the optimistic DaD logic.  However, two things strike me:

1) even beforehand we'd only get a NULL neigh when the device pointer
   is NULL, that can't happen here, rt->rt6i_dev will be non-NULL always
   in these circumstances, it's forced to be net->loopback_dev by the
   ip6_dst_alloc() call made by addrconf_dst_alloc() (the code back
   in 2.6.21 does the same exact thing, just inline)

2) on top of that it's looking up a neighbour using rt->rt6i_gateway
   as the key, that's either bogus or pointless.  As far as I can tell
   it's uninitialized at this point and therefore always all-zeros.

My quick hunch is that the neigh should be looked up based upon 'addr', and
that ipv6_add_addr() should test if the neigh is resolved in order to determine
if the optimistic flag should be cleared.  That matches the logic in the comment
"do not yet know the link layer address of our nexthop router."

Neil, any chance you can help me unravel this?

Thanks.

^ permalink raw reply

* Re: Cross posting
From: Andrew Beverley @ 2011-12-27 19:00 UTC (permalink / raw)
  To: John A. Sullivan III; +Cc: lartc, netdev
In-Reply-To: <1324938391.4401.75.camel@denise.theartistscloset.com>

On Mon, 2011-12-26 at 17:26 -0500, John A. Sullivan III wrote:
> Hello, all.  I still find myself posting most of my questions to
> netdev@vger.kernel.org since I'm not quite sure who has found there way
> to lartc yet.
> 
> I know it is normally considered rude to cross post but would it be a
> good idea to cross post until lartc builds up a support community of its
> own and to help build its archives? Thoughts? Comments? Insults? Thanks

I personally think that in this case it would be a good thing to
cross-post.

Andy

^ permalink raw reply

* [PATCH v2] net: fec: Adjust ENET MDIO timeouts
From: Fabio Estevam @ 2011-12-27 18:53 UTC (permalink / raw)
  To: netdev; +Cc: davem, shawn.guo, kernel, Rogerio Pimentel, Fabio Estevam
In-Reply-To: <1324997198-2241-1-git-send-email-festevam@gmail.com>

On extensive NFS boots on a mx6qsabrelite board it was noted that "FEC: MDIO read timeout" were occuring,
which caused failure on loading the FEC driver.

The original FEC_MII_TIMEOUT was set to 1 ms, which is too low when passed to the usecs_to_jiffies macro.

On ARM one jiffy is 10ms, so use a timeout of 30ms, which corresponds to 3 jiffies.

After running extensive NFS boots, the MDIO timeouts do not occur anymore with this change.

Signed-off-by: Rogerio Pimentel <rogerio.pimentel@freescale.com>
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v1:
- Keep the timeout unit in us.

 drivers/net/ethernet/freescale/fec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index c136230..c54cc62 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -255,7 +255,7 @@ struct fec_enet_private {
 #define FEC_MMFR_TA		(2 << 16)
 #define FEC_MMFR_DATA(v)	(v & 0xffff)
 
-#define FEC_MII_TIMEOUT		1000 /* us */
+#define FEC_MII_TIMEOUT		30000 /* us */
 
 /* Transmitter timeout */
 #define TX_TIMEOUT (2 * HZ)
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor discovery
From: David Miller @ 2011-12-27 18:52 UTC (permalink / raw)
  To: shemminger; +Cc: kumba, netdev, linux-mips
In-Reply-To: <20111227103408.01aad10e@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 27 Dec 2011 10:34:08 -0800

> On Tue, 27 Dec 2011 00:06:15 -0500
> Joshua Kinard <kumba@gentoo.org> wrote:
> 
>> @@ -95,7 +95,7 @@ struct mace_video {
>>   * Ethernet interface
>>   */
>>  struct mace_ethernet {
>> -	volatile unsigned long mac_ctrl;
>> +	volatile u64 mac_ctrl;
>>  	volatile unsigned long int_stat;
>>  	volatile unsigned long dma_ctrl;
>>  	volatile unsigned long timer;
> 
> 
> This device driver writer needs to read:
>   Documentation/volatile-considered-harmful.txt

This driver has a lot of problems, some of which I've made Joshua aware of
already.

^ permalink raw reply

* Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor discovery
From: Stephen Hemminger @ 2011-12-27 18:34 UTC (permalink / raw)
  To: Joshua Kinard; +Cc: netdev, Linux MIPS List
In-Reply-To: <4EF95247.7000403@gentoo.org>

On Tue, 27 Dec 2011 00:06:15 -0500
Joshua Kinard <kumba@gentoo.org> wrote:

> @@ -95,7 +95,7 @@ struct mace_video {
>   * Ethernet interface
>   */
>  struct mace_ethernet {
> -	volatile unsigned long mac_ctrl;
> +	volatile u64 mac_ctrl;
>  	volatile unsigned long int_stat;
>  	volatile unsigned long dma_ctrl;
>  	volatile unsigned long timer;


This device driver writer needs to read:
  Documentation/volatile-considered-harmful.txt

^ permalink raw reply

* Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor discovery
From: David Miller @ 2011-12-27 18:17 UTC (permalink / raw)
  To: kumba; +Cc: netdev, linux-mips
In-Reply-To: <4EF95247.7000403@gentoo.org>

From: Joshua Kinard <kumba@gentoo.org>
Date: Tue, 27 Dec 2011 00:06:15 -0500

> SGI IP32 (O2)'s ethernet driver (meth) lacks a set_rx_mode function, which
> prevents IPv6 from working completely because any ICMPv6 neighbor
> solicitation requests aren't picked up by the driver.  So the machine can
> ping out and connect to other systems, but other systems will have a very
> hard time connecting to the O2.
> 
> Signed-off-by: Joshua Kinard <kumba@gentoo.org>

Applied to net-next, thanks.

^ permalink raw reply

* RE: [PATCH net-next V2 0/2] igb: ptp hardware clock
From: Ronciak, John @ 2011-12-27 18:17 UTC (permalink / raw)
  To: Richard Cochran, netdev@vger.kernel.org
  Cc: e1000-devel@lists.sourceforge.net, Keller, Jacob E,
	Kirsher, Jeffrey T, John Stultz, Thomas Gleixner
In-Reply-To: <cover.1324901880.git.richardcochran@gmail.com>

> I have tested the 82580 with good results. However, I don't have the
> 82576 and so would appreciate testing and feedback.
> 
Richard,

We'll test this on an 82576 but that won't happen until next week we staff is back from the holidays.

Cheers,
John

^ permalink raw reply

* Re: [PATCH] packet: fix dev refcnt leak when bind fail
From: David Miller @ 2011-12-27 18:15 UTC (permalink / raw)
  To: weiyj.lk; +Cc: netdev
In-Reply-To: <CAPgLHd9DPsxBnhH_xvuApjy32X--jxYiDn4DmxmR-Uqq8wJP+A@mail.gmail.com>

From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Tue, 27 Dec 2011 12:28:31 +0800

> cc netdev@vger.kernel.org
> 
>> From: Wei Yongjun <weiyj.lk@gmail.com>
>> Date: Mon, 26 Dec 2011 17:02:59 +0800
>>
>>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>>
>>> If bind is fail such as bind is called after set PACKET_FANOUT
>>> sock option, the dev refcnt will leak.
>>>
>>> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> The device pointer is stored in ->prot_hook.dev
>>
>> Therefore, it will be released at the latest in packet_release() or
>> earlier if another bind() attempt is done on the socket.
>>
>> There is no leak here as far as I can see.
> 
> Hi David,
> 
> In packet_do_bind(), if po->fanout is set, the device pointer may not be
> stored in ->prot_hook.dev, see the following code:
> 
> static int packet_do_bind(struct sock *sk, struct net_device *dev, ...)
> {
>    struct packet_sock *po = pkt_sk(sk);
> 
>    if (po->fanout)
>        return -EINVAL;
>    ...
>    po->prot_hook.dev = dev;
>    ...
> }
> 
> The leak is exists only if po->fanout is set.

Then it sounds a lot simpler for packet_do_bind() (one location) to
manage this reference drop, rather than every single call site of
packet_do_bind().

Look how complicated you had to make the tests in those call sites
to get the logic right, it's must easier to just add the dev_put()
right at that po->fanout test.

^ permalink raw reply

* Re: [PATCH] net: fec: Adjust ENET MDIO timeouts
From: David Miller @ 2011-12-27 18:13 UTC (permalink / raw)
  To: festevam; +Cc: netdev, shawn.guo, kernel, rogerio.pimentel, fabio.estevam
In-Reply-To: <1324997198-2241-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <festevam@gmail.com>
Date: Tue, 27 Dec 2011 12:46:38 -0200

> -#define FEC_MII_TIMEOUT		1000 /* us */
> +#define FEC_MII_TIMEOUT		30 /* ms */

I don't see why you must use msecs_to_jiffies(), why not just change
FEC_MII_TIMEOUT to 30000?

That's a one-line, easy to validate, change.

Whereas if you also change the units of the timeout I have to go
through the entire driver to audit your patch, making sure none of the
FEC_MII_TIMEOUT uses in the driver have been missed.

When you make multiple changes at once, it leads to more work for the
patch reviewer, please avoid this whenever possible.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 00/23] TIPC: Updates for what will be v3.3
From: David Miller @ 2011-12-27 18:08 UTC (permalink / raw)
  To: paul.gortmaker; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Tue, 27 Dec 2011 12:39:19 -0500

> Here are some more updates for TIPC, based on the backlog of stuff
> that was unfortunately stacked away in sourceforge a while ago.
> 
> Given the timing, please feel free to defer these to the for-3.4
> net-next if there isn't a convenient opportunity to process it
> before 3.2 is officially released.  I can easily resend later.
> 
> Fortunately, the SF backlog will finally be 100% gone with the
> pending stuff that will be sent in the next year for the 3.4
> net-next, so that will be nice to have that task closed out.
> 
> I just finished testing this over the wire between an x86-32 and
> an x86-64 box, with both taking turns being server and client.

Pulled, thanks Paul.

Two suggestions:

1) Please rename things like "struct media" to something more global
   namespace friendly like "struct tipc_media"

2) We really should get rid of the ad-hoc debug logging facility tipc
   uses.  It's not like there aren't tons of generic, well maintained,
   feature-full logging facilities available.  There is no reason for
   tipc to have it's own special, private, logginc facility.

   If the generic facilities lack some feature, add it and submit
   it for inclusion to linux-kernel so everyone can use it, not just
   TIPC.

^ permalink raw reply

* Re: [PATCH -next] net: calxeda xgmac ethernet driver add missing HAS_IOMEM dependency
From: David Miller @ 2011-12-27 18:01 UTC (permalink / raw)
  To: rob.herring; +Cc: heiko.carstens, netdev, linux-kernel
In-Reply-To: <4EF9D327.3070808@calxeda.com>

From: Rob Herring <rob.herring@calxeda.com>
Date: Tue, 27 Dec 2011 08:16:07 -0600

> On 12/27/2011 08:07 AM, Heiko Carstens wrote:
>> Fix allyesconfig build on architectures without IOMEM:
>> 
>> drivers/net/ethernet/calxeda/xgmac.c:1800:2:
>>   error: implicit declaration of function 'iounmap' [-Werror=implicit-function-declaration]
>> 
>> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
>> Cc: Rob Herring <rob.herring@calxeda.com>
> 
> Thanks.
> 
> Acked-by: Rob Herring <rob.herring@calxeda.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] net/ucc_geth: some fix in current kernel
From: David Miller @ 2011-12-27 17:59 UTC (permalink / raw)
  To: X.Xie; +Cc: linuxppc-dev, netdev, leoli, Haiying.Wang
In-Reply-To: <1324979328-32081-1-git-send-email-X.Xie@freescale.com>

From: Xie Xiaobo <X.Xie@freescale.com>
Date: Tue, 27 Dec 2011 17:48:48 +0800

> * Revert commit "ucc_geth: Fix hangs after switching from full to half duplex"
>   This commit impacted the driver in all link state change more than
>   duplex change.
> * Change some parameters.
>   Increased the BD ring length.
> 
> Signed-off-by: Haiying Wang <Haiying.Wang@freescale.com>
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>

Please, one major change per patch.

^ permalink raw reply

* Re: [PATCH] skge: restore multicast rx filter on resume
From: Stephen Hemminger @ 2011-12-27 17:56 UTC (permalink / raw)
  To: David Miller; +Cc: florz, netdev
In-Reply-To: <20111226.151754.417402855307646295.davem@davemloft.net>

On Mon, 26 Dec 2011 15:17:54 -0500 (EST)
David Miller <davem@davemloft.net> wrote:

> From: Florian Zumbiehl <florz@florz.de>
> Date: Sun, 25 Dec 2011 09:18:33 +0100
> 
> > skge: restore multicast rx filter on resume
> >     
> > Signed-off-by: Florian Zumbiehl <florz@florz.de>
> 
> Stephen?  This one looks fine to me.
> 
> > diff --git a/drivers/net/skge.c b/drivers/net/skge.c
> > index f4be5c7..cd968e5 100644
> > --- a/drivers/net/skge.c
> > +++ b/drivers/net/skge.c
> > @@ -4046,6 +4046,7 @@ static int skge_resume(struct device *dev)
> >  				dev_close(dev);
> >  				goto out;
> >  			}
> > +			skge_set_multicast(dev);
> >  		}
> >  	}
> >  out:

Right idea, but it needs to be done in a different spot to catch
all the other device restart cases like changing settings.

^ permalink raw reply

* [PATCH net-next 23/23] tipc: Allow use of buf_seqno() helper routine by unicast links
From: Paul Gortmaker @ 2011-12-27 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, allan.stephens, ying.xue, Paul Gortmaker
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Allan Stephens <allan.stephens@windriver.com>

Migrates the buf_seqno() helper routine from broadcast link level to
unicast link level so that it can be used both types of TIPC links.
This is a cosmetic change only, and does not affect the operation of TIPC.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |    5 -----
 net/tipc/link.c  |   39 +++++++++++++++++++--------------------
 net/tipc/link.h  |    5 +++++
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 15eb744..048b7a3 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -115,11 +115,6 @@ static void tipc_nmap_diff(struct tipc_node_map *nm_a,
 			   struct tipc_node_map *nm_b,
 			   struct tipc_node_map *nm_diff);
 
-static u32 buf_seqno(struct sk_buff *buf)
-{
-	return msg_seqno(buf_msg(buf));
-}
-
 static u32 bcbuf_acks(struct sk_buff *buf)
 {
 	return (u32)(unsigned long)TIPC_SKB_CB(buf)->handle;
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 4eff342..853b286 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -130,7 +130,7 @@ static void link_init_max_pkt(struct link *l_ptr)
 static u32 link_next_sent(struct link *l_ptr)
 {
 	if (l_ptr->next_out)
-		return msg_seqno(buf_msg(l_ptr->next_out));
+		return buf_seqno(l_ptr->next_out);
 	return mod(l_ptr->next_out_no);
 }
 
@@ -1354,7 +1354,7 @@ u32 tipc_link_push_packet(struct link *l_ptr)
 	if (r_q_size && buf) {
 		u32 last = lesser(mod(r_q_head + r_q_size),
 				  link_last_sent(l_ptr));
-		u32 first = msg_seqno(buf_msg(buf));
+		u32 first = buf_seqno(buf);
 
 		while (buf && less(first, r_q_head)) {
 			first = mod(first + 1);
@@ -1403,7 +1403,7 @@ u32 tipc_link_push_packet(struct link *l_ptr)
 	if (buf) {
 		struct tipc_msg *msg = buf_msg(buf);
 		u32 next = msg_seqno(msg);
-		u32 first = msg_seqno(buf_msg(l_ptr->first_out));
+		u32 first = buf_seqno(l_ptr->first_out);
 
 		if (mod(next - first) < l_ptr->queue_limit[0]) {
 			msg_set_ack(msg, mod(l_ptr->next_in_no - 1));
@@ -1558,7 +1558,7 @@ void tipc_link_retransmit(struct link *l_ptr, struct sk_buff *buf,
 		} else {
 			tipc_bearer_schedule(l_ptr->b_ptr, l_ptr);
 			l_ptr->stats.bearer_congs++;
-			l_ptr->retransm_queue_head = msg_seqno(buf_msg(buf));
+			l_ptr->retransm_queue_head = buf_seqno(buf);
 			l_ptr->retransm_queue_size = retransmits;
 			return;
 		}
@@ -1579,7 +1579,7 @@ static struct sk_buff *link_insert_deferred_queue(struct link *l_ptr,
 	if (l_ptr->oldest_deferred_in == NULL)
 		return buf;
 
-	seq_no = msg_seqno(buf_msg(l_ptr->oldest_deferred_in));
+	seq_no = buf_seqno(l_ptr->oldest_deferred_in);
 	if (seq_no == mod(l_ptr->next_in_no)) {
 		l_ptr->newest_deferred_in->next = buf;
 		buf = l_ptr->oldest_deferred_in;
@@ -1738,7 +1738,7 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
 
 		crs = l_ptr->first_out;
 		while ((crs != l_ptr->next_out) &&
-		       less_eq(msg_seqno(buf_msg(crs)), ackd)) {
+		       less_eq(buf_seqno(crs), ackd)) {
 			struct sk_buff *next = crs->next;
 
 			buf_discard(crs);
@@ -1861,7 +1861,7 @@ u32 tipc_link_defer_pkt(struct sk_buff **head,
 {
 	struct sk_buff *prev = NULL;
 	struct sk_buff *crs = *head;
-	u32 seq_no = msg_seqno(buf_msg(buf));
+	u32 seq_no = buf_seqno(buf);
 
 	buf->next = NULL;
 
@@ -1872,7 +1872,7 @@ u32 tipc_link_defer_pkt(struct sk_buff **head,
 	}
 
 	/* Last ? */
-	if (less(msg_seqno(buf_msg(*tail)), seq_no)) {
+	if (less(buf_seqno(*tail), seq_no)) {
 		(*tail)->next = buf;
 		*tail = buf;
 		return 1;
@@ -1909,7 +1909,7 @@ u32 tipc_link_defer_pkt(struct sk_buff **head,
 static void link_handle_out_of_seq_msg(struct link *l_ptr,
 				       struct sk_buff *buf)
 {
-	u32 seq_no = msg_seqno(buf_msg(buf));
+	u32 seq_no = buf_seqno(buf);
 
 	if (likely(msg_user(buf_msg(buf)) == LINK_PROTOCOL)) {
 		link_recv_proto_msg(l_ptr, buf);
@@ -1971,10 +1971,10 @@ void tipc_link_send_proto_msg(struct link *l_ptr, u32 msg_typ, int probe_msg,
 		if (!tipc_link_is_up(l_ptr))
 			return;
 		if (l_ptr->next_out)
-			next_sent = msg_seqno(buf_msg(l_ptr->next_out));
+			next_sent = buf_seqno(l_ptr->next_out);
 		msg_set_next_sent(msg, next_sent);
 		if (l_ptr->oldest_deferred_in) {
-			u32 rec = msg_seqno(buf_msg(l_ptr->oldest_deferred_in));
+			u32 rec = buf_seqno(l_ptr->oldest_deferred_in);
 			gap = mod(rec - mod(l_ptr->next_in_no));
 		}
 		msg_set_seq_gap(msg, gap);
@@ -2589,7 +2589,7 @@ int tipc_link_recv_fragment(struct sk_buff **pending, struct sk_buff **fb,
 
 	/* Is there an incomplete message waiting for this fragment? */
 
-	while (pbuf && ((msg_seqno(buf_msg(pbuf)) != long_msg_seq_no) ||
+	while (pbuf && ((buf_seqno(pbuf) != long_msg_seq_no) ||
 			(msg_orignode(fragm) != msg_orignode(buf_msg(pbuf))))) {
 		prev = pbuf;
 		pbuf = pbuf->next;
@@ -3112,13 +3112,12 @@ static void link_print(struct link *l_ptr, const char *str)
 	tipc_printf(buf, "NXI(%u):", mod(l_ptr->next_in_no));
 	tipc_printf(buf, "SQUE");
 	if (l_ptr->first_out) {
-		tipc_printf(buf, "[%u..", msg_seqno(buf_msg(l_ptr->first_out)));
+		tipc_printf(buf, "[%u..", buf_seqno(l_ptr->first_out));
 		if (l_ptr->next_out)
-			tipc_printf(buf, "%u..",
-				    msg_seqno(buf_msg(l_ptr->next_out)));
-		tipc_printf(buf, "%u]", msg_seqno(buf_msg(l_ptr->last_out)));
-		if ((mod(msg_seqno(buf_msg(l_ptr->last_out)) -
-			 msg_seqno(buf_msg(l_ptr->first_out)))
+			tipc_printf(buf, "%u..", buf_seqno(l_ptr->next_out));
+		tipc_printf(buf, "%u]", buf_seqno(l_ptr->last_out));
+		if ((mod(buf_seqno(l_ptr->last_out) -
+			 buf_seqno(l_ptr->first_out))
 		     != (l_ptr->out_queue_size - 1)) ||
 		    (l_ptr->last_out->next != NULL)) {
 			tipc_printf(buf, "\nSend queue inconsistency\n");
@@ -3130,8 +3129,8 @@ static void link_print(struct link *l_ptr, const char *str)
 		tipc_printf(buf, "[]");
 	tipc_printf(buf, "SQSIZ(%u)", l_ptr->out_queue_size);
 	if (l_ptr->oldest_deferred_in) {
-		u32 o = msg_seqno(buf_msg(l_ptr->oldest_deferred_in));
-		u32 n = msg_seqno(buf_msg(l_ptr->newest_deferred_in));
+		u32 o = buf_seqno(l_ptr->oldest_deferred_in);
+		u32 n = buf_seqno(l_ptr->newest_deferred_in);
 		tipc_printf(buf, ":RQUE[%u..%u]", o, n);
 		if (l_ptr->deferred_inqueue_sz != mod((n + 1) - o)) {
 			tipc_printf(buf, ":RQSIZ(%u)",
diff --git a/net/tipc/link.h b/net/tipc/link.h
index 7879239..d35b0f3 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -254,6 +254,11 @@ void tipc_link_retransmit(struct link *l_ptr, struct sk_buff *start, u32 retrans
  * Link sequence number manipulation routines (uses modulo 2**16 arithmetic)
  */
 
+static inline u32 buf_seqno(struct sk_buff *buf)
+{
+	return msg_seqno(buf_msg(buf));
+}
+
 static inline u32 mod(u32 x)
 {
 	return x & 0xffffu;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH net-next 22/23] tipc: Ignore broadcast acknowledgements that are out-of-range
From: Paul Gortmaker @ 2011-12-27 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, allan.stephens, ying.xue, Paul Gortmaker
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Allan Stephens <allan.stephens@windriver.com>

Adds checks to TIPC's broadcast link so that it ignores any
acknowledgement message containing a sequence number that does not
correspond to an unacknowledged message currently in the broadcast
link's transmit queue.

This change prevents the broadcast link from becoming stalled if a
newly booted node receives stale broadcast link acknowledgement
information from another node that has not yet fully synchronized
its end of the broadcast link to reflect the current state of the
new node's end.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |   35 ++++++++++++++++++++++++++++-------
 net/tipc/link.c  |    6 ++----
 net/tipc/link.h  |    6 ++++++
 net/tipc/node.c  |    3 +--
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 4609819..15eb744 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -237,14 +237,36 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 	struct sk_buff *next;
 	unsigned int released = 0;
 
-	if (less_eq(acked, n_ptr->bclink.acked))
-		return;
-
 	spin_lock_bh(&bc_lock);
 
-	/* Skip over packets that node has previously acknowledged */
-
+	/* Bail out if tx queue is empty (no clean up is required) */
 	crs = bcl->first_out;
+	if (!crs)
+		goto exit;
+
+	/* Determine which messages need to be acknowledged */
+	if (acked == INVALID_LINK_SEQ) {
+		/*
+		 * Contact with specified node has been lost, so need to
+		 * acknowledge sent messages only (if other nodes still exist)
+		 * or both sent and unsent messages (otherwise)
+		 */
+		if (bclink->bcast_nodes.count)
+			acked = bcl->fsm_msg_cnt;
+		else
+			acked = bcl->next_out_no;
+	} else {
+		/*
+		 * Bail out if specified sequence number does not correspond
+		 * to a message that has been sent and not yet acknowledged
+		 */
+		if (less(acked, buf_seqno(crs)) ||
+		    less(bcl->fsm_msg_cnt, acked) ||
+		    less_eq(acked, n_ptr->bclink.acked))
+			goto exit;
+	}
+
+	/* Skip over packets that node has previously acknowledged */
 	while (crs && less_eq(buf_seqno(crs), n_ptr->bclink.acked))
 		crs = crs->next;
 
@@ -255,8 +277,6 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 
 		if (crs != bcl->next_out)
 			bcbuf_decr_acks(crs);
-		else if (bclink->bcast_nodes.count)
-			break;
 		else {
 			bcbuf_set_acks(crs, 0);
 			bcl->next_out = next;
@@ -281,6 +301,7 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 	}
 	if (unlikely(released && !list_empty(&bcl->waiting_ports)))
 		tipc_link_wakeup_ports(bcl, 0);
+exit:
 	spin_unlock_bh(&bc_lock);
 }
 
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 332915e..4eff342 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1733,10 +1733,8 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
 
 		/* Release acked messages */
 
-		if (less(n_ptr->bclink.acked, msg_bcast_ack(msg))) {
-			if (tipc_node_is_up(n_ptr) && n_ptr->bclink.supported)
-				tipc_bclink_acknowledge(n_ptr, msg_bcast_ack(msg));
-		}
+		if (tipc_node_is_up(n_ptr) && n_ptr->bclink.supported)
+			tipc_bclink_acknowledge(n_ptr, msg_bcast_ack(msg));
 
 		crs = l_ptr->first_out;
 		while ((crs != l_ptr->next_out) &&
diff --git a/net/tipc/link.h b/net/tipc/link.h
index e56cb53..7879239 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -45,6 +45,12 @@
 #define PUSH_FINISHED 2
 
 /*
+ * Out-of-range value for link sequence numbers
+ */
+
+#define INVALID_LINK_SEQ 0x10000
+
+/*
  * Link states
  */
 
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 1861ae9..e530de2 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -351,8 +351,7 @@ static void node_lost_contact(struct tipc_node *n_ptr)
 		}
 
 		tipc_bclink_remove_node(n_ptr->addr);
-		tipc_bclink_acknowledge(n_ptr,
-					mod(n_ptr->bclink.acked + 10000));
+		tipc_bclink_acknowledge(n_ptr, INVALID_LINK_SEQ);
 		if (n_ptr->addr < tipc_own_addr)
 			tipc_own_tag--;
 
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH net-next 21/23] tipc: Flush unsent broadcast messages when contact with last node is lost
From: Paul Gortmaker @ 2011-12-27 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, allan.stephens, ying.xue, Paul Gortmaker
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Allan Stephens <allan.stephens@windriver.com>

Adds code to release any unsent broadcast messages in the broadcast link
transmit queue if TIPC loses contact with its only neighboring node.
Previously, a broadcast link that was in the congested state would hold
on to the unsent messages, even though the messages were now undeliverable.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index dd990b0..4609819 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -252,7 +252,17 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 
 	while (crs && less_eq(buf_seqno(crs), acked)) {
 		next = crs->next;
-		bcbuf_decr_acks(crs);
+
+		if (crs != bcl->next_out)
+			bcbuf_decr_acks(crs);
+		else if (bclink->bcast_nodes.count)
+			break;
+		else {
+			bcbuf_set_acks(crs, 0);
+			bcl->next_out = next;
+			bclink_set_last_sent();
+		}
+
 		if (bcbuf_acks(crs) == 0) {
 			bcl->first_out = next;
 			bcl->out_queue_size--;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH net-next 20/23] tipc: Minor optimization of broadcast link transmit queue statistic
From: Paul Gortmaker @ 2011-12-27 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, allan.stephens, ying.xue, Paul Gortmaker
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Allan Stephens <allan.stephens@windriver.com>

The two broadcast link statistics fields that are used to derive the
average length of that link's transmit queue are now updated only after
a successful attempt to send a broadcast message, since there is no need
to update these values when an unsuccessful send attempt leaves the
queue unchanged.

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 8f58df2..dd990b0 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -424,11 +424,11 @@ int tipc_bclink_send_msg(struct sk_buff *buf)
 	}
 
 	res = tipc_link_send_buf(bcl, buf);
-	if (likely(res > 0))
+	if (likely(res >= 0)) {
 		bclink_set_last_sent();
-
-	bcl->stats.queue_sz_counts++;
-	bcl->stats.accu_queue_sz += bcl->out_queue_size;
+		bcl->stats.queue_sz_counts++;
+		bcl->stats.accu_queue_sz += bcl->out_queue_size;
+	}
 exit:
 	spin_unlock_bh(&bc_lock);
 	return res;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH net-next 19/23] tipc: Handle broadcast attempt when no neighboring nodes exist
From: Paul Gortmaker @ 2011-12-27 17:39 UTC (permalink / raw)
  To: davem; +Cc: netdev, allan.stephens, ying.xue, Paul Gortmaker
In-Reply-To: <1325007582-31610-1-git-send-email-paul.gortmaker@windriver.com>

From: Allan Stephens <allan.stephens@windriver.com>

Adds a check to detect when an attempt is made to send a message
via the broadcast link and no neighboring nodes are currently available
to receive it. Rather than wasting effort passing the message to the
broadcast link and broadcast bearer, who will only throw it away,
TIPC now frees the message immediately and reports success (i.e. the
message has been delivered to all available destinations).

Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 net/tipc/bcast.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c
index 5ca8fdd..8f58df2 100644
--- a/net/tipc/bcast.c
+++ b/net/tipc/bcast.c
@@ -417,13 +417,19 @@ int tipc_bclink_send_msg(struct sk_buff *buf)
 
 	spin_lock_bh(&bc_lock);
 
+	if (!bclink->bcast_nodes.count) {
+		res = msg_data_sz(buf_msg(buf));
+		buf_discard(buf);
+		goto exit;
+	}
+
 	res = tipc_link_send_buf(bcl, buf);
 	if (likely(res > 0))
 		bclink_set_last_sent();
 
 	bcl->stats.queue_sz_counts++;
 	bcl->stats.accu_queue_sz += bcl->out_queue_size;
-
+exit:
 	spin_unlock_bh(&bc_lock);
 	return res;
 }
-- 
1.7.4.4

^ permalink raw reply related


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