All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlad Yasevich <vyasevich@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jan Ariyasu <jan.ariyasu@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jan Ariyasu <jan.ariyasu@hp.com>
Subject: Re: [PATCH 00/13] SCTP: Enable netns
Date: Mon, 06 Aug 2012 20:06:58 +0000	[thread overview]
Message-ID: <502023E2.70207@gmail.com> (raw)
In-Reply-To: <874noflrzd.fsf@xmission.com>

On 08/06/2012 03:50 PM, Eric W. Biederman wrote:
> Vlad Yasevich <vyasevich@gmail.com> writes:
>
>
>> Hi Eric
>>
>> Associations are looked up by ports, but then verifyed by addresses.
>> Also, associations belong to sockets and simply validating the socket
>> namespace should be sufficient.
>
> True.  Your set of patches isn't quite as likely to malfunction as it
> looked at first glance.  It requires address reuse which happens accross
> namespaces but not too frequently.

Last time I looked at Jan's patches, I though she took care of the 
address re-use issue.  It isn't technically necessary to include 
namespace into the hash mix, but I think it will make chains shorter 
when namespaces are involved.  Might be interesting to look.

>
> As for validating the socket namespace I agree that is the fix and my
> patchset winds up doing it.

Yes, I saw that.

>
>>> The downside with my version is that it does not make all of the sctp
>>> tunables per network namespace the way yours does, but making all of
>>> the tunables per network namespace should be straight forward from
>>> my base.
>>>
>>> My patchset also misses some nice to haves like making the association
>>> id allocation per network namespace.  It is not important for
>>> correctness of the code but it might allow an information leak between
>>> namespaces.
>>
>> Hmm.. this one might be nice to have not from the perspective of leak,
>> but from resource limitation.  Without this, once the id space is
>> global is can be exhausted faster.
>
> It takes a lot of associtations to exhaust the id space, but I have no
> fundamental problems problems with the id allocation being per
> namespace.  I had actually overlooked the local association id when I
> did my patches.   After looking it became clear that making the
> association id global was not necessary so I left it.
>
> The sctp association id is a strange beast.  My personal inclination is
> that the sctp association id really ought to be per sctp socket, but I
> have not looked enough at the sctp userspace API to see if that works in
> practice.  Shrug.
>
> Mostly I am in favor of simple and correct.

Technically association id must be unique within a namespace.  Having 
global id space may be simpler and correct enough as there would be no 
duplication of ids between namespaces.  The only thing of value the 
per/namespace id space provides is that it restored the theoretical 
maximum on sctp associations one can have.

However, this means teaching IDR about namespaces... :)

We can skip it for now.

-vlad
>
> Eric
>


WARNING: multiple messages have this Message-ID (diff)
From: Vlad Yasevich <vyasevich@gmail.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jan Ariyasu <jan.ariyasu@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-sctp@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Jan Ariyasu <jan.ariyasu@hp.com>
Subject: Re: [PATCH 00/13] SCTP: Enable netns
Date: Mon, 06 Aug 2012 16:06:58 -0400	[thread overview]
Message-ID: <502023E2.70207@gmail.com> (raw)
In-Reply-To: <874noflrzd.fsf@xmission.com>

On 08/06/2012 03:50 PM, Eric W. Biederman wrote:
> Vlad Yasevich <vyasevich@gmail.com> writes:
>
>
>> Hi Eric
>>
>> Associations are looked up by ports, but then verifyed by addresses.
>> Also, associations belong to sockets and simply validating the socket
>> namespace should be sufficient.
>
> True.  Your set of patches isn't quite as likely to malfunction as it
> looked at first glance.  It requires address reuse which happens accross
> namespaces but not too frequently.

Last time I looked at Jan's patches, I though she took care of the 
address re-use issue.  It isn't technically necessary to include 
namespace into the hash mix, but I think it will make chains shorter 
when namespaces are involved.  Might be interesting to look.

>
> As for validating the socket namespace I agree that is the fix and my
> patchset winds up doing it.

Yes, I saw that.

>
>>> The downside with my version is that it does not make all of the sctp
>>> tunables per network namespace the way yours does, but making all of
>>> the tunables per network namespace should be straight forward from
>>> my base.
>>>
>>> My patchset also misses some nice to haves like making the association
>>> id allocation per network namespace.  It is not important for
>>> correctness of the code but it might allow an information leak between
>>> namespaces.
>>
>> Hmm.. this one might be nice to have not from the perspective of leak,
>> but from resource limitation.  Without this, once the id space is
>> global is can be exhausted faster.
>
> It takes a lot of associtations to exhaust the id space, but I have no
> fundamental problems problems with the id allocation being per
> namespace.  I had actually overlooked the local association id when I
> did my patches.   After looking it became clear that making the
> association id global was not necessary so I left it.
>
> The sctp association id is a strange beast.  My personal inclination is
> that the sctp association id really ought to be per sctp socket, but I
> have not looked enough at the sctp userspace API to see if that works in
> practice.  Shrug.
>
> Mostly I am in favor of simple and correct.

Technically association id must be unique within a namespace.  Having 
global id space may be simpler and correct enough as there would be no 
duplication of ids between namespaces.  The only thing of value the 
per/namespace id space provides is that it restored the theoretical 
maximum on sctp associations one can have.

However, this means teaching IDR about namespaces... :)

We can skip it for now.

-vlad
>
> Eric
>


  reply	other threads:[~2012-08-06 20:06 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-04 21:30 [PATCH 00/13] SCTP: Enable netns Jan Ariyasu
2012-08-04 21:30 ` Jan Ariyasu
2012-08-04 23:57 ` David Miller
2012-08-04 23:57   ` David Miller
2012-08-06 18:20 ` Eric W. Biederman
2012-08-06 18:20   ` Eric W. Biederman
2012-08-06 18:38   ` [PATCH net-next 0/9] sctp: Basic support for network namespaces Eric W. Biederman
2012-08-06 18:38     ` Eric W. Biederman
2012-08-06 18:39     ` [PATCH net-next 1/9] sctp: Make the port hash table use struct net in it's key Eric W. Biederman
2012-08-06 18:39       ` Eric W. Biederman
2012-08-15  3:18       ` Vlad Yasevich
2012-08-15  3:18         ` Vlad Yasevich
2012-08-06 18:40     ` [PATCH net-next 2/9] sctp: Make the endpoint hashtable handle multiple network namespaces Eric W. Biederman
2012-08-06 18:40       ` Eric W. Biederman
2012-08-15  3:18       ` Vlad Yasevich
2012-08-15  3:18         ` Vlad Yasevich
2012-08-06 18:41     ` [PATCH net-next 3/9] sctp: Make the association " Eric W. Biederman
2012-08-06 18:41       ` Eric W. Biederman
2012-08-15  3:18       ` Vlad Yasevich
2012-08-15  3:18         ` Vlad Yasevich
2012-08-06 18:42     ` [PATCH net-next 4/9] sctp: Make the address lists per network namespace Eric W. Biederman
2012-08-06 18:42       ` Eric W. Biederman
2012-08-15  3:19       ` Vlad Yasevich
2012-08-15  3:19         ` Vlad Yasevich
2012-08-06 18:43     ` [PATCH net-next 5/9] sctp: Make the ctl_sock " Eric W. Biederman
2012-08-06 18:43       ` Eric W. Biederman
2012-08-15  3:19       ` Vlad Yasevich
2012-08-15  3:19         ` Vlad Yasevich
2012-08-06 18:44     ` [PATCH net-next 6/9] sctp: Move the percpu sockets counter out of sctp_proc_init Eric W. Biederman
2012-08-06 18:44       ` Eric W. Biederman
2012-08-15  3:19       ` Vlad Yasevich
2012-08-15  3:19         ` Vlad Yasevich
2012-08-06 18:45     ` [PATCH net-next 7/9] sctp: Make the proc files per network namespace Eric W. Biederman
2012-08-06 18:45       ` Eric W. Biederman
2012-08-15  3:19       ` Vlad Yasevich
2012-08-15  3:19         ` Vlad Yasevich
2012-08-06 18:46     ` [PATCH net-next 8/9] sctp: Enable sctp in all network namespaces Eric W. Biederman
2012-08-06 18:46       ` Eric W. Biederman
2012-08-15  3:20       ` Vlad Yasevich
2012-08-15  3:20         ` Vlad Yasevich
2012-08-06 18:47     ` [PATCH net-next 9/9] sctp: Make the mib per network namespace Eric W. Biederman
2012-08-06 18:47       ` Eric W. Biederman
2012-08-15  3:20       ` Vlad Yasevich
2012-08-15  3:20         ` Vlad Yasevich
2012-08-07 17:17     ` [PATCH net-next 0/7] sctp: network namespace support Part 2: per net tunables Eric W. Biederman
2012-08-07 17:17       ` Eric W. Biederman
2012-08-07 17:23       ` [PATCH net-next 1/7] sctp: Add infrastructure for per net sysctls Eric W. Biederman
2012-08-07 17:23         ` Eric W. Biederman
2012-08-15  3:20         ` Vlad Yasevich
2012-08-15  3:20           ` Vlad Yasevich
2012-08-07 17:25       ` [PATCH net-next 2/7] sctp: Push struct net down to sctp_chunk_event_lookup Eric W. Biederman
2012-08-07 17:25         ` Eric W. Biederman
2012-08-07 17:26       ` [PATCH net-next 3/7] sctp: Push struct net down into sctp_transport_init Eric W. Biederman
2012-08-07 17:26         ` Eric W. Biederman
2012-08-07 17:27       ` [PATCH net-next 4/7] sctp: Push struct net down into sctp_in_scope Eric W. Biederman
2012-08-07 17:27         ` Eric W. Biederman
2012-08-07 17:28       ` [PATCH net-next 5/7] sctp: Push struct net down into all of the state machine functions Eric W. Biederman
2012-08-07 17:29       ` [PATCH net-next 6/7] sctp: Push struct net down into sctp_verify_ext_param Eric W. Biederman
2012-08-07 17:29         ` Eric W. Biederman
2012-08-07 17:29       ` [PATCH net-next 7/7] sctp: Make sysctl tunables per net Eric W. Biederman
2012-08-07 17:29         ` Eric W. Biederman
2012-08-09  6:20       ` [PATCH net-next 0/7] sctp: network namespace support Part 2: per net tunables David Miller
2012-08-09  6:20         ` David Miller
2012-08-09 14:07         ` Vlad Yasevich
2012-08-09 14:07           ` Vlad Yasevich
2012-08-14 21:14         ` David Miller
2012-08-14 21:14           ` David Miller
2012-08-15  3:16           ` Vlad Yasevich
2012-08-15  3:16             ` Vlad Yasevich
2012-08-15  3:21       ` Vlad Yasevich
2012-08-15  3:21         ` Vlad Yasevich
2012-08-15  6:10       ` David Miller
2012-08-15  6:10         ` David Miller
2012-08-06 19:21   ` [PATCH 00/13] SCTP: Enable netns Vlad Yasevich
2012-08-06 19:21     ` Vlad Yasevich
2012-08-06 19:50     ` Eric W. Biederman
2012-08-06 19:50       ` Eric W. Biederman
2012-08-06 20:06       ` Vlad Yasevich [this message]
2012-08-06 20:06         ` Vlad Yasevich
2012-08-06 20:47       ` David Miller
2012-08-06 20:47         ` David Miller
2012-08-06 21:39         ` Vlad Yasevich
2012-08-06 21:39           ` Vlad Yasevich
2012-08-06 23:06           ` Eric W. Biederman
2012-08-06 23:06             ` Eric W. Biederman
2012-08-15  3:23 ` Vlad Yasevich
2012-08-15  3:23   ` Vlad Yasevich

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=502023E2.70207@gmail.com \
    --to=vyasevich@gmail.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=jan.ariyasu@gmail.com \
    --cc=jan.ariyasu@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /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.