Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-12-19 Thread Neil Horman
On Sat, Dec 17, 2016 at 05:56:51PM +0800, Xin Long wrote: > On Wed, Aug 24, 2016 at 6:38 PM, Neil Horman wrote: > > On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: > >> >> > Ah, I see what you're doing. Ok, this makes some sense, at least on > >> >> > the receive > >> >> > side, when y

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-12-17 Thread Xin Long
On Wed, Aug 24, 2016 at 6:38 PM, Neil Horman wrote: > On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: >> >> > Ah, I see what you're doing. Ok, this makes some sense, at least on >> >> > the receive >> >> > side, when you get a cookie unpacked and modify the remote peers >> >> > addres

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-09-02 Thread 'Marcelo Ricardo Leitner'
On Fri, Sep 02, 2016 at 02:25:42PM +, David Laight wrote: > From: Marcelo Ricardo Leitner > > Sent: 02 September 2016 14:47 > ... > > > Consider the following network: > > > > > > +---+--+- > > > | | | >

RE: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-09-02 Thread David Laight
From: Marcelo Ricardo Leitner > Sent: 02 September 2016 14:47 ... > > Consider the following network: > > > > +---+--+- > > | | | > > x.x.1.1 x.x.1.2y.y.1.2 > > 10.1.1.1

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-09-02 Thread Marcelo Ricardo Leitner
On Fri, Sep 02, 2016 at 01:22:05PM +, David Laight wrote: > From: Of Xin Long > > Sent: 25 August 2016 05:04 > ... > > But I still prefer the current patch. > > 1. This issue only happens when server bind 'ANY' addresses. > > we don't need to add any new members to struct sctp_sockaddr_entr

RE: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-09-02 Thread David Laight
From: Of Xin Long > Sent: 25 August 2016 05:04 ... > But I still prefer the current patch. > 1. This issue only happens when server bind 'ANY' addresses. > we don't need to add any new members to struct sctp_sockaddr_entry. > especially if it's a really corner issue, we fix this as an impr

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-25 Thread Marcelo Ricardo Leitner
On Thu, Aug 25, 2016 at 12:03:30PM +0800, Xin Long wrote: > > Or add a refcnt to its members. > > NETDEV_UP, it gets a ++ if it's already there > > NETDEV_DOWN, it gets a -- and cleans it up if it reaches 0 > > And the rest probably could stay the same. > > > Yes, it could also avoid the issue of

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-24 Thread Xin Long
> Or add a refcnt to its members. > NETDEV_UP, it gets a ++ if it's already there > NETDEV_DOWN, it gets a -- and cleans it up if it reaches 0 > And the rest probably could stay the same. > Yes, it could also avoid the issue of amounts of duplicate addrs. or add a nic index variable to its member

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-24 Thread Marcelo Ricardo Leitner
On Mon, Aug 22, 2016 at 10:25:38AM -0400, Neil Horman wrote: > On Sat, Aug 20, 2016 at 02:41:01PM +0800, Xin Long wrote: > > > Ah, I see what you're doing. Ok, this makes some sense, at least on the > > > receive > > > side, when you get a cookie unpacked and modify the remote peers address > >

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-24 Thread Neil Horman
On Wed, Aug 24, 2016 at 01:14:27PM +0800, Xin Long wrote: > >> > Ah, I see what you're doing. Ok, this makes some sense, at least on the > >> > receive > >> > side, when you get a cookie unpacked and modify the remote peers address > >> > list, > >> > it makes sense to check for duplicates. On

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-23 Thread Xin Long
>> > Ah, I see what you're doing. Ok, this makes some sense, at least on the >> > receive >> > side, when you get a cookie unpacked and modify the remote peers address >> > list, >> > it makes sense to check for duplicates. On the local side however, I >> > would, >> > instead of checking it w

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-22 Thread Neil Horman
On Sat, Aug 20, 2016 at 02:41:01PM +0800, Xin Long wrote: > > Ah, I see what you're doing. Ok, this makes some sense, at least on the > > receive > > side, when you get a cookie unpacked and modify the remote peers address > > list, > > it makes sense to check for duplicates. On the local side

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-19 Thread Xin Long
> Ah, I see what you're doing. Ok, this makes some sense, at least on the > receive > side, when you get a cookie unpacked and modify the remote peers address list, > it makes sense to check for duplicates. On the local side however, I would, > instead of checking it when the list gets copied, I

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-19 Thread Neil Horman
On Fri, Aug 19, 2016 at 07:30:22PM +0800, Xin Long wrote: > From: Xin Long > > sctp.local_addr_list is a global address list that is supposed to include > all the local addresses. sctp updates this list according to NETDEV_UP/ > NETDEV_DOWN notifications. > > However, if multiple NICs have the s

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-19 Thread Xin Long
> Under what valid use case will multiple interfaces have the same network > address? > Hi, Neil. I'm not sure the specific valid use case. The point is, do we trust the sctp global addr list has no duplicate address ? In one of users' computer, he found hundreds of duplicate addresses in INIT_ACK

Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-19 Thread Neil Horman
On Fri, Aug 19, 2016 at 07:30:22PM +0800, Xin Long wrote: > From: Xin Long > > sctp.local_addr_list is a global address list that is supposed to include > all the local addresses. sctp updates this list according to NETDEV_UP/ > NETDEV_DOWN notifications. > > However, if multiple NICs have the s

[PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list

2016-08-19 Thread Xin Long
From: Xin Long sctp.local_addr_list is a global address list that is supposed to include all the local addresses. sctp updates this list according to NETDEV_UP/ NETDEV_DOWN notifications. However, if multiple NICs have the same address, the global list will have duplicate addresses. Even if for