Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Use list_head-s in inetpeer.c
From: Pavel Emelyanov @ 2007-11-12  8:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, devel
In-Reply-To: <20071110.213107.94486766.davem@davemloft.net>

David Miller wrote:
> From: Pavel Emelyanov <xemul@openvz.org>
> Date: Sat, 10 Nov 2007 17:32:58 +0300
> 
>> The inetpeer.c tracks the LRU list of inet_perr-s, but makes
>> it by hands. Use the list_head-s for this.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> 
> This makes every inetpeer struct consume 8 more bytes, and on some
> systems we have can have many of these objects active.  That space
> savings is why this was done the way it was.

No. I remove _two_ pointers unused_next and unused_prevp, and add
the list_head, which is _two_ pointers as well. I've even checked the
compilation on both i386 and x86_64 - the sizeof(struct inet_peer) 
is not changed.

You must have overlooked the unused_prevp member, because it is 
declared in the same line as the unused_next. Or I miss something else?

> It would be nice to have "tailq" like interfaces in linux/list.h
> for situations like this.
> 
> Please do not submit a patch implementing that until the 2.6.25
> merge window, however, thanks.

If my explanation above is correct, should I delay this patch until
the 2.6.25 anyway?

Thanks,
Pavel

^ permalink raw reply

* Re: [PATCH] Use list_head-s in inetpeer.c
From: David Miller @ 2007-11-12  9:42 UTC (permalink / raw)
  To: xemul; +Cc: netdev, devel
In-Reply-To: <47381573.5020506@openvz.org>

From: Pavel Emelyanov <xemul@openvz.org>
Date: Mon, 12 Nov 2007 11:57:23 +0300

> No. I remove _two_ pointers unused_next and unused_prevp, and add
> the list_head, which is _two_ pointers as well. I've even checked the
> compilation on both i386 and x86_64 - the sizeof(struct inet_peer) 
> is not changed.
> 
> You must have overlooked the unused_prevp member, because it is 
> declared in the same line as the unused_next. Or I miss something else?

I missed that, please resubmit your patch.

Thanks.

^ permalink raw reply

* Re: [PATCH] NET: Add the helper kernel_sock_shutdown()
From: David Howells @ 2007-11-12 12:22 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: dhowells, netdev, Paul Clements, Mark Fasheh, Steve French,
	David S Miller
In-Reply-To: <20071109000136.4700.8891.stgit@heimdal.trondhjem.org>

Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> take a SHUT_RD/SHUT_WR/SHUT_RDWR argument instead of the

Hmmm...  Why SHUT_*?  Why not SHUTDOWN_*?

David

^ permalink raw reply

* Re: [PATCH] NET: Add the helper kernel_sock_shutdown()
From: Trond Myklebust @ 2007-11-12 13:33 UTC (permalink / raw)
  To: David Howells
  Cc: netdev, Paul Clements, Mark Fasheh, Steve French, David S Miller
In-Reply-To: <4145.1194870138@redhat.com>


On Mon, 2007-11-12 at 12:22 +0000, David Howells wrote:
> Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> 
> > take a SHUT_RD/SHUT_WR/SHUT_RDWR argument instead of the
> 
> Hmmm...  Why SHUT_*?  Why not SHUTDOWN_*?

SHUT_RD/SHUT_WR/SHUT_RDWR are the traditional names for these constants
(see 'man 3 shutdown') and so should be easier to remember. I didn't
however feel comfortable naming the function kernel_shutdown() or
shutdown(), since those have other connotations, hence
kernel_sock_shutdown().


^ permalink raw reply

* [PATCH] (resubmit) Use list_head-s in inetpeer.c
From: Pavel Emelyanov @ 2007-11-12 14:26 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List, devel

The inetpeer.c tracks the LRU list of inet_perr-s, but makes
it by hands. Use the list_head-s for this.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>

---

diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index aa10a81..ad8404b 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -22,7 +22,7 @@ struct inet_peer
 	__be32			v4daddr;	/* peer's address */
 	__u16			avl_height;
 	__u16			ip_id_count;	/* IP ID for the next packet */
-	struct inet_peer	*unused_next, **unused_prevp;
+	struct list_head	unused;
 	__u32			dtime;		/* the time of last use of not
 						 * referenced entries */
 	atomic_t		refcnt;
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 771031d..af99519 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -61,7 +61,7 @@
  *  4.  Global variable peer_total is modified under the pool lock.
  *  5.  struct inet_peer fields modification:
  *		avl_left, avl_right, avl_parent, avl_height: pool lock
- *		unused_next, unused_prevp: unused node list lock
+ *		unused: unused node list lock
  *		refcnt: atomically against modifications on other CPU;
  *		   usually under some other lock to prevent node disappearing
  *		dtime: unused node list lock
@@ -94,8 +94,7 @@ int inet_peer_maxttl __read_mostly = 10 * 60 * HZ;	/* usual time to live: 10 min
 int inet_peer_gc_mintime __read_mostly = 10 * HZ;
 int inet_peer_gc_maxtime __read_mostly = 120 * HZ;
 
-static struct inet_peer *inet_peer_unused_head;
-static struct inet_peer **inet_peer_unused_tailp = &inet_peer_unused_head;
+static LIST_HEAD(unused_peers);
 static DEFINE_SPINLOCK(inet_peer_unused_lock);
 
 static void peer_check_expire(unsigned long dummy);
@@ -138,15 +137,7 @@ void __init inet_initpeers(void)
 static void unlink_from_unused(struct inet_peer *p)
 {
 	spin_lock_bh(&inet_peer_unused_lock);
-	if (p->unused_prevp != NULL) {
-		/* On unused list. */
-		*p->unused_prevp = p->unused_next;
-		if (p->unused_next != NULL)
-			p->unused_next->unused_prevp = p->unused_prevp;
-		else
-			inet_peer_unused_tailp = p->unused_prevp;
-		p->unused_prevp = NULL; /* mark it as removed */
-	}
+	list_del_init(&p->unused);
 	spin_unlock_bh(&inet_peer_unused_lock);
 }
 
@@ -337,24 +328,24 @@ static void unlink_from_pool(struct inet_peer *p)
 /* May be called with local BH enabled. */
 static int cleanup_once(unsigned long ttl)
 {
-	struct inet_peer *p;
+	struct inet_peer *p = NULL;
 
 	/* Remove the first entry from the list of unused nodes. */
 	spin_lock_bh(&inet_peer_unused_lock);
-	p = inet_peer_unused_head;
-	if (p != NULL) {
-		__u32 delta = (__u32)jiffies - p->dtime;
+	if (!list_empty(&unused_peers)) {
+		__u32 delta;
+
+		p = list_first_entry(&unused_peers, struct inet_peer, unused);
+		delta = (__u32)jiffies - p->dtime;
+
 		if (delta < ttl) {
 			/* Do not prune fresh entries. */
 			spin_unlock_bh(&inet_peer_unused_lock);
 			return -1;
 		}
-		inet_peer_unused_head = p->unused_next;
-		if (p->unused_next != NULL)
-			p->unused_next->unused_prevp = p->unused_prevp;
-		else
-			inet_peer_unused_tailp = p->unused_prevp;
-		p->unused_prevp = NULL; /* mark as not on the list */
+
+		list_del_init(&p->unused);
+
 		/* Grab an extra reference to prevent node disappearing
 		 * before unlink_from_pool() call. */
 		atomic_inc(&p->refcnt);
@@ -412,7 +403,7 @@ struct inet_peer *inet_getpeer(__be32 daddr, int create)
 
 	/* Link the node. */
 	link_to_pool(n);
-	n->unused_prevp = NULL; /* not on the list */
+	INIT_LIST_HEAD(&n->unused);
 	peer_total++;
 	write_unlock_bh(&peer_pool_lock);
 
@@ -467,10 +458,7 @@ void inet_putpeer(struct inet_peer *p)
 {
 	spin_lock_bh(&inet_peer_unused_lock);
 	if (atomic_dec_and_test(&p->refcnt)) {
-		p->unused_prevp = inet_peer_unused_tailp;
-		p->unused_next = NULL;
-		*inet_peer_unused_tailp = p;
-		inet_peer_unused_tailp = &p->unused_next;
+		list_add_tail(&p->unused, &unused_peers);
 		p->dtime = (__u32)jiffies;
 	}
 	spin_unlock_bh(&inet_peer_unused_lock);

^ permalink raw reply related

* Re: [PATCH] NET: Add the helper kernel_sock_shutdown()
From: David Howells @ 2007-11-12 14:31 UTC (permalink / raw)
  To: Trond Myklebust
  Cc: dhowells, netdev, Paul Clements, Mark Fasheh, Steve French,
	David S Miller
In-Reply-To: <1194874383.7465.8.camel@heimdal.trondhjem.org>

Trond Myklebust <Trond.Myklebust@netapp.com> wrote:

> SHUT_RD/SHUT_WR/SHUT_RDWR are the traditional names for these constants
> (see 'man 3 shutdown') and so should be easier to remember.

Good point.

David

^ permalink raw reply

* Re: [Lksctp-developers] [PATCH] Fix memory leak in discard case of sctp_sf_abort_violation()
From: Vlad Yasevich @ 2007-11-12 15:14 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: lksctp developers, netdev, Linux Kernel Mailing List,
	Sridhar Samudrala
In-Reply-To: <200711112357.49577.jesper.juhl@gmail.com>

Jesper Juhl wrote:
> From: Jesper Juhl <jesper.juhl@gmail.com>
> 
> In net/sctp/sm_statefuns.c::sctp_sf_abort_violation() we may leak 
> the storage allocated for 'abort' by returning from the function 
> without using or freeing it. This happens in case 
> "sctp_auth_recv_cid(SCTP_CID_ABORT, asoc)" is true and we jump to 
> the 'discard' label.
> Spotted by the Coverity checker.
> 
> The simple fix is to simply move the creation of the "abort chunk" 
> to after the possible jump to the 'discard' label. This way we don't   
> even have to allocate the memory at all in the problem case.
> 
> 
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>

Thanks.  I've applied this to my tree.

-vlad

^ permalink raw reply

* [patch 1/1][NETNS][IPV6] protect addrconf from loopback registration
From: Daniel Lezcano @ 2007-11-12 15:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, den, xemul, ebiederm, containers, yoshfuji,
	Benjamin Thery
In-Reply-To: <20071112151953.052335971@mai.toulouse-stg.fr.ibm.com>

[-- Attachment #1: protect-addrconf-from-loopback-reg-notifications.patch --]
[-- Type: text/plain, Size: 2167 bytes --]

The loopback is now dynamically allocated. The ipv6 code was written
considering the loopback is allocated before the ipv6 protocol 
initialization. This is still the case when we don't use multiple
network namespaces.

In the case of the network namespaces, ipv6 notification handler is
already setup and active (done by the initial network namespace), 
so when a network namespace is created, a new instance of the 
loopback device, via dynamic allocation, will trigger a REGISTER event
to addrconf_notify and this one will try to setup the network device
while the ipv6 protocol is not yet initialized for the network namespace.

Because the ipv6 is relying on the fact that the loopback device will
not trigger REGISTER/UNREGISTER events, I just protect the addrconf_notify
function when the loopback register event is triggered.

In the case of multiple network namespaces, the usual ipv6 protocol 
initialization will be done after the loopback initialization with 
the subsystem registration mechanism.

Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Signed-off-by: Benjamin Thery <benjamin.thery@bull.net>
---
 net/ipv6/addrconf.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-2.6-netns/net/ipv6/addrconf.c
===================================================================
--- linux-2.6-netns.orig/net/ipv6/addrconf.c
+++ linux-2.6-netns/net/ipv6/addrconf.c
@@ -2272,7 +2272,8 @@ static int addrconf_notify(struct notifi
 
 	switch(event) {
 	case NETDEV_REGISTER:
-		if (!idev && dev->mtu >= IPV6_MIN_MTU) {
+		if (!(dev->flags & IFF_LOOPBACK) &&
+		    !idev && dev->mtu >= IPV6_MIN_MTU) {
 			idev = ipv6_add_dev(dev);
 			if (!idev)
 				return notifier_from_errno(-ENOMEM);
@@ -2366,11 +2367,15 @@ static int addrconf_notify(struct notifi
 		/* MTU falled under IPV6_MIN_MTU. Stop IPv6 on this interface. */
 
 	case NETDEV_DOWN:
+		addrconf_ifdown(dev, 0);
+		break;
+
 	case NETDEV_UNREGISTER:
 		/*
 		 *	Remove all addresses from this interface.
 		 */
-		addrconf_ifdown(dev, event != NETDEV_DOWN);
+		if (!(dev->flags & IFF_LOOPBACK))
+			addrconf_ifdown(dev, 1);
 		break;
 
 	case NETDEV_CHANGENAME:

-- 

^ permalink raw reply

* Problem with frame time stamping
From: Antoine Zen-Ruffinen @ 2007-11-12 15:42 UTC (permalink / raw)
  To: netdev, linux-net, netfilter-devel; +Cc: patrik.arlos

Dear all,

I'm writing a network analyzer software using Linux and I need a VERY
precise frame time stamping. Therefor I am planing to add my own time
stamping algorithm on a modified network driver. For test purpose I
did so :

	skb->tstamp.tv64 = 0x00010002;
	netif_rx(skb);

On the user side, I ask for the timestamp that way :

	...
	sock = socket(AF_PACKET, SOCK_RAW, type);
	...
	//bind this socket with the interface using my modified driver.
	...
	recvByteCount = recv(sock, buffer, 1514, 0);
    	ioctl(sock, SIOCGSTAMP, &timeStamp);

I was surprised to see that the var timeStamp was still holding a
count of second since  year 1970.

Investigating a bit into the kernel code, I found that ioctl(sock,
SIOCGSTAMP, ...) was giving just the current kernel time using
ktime_get_real().

Are my investigation wrong ?
Is that a bug in kernel code ?
Is there an other way to access skb.tstamp from user side ?

Thank's to any one that can help me !

Antoine Zen-Ruffinen

^ permalink raw reply

* Re: [patch 1/1][NETNS][IPV6] protect addrconf from loopback registration
From: Denis V. Lunev @ 2007-11-12 16:05 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: davem, netdev, xemul, ebiederm, containers, yoshfuji,
	Benjamin Thery
In-Reply-To: <20071112152403.273795630@mai.toulouse-stg.fr.ibm.com>

Daniel Lezcano wrote:
> The loopback is now dynamically allocated. The ipv6 code was written
> considering the loopback is allocated before the ipv6 protocol 
> initialization. This is still the case when we don't use multiple
> network namespaces.
> 
> In the case of the network namespaces, ipv6 notification handler is
> already setup and active (done by the initial network namespace), 
> so when a network namespace is created, a new instance of the 
> loopback device, via dynamic allocation, will trigger a REGISTER event
> to addrconf_notify and this one will try to setup the network device
> while the ipv6 protocol is not yet initialized for the network namespace.
> 
> Because the ipv6 is relying on the fact that the loopback device will
> not trigger REGISTER/UNREGISTER events, I just protect the addrconf_notify
> function when the loopback register event is triggered.
> 
> In the case of multiple network namespaces, the usual ipv6 protocol 
> initialization will be done after the loopback initialization with 
> the subsystem registration mechanism.
> 
> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
> Signed-off-by: Benjamin Thery <benjamin.thery@bull.net>
> ---
>  net/ipv6/addrconf.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6-netns/net/ipv6/addrconf.c
> ===================================================================
> --- linux-2.6-netns.orig/net/ipv6/addrconf.c
> +++ linux-2.6-netns/net/ipv6/addrconf.c
> @@ -2272,7 +2272,8 @@ static int addrconf_notify(struct notifi
>  
>  	switch(event) {
>  	case NETDEV_REGISTER:
> -		if (!idev && dev->mtu >= IPV6_MIN_MTU) {
> +		if (!(dev->flags & IFF_LOOPBACK) &&
> +		    !idev && dev->mtu >= IPV6_MIN_MTU) {
>  			idev = ipv6_add_dev(dev);
>  			if (!idev)
>  				return notifier_from_errno(-ENOMEM);
> @@ -2366,11 +2367,15 @@ static int addrconf_notify(struct notifi
>  		/* MTU falled under IPV6_MIN_MTU. Stop IPv6 on this interface. */
>  
>  	case NETDEV_DOWN:
> +		addrconf_ifdown(dev, 0);
> +		break;
> +
>  	case NETDEV_UNREGISTER:
>  		/*
>  		 *	Remove all addresses from this interface.
>  		 */
> -		addrconf_ifdown(dev, event != NETDEV_DOWN);
> +		if (!(dev->flags & IFF_LOOPBACK))
> +			addrconf_ifdown(dev, 1);
>  		break;
>  
>  	case NETDEV_CHANGENAME:
> 

why should we care on down? we are destroying the device. It should
gone. All references to it should also gone. So, we should perform the
cleaning and remove all IPv6 addresses, so notifier should also work.

The code relies on the "persistent" loopback and this is a _bad_ thing.
This is longstanding bug in the code, that the dst_entry should have a
valid reference to a device. This is the only purpose for a loopback
persistence. Though, at the namespace death no such entries must be and
this will be checked during unregister process. This patch definitely
breaks this assumption :(

Namespaces are good to catch leakage using standard codepaths, so they
should be preserved as much as possible. So, _all_ normal down code
should be called for a loopback device in other than init_net context.

Regards,
	Den

^ permalink raw reply

* Re: [patch 1/1][NETNS][IPV6] protect addrconf from loopback registration
From: Daniel Lezcano @ 2007-11-12 16:11 UTC (permalink / raw)
  To: Denis V. Lunev
  Cc: davem, netdev, xemul, ebiederm, containers, yoshfuji,
	Benjamin Thery
In-Reply-To: <473879C3.5020301@sw.ru>

Denis V. Lunev wrote:
> Daniel Lezcano wrote:
>> The loopback is now dynamically allocated. The ipv6 code was written
>> considering the loopback is allocated before the ipv6 protocol 
>> initialization. This is still the case when we don't use multiple
>> network namespaces.
>>
>> In the case of the network namespaces, ipv6 notification handler is
>> already setup and active (done by the initial network namespace), 
>> so when a network namespace is created, a new instance of the 
>> loopback device, via dynamic allocation, will trigger a REGISTER event
>> to addrconf_notify and this one will try to setup the network device
>> while the ipv6 protocol is not yet initialized for the network namespace.
>>
>> Because the ipv6 is relying on the fact that the loopback device will
>> not trigger REGISTER/UNREGISTER events, I just protect the addrconf_notify
>> function when the loopback register event is triggered.
>>
>> In the case of multiple network namespaces, the usual ipv6 protocol 
>> initialization will be done after the loopback initialization with 
>> the subsystem registration mechanism.
>>
>> Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
>> Signed-off-by: Benjamin Thery <benjamin.thery@bull.net>
>> ---
>>  net/ipv6/addrconf.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> Index: linux-2.6-netns/net/ipv6/addrconf.c
>> ===================================================================
>> --- linux-2.6-netns.orig/net/ipv6/addrconf.c
>> +++ linux-2.6-netns/net/ipv6/addrconf.c
>> @@ -2272,7 +2272,8 @@ static int addrconf_notify(struct notifi
>>  
>>  	switch(event) {
>>  	case NETDEV_REGISTER:
>> -		if (!idev && dev->mtu >= IPV6_MIN_MTU) {
>> +		if (!(dev->flags & IFF_LOOPBACK) &&
>> +		    !idev && dev->mtu >= IPV6_MIN_MTU) {
>>  			idev = ipv6_add_dev(dev);
>>  			if (!idev)
>>  				return notifier_from_errno(-ENOMEM);
>> @@ -2366,11 +2367,15 @@ static int addrconf_notify(struct notifi
>>  		/* MTU falled under IPV6_MIN_MTU. Stop IPv6 on this interface. */
>>  
>>  	case NETDEV_DOWN:
>> +		addrconf_ifdown(dev, 0);
>> +		break;
>> +
>>  	case NETDEV_UNREGISTER:
>>  		/*
>>  		 *	Remove all addresses from this interface.
>>  		 */
>> -		addrconf_ifdown(dev, event != NETDEV_DOWN);
>> +		if (!(dev->flags & IFF_LOOPBACK))
>> +			addrconf_ifdown(dev, 1);
>>  		break;
>>  
>>  	case NETDEV_CHANGENAME:
>>
> 
> why should we care on down? we are destroying the device. It should
> gone. All references to it should also gone. So, we should perform the
> cleaning and remove all IPv6 addresses, so notifier should also work.

We need to take care of netdev down, someone can put the loopback down 
if he wants.

> The code relies on the "persistent" loopback and this is a _bad_ thing.
> This is longstanding bug in the code, that the dst_entry should have a
> valid reference to a device. This is the only purpose for a loopback
> persistence. Though, at the namespace death no such entries must be and
> this will be checked during unregister process. This patch definitely
> breaks this assumption :(
> 
> Namespaces are good to catch leakage using standard codepaths, so they
> should be preserved as much as possible. So, _all_ normal down code
> should be called for a loopback device in other than init_net context.

I agree with you, this is a bug in ipv6 and the loopback; when playing 
with ipv6 we found that the loopback is still referenced 9 times when 
the system is shutdown.

The purpose of this patch is to protect the __actual__ code from the new 
loopback behavior. We are looking at a more generic approach with the 
namespace for ipv6, as you mentioned, namespaces are good for network 
leakage detection as we create several instances of the network stack.

^ permalink raw reply

* Possible bug in bonding
From: Moni Shoua @ 2007-11-12 16:26 UTC (permalink / raw)
  To: Jay Vosburgh, Jeff Garzik; +Cc: Moni Levy, Or Gerlitz, netdev


Hi,

As reported last week by Or Gerlitz and confirmed by me, there is a kernel crash 
when trying to unenslave all ib slaves (which leads to bonding master destruction).
I also found that it happens with Ethernet slaves if following the steps:

1. unenslaving all slaves via sysfs
2. deleting the bonding master via sysfs


I see that in commit 6603a6f25e4bca922a7dfbf0bf03072d98850176 
the order of the bonding master destruction sequence was changed 
in 2 places (a part of the commitdiff is below).

Although I might be missing something it looks like a bug to me
to dereference the dev pointer after unregistering it. Am I right?

I also see that the kernel crash doesn't happen when reverting from this 
(part of) the patch.

I'd like to have your opinion please.

thanks
 
    MoniS

---------------------------------------------------------

commit 6603a6f25e4bca922a7dfbf0bf03072d98850176
Author: Jay Vosburgh <fubar@us.ibm.com>
Date:   Wed Oct 17 17:37:50 2007 -0700

    bonding: Convert more locks to _bh, acquire rtnl, for new locking
    
    	Convert more lock acquisitions to _bh flavor to avoid deadlock
    with workqueue activity and add acquisition of RTNL in appropriate places.
    Affects ALB mode, as well as core bonding functions and sysfs.
    
    Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
    Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
    Signed-off-by: Jeff Garzik <jeff@garzik.org>


--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1846,9 +1846,9 @@ int bond_release(struct net_device *bond
 */
 void bond_destroy(struct bonding *bond)
 {
+	unregister_netdevice(bond->dev);
 	bond_deinit(bond->dev);
 	bond_destroy_sysfs_entry(bond);
-	unregister_netdevice(bond->dev);
 }

.
.
.

@@ -4473,8 +4473,8 @@ static void bond_free_all(void)
 		bond_mc_list_destroy(bond);
 		/* Release the bonded slaves */
 		bond_release_all(bond_dev);
-		bond_deinit(bond_dev);
 		unregister_netdevice(bond_dev);
+		bond_deinit(bond_dev);
 	}
 
 


^ permalink raw reply

* [GIT PATCHES v2] SCTP bug fixes
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers

Hi David

I've updated the series of SCTP bugfixes.  These pass all of the regression
tests.

You can pull the changes from:
 master.kernel.org:/pub/scm/linux/kernel/git/vxy/lksctp-dev.git pending

Summary of changes:

Jesper Juhl (1):
      Fix memory leak in discard case of sctp_sf_abort_violation()

Vlad Yasevich (11):
      SCTP: Fix difference cases of retransmit.
      SCTP: Update RCU handling during the ADD-IP case
      SCTP: Correctly disable ADD-IP when AUTH is not supported.
      SCTP: Allow ADD_IP to work with AUTH for backward compatibility.
      SCTP: Fix a potential race between timers and receive path.
      SCTP: Use hashed lookup when looking for an association.
      SCTP: Convert custom hash lists to use hlist.
      SCTP: Make sctp_verify_param return multiple indications.
      SCTP: Fix PR-SCTP to deliver all the accumulated ordered chunks
      SCTP: Clean-up some defines for regressions tests.
      SCTP: Always flush the queue when uncorcking.

Wei Yongjun (2):
      SCTP : Fix bad formatted comment in outqueue.c
      SCTP : Fix to process bundled ASCONF chunk correctly

 include/net/sctp/command.h   |    1 +
 include/net/sctp/constants.h |    3 +
 include/net/sctp/sctp.h      |    5 +-
 include/net/sctp/structs.h   |   22 +++---
 net/sctp/associola.c         |   10 ++-
 net/sctp/bind_addr.c         |   13 +---
 net/sctp/endpointola.c       |   35 ++++++---
 net/sctp/input.c             |   43 +++--------
 net/sctp/inqueue.c           |    4 +
 net/sctp/outqueue.c          |   41 +++++-----
 net/sctp/proc.c              |    6 +-
 net/sctp/protocol.c          |    7 +-
 net/sctp/sm_make_chunk.c     |  170 ++++++++++++++++++++++-------------------
 net/sctp/sm_sideeffect.c     |   10 ++-
 net/sctp/sm_statefuns.c      |   12 ++--
 net/sctp/socket.c            |   16 ++---
 net/sctp/sysctl.c            |    9 ++
 net/sctp/transport.c         |    5 +-
 net/sctp/ulpqueue.c          |    2 +-
 19 files changed, 220 insertions(+), 194 deletions(-)




^ permalink raw reply

* [PATCH 01/14] SCTP : Fix bad formatted comment in outqueue.c
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Wei Yongjun, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

From: Wei Yongjun <yjwei@cn.fujitsu.com>

Just fix the bad format of the comment in outqueue.c.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/outqueue.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 28f4fe7..e315c6c 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -641,7 +641,8 @@ static int sctp_outq_flush_rtx(struct sctp_outq *q, struct sctp_packet *pkt,
 
 		/* If we are here due to a retransmit timeout or a fast
 		 * retransmit and if there are any chunks left in the retransmit
-		 * queue that could not fit in the PMTU sized packet, they need			 * to be marked as ineligible for a subsequent fast retransmit.
+		 * queue that could not fit in the PMTU sized packet, they need
+		 * to be marked as ineligible for a subsequent fast retransmit.
 		 */
 		if (rtx_timeout && !lchunk) {
 			list_for_each(lchunk1, lqueue) {
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 03/14] SCTP: Fix difference cases of retransmit.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

Commit d0ce92910bc04e107b2f3f2048f07e94f570035d broke several retransmit
cases including fast retransmit.  The reason is that we should
only delay by rto while doing retranmists as a result of a timeout.
Retransmit as a result of path mtu discover, fast retransmit, or
other evernts that should trigger immidiate retransmissions got broken.

Also, since rto is doubled prior to marking of packets elegable for
retransmission, we never marked correct chunks anyway.

The fix is provide a reason for a given retransmission so that we
can mark chunks appropriately and to save the old rto value to do
comparisons against.

All regressions tests passed with this code.

Spotted by Wei Yongjun <yjwei@cn.fujitsu.com>

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/command.h   |    1 +
 include/net/sctp/constants.h |    1 +
 include/net/sctp/sctp.h      |    1 +
 include/net/sctp/structs.h   |    5 +++--
 net/sctp/outqueue.c          |   33 +++++++++++++++++----------------
 net/sctp/sm_sideeffect.c     |   10 +++++++++-
 net/sctp/sm_statefuns.c      |    2 +-
 net/sctp/transport.c         |    5 +++--
 8 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index b873336..c1f7976 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -103,6 +103,7 @@ typedef enum {
 	SCTP_CMD_ASSOC_CHANGE,	 /* generate and send assoc_change event */
 	SCTP_CMD_ADAPTATION_IND, /* generate and send adaptation event */
 	SCTP_CMD_ASSOC_SHKEY,    /* generate the association shared keys */
+	SCTP_CMD_T1_RETRAN,	 /* Mark for retransmission after T1 timeout  */
 	SCTP_CMD_LAST
 } sctp_verb_t;
 
diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index da8354e..73fbdf6 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -407,6 +407,7 @@ typedef enum {
 	SCTP_RTXR_T3_RTX,
 	SCTP_RTXR_FAST_RTX,
 	SCTP_RTXR_PMTUD,
+	SCTP_RTXR_T1_RTX,
 } sctp_retransmit_reason_t;
 
 /* Reasons to lower cwnd. */
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 93eb708..7082730 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -267,6 +267,7 @@ enum
 	SCTP_MIB_T5_SHUTDOWN_GUARD_EXPIREDS,
 	SCTP_MIB_DELAY_SACK_EXPIREDS,
 	SCTP_MIB_AUTOCLOSE_EXPIREDS,
+	SCTP_MIB_T1_RETRANSMITS,
 	SCTP_MIB_T3_RETRANSMITS,
 	SCTP_MIB_PMTUD_RETRANSMITS,
 	SCTP_MIB_FAST_RETRANSMITS,
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index ef892e0..482c2aa 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -873,10 +873,11 @@ struct sctp_transport {
 	 * address list derived from the INIT or INIT ACK chunk, a
 	 * number of data elements needs to be maintained including:
 	 */
-	__u32 rtt;		/* This is the most recent RTT.	 */
-
 	/* RTO	       : The current retransmission timeout value.  */
 	unsigned long rto;
+	unsigned long last_rto;
+
+	__u32 rtt;		/* This is the most recent RTT.	 */
 
 	/* RTTVAR      : The current RTT variation.  */
 	__u32 rttvar;
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index e315c6c..99a3db5 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -382,7 +382,7 @@ static void sctp_insert_list(struct list_head *head, struct list_head *new)
 /* Mark all the eligible packets on a transport for retransmission.  */
 void sctp_retransmit_mark(struct sctp_outq *q,
 			  struct sctp_transport *transport,
-			  __u8 fast_retransmit)
+			  __u8 reason)
 {
 	struct list_head *lchunk, *ltemp;
 	struct sctp_chunk *chunk;
@@ -412,20 +412,20 @@ void sctp_retransmit_mark(struct sctp_outq *q,
 			continue;
 		}
 
-		/* If we are doing retransmission due to a fast retransmit,
-		 * only the chunk's that are marked for fast retransmit
-		 * should be added to the retransmit queue.  If we are doing
-		 * retransmission due to a timeout or pmtu discovery, only the
-		 * chunks that are not yet acked should be added to the
-		 * retransmit queue.
+		/* If we are doing  retransmission due to a timeout or pmtu
+		 * discovery, only the  chunks that are not yet acked should
+		 * be added to the retransmit queue.
 		 */
-		if ((fast_retransmit && (chunk->fast_retransmit > 0)) ||
-		   (!fast_retransmit && !chunk->tsn_gap_acked)) {
+		if ((reason == SCTP_RTXR_FAST_RTX  &&
+			    (chunk->fast_retransmit > 0)) ||
+		    (reason != SCTP_RTXR_FAST_RTX  && !chunk->tsn_gap_acked)) {
 			/* If this chunk was sent less then 1 rto ago, do not
 			 * retransmit this chunk, but give the peer time
-			 * to acknowlege it.
+			 * to acknowlege it.  Do this only when
+			 * retransmitting due to T3 timeout.
 			 */
-			if ((jiffies - chunk->sent_at) < transport->rto)
+			if (reason == SCTP_RTXR_T3_RTX &&
+			    (jiffies - chunk->sent_at) < transport->last_rto)
 				continue;
 
 			/* RFC 2960 6.2.1 Processing a Received SACK
@@ -467,10 +467,10 @@ void sctp_retransmit_mark(struct sctp_outq *q,
 		}
 	}
 
-	SCTP_DEBUG_PRINTK("%s: transport: %p, fast_retransmit: %d, "
+	SCTP_DEBUG_PRINTK("%s: transport: %p, reason: %d, "
 			  "cwnd: %d, ssthresh: %d, flight_size: %d, "
 			  "pba: %d\n", __FUNCTION__,
-			  transport, fast_retransmit,
+			  transport, reason,
 			  transport->cwnd, transport->ssthresh,
 			  transport->flight_size,
 			  transport->partial_bytes_acked);
@@ -484,7 +484,6 @@ void sctp_retransmit(struct sctp_outq *q, struct sctp_transport *transport,
 		     sctp_retransmit_reason_t reason)
 {
 	int error = 0;
-	__u8 fast_retransmit = 0;
 
 	switch(reason) {
 	case SCTP_RTXR_T3_RTX:
@@ -499,16 +498,18 @@ void sctp_retransmit(struct sctp_outq *q, struct sctp_transport *transport,
 	case SCTP_RTXR_FAST_RTX:
 		SCTP_INC_STATS(SCTP_MIB_FAST_RETRANSMITS);
 		sctp_transport_lower_cwnd(transport, SCTP_LOWER_CWND_FAST_RTX);
-		fast_retransmit = 1;
 		break;
 	case SCTP_RTXR_PMTUD:
 		SCTP_INC_STATS(SCTP_MIB_PMTUD_RETRANSMITS);
 		break;
+	case SCTP_RTXR_T1_RTX:
+		SCTP_INC_STATS(SCTP_MIB_T1_RETRANSMITS);
+		break;
 	default:
 		BUG();
 	}
 
-	sctp_retransmit_mark(q, transport, fast_retransmit);
+	sctp_retransmit_mark(q, transport, reason);
 
 	/* PR-SCTP A5) Any time the T3-rtx timer expires, on any destination,
 	 * the sender SHOULD try to advance the "Advanced.Peer.Ack.Point" by
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index bbdc938..78d1a8a 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -453,6 +453,7 @@ static void sctp_do_8_2_transport_strike(struct sctp_association *asoc,
 	 * maximum value discussed in rule C7 above (RTO.max) may be
 	 * used to provide an upper bound to this doubling operation.
 	 */
+	transport->last_rto = transport->rto;
 	transport->rto = min((transport->rto * 2), transport->asoc->rto_max);
 }
 
@@ -1267,6 +1268,12 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
 			sctp_ootb_pkt_free(packet);
 			break;
 
+		case SCTP_CMD_T1_RETRAN:
+			/* Mark a transport for retransmission.  */
+			sctp_retransmit(&asoc->outqueue, cmd->obj.transport,
+					SCTP_RTXR_T1_RTX);
+			break;
+
 		case SCTP_CMD_RETRAN:
 			/* Mark a transport for retransmission.  */
 			sctp_retransmit(&asoc->outqueue, cmd->obj.transport,
@@ -1393,7 +1400,8 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
 			list_for_each(pos, &asoc->peer.transport_addr_list) {
 				t = list_entry(pos, struct sctp_transport,
 					       transports);
-				sctp_retransmit_mark(&asoc->outqueue, t, 0);
+				sctp_retransmit_mark(&asoc->outqueue, t,
+					    SCTP_RTXR_T1_RTX);
 			}
 
 			sctp_add_cmd_sf(commands,
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index f01b408..a66075a 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2305,7 +2305,7 @@ static sctp_disposition_t sctp_sf_do_5_2_6_stale(const struct sctp_endpoint *ep,
 	/* If we've sent any data bundled with COOKIE-ECHO we will need to
 	 * resend
 	 */
-	sctp_add_cmd_sf(commands, SCTP_CMD_RETRAN,
+	sctp_add_cmd_sf(commands, SCTP_CMD_T1_RETRAN,
 			SCTP_TRANSPORT(asoc->peer.primary_path));
 
 	/* Cast away the const modifier, as we want to just
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 5f467c9..d55ce83 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -74,8 +74,8 @@ static struct sctp_transport *sctp_transport_init(struct sctp_transport *peer,
 	 * given destination transport address, set RTO to the protocol
 	 * parameter 'RTO.Initial'.
 	 */
+	peer->last_rto = peer->rto = msecs_to_jiffies(sctp_rto_initial);
 	peer->rtt = 0;
-	peer->rto = msecs_to_jiffies(sctp_rto_initial);
 	peer->rttvar = 0;
 	peer->srtt = 0;
 	peer->rto_pending = 0;
@@ -385,6 +385,7 @@ void sctp_transport_update_rto(struct sctp_transport *tp, __u32 rtt)
 		tp->rto = tp->asoc->rto_max;
 
 	tp->rtt = rtt;
+	tp->last_rto = tp->rto;
 
 	/* Reset rto_pending so that a new RTT measurement is started when a
 	 * new data chunk is sent.
@@ -578,7 +579,7 @@ void sctp_transport_reset(struct sctp_transport *t)
 	 */
 	t->cwnd = min(4*asoc->pathmtu, max_t(__u32, 2*asoc->pathmtu, 4380));
 	t->ssthresh = asoc->peer.i.a_rwnd;
-	t->rto = asoc->rto_initial;
+	t->last_rto = t->rto = asoc->rto_initial;
 	t->rtt = 0;
 	t->srtt = 0;
 	t->rttvar = 0;
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 02/14] SCTP : Fix to process bundled ASCONF chunk correctly
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Wei Yongjun, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

From: Wei Yongjun <yjwei@cn.fujitsu.com>

If ASCONF chunk is bundled with other chunks as the first chunk, when
process the ASCONF parameters, full packet data will be process as the
parameters of the ASCONF chunk, not only the real parameters. So if you
send a ASCONF chunk bundled with other chunks, you will get an unexpect
result.
This problem also exists when ASCONF-ACK chunk is bundled with other chunks.

This patch fix this problem.

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_make_chunk.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index c377e4e..5de4729 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2848,10 +2848,11 @@ struct sctp_chunk *sctp_process_asconf(struct sctp_association *asoc,
 
 	__be16	err_code;
 	int	length = 0;
-	int	chunk_len = asconf->skb->len;
+	int	chunk_len;
 	__u32	serial;
 	int	all_param_pass = 1;
 
+	chunk_len = ntohs(asconf->chunk_hdr->length) - sizeof(sctp_chunkhdr_t);
 	hdr = (sctp_addiphdr_t *)asconf->skb->data;
 	serial = ntohl(hdr->serial);
 
@@ -2990,7 +2991,7 @@ static __be16 sctp_get_asconf_response(struct sctp_chunk *asconf_ack,
 	sctp_addip_param_t	*asconf_ack_param;
 	sctp_errhdr_t		*err_param;
 	int			length;
-	int			asconf_ack_len = asconf_ack->skb->len;
+	int			asconf_ack_len;
 	__be16			err_code;
 
 	if (no_err)
@@ -2998,6 +2999,9 @@ static __be16 sctp_get_asconf_response(struct sctp_chunk *asconf_ack,
 	else
 		err_code = SCTP_ERROR_REQ_REFUSED;
 
+	asconf_ack_len = ntohs(asconf_ack->chunk_hdr->length) -
+			     sizeof(sctp_chunkhdr_t);
+
 	/* Skip the addiphdr from the asconf_ack chunk and store a pointer to
 	 * the first asconf_ack parameter.
 	 */
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 04/14] SCTP: Update RCU handling during the ADD-IP case
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

After learning more about rcu, it looks like the ADD-IP hadling
doesn't need to call call_rcu_bh.  All the rcu critical sections
use rcu_read_lock, so using call_rcu_bh is wrong here.
Now, restore the local_bh_disable() code blocks and use normal
call_rcu() calls.  Also restore the missing return statement.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/structs.h |    4 +---
 net/sctp/bind_addr.c       |   13 +++----------
 net/sctp/sm_make_chunk.c   |    6 +++++-
 net/sctp/socket.c          |    2 +-
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 482c2aa..a177017 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1185,9 +1185,7 @@ int sctp_bind_addr_copy(struct sctp_bind_addr *dest,
 			int flags);
 int sctp_add_bind_addr(struct sctp_bind_addr *, union sctp_addr *,
 		       __u8 use_as_src, gfp_t gfp);
-int sctp_del_bind_addr(struct sctp_bind_addr *, union sctp_addr *,
-			void fastcall (*rcu_call)(struct rcu_head *,
-					  void (*func)(struct rcu_head *)));
+int sctp_del_bind_addr(struct sctp_bind_addr *, union sctp_addr *);
 int sctp_bind_addr_match(struct sctp_bind_addr *, const union sctp_addr *,
 			 struct sctp_sock *);
 union sctp_addr *sctp_find_unmatch_addr(struct sctp_bind_addr	*bp,
diff --git a/net/sctp/bind_addr.c b/net/sctp/bind_addr.c
index dfffa94..cae95af 100644
--- a/net/sctp/bind_addr.c
+++ b/net/sctp/bind_addr.c
@@ -180,9 +180,7 @@ int sctp_add_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *new,
 /* Delete an address from the bind address list in the SCTP_bind_addr
  * structure.
  */
-int sctp_del_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *del_addr,
-			void fastcall (*rcu_call)(struct rcu_head *head,
-					 void (*func)(struct rcu_head *head)))
+int sctp_del_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *del_addr)
 {
 	struct sctp_sockaddr_entry *addr, *temp;
 
@@ -198,15 +196,10 @@ int sctp_del_bind_addr(struct sctp_bind_addr *bp, union sctp_addr *del_addr,
 		}
 	}
 
-	/* Call the rcu callback provided in the args.  This function is
-	 * called by both BH packet processing and user side socket option
-	 * processing, but it works on different lists in those 2 contexts.
-	 * Each context provides it's own callback, whether call_rcu_bh()
-	 * or call_rcu(), to make sure that we wait for an appropriate time.
-	 */
 	if (addr && !addr->valid) {
-		rcu_call(&addr->rcu, sctp_local_addr_free);
+		call_rcu(&addr->rcu, sctp_local_addr_free);
 		SCTP_DBG_OBJCNT_DEC(addr);
+		return 0;
 	}
 
 	return -EINVAL;
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 5de4729..c60564d 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2953,13 +2953,17 @@ static int sctp_asconf_param_success(struct sctp_association *asoc,
 		/* This is always done in BH context with a socket lock
 		 * held, so the list can not change.
 		 */
+		local_bh_disable();
 		list_for_each_entry(saddr, &bp->address_list, list) {
 			if (sctp_cmp_addr_exact(&saddr->a, &addr))
 				saddr->use_as_src = 1;
 		}
+		local_bh_enable();
 		break;
 	case SCTP_PARAM_DEL_IP:
-		retval = sctp_del_bind_addr(bp, &addr, call_rcu_bh);
+		local_bh_disable();
+		retval = sctp_del_bind_addr(bp, &addr);
+		local_bh_enable();
 		list_for_each(pos, &asoc->peer.transport_addr_list) {
 			transport = list_entry(pos, struct sctp_transport,
 						 transports);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index a7ecf31..6ce9b49 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -660,7 +660,7 @@ static int sctp_bindx_rem(struct sock *sk, struct sockaddr *addrs, int addrcnt)
 		 * socket routing and failover schemes. Refer to comments in
 		 * sctp_do_bind(). -daisy
 		 */
-		retval = sctp_del_bind_addr(bp, sa_addr, call_rcu);
+		retval = sctp_del_bind_addr(bp, sa_addr);
 
 		addr_buf += af->sockaddr_len;
 err_bindx_rem:
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 05/14] SCTP: Correctly disable ADD-IP when AUTH is not supported.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/structs.h |    1 -
 net/sctp/associola.c       |    2 +-
 net/sctp/sm_make_chunk.c   |    5 +++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index a177017..41f1039 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1540,7 +1540,6 @@ struct sctp_association {
 		__u8    asconf_capable;  /* Does peer support ADDIP? */
 		__u8    prsctp_capable;  /* Can peer do PR-SCTP? */
 		__u8	auth_capable;	 /* Is peer doing SCTP-AUTH? */
-		__u8	addip_capable;	 /* Can peer do ADD-IP */
 
 		__u32   adaptation_ind;	 /* Adaptation Code point. */
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 03158e3..eaad5c5 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -265,7 +265,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	/* Assume that the peer recongizes ASCONF until reported otherwise
 	 * via an ERROR chunk.
 	 */
-	asoc->peer.asconf_capable = 1;
+	asoc->peer.asconf_capable = 0;
 
 	/* Create an input queue.  */
 	sctp_inq_init(&asoc->base.inqueue);
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index c60564d..2ff3a3d 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1847,7 +1847,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
 			    break;
 		    case SCTP_CID_ASCONF:
 		    case SCTP_CID_ASCONF_ACK:
-			    asoc->peer.addip_capable = 1;
+			    asoc->peer.asconf_capable = 1;
 			    break;
 		    default:
 			    break;
@@ -2138,10 +2138,11 @@ int sctp_process_init(struct sctp_association *asoc, sctp_cid_t cid,
 	/* If the peer claims support for ADD-IP without support
 	 * for AUTH, disable support for ADD-IP.
 	 */
-	if (asoc->peer.addip_capable && !asoc->peer.auth_capable) {
+	if (asoc->peer.asconf_capable && !asoc->peer.auth_capable) {
 		asoc->peer.addip_disabled_mask |= (SCTP_PARAM_ADD_IP |
 						  SCTP_PARAM_DEL_IP |
 						  SCTP_PARAM_SET_PRIMARY);
+		asoc->peer.asconf_capable = 0;
 	}
 
 	/* Walk list of transports, removing transports in the UNKNOWN state. */
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 06/14] SCTP: Allow ADD_IP to work with AUTH for backward compatibility.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

This patch adds a tunable that will allow ADD_IP to work without
AUTH for backward compatibility.  The default value is off since
the default value for ADD_IP is off as well.  People who need
to use ADD-IP with older implementations take risks of connection
hijacking and should consider upgrading or turning this tunable on.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/structs.h |    2 ++
 net/sctp/associola.c       |    8 ++++++--
 net/sctp/protocol.c        |    1 +
 net/sctp/sm_make_chunk.c   |    4 +++-
 net/sctp/sysctl.c          |    9 +++++++++
 5 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 41f1039..44f2672 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -212,6 +212,7 @@ extern struct sctp_globals {
 	
 	/* Flag to indicate if addip is enabled. */
 	int addip_enable;
+	int addip_noauth_enable;
 
 	/* Flag to indicate if PR-SCTP is enabled. */
 	int prsctp_enable;
@@ -249,6 +250,7 @@ extern struct sctp_globals {
 #define sctp_local_addr_list		(sctp_globals.local_addr_list)
 #define sctp_local_addr_lock		(sctp_globals.addr_list_lock)
 #define sctp_addip_enable		(sctp_globals.addip_enable)
+#define sctp_addip_noauth		(sctp_globals.addip_noauth_enable)
 #define sctp_prsctp_enable		(sctp_globals.prsctp_enable)
 #define sctp_auth_enable		(sctp_globals.auth_enable)
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index eaad5c5..013e3d3 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -262,10 +262,14 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
 	 */
 	asoc->peer.sack_needed = 1;
 
-	/* Assume that the peer recongizes ASCONF until reported otherwise
-	 * via an ERROR chunk.
+	/* Assume that the peer will tell us if he recognizes ASCONF
+	 * as part of INIT exchange.
+	 * The sctp_addip_noauth option is there for backward compatibilty
+	 * and will revert old behavior.
 	 */
 	asoc->peer.asconf_capable = 0;
+	if (sctp_addip_noauth)
+		asoc->peer.asconf_capable = 1;
 
 	/* Create an input queue.  */
 	sctp_inq_init(&asoc->base.inqueue);
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 40c1a47..ecfab03 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1179,6 +1179,7 @@ SCTP_STATIC __init int sctp_init(void)
 
 	/* Disable ADDIP by default. */
 	sctp_addip_enable = 0;
+	sctp_addip_noauth = 0;
 
 	/* Enable PR-SCTP by default. */
 	sctp_prsctp_enable = 1;
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 2ff3a3d..43e8de1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -2137,8 +2137,10 @@ int sctp_process_init(struct sctp_association *asoc, sctp_cid_t cid,
 
 	/* If the peer claims support for ADD-IP without support
 	 * for AUTH, disable support for ADD-IP.
+	 * Do this only if backward compatible mode is turned off.
 	 */
-	if (asoc->peer.asconf_capable && !asoc->peer.auth_capable) {
+	if (!sctp_addip_noauth &&
+	     (asoc->peer.asconf_capable && !asoc->peer.auth_capable)) {
 		asoc->peer.addip_disabled_mask |= (SCTP_PARAM_ADD_IP |
 						  SCTP_PARAM_DEL_IP |
 						  SCTP_PARAM_SET_PRIMARY);
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index 0669778..da4f157 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -263,6 +263,15 @@ static ctl_table sctp_table[] = {
 		.proc_handler	= &proc_dointvec,
 		.strategy	= &sysctl_intvec
 	},
+	{
+		.ctl_name	= CTL_UNNUMBERED,
+		.procname	= "addip_noauth_enable",
+		.data		= &sctp_addip_noauth,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= &proc_dointvec,
+		.strategy	= &sysctl_intvec
+	},
 	{ .ctl_name = 0 }
 };
 
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 07/14] SCTP: Fix a potential race between timers and receive path.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

There is a possible race condition where the timer code will
free the association and the next packet in the queue will also
attempt to free the same association.

The example is, when we receive an ABORT at about the same time
as the retransmission timer fires.  If the timer wins the race,
it will free the association.  Once it releases the lock, the
queue processing will recieve the ABORT and will try to free
the association again.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/inqueue.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c
index f10fe7f..cf4b7eb 100644
--- a/net/sctp/inqueue.c
+++ b/net/sctp/inqueue.c
@@ -90,6 +90,10 @@ void sctp_inq_free(struct sctp_inq *queue)
 void sctp_inq_push(struct sctp_inq *q, struct sctp_chunk *chunk)
 {
 	/* Directly call the packet handling routine. */
+	if (chunk->rcvr->dead) {
+		sctp_chunk_free(chunk);
+		return;
+	}
 
 	/* We are now calling this either from the soft interrupt
 	 * or from the backlog processing.
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 08/14] SCTP: Use hashed lookup when looking for an association.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

A SCTP endpoint may have a lot of associations on them and walking
the list is fairly inefficient.  Instead, use a hashed lookup,
and filter out the hash list based on the endopoing we already have.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/endpointola.c |   34 ++++++++++++++++++++++------------
 1 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 2d2d81e..68f0556 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -328,24 +328,34 @@ static struct sctp_association *__sctp_endpoint_lookup_assoc(
 	const union sctp_addr *paddr,
 	struct sctp_transport **transport)
 {
+	struct sctp_association *asoc = NULL;
+	struct sctp_transport *t = NULL;
+	struct sctp_hashbucket *head;
+	struct sctp_ep_common *epb;
+	int hash;
 	int rport;
-	struct sctp_association *asoc;
-	struct list_head *pos;
 
+	*transport = NULL;
 	rport = ntohs(paddr->v4.sin_port);
 
-	list_for_each(pos, &ep->asocs) {
-		asoc = list_entry(pos, struct sctp_association, asocs);
-		if (rport == asoc->peer.port) {
-			*transport = sctp_assoc_lookup_paddr(asoc, paddr);
-
-			if (*transport)
-				return asoc;
+	hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
+	head = &sctp_assoc_hashtable[hash];
+	read_lock(&head->lock);
+	for (epb = head->chain; epb; epb = epb->next) {
+		asoc = sctp_assoc(epb);
+		if (asoc->ep != ep || rport != asoc->peer.port)
+			goto next;
+
+		t = sctp_assoc_lookup_paddr(asoc, paddr);
+		if (t) {
+			*transport = t;
+			break;
 		}
+next:
+		asoc = NULL;
 	}
-
-	*transport = NULL;
-	return NULL;
+	read_unlock(&head->lock);
+	return asoc;
 }
 
 /* Lookup association on an endpoint based on a peer address.  BH-safe.  */
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 09/14] SCTP: Convert custom hash lists to use hlist.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

Convert the custom hash list traversals to use hlist functions.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/sctp.h    |    3 +++
 include/net/sctp/structs.h |   10 ++++------
 net/sctp/endpointola.c     |    3 ++-
 net/sctp/input.c           |   43 +++++++++++--------------------------------
 net/sctp/proc.c            |    6 ++++--
 net/sctp/protocol.c        |    6 +++---
 net/sctp/socket.c          |   14 +++++---------
 7 files changed, 32 insertions(+), 53 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 7082730..67c997c 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -665,6 +665,9 @@ static inline int sctp_vtag_hashfn(__u16 lport, __u16 rport, __u32 vtag)
 	return (h & (sctp_assoc_hashsize-1));
 }
 
+#define sctp_for_each_hentry(epb, node, head) \
+	hlist_for_each_entry(epb, node, head, node)
+
 /* Is a socket of this style? */
 #define sctp_style(sk, style) __sctp_style((sk), (SCTP_SOCKET_##style))
 static inline int __sctp_style(const struct sock *sk, sctp_socket_type_t style)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 44f2672..eb3113c 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -100,20 +100,19 @@ struct crypto_hash;
 struct sctp_bind_bucket {
 	unsigned short	port;
 	unsigned short	fastreuse;
-	struct sctp_bind_bucket *next;
-	struct sctp_bind_bucket **pprev;
+	struct hlist_node	node;
 	struct hlist_head	owner;
 };
 
 struct sctp_bind_hashbucket {
 	spinlock_t	lock;
-	struct sctp_bind_bucket	*chain;
+	struct hlist_head	chain;
 };
 
 /* Used for hashing all associations.  */
 struct sctp_hashbucket {
 	rwlock_t	lock;
-	struct sctp_ep_common  *chain;
+	struct hlist_head	chain;
 } __attribute__((__aligned__(8)));
 
 
@@ -1230,8 +1229,7 @@ typedef enum {
 
 struct sctp_ep_common {
 	/* Fields to help us manage our entries in the hash tables. */
-	struct sctp_ep_common *next;
-	struct sctp_ep_common **pprev;
+	struct hlist_node node;
 	int hashent;
 
 	/* Runtime type information.  What kind of endpoint is this? */
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 68f0556..de6f505 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -332,6 +332,7 @@ static struct sctp_association *__sctp_endpoint_lookup_assoc(
 	struct sctp_transport *t = NULL;
 	struct sctp_hashbucket *head;
 	struct sctp_ep_common *epb;
+	struct hlist_node *node;
 	int hash;
 	int rport;
 
@@ -341,7 +342,7 @@ static struct sctp_association *__sctp_endpoint_lookup_assoc(
 	hash = sctp_assoc_hashfn(ep->base.bind_addr.port, rport);
 	head = &sctp_assoc_hashtable[hash];
 	read_lock(&head->lock);
-	for (epb = head->chain; epb; epb = epb->next) {
+	sctp_for_each_hentry(epb, node, &head->chain) {
 		asoc = sctp_assoc(epb);
 		if (asoc->ep != ep || rport != asoc->peer.port)
 			goto next;
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 86503e7..91ae463 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -656,7 +656,6 @@ discard:
 /* Insert endpoint into the hash table.  */
 static void __sctp_hash_endpoint(struct sctp_endpoint *ep)
 {
-	struct sctp_ep_common **epp;
 	struct sctp_ep_common *epb;
 	struct sctp_hashbucket *head;
 
@@ -666,12 +665,7 @@ static void __sctp_hash_endpoint(struct sctp_endpoint *ep)
 	head = &sctp_ep_hashtable[epb->hashent];
 
 	sctp_write_lock(&head->lock);
-	epp = &head->chain;
-	epb->next = *epp;
-	if (epb->next)
-		(*epp)->pprev = &epb->next;
-	*epp = epb;
-	epb->pprev = epp;
+	hlist_add_head(&epb->node, &head->chain);
 	sctp_write_unlock(&head->lock);
 }
 
@@ -691,19 +685,15 @@ static void __sctp_unhash_endpoint(struct sctp_endpoint *ep)
 
 	epb = &ep->base;
 
+	if (hlist_unhashed(&epb->node))
+		return;
+
 	epb->hashent = sctp_ep_hashfn(epb->bind_addr.port);
 
 	head = &sctp_ep_hashtable[epb->hashent];
 
 	sctp_write_lock(&head->lock);
-
-	if (epb->pprev) {
-		if (epb->next)
-			epb->next->pprev = epb->pprev;
-		*epb->pprev = epb->next;
-		epb->pprev = NULL;
-	}
-
+	__hlist_del(&epb->node);
 	sctp_write_unlock(&head->lock);
 }
 
@@ -721,12 +711,13 @@ static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(const union sctp_addr *l
 	struct sctp_hashbucket *head;
 	struct sctp_ep_common *epb;
 	struct sctp_endpoint *ep;
+	struct hlist_node *node;
 	int hash;
 
 	hash = sctp_ep_hashfn(ntohs(laddr->v4.sin_port));
 	head = &sctp_ep_hashtable[hash];
 	read_lock(&head->lock);
-	for (epb = head->chain; epb; epb = epb->next) {
+	sctp_for_each_hentry(epb, node, &head->chain) {
 		ep = sctp_ep(epb);
 		if (sctp_endpoint_is_match(ep, laddr))
 			goto hit;
@@ -744,7 +735,6 @@ hit:
 /* Insert association into the hash table.  */
 static void __sctp_hash_established(struct sctp_association *asoc)
 {
-	struct sctp_ep_common **epp;
 	struct sctp_ep_common *epb;
 	struct sctp_hashbucket *head;
 
@@ -756,12 +746,7 @@ static void __sctp_hash_established(struct sctp_association *asoc)
 	head = &sctp_assoc_hashtable[epb->hashent];
 
 	sctp_write_lock(&head->lock);
-	epp = &head->chain;
-	epb->next = *epp;
-	if (epb->next)
-		(*epp)->pprev = &epb->next;
-	*epp = epb;
-	epb->pprev = epp;
+	hlist_add_head(&epb->node, &head->chain);
 	sctp_write_unlock(&head->lock);
 }
 
@@ -790,14 +775,7 @@ static void __sctp_unhash_established(struct sctp_association *asoc)
 	head = &sctp_assoc_hashtable[epb->hashent];
 
 	sctp_write_lock(&head->lock);
-
-	if (epb->pprev) {
-		if (epb->next)
-			epb->next->pprev = epb->pprev;
-		*epb->pprev = epb->next;
-		epb->pprev = NULL;
-	}
-
+	__hlist_del(&epb->node);
 	sctp_write_unlock(&head->lock);
 }
 
@@ -822,6 +800,7 @@ static struct sctp_association *__sctp_lookup_association(
 	struct sctp_ep_common *epb;
 	struct sctp_association *asoc;
 	struct sctp_transport *transport;
+	struct hlist_node *node;
 	int hash;
 
 	/* Optimize here for direct hit, only listening connections can
@@ -830,7 +809,7 @@ static struct sctp_association *__sctp_lookup_association(
 	hash = sctp_assoc_hashfn(ntohs(local->v4.sin_port), ntohs(peer->v4.sin_port));
 	head = &sctp_assoc_hashtable[hash];
 	read_lock(&head->lock);
-	for (epb = head->chain; epb; epb = epb->next) {
+	sctp_for_each_hentry(epb, node, &head->chain) {
 		asoc = sctp_assoc(epb);
 		transport = sctp_assoc_is_match(asoc, local, peer);
 		if (transport)
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index e4cd841..2499732 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -225,6 +225,7 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v)
 	struct sctp_ep_common *epb;
 	struct sctp_endpoint *ep;
 	struct sock *sk;
+	struct hlist_node *node;
 	int    hash = *(loff_t *)v;
 
 	if (hash >= sctp_ep_hashsize)
@@ -233,7 +234,7 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v)
 	head = &sctp_ep_hashtable[hash];
 	sctp_local_bh_disable();
 	read_lock(&head->lock);
-	for (epb = head->chain; epb; epb = epb->next) {
+	sctp_for_each_hentry(epb, node, &head->chain) {
 		ep = sctp_ep(epb);
 		sk = epb->sk;
 		seq_printf(seq, "%8p %8p %-3d %-3d %-4d %-5d %5d %5lu ", ep, sk,
@@ -328,6 +329,7 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
 	struct sctp_ep_common *epb;
 	struct sctp_association *assoc;
 	struct sock *sk;
+	struct hlist_node *node;
 	int    hash = *(loff_t *)v;
 
 	if (hash >= sctp_assoc_hashsize)
@@ -336,7 +338,7 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
 	head = &sctp_assoc_hashtable[hash];
 	sctp_local_bh_disable();
 	read_lock(&head->lock);
-	for (epb = head->chain; epb; epb = epb->next) {
+	sctp_for_each_hentry(epb, node, &head->chain) {
 		assoc = sctp_assoc(epb);
 		sk = epb->sk;
 		seq_printf(seq,
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index ecfab03..d50f610 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1137,7 +1137,7 @@ SCTP_STATIC __init int sctp_init(void)
 	}
 	for (i = 0; i < sctp_assoc_hashsize; i++) {
 		rwlock_init(&sctp_assoc_hashtable[i].lock);
-		sctp_assoc_hashtable[i].chain = NULL;
+		INIT_HLIST_HEAD(&sctp_assoc_hashtable[i].chain);
 	}
 
 	/* Allocate and initialize the endpoint hash table.  */
@@ -1151,7 +1151,7 @@ SCTP_STATIC __init int sctp_init(void)
 	}
 	for (i = 0; i < sctp_ep_hashsize; i++) {
 		rwlock_init(&sctp_ep_hashtable[i].lock);
-		sctp_ep_hashtable[i].chain = NULL;
+		INIT_HLIST_HEAD(&sctp_ep_hashtable[i].chain);
 	}
 
 	/* Allocate and initialize the SCTP port hash table.  */
@@ -1170,7 +1170,7 @@ SCTP_STATIC __init int sctp_init(void)
 	}
 	for (i = 0; i < sctp_port_hashsize; i++) {
 		spin_lock_init(&sctp_port_hashtable[i].lock);
-		sctp_port_hashtable[i].chain = NULL;
+		INIT_HLIST_HEAD(&sctp_port_hashtable[i].chain);
 	}
 
 	printk(KERN_INFO "SCTP: Hash tables configured "
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 6ce9b49..ff8bc95 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -5307,6 +5307,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 {
 	struct sctp_bind_hashbucket *head; /* hash list */
 	struct sctp_bind_bucket *pp; /* hash list port iterator */
+	struct hlist_node *node;
 	unsigned short snum;
 	int ret;
 
@@ -5331,7 +5332,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 			index = sctp_phashfn(rover);
 			head = &sctp_port_hashtable[index];
 			sctp_spin_lock(&head->lock);
-			for (pp = head->chain; pp; pp = pp->next)
+			sctp_for_each_hentry(pp, node, &head->chain)
 				if (pp->port == rover)
 					goto next;
 			break;
@@ -5358,7 +5359,7 @@ static long sctp_get_port_local(struct sock *sk, union sctp_addr *addr)
 		 */
 		head = &sctp_port_hashtable[sctp_phashfn(snum)];
 		sctp_spin_lock(&head->lock);
-		for (pp = head->chain; pp; pp = pp->next) {
+		sctp_for_each_hentry(pp, node, &head->chain) {
 			if (pp->port == snum)
 				goto pp_found;
 		}
@@ -5702,10 +5703,7 @@ static struct sctp_bind_bucket *sctp_bucket_create(
 		pp->port = snum;
 		pp->fastreuse = 0;
 		INIT_HLIST_HEAD(&pp->owner);
-		if ((pp->next = head->chain) != NULL)
-			pp->next->pprev = &pp->next;
-		head->chain = pp;
-		pp->pprev = &head->chain;
+		hlist_add_head(&pp->node, &head->chain);
 	}
 	return pp;
 }
@@ -5714,9 +5712,7 @@ static struct sctp_bind_bucket *sctp_bucket_create(
 static void sctp_bucket_destroy(struct sctp_bind_bucket *pp)
 {
 	if (pp && hlist_empty(&pp->owner)) {
-		if (pp->next)
-			pp->next->pprev = pp->pprev;
-		*(pp->pprev) = pp->next;
+		__hlist_del(&pp->node);
 		kmem_cache_free(sctp_bucket_cachep, pp);
 		SCTP_DBG_OBJCNT_DEC(bind_bucket);
 	}
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 11/14] SCTP: Fix PR-SCTP to deliver all the accumulated ordered chunks
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

There is a small bug when we process a FWD-TSN.  We'll deliver
anything upto the current next expected SSN.  However, if the
next expected is already in the queue, it will take another
chunk to trigger its delivery.  The fix is to simply check
the current queued SSN is the next expected one.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/ulpqueue.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index 4be92d0..4908041 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -862,7 +862,7 @@ static inline void sctp_ulpq_reap_ordered(struct sctp_ulpq *ulpq, __u16 sid)
 			continue;
 
 		/* see if this ssn has been marked by skipping */
-		if (!SSN_lt(cssn, sctp_ssn_peek(in, csid)))
+		if (!SSN_lte(cssn, sctp_ssn_peek(in, csid)))
 			break;
 
 		__skb_unlink(pos, &ulpq->lobby);
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 13/14] SCTP: Always flush the queue when uncorcking.
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

When the code calls uncork, trigger a queue flush, even
if the queue was not corked.  Most callers that explicitely
cork the queue will have additinal checks to see if they
corked it.  Callers who do not cork the queue expect packets
to flow when they call uncork.

The scneario that showcased this bug happend when we were not
able to bundle DATA with outgoing COOKIE-ECHO.  As a result
the data just sat in the outqueue and did not get transmitted.
The application expected a response, but nothing happened.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/outqueue.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 99a3db5..fa76f23 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -662,10 +662,9 @@ static int sctp_outq_flush_rtx(struct sctp_outq *q, struct sctp_packet *pkt,
 int sctp_outq_uncork(struct sctp_outq *q)
 {
 	int error = 0;
-	if (q->cork) {
+	if (q->cork)
 		q->cork = 0;
-		error = sctp_outq_flush(q, 0);
-	}
+	error = sctp_outq_flush(q, 0);
 	return error;
 }
 
-- 
1.5.2.4


^ permalink raw reply related

* [PATCH 14/14] Fix memory leak in discard case of sctp_sf_abort_violation()
From: Vlad Yasevich @ 2007-11-12 16:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, lksctp-developers, Jesper Juhl, Vlad Yasevich
In-Reply-To: <11948855091808-git-send-email-vladislav.yasevich@hp.com>

From: Jesper Juhl <jesper.juhl@gmail.com>

In net/sctp/sm_statefuns.c::sctp_sf_abort_violation() we may leak
the storage allocated for 'abort' by returning from the function
without using or freeing it. This happens in case
"sctp_auth_recv_cid(SCTP_CID_ABORT, asoc)" is true and we jump to
the 'discard' label.
Spotted by the Coverity checker.

The simple fix is to simply move the creation of the "abort chunk"
to after the possible jump to the 'discard' label. This way we don't
even have to allocate the memory at all in the problem case.

Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 net/sctp/sm_statefuns.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index a66075a..5ebbe80 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4064,11 +4064,6 @@ static sctp_disposition_t sctp_sf_abort_violation(
 	struct sctp_chunk *chunk =  arg;
 	struct sctp_chunk *abort = NULL;
 
-	/* Make the abort chunk. */
-	abort = sctp_make_abort_violation(asoc, chunk, payload, paylen);
-	if (!abort)
-		goto nomem;
-
 	/* SCTP-AUTH, Section 6.3:
 	 *    It should be noted that if the receiver wants to tear
 	 *    down an association in an authenticated way only, the
@@ -4083,6 +4078,11 @@ static sctp_disposition_t sctp_sf_abort_violation(
 	if (sctp_auth_recv_cid(SCTP_CID_ABORT, asoc))
 		goto discard;
 
+	/* Make the abort chunk. */
+	abort = sctp_make_abort_violation(asoc, chunk, payload, paylen);
+	if (!abort)
+		goto nomem;
+
 	if (asoc) {
 		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
 		SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS);
-- 
1.5.2.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