All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: "'Xin Long'" <lucien.xin@gmail.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	network dev <netdev@vger.kernel.org>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
	davem <davem@davemloft.net>, Vlad Yasevich <vyasevich@gmail.com>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>
Subject: Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
Date: Fri, 2 Sep 2016 10:46:57 -0300	[thread overview]
Message-ID: <20160902134657.GB11159@localhost.localdomain> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB00EF186@AcuExch.aculab.com>

On Fri, Sep 02, 2016 at 01:22:05PM +0000, 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_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 ?
> 
> I want to know what kind of weed the SCTP authors were smoking when they
> decided it was appropriate to put all of a systems IP addresses in the
> cookie and cookie-ack messages.
> 

The 'best effort' one I guess :-)

> It would be nice to have the local addresses selected by the kernel on the
> basis of the remote address - removing the necessity of the application
> to know the current network topology (and IP addresses) in order to bind
> to the correct local addresses before making an outward call.
> 
> This sort of requires that local addresses for a connection are more of a
> property of the routing table than anything else.
> 
> Consider the following network:
> 
>     ----+---------------+----------------------+---------
>         |               |                      |
>      x.x.1.1         x.x.1.2                y.y.1.2
>     10.1.1.1        10.1.1.2               10.1.1.2
>         |               |                      |
>     ----+---------------+                      +---------
> 
> A connection from x.x.1.1 to x.x.1.2 needs to specify the 10.1.1.* addresses,
> but a connection for x.x.1.1 to y.y.1.2 must not.
> 

Exactly. Your example is an exact match with the issue I had last week.
That 10.1.1.2 of yours, may very well be 192.168.122.1 from libvirt.
Then the host may attempt to send the packet to itself, and abort the
association due to that.

I hated this because it took me a big while to find out about it. 

I guess they just didn't predict this situation of duplicated/internal
addresses. Otherwise, the address is there, reachable or not. It's in
the best effort situation.

> I'm not at sure whether it is possible to setup listener(s) on x.x.1.1
> that can accept connections from both x.x.1.2 and y.y.1.2.

You mean without an explicit bind()?

  Marcelo

WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: 'Xin Long' <lucien.xin@gmail.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	network dev <netdev@vger.kernel.org>,
	"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
	davem <davem@davemloft.net>, Vlad Yasevich <vyasevich@gmail.com>,
	"daniel@iogearbox.net" <daniel@iogearbox.net>
Subject: Re: [PATCH net 2/2] sctp: not copying duplicate addrs to the assoc's bind address list
Date: Fri, 02 Sep 2016 13:46:57 +0000	[thread overview]
Message-ID: <20160902134657.GB11159@localhost.localdomain> (raw)
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB00EF186@AcuExch.aculab.com>

On Fri, Sep 02, 2016 at 01:22:05PM +0000, 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_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 ?
> 
> I want to know what kind of weed the SCTP authors were smoking when they
> decided it was appropriate to put all of a systems IP addresses in the
> cookie and cookie-ack messages.
> 

The 'best effort' one I guess :-)

> It would be nice to have the local addresses selected by the kernel on the
> basis of the remote address - removing the necessity of the application
> to know the current network topology (and IP addresses) in order to bind
> to the correct local addresses before making an outward call.
> 
> This sort of requires that local addresses for a connection are more of a
> property of the routing table than anything else.
> 
> Consider the following network:
> 
>     ----+---------------+----------------------+---------
>         |               |                      |
>      x.x.1.1         x.x.1.2                y.y.1.2
>     10.1.1.1        10.1.1.2               10.1.1.2
>         |               |                      |
>     ----+---------------+                      +---------
> 
> A connection from x.x.1.1 to x.x.1.2 needs to specify the 10.1.1.* addresses,
> but a connection for x.x.1.1 to y.y.1.2 must not.
> 

Exactly. Your example is an exact match with the issue I had last week.
That 10.1.1.2 of yours, may very well be 192.168.122.1 from libvirt.
Then the host may attempt to send the packet to itself, and abort the
association due to that.

I hated this because it took me a big while to find out about it. 

I guess they just didn't predict this situation of duplicated/internal
addresses. Otherwise, the address is there, reachable or not. It's in
the best effort situation.

> I'm not at sure whether it is possible to setup listener(s) on x.x.1.1
> that can accept connections from both x.x.1.2 and y.y.1.2.

You mean without an explicit bind()?

  Marcelo


  reply	other threads:[~2016-09-02 13:47 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
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 [this message]
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=20160902134657.GB11159@localhost.localdomain \
    --to=marcelo.leitner@gmail.com \
    --cc=David.Laight@ACULAB.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.