All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
	network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem <davem@davemloft.net>,
	Vlad Yasevich <vyasevich@gmail.com>,
	daniel@iogearbox.net
Subject: Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
Date: Thu, 25 Aug 2016 12:10:46 +0000	[thread overview]
Message-ID: <20160825121046.GA11159@localhost.localdomain> (raw)
In-Reply-To: <CADvbK_dJbaze7qMsDp-7x0xANsXZX=JfYBNhKOo=2VgbM_ywZg@mail.gmail.com>

On Thu, Aug 25, 2016 at 12:03:30PM +0800, Xin Long wrote:
> > Or add a refcnt to its members. </idea>
> > 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 members.
> 
> 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 improvement.
> 
> 2. It's yet two issues  here, the duplicate addrs may be from
>    a) different local NICs.
>    b) the same one NIC.
>    It may be unexpectable to filter them in NETDEV_UP/DOWN events.
> 
> 3. We check it only when sctp really binds it, just like sctp_do_bind.
> 
> What do you think ?

Yep, +1. LGTM the current patch, thanks.


WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>,
	network dev <netdev@vger.kernel.org>,
	linux-sctp@vger.kernel.org, davem <davem@davemloft.net>,
	Vlad Yasevich <vyasevich@gmail.com>,
	daniel@iogearbox.net
Subject: Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
Date: Thu, 25 Aug 2016 09:10:46 -0300	[thread overview]
Message-ID: <20160825121046.GA11159@localhost.localdomain> (raw)
In-Reply-To: <CADvbK_dJbaze7qMsDp-7x0xANsXZX=JfYBNhKOo=2VgbM_ywZg@mail.gmail.com>

On Thu, Aug 25, 2016 at 12:03:30PM +0800, Xin Long wrote:
> > Or add a refcnt to its members. </idea>
> > 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 members.
> 
> 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 improvement.
> 
> 2. It's yet two issues  here, the duplicate addrs may be from
>    a) different local NICs.
>    b) the same one NIC.
>    It may be unexpectable to filter them in NETDEV_UP/DOWN events.
> 
> 3. We check it only when sctp really binds it, just like sctp_do_bind.
> 
> What do you think ?

Yep, +1. LGTM the current patch, thanks.

  reply	other threads:[~2016-08-25 12:10 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19 11:30 [PATCH net 0/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long
2016-08-19 11:30 ` Xin Long
2016-08-19 11:30 ` [PATCH net 1/2] sctp: reduce indent level in sctp_copy_local_addr_list Xin Long
2016-08-19 11:30   ` Xin Long
2016-08-19 11:30   ` [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list Xin Long
2016-08-19 11:30     ` Xin Long
2016-08-19 13:30     ` Neil Horman
2016-08-19 13:30       ` Neil Horman
2016-08-19 15:16       ` Xin Long
2016-08-19 15:16         ` Xin Long
2016-08-19 17:50     ` Neil Horman
2016-08-19 17:50       ` Neil Horman
2016-08-20  6:41       ` Xin Long
2016-08-20  6:41         ` Xin Long
2016-08-22 14:25         ` Neil Horman
2016-08-22 14:25           ` Neil Horman
2016-08-24  5:14           ` Xin Long
2016-08-24  5:14             ` Xin Long
2016-08-24 10:38             ` Neil Horman
2016-08-24 10:38               ` Neil Horman
2016-12-17  9:56               ` Xin Long
2016-12-17  9:56                 ` Xin Long
2016-12-19 12:35                 ` Neil Horman
2016-12-19 12:35                   ` Neil Horman
2016-08-24 11:23           ` Marcelo Ricardo Leitner
2016-08-24 11:23             ` Marcelo Ricardo Leitner
2016-08-25  4:03             ` Xin Long
2016-08-25  4:03               ` Xin Long
2016-08-25 12:10               ` Marcelo Ricardo Leitner [this message]
2016-08-25 12:10                 ` Marcelo Ricardo Leitner
2016-09-02 13:22               ` David Laight
2016-09-02 13:22                 ` David Laight
2016-09-02 13:46                 ` Marcelo Ricardo Leitner
2016-09-02 13:46                   ` Marcelo Ricardo Leitner
2016-09-02 14:25                   ` David Laight
2016-09-02 14:25                     ` David Laight
2016-09-02 14:44                     ` 'Marcelo Ricardo Leitner'
2016-09-02 14:44                       ` 'Marcelo Ricardo Leitner'

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160825121046.GA11159@localhost.localdomain \
    --to=marcelo.leitner@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.