Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH] Documentation: charger: max14577: Update the date of introducing ABI
From: Sebastian Reichel @ 2014-10-03 14:29 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Lee Jones
In-Reply-To: <1412251954-29912-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

Hi,

On Thu, Oct 02, 2014 at 02:12:34PM +0200, Krzysztof Kozlowski wrote:
> Update the date of introducing max14577 charger's ABI (fast_charge_timer
> sysfs entry) to approximate date of kernel release which actually
> introduces this.
> 
> The old date came from previous driver submissions.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

http://git.infradead.org/battery-2.6.git/commit/c8c5ebcc5e33a0ae65b6b1cba8d257d423f25da0

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Jens Axboe @ 2014-10-03 14:28 UTC (permalink / raw)
  To: Thanos Makatos, linux-fsdevel@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	jlayton@poochiereds.net, bfields@fieldses.org, jack@suse.cz
In-Reply-To: <2368A3FCF9F7214298E53C823B0A48EC0423DFE3@AMSPEX01CL02.citrite.net>

On 2014-10-03 03:25, Thanos Makatos wrote:
>>> -		fsync_bdev(bdev);
>>> -		invalidate_bdev(bdev);
>>> +		flush_buffer_cache(bdev);
>>> +		if (BLKFLSBUF2 == cmd)
>>> +			return invalidate_inode_pages2(
>>> +					bdev->bd_inode->i_mapping);
>>>   		return 0;
>>
>> We're currently ignoring the buffer cache sync and invalidation (which is odd),
>> but at least being consistent would be good.
>>
>> Might also need a filemap_write_and_wait() to sync before invalidation.
>
> (I've got zero knowledge in this area, so excuse my ignorance!)
>
> Does filemap_write_and_wait() writes back modified, memory-mapped pages? If so,
> isn't there a race condition? Or have I got it completely wrong?

There's no race to be concerned of for this ioctl. Any page dirtied 
before you make the call will be synced, any page dirtied after may not. 
This is no different than what would happen on the buffer cache side in 
the current ioctl.

-- 
Jens Axboe


^ permalink raw reply

* Re: [RFC PATCH net-next v3 1/4] netns: add genl cmd to add and get peer netns ids
From: Nicolas Dichtel @ 2014-10-03 12:22 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	luto-kltTT9wpgjJwATOyAt5JVQ, cwang-xCSkyg8dI+0RB7SZvlqPiA
In-Reply-To: <87tx3mmflp.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

Le 02/10/2014 21:33, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
>
>> With this patch, a user can define an id for a peer netns by providing a FD or a
>> PID. These ids are local to netns (ie valid only into one netns).
>>
>> This will be useful for netlink messages when a x-netns interface is
>> dumped.
>
> You have a "id -> struct net *" table but you don't have a
> "struct net * -> id" table which looks like it will impact the
> performance of peernet2id at scale.
It is indirectly stores in 'struct idr'. It can be optimized later, with a
proper algorithm to find quickly this 'struct net *' (hash table? something
else?). A basic algorithm will not be more scalable than the current
idr_for_each().

^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Nicolas Dichtel @ 2014-10-03 12:22 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Andy Lutomirski, Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
	David S. Miller, Stephen Hemminger, Andrew Morton, Cong Wang
In-Reply-To: <8761g2nurx.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

Le 02/10/2014 21:20, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
>
>> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>>> Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
>>>
>>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>>> Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
>>>>>
>>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>>> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>>>>>>> I see two ways to go with this.
>>>>>>>
>>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>>      network namespaces.  The table would need to be populated manually by
>>>>>>>      the likes of ip netns add.
>>>>>>>
>>>>>>>      That flips the order of assignment and makes this idea solid.
>>>> I have a preference for this solution, because it allows to have a full
>>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>>> it saves a lot of time to avoid another request to get all informations.
>>>
>>> My practical question is how often does it happen that we care?
>> In fact, I don't think that scenarii with a lot of netns have a full mesh of
>> x-netns interfaces. It will be more one "link" netns with the physical
>> interface and all other with one interface with the link part in this "link"
>> netns. Hence, only one nsid is needing in each netns.
>
> I will buy that a full mesh is unlikely.
>
> For people doing simulations anything physical has a limited number of
> links.
>
> For people wanting all to all connectivity setting up an internal
> macvlan (or the equivalent) is likely much simpler and more efficient
> that a full mesh.
>
> So the question in my mind is how do we create these identifiers at need
> (when we create the cross network namespace links) instead of at network
> namespace creation time.  I don't see an answer to that in your patches,
> and perhaps it obvious.
For me, it is the responsability of the user who creates the netns. He should
know what will be done with this new netns, hence he may or may not define an
id. It's also possible to delegate this to the user who will create the tunnel.
In other words, it's part of the configuration.

^ permalink raw reply

* RE: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Thanos Makatos @ 2014-10-03  9:25 UTC (permalink / raw)
  To: 'Jens Axboe',
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org,
	bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org,
	jack-AlSwsSmVLrQ@public.gmane.org
In-Reply-To: <542DAEAC.8010203-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>

> > -		fsync_bdev(bdev);
> > -		invalidate_bdev(bdev);
> > +		flush_buffer_cache(bdev);
> > +		if (BLKFLSBUF2 == cmd)
> > +			return invalidate_inode_pages2(
> > +					bdev->bd_inode->i_mapping);
> >  		return 0;
> 
> We're currently ignoring the buffer cache sync and invalidation (which is odd),
> but at least being consistent would be good.
> 
> Might also need a filemap_write_and_wait() to sync before invalidation.

(I've got zero knowledge in this area, so excuse my ignorance!)

Does filemap_write_and_wait() writes back modified, memory-mapped pages? If so, isn't there a race condition? Or have I got it completely wrong?

^ permalink raw reply

* RE: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Thanos Makatos @ 2014-10-03  9:00 UTC (permalink / raw)
  To: 'Dave Chinner', Jens Axboe
  Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org, jlayton@poochiereds.net,
	bfields@fieldses.org, jack@suse.cz
In-Reply-To: <20141003052737.GL24490@dastard>

> > > This patch introduces a new ioctl called BLKFLUSHBUFS2, which is
> > > pretty
> 
> What a horrible name. Whatever happened to naming ioctls interfaces after
> their function? i.e. BLKFLUSHINVAL?

Indeed it's not a good name, I'm open to suggestions!

^ permalink raw reply

* [PATCHv8.2] fanotify: enable close-on-exec on events' fd when requested in fanotify_init()
From: Yann Droneaud @ 2014-10-03  8:39 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Yann Droneaud, Jan Kara, Heinrich Schuchardt, Eric Paris,
	Richard Guy Briggs, Al Viro, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	stable-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Lino Sanfilippo, Valdis Kletnieks, Michael Kerrisk-manpages,
	Mihai Donțu, Pádraig Brady
In-Reply-To: <20141002124652.c877efeb35d07064e520a702-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>

According to commit 80af258867648 ('fanotify: groups can specify
their f_flags for new fd'), file descriptors created as part of
file access notification events inherit flags from the
event_f_flags argument passed to syscall fanotify_init(2)[1].

Unfortunately O_CLOEXEC is currently silently ignored.

Indeed, event_f_flags are only given to dentry_open(), which only
seems to care about O_ACCMODE and O_PATH in do_dentry_open(),
O_DIRECT in open_check_o_direct() and O_LARGEFILE in
generic_file_open().

It's a pity, since, according to some lookup on various search
engines and http://codesearch.debian.net/, there's already some
userspace code which use O_CLOEXEC:

- in systemd's readahead[2]:

    fanotify_fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK, O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);

- in clsync[3]:

    #define FANOTIFY_EVFLAGS (O_LARGEFILE|O_RDONLY|O_CLOEXEC)

    int fanotify_d = fanotify_init(FANOTIFY_FLAGS, FANOTIFY_EVFLAGS);

- in examples [4] from "Filesystem monitoring in the Linux
  kernel" article[5] by Aleksander Morgado:

    if ((fanotify_fd = fanotify_init (FAN_CLOEXEC,
                                      O_RDONLY | O_CLOEXEC | O_LARGEFILE)) < 0)

Additionally, since commit 48149e9d3a7e ('fanotify: check file
flags passed in fanotify_init'). having O_CLOEXEC as part of
fanotify_init() second argument is expressly allowed.

So it seems expected to set close-on-exec flag on the file
descriptors if userspace is allowed to request it with O_CLOEXEC.

But Andrew Morton raised[6] the concern that enabling now
close-on-exec might break existing applications which ask for
O_CLOEXEC but expect the file descriptor to be inherited
across exec().

In the other hand, as reported by Mihai Donțu[7], not setting
close-on-exec on the file descriptor returned as part of file
access notify can break applications due to deadlock.
So close-on-exec is needed for most applications.

More, applications asking for close-on-exec are likely expecting
it to be enabled, relying on O_CLOEXEC being effective. If not,
it might weaken their security, as noted by Jan Kara[8].

So this patch replaces call to macro get_unused_fd() by a call
to function get_unused_fd_flags() with event_f_flags value as
argument. This way O_CLOEXEC flag in the second argument of
fanotify_init(2) syscall is interpreted and close-on-exec
get enabled when requested.

[1] http://man7.org/linux/man-pages/man2/fanotify_init.2.html
[2] http://cgit.freedesktop.org/systemd/systemd/tree/src/readahead/readahead-collect.c?id=v208#n294
[3] https://github.com/xaionaro/clsync/blob/v0.2.1/sync.c#L1631
    https://github.com/xaionaro/clsync/blob/v0.2.1/configuration.h#L38
[4] http://www.lanedo.com/~aleksander/fanotify/fanotify-example.c
[5] http://www.lanedo.com/2013/filesystem-monitoring-linux-kernel/
[6] http://lkml.kernel.org/r/20141001153621.65e9258e65a6167bf2e4cb50@linux-foundation.org
[7] http://lkml.kernel.org/r/20141002095046.3715eb69@mdontu-l
[8] http://lkml.kernel.org/r/20141002104410.GB19748-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org

Link: http://lkml.kernel.org/r/cover.1411562410.git.ydroneaud-RlY5vtjFyJ1hl2p70BpVqQ@public.gmane.orgm
Cc: Mihai Donțu <mihai.dontu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pádraig Brady <P@draigBrady.com>
Cc: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
Cc: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Cc: Valdis Kletnieks <Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org>
Cc: Michael Kerrisk-manpages <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Lino Sanfilippo <LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org>
Cc: Richard Guy Briggs <rgb-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>
Reviewed by: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
Tested-by: Heinrich Schuchardt <xypron.glpk-Mmb7MZpHnFY@public.gmane.org>
Signed-off-by: Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org>
---
Hi Andrew,

> Fair enough, it sounds like the risk is acceptable.
>

OK.

> Can we get a new version sent out with all this new info appropriately
> changelogged?
>

Of course !

Please find an updated patch with revamped commit message.

Changes from v8.1:

- added more Cc:
- added Reviewed-by:
- rewrote commit message.

 fs/notify/fanotify/fanotify_user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index b13992a41bd9..c991616acca9 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -78,7 +78,7 @@ static int create_fd(struct fsnotify_group *group,
 
 	pr_debug("%s: group=%p event=%p\n", __func__, group, event);
 
-	client_fd = get_unused_fd();
+	client_fd = get_unused_fd_flags(group->fanotify_data.f_flags);
 	if (client_fd < 0)
 		return client_fd;
 
-- 
1.9.3

^ permalink raw reply related

* FUND TRANSFER.
From: martins summers @ 2014-10-03  6:44 UTC (permalink / raw)

In-Reply-To: <1163542406.248052.1412318680433.JavaMail.yahoo-MAmEC6bCamZ4twUfcxIKQ5OW+3bF1jUfVpNB7YpNyf8@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 60 bytes --]





Kindly read the attached Letter and get back to me asap 

[-- Attachment #2: CONTACT HSBC FOR FUND TRANSFER..doc --]
[-- Type: application/msword, Size: 47104 bytes --]

^ permalink raw reply

* Re: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Dave Chinner @ 2014-10-03  5:27 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Thanos Makatos, linux-fsdevel, linux-kernel, linux-api, jlayton,
	bfields, jack
In-Reply-To: <542DAEAC.8010203@kernel.dk>

On Thu, Oct 02, 2014 at 01:59:40PM -0600, Jens Axboe wrote:
> On 10/02/2014 10:09 AM, Thanos Makatos wrote:
> > This patch introduces a new ioctl called BLKFLUSHBUFS2, which is pretty

What a horrible name. Whatever happened to naming ioctls interfaces
after their function? i.e. BLKFLUSHINVAL?

Cheers,

Dave?

-- 
Dave Chinner
david@fromorbit.com

^ permalink raw reply

* [PATCH v4] mm: add mremap flag for preserving the old mapping
From: Daniel Micay @ 2014-10-03  3:41 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, linux-api, akpm, jasone, luto, Daniel Micay

This introduces the MREMAP_RETAIN flag for preserving the source mapping
when MREMAP_MAYMOVE moves the pages to a new destination. Accesses to
the source mapping will fault and map in fresh zeroed pages.

It is currently limited to writable MAP_PRIVATE|MAP_ANONYMOUS mappings
and will return EFAULT when used on anything else. This covers the
intended use case in general purpose allocators.

For consistency, the old_len >= new_len case could decommit the pages
instead of unmapping. However, userspace can accomplish the same thing
via madvise and the flag is coherent without the additional complexity.

Motivation:

TCMalloc and jemalloc avoid releasing virtual memory in order to reduce
virtual memory fragmentation. A call to munmap or mremap would leave a
hole in the address space. Instead, unused pages are lazily returned to
the operating system via MADV_DONTNEED.

Since mremap cannot be used to elide copies, TCMalloc and jemalloc end
up being significantly slower for patterns like repeated vector / hash
table reallocations. Consider the typical vector building pattern:

    #include <string.h>
    #include <stdlib.h>

    int main(void) {
        for (size_t i = 0; i < 100; i++) {
            void *ptr = NULL;
            size_t old_size = 0;
            for (size_t size = 4; size < (1 << 30); size *= 2) {
                ptr = realloc(ptr, size);
                if (!ptr) return 1;
                memset(ptr + old_size, 0xff, size - old_size);
                old_size = size;
            }
            free(ptr);
        }
    }

Transparent huge pages disabled:

glibc (baseline, uses mremap already): 15.051s
jemalloc without MREMAP_RETAIN: 38.540s
jemalloc with MREMAP_RETAIN: 15.086s

Transparent huge pages enabled:

glibc (baseline, uses mremap already): 8.464s
jemalloc without MREMAP_RETAIN: 18.230s
jemalloc with MREMAP_RETAIN: 6.696s

In practice, in-place growth never occurs for huge allocations because
the heap grows in the downwards direction for all 3 allocators. TCMalloc
and jemalloc pay for enormous copies while glibc is only spending time
writing new elements to the vector. Even if it was grown in the other
direction, real-world applications would end up blocking in-place growth
with new allocations.

The allocators could attempt to map the source location again after an
mremap call, but there is no guarantee of success in a multi-threaded
program and fragmentating memory over time is considered unacceptable.

Signed-off-by: Daniel Micay <danielmicay@gmail.com>
---
 include/linux/huge_mm.h   |  2 +-
 include/linux/mm.h        |  6 ++++++
 include/uapi/linux/mman.h |  1 +
 mm/huge_memory.c          |  4 ++--
 mm/memory.c               |  2 +-
 mm/mmap.c                 | 12 +++++++++++
 mm/mremap.c               | 52 +++++++++++++++++++++++++++++++----------------
 7 files changed, 57 insertions(+), 22 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 63579cb..3c13b20 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -143,7 +143,7 @@ static inline void vma_adjust_trans_huge(struct vm_area_struct *vma,
 					 unsigned long end,
 					 long adjust_next)
 {
-	if (!vma->anon_vma || vma->vm_ops)
+	if (!vma->anon_vma || (vma->vm_ops && !vma->vm_ops->allow_huge_pages))
 		return;
 	__vma_adjust_trans_huge(vma, start, end, adjust_next);
 }
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8981cc8..1e61036 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -273,6 +273,12 @@ struct vm_operations_struct {
 	/* called by sys_remap_file_pages() to populate non-linear mapping */
 	int (*remap_pages)(struct vm_area_struct *vma, unsigned long addr,
 			   unsigned long size, pgoff_t pgoff);
+
+	/* Check if the mapping may be duplicated by MREMAP_RETAIN */
+	bool (*may_duplicate)(struct vm_area_struct *vma);
+
+	/* if there is no vm_ops table, this is considered true */
+	bool allow_huge_pages;
 };
 
 struct mmu_gather;
diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
index ade4acd..4e9a546 100644
--- a/include/uapi/linux/mman.h
+++ b/include/uapi/linux/mman.h
@@ -5,6 +5,7 @@
 
 #define MREMAP_MAYMOVE	1
 #define MREMAP_FIXED	2
+#define MREMAP_RETAIN	4
 
 #define OVERCOMMIT_GUESS		0
 #define OVERCOMMIT_ALWAYS		1
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index d9a21d06..350b478 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2077,7 +2077,7 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma)
 		 * page fault if needed.
 		 */
 		return 0;
-	if (vma->vm_ops)
+	if ((vma->vm_ops && !vma->vm_ops->allow_huge_pages))
 		/* khugepaged not yet working on file or special mappings */
 		return 0;
 	VM_BUG_ON(vma->vm_flags & VM_NO_THP);
@@ -2405,7 +2405,7 @@ static bool hugepage_vma_check(struct vm_area_struct *vma)
 	    (vma->vm_flags & VM_NOHUGEPAGE))
 		return false;
 
-	if (!vma->anon_vma || vma->vm_ops)
+	if (!vma->anon_vma || (vma->vm_ops && !vma->vm_ops->allow_huge_pages))
 		return false;
 	if (is_vma_temporary_stack(vma))
 		return false;
diff --git a/mm/memory.c b/mm/memory.c
index e229970..c181401 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3275,7 +3275,7 @@ static int __handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 		return VM_FAULT_OOM;
 	if (pmd_none(*pmd) && transparent_hugepage_enabled(vma)) {
 		int ret = VM_FAULT_FALLBACK;
-		if (!vma->vm_ops)
+		if (!vma->vm_ops || vma->vm_ops->allow_huge_pages)
 			ret = do_huge_pmd_anonymous_page(mm, vma, address,
 					pmd, flags);
 		if (!(ret & VM_FAULT_FALLBACK))
diff --git a/mm/mmap.c b/mm/mmap.c
index c0a3637..6b644fe 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1500,6 +1500,16 @@ static inline int accountable_mapping(struct file *file, vm_flags_t vm_flags)
 	return (vm_flags & (VM_NORESERVE | VM_SHARED | VM_WRITE)) == VM_WRITE;
 }
 
+static bool anon_may_duplicate(struct vm_area_struct *vma)
+{
+	return vma->vm_flags & VM_WRITE && !(vma->vm_flags & VM_SHARED);
+}
+
+static const struct vm_operations_struct anon_vmops = {
+	.may_duplicate = anon_may_duplicate,
+	.allow_huge_pages = true
+};
+
 unsigned long mmap_region(struct file *file, unsigned long addr,
 		unsigned long len, vm_flags_t vm_flags, unsigned long pgoff)
 {
@@ -1569,6 +1579,8 @@ munmap_back:
 	vma->vm_flags = vm_flags;
 	vma->vm_page_prot = vm_get_page_prot(vm_flags);
 	vma->vm_pgoff = pgoff;
+	if (!file)
+		vma->vm_ops = &anon_vmops;
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 
 	if (file) {
diff --git a/mm/mremap.c b/mm/mremap.c
index 05f1180..ca7a662 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -235,7 +235,8 @@ unsigned long move_page_tables(struct vm_area_struct *vma,
 
 static unsigned long move_vma(struct vm_area_struct *vma,
 		unsigned long old_addr, unsigned long old_len,
-		unsigned long new_len, unsigned long new_addr, bool *locked)
+		unsigned long new_len, unsigned long new_addr, bool retain,
+		bool *locked)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct vm_area_struct *new_vma;
@@ -287,15 +288,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 		old_len = new_len;
 		old_addr = new_addr;
 		new_addr = -ENOMEM;
-	}
-
-	/* Conceal VM_ACCOUNT so old reservation is not undone */
-	if (vm_flags & VM_ACCOUNT) {
-		vma->vm_flags &= ~VM_ACCOUNT;
-		excess = vma->vm_end - vma->vm_start - old_len;
-		if (old_addr > vma->vm_start &&
-		    old_addr + old_len < vma->vm_end)
-			split = 1;
+		retain = false;
 	}
 
 	/*
@@ -310,6 +303,19 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 	hiwater_vm = mm->hiwater_vm;
 	vm_stat_account(mm, vma->vm_flags, vma->vm_file, new_len>>PAGE_SHIFT);
 
+	/* Leave the old mapping in place for MREMAP_RETAIN */
+	if (retain)
+		goto out;
+
+	/* Conceal VM_ACCOUNT so old reservation is not undone */
+	if (vm_flags & VM_ACCOUNT) {
+		vma->vm_flags &= ~VM_ACCOUNT;
+		excess = vma->vm_end - vma->vm_start - old_len;
+		if (old_addr > vma->vm_start &&
+		    old_addr + old_len < vma->vm_end)
+			split = 1;
+	}
+
 	if (do_munmap(mm, old_addr, old_len) < 0) {
 		/* OOM: unable to split vma, just get accounts right */
 		vm_unacct_memory(excess >> PAGE_SHIFT);
@@ -324,6 +330,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 			vma->vm_next->vm_flags |= VM_ACCOUNT;
 	}
 
+out:
 	if (vm_flags & VM_LOCKED) {
 		mm->locked_vm += new_len >> PAGE_SHIFT;
 		*locked = true;
@@ -333,7 +340,8 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 }
 
 static struct vm_area_struct *vma_to_resize(unsigned long addr,
-	unsigned long old_len, unsigned long new_len, unsigned long *p)
+	unsigned long old_len, unsigned long new_len, bool retain,
+	unsigned long *p)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma = find_vma(mm, addr);
@@ -348,6 +356,11 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
 	if (old_len > vma->vm_end - addr)
 		goto Efault;
 
+	/* Forbid MREMAP_RETAIN if not explicitly permitted by the mapping */
+	if (retain && !(vma->vm_ops && vma->vm_ops->may_duplicate &&
+	    vma->vm_ops->may_duplicate(vma)))
+		goto Efault;
+
 	/* Need to be careful about a growing mapping */
 	if (new_len > old_len) {
 		unsigned long pgoff;
@@ -392,7 +405,8 @@ Eagain:
 }
 
 static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
-		unsigned long new_addr, unsigned long new_len, bool *locked)
+		unsigned long new_addr, unsigned long new_len, bool retain,
+		bool *locked)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
@@ -426,7 +440,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
 		old_len = new_len;
 	}
 
-	vma = vma_to_resize(addr, old_len, new_len, &charged);
+	vma = vma_to_resize(addr, old_len, new_len, retain, &charged);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out;
@@ -442,7 +456,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
 	if (ret & ~PAGE_MASK)
 		goto out1;
 
-	ret = move_vma(vma, addr, old_len, new_len, new_addr, locked);
+	ret = move_vma(vma, addr, old_len, new_len, new_addr, retain, locked);
 	if (!(ret & ~PAGE_MASK))
 		goto out;
 out1:
@@ -482,7 +496,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	unsigned long charged = 0;
 	bool locked = false;
 
-	if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE))
+	if (flags & ~(MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_RETAIN))
 		return ret;
 
 	if (flags & MREMAP_FIXED && !(flags & MREMAP_MAYMOVE))
@@ -506,7 +520,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 
 	if (flags & MREMAP_FIXED) {
 		ret = mremap_to(addr, old_len, new_addr, new_len,
-				&locked);
+				flags & MREMAP_RETAIN, &locked);
 		goto out;
 	}
 
@@ -526,7 +540,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 	/*
 	 * Ok, we need to grow..
 	 */
-	vma = vma_to_resize(addr, old_len, new_len, &charged);
+	vma = vma_to_resize(addr, old_len, new_len, flags & MREMAP_RETAIN,
+			    &charged);
 	if (IS_ERR(vma)) {
 		ret = PTR_ERR(vma);
 		goto out;
@@ -575,7 +590,8 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
 			goto out;
 		}
 
-		ret = move_vma(vma, addr, old_len, new_len, new_addr, &locked);
+		ret = move_vma(vma, addr, old_len, new_len, new_addr,
+			       flags & MREMAP_RETAIN, &locked);
 	}
 out:
 	if (ret & ~PAGE_MASK)
-- 
2.1.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* Re: [PATCH v3] mm: add mremap flag for preserving the old mapping
From: Daniel Micay @ 2014-10-02 21:09 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton,
	Jason Evans, Linux API
In-Reply-To: <CALCETrVHgvhAN3neoOpJEk94uM7QKm2izZpp+=1UA6qieaQiTQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

On 30/09/14 01:49 PM, Andy Lutomirski wrote:
> 
> I think it might pay to add an explicit vm_op to authorize
> duplication, especially for non-cow mappings.  IOW this kind of
> extension seems quite magical for anything that doesn't have the
> normal COW semantics, including for plain old read-only mappings.

Adding a vm_ops table to MAP_PRIVATE|MAP_ANONYMOUS mappings has a
significant performance impact. I haven't yet narrowed it down, but
there's at least one code path a check of `!vma->vm_ops` for the fast
path. One is for transparent huge page faults, so the performance impact
makes sense. I'll use a simpler implementation for now since the
requirements are very narrow / simple.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH RFC] introduce ioctl to completely invalidate page cache
From: Jens Axboe @ 2014-10-02 19:59 UTC (permalink / raw)
  To: Thanos Makatos, linux-fsdevel
  Cc: linux-kernel, linux-api, jlayton, bfields, jack
In-Reply-To: <1412266184-23776-1-git-send-email-thanos.makatos@citrix.com>

On 10/02/2014 10:09 AM, Thanos Makatos wrote:
> This patch introduces a new ioctl called BLKFLUSHBUFS2, which is pretty
> similar to BLKFLUSHBUFS except that is also invalidates the page cache.
> This allows for a complete invalidation of the cached data of a
> particular block device, which might be useful for cases like
> synchronising the caches of an iSCSI block device used by multiple
> hosts.
> 
> Signed-off-by: Thanos Makatos <thanos.makatos@citrix.com>
> ---
>  block/compat_ioctl.c    |    1 +
>  block/ioctl.c           |   13 +++++++++++--
>  include/uapi/linux/fs.h |    1 +
>  3 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/block/compat_ioctl.c b/block/compat_ioctl.c
> index 18b282c..672388ab 100644
> --- a/block/compat_ioctl.c
> +++ b/block/compat_ioctl.c
> @@ -688,6 +688,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  	case BLKDISCARDZEROES:
>  		return compat_put_uint(arg, bdev_discard_zeroes_data(bdev));
>  	case BLKFLSBUF:
> +	case BLKFLSBUF2:
>  	case BLKROSET:
>  	case BLKDISCARD:
>  	case BLKSECDISCARD:
> diff --git a/block/ioctl.c b/block/ioctl.c
> index d6cda81..0c427a7 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -268,6 +268,12 @@ static inline int is_unrecognized_ioctl(int ret)
>  		ret == -ENOIOCTLCMD;
>  }
>  
> +static void flush_buffer_cache(struct block_device *bdev)
> +{
> +	fsync_bdev(bdev);
> +	invalidate_bdev(bdev);
> +}
> +
>  /*
>   * always keep this in sync with compat_blkdev_ioctl()
>   */
> @@ -282,6 +288,7 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
>  
>  	switch(cmd) {
>  	case BLKFLSBUF:
> +	case BLKFLSBUF2:
>  		if (!capable(CAP_SYS_ADMIN))
>  			return -EACCES;
>  
> @@ -289,8 +296,10 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
>  		if (!is_unrecognized_ioctl(ret))
>  			return ret;
>  
> -		fsync_bdev(bdev);
> -		invalidate_bdev(bdev);
> +		flush_buffer_cache(bdev);
> +		if (BLKFLSBUF2 == cmd)
> +			return invalidate_inode_pages2(
> +					bdev->bd_inode->i_mapping);
>  		return 0;

We're currently ignoring the buffer cache sync and invalidation (which
is odd), but at least being consistent would be good.

Might also need a filemap_write_and_wait() to sync before invalidation.

-- 
Jens Axboe


^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Andy Lutomirski @ 2014-10-02 19:48 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Andrew Morton, Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
	David S. Miller
In-Reply-To: <877g0il0gd.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

On Thu, Oct 2, 2014 at 12:45 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
>
>> On Thu, Oct 2, 2014 at 12:20 PM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>
>>>> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>>>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>>>
>>>>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>>>>
>>>>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>>>> I see two ways to go with this.
>>>>>>>>>
>>>>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>>>>     network namespaces.  The table would need to be populated manually by
>>>>>>>>>     the likes of ip netns add.
>>>>>>>>>
>>>>>>>>>     That flips the order of assignment and makes this idea solid.
>>>>>> I have a preference for this solution, because it allows to have a full
>>>>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>>>>> it saves a lot of time to avoid another request to get all informations.
>>>>>
>>>>> My practical question is how often does it happen that we care?
>>>> In fact, I don't think that scenarii with a lot of netns have a full mesh of
>>>> x-netns interfaces. It will be more one "link" netns with the physical
>>>> interface and all other with one interface with the link part in this "link"
>>>> netns. Hence, only one nsid is needing in each netns.
>>>
>>> I will buy that a full mesh is unlikely.
>>>
>>> For people doing simulations anything physical has a limited number of
>>> links.
>>>
>>> For people wanting all to all connectivity setting up an internal
>>> macvlan (or the equivalent) is likely much simpler and more efficient
>>> that a full mesh.
>>>
>>> So the question in my mind is how do we create these identifiers at need
>>> (when we create the cross network namespace links) instead of at network
>>> namespace creation time.  I don't see an answer to that in your patches,
>>> and perhaps it obvious.
>>>
>>
>> I wonder whether part of the problem is that we're thinking about
>> scoping wrong.  What if we made the hierarchy more explicit?
>>
>> For example, we could give each netns an admin-assigned identifier
>> (e.g. a 64-bit number, maybe required to be unique, maybe not)
>> relative to its containing userns.  Then we could come up with a way
>> to identify user namespaces (i.e. inode number relative to containing
>> user ns, if that's well-defined).
>
> If as suggested we only assign ids when a tunnel (or equivalent) is
> created between two network namespaces the space cost is a non-issue.
> The ids become at worst a constant factor addition to the cost of the
> tunnel.
>
> To keep things simple we may want to assign a free id (if one does not
> exist) when we connect a tunnel to a network namespace.
>
>> From user code's perspective, netnses that are in the requester's
>> userns or its descendents are identified by a path through a (possibly
>> zero-length) sequence of userns ids followed by a netns id.  Netnses
>> outside the requester's userns hierarchy cannot be named at all.
>>
>> Would this make sense?
>
> Nope.  What happens if I migrate 2 of the 4 network namespaces in a user
> namespace?  The migration potentially fails.  Application migration does
> not require user namespace migration.

Hmm.  I guess that, as long as those network namespaces aren't
connected to anything else, migrating like that makes sense and ought
to work.  Fair enough.

--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCHv8.1] fanotify: enable close-on-exec on events' fd when requested in fanotify_init()
From: Andrew Morton @ 2014-10-02 19:46 UTC (permalink / raw)
  To: Jan Kara
  Cc: Yann Droneaud, Heinrich Schuchardt, Eric Paris,
	Richard Guy Briggs, Al Viro, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	stable-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Lino Sanfilippo, Valdis Kletnieks, Michael Kerrisk-manpages
In-Reply-To: <20141002104410.GB19748-+0h/O2h83AeN3ZZ/Hiejyg@public.gmane.org>

On Thu, 2 Oct 2014 12:44:10 +0200 Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org> wrote:

> On Wed 01-10-14 15:36:21, Andrew Morton wrote:
> > On Mon, 29 Sep 2014 10:49:15 +0200 Yann Droneaud <ydroneaud-RlY5vtjFyJ3QT0dZR+AlfA@public.gmane.org> wrote:
> > 
> > > According to commit 80af258867648 ('fanotify: groups can specify
> > > their f_flags for new fd'), file descriptors created as part of
> > > file access notification events inherit flags from the
> > > event_f_flags argument passed to syscall fanotify_init(2).
> > > 
> > > So while it is legal for userspace to call fanotify_init() with
> > > O_CLOEXEC as part of its second argument, O_CLOEXEC is currently
> > > silently ignored.
> > > 
> > > Indeed event_f_flags are only given to dentry_open(), which only
> > > seems to care about O_ACCMODE and O_PATH in do_dentry_open(),
> > > O_DIRECT in open_check_o_direct() and O_LARGEFILE in
> > > generic_file_open().
> > > 
> > > But it seems logical to set close-on-exec flag on the file
> > > descriptor if userspace is allowed to request it with O_CLOEXEC.
> > > 
> > > In fact, according to some lookup on http://codesearch.debian.net/
> > > and various search engine, there's already some userspace code
> > > requesting it:
> > > 
> > > - in systemd's readahead[2]:
> > > 
> > >     fanotify_fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK, O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);
> > > 
> > > - in clsync[3]:
> > > 
> > >     #define FANOTIFY_EVFLAGS (O_LARGEFILE|O_RDONLY|O_CLOEXEC)
> > > 
> > >     int fanotify_d = fanotify_init(FANOTIFY_FLAGS, FANOTIFY_EVFLAGS);
> > > 
> > > - in examples [4] from "Filesystem monitoring in the Linux
> > >   kernel" article[5] by Aleksander Morgado:
> > > 
> > >     if ((fanotify_fd = fanotify_init (FAN_CLOEXEC,
> > >                                       O_RDONLY | O_CLOEXEC | O_LARGEFILE)) < 0)
> > 
> > So we have a number of apps which are setting O_CLOEXEC, but it doesn't
> > actually work.  With this change it *will* work, so the behaviour of
> > those apps might change, possibly breaking them?
>   Possibly. OTOH I'd dare to say that most of the apps specifying O_CLOEXEC
> want that behavior and their security may be weakened by the fact that
> O_CLOEXEC is ignored. So we are weighting possible security issues for apps
> doing things right (and Mihai mentioned in this thread that at least he has
> an application which needs O_CLOEXEC working) against possible breakage for
> apps which just randomly set O_CLOEXEC without wanting. So I'm really for
> fixing O_CLOEXEC behavior.

Fair enough, it sounds like the risk is acceptable.

Can we get a new version sent out with all this new info appropriately
changelogged?

^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-10-02 19:45 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Andrew Morton, Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
	David S. Miller
In-Reply-To: <CALCETrWxqzUF1x+TmW5G4kuHPP+sUtiRaT6dpZ0mQTJ217QB5w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Andy Lutomirski <luto@amacapital.net> writes:

> On Thu, Oct 2, 2014 at 12:20 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>
>>> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>>
>>>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>>>
>>>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>>> I see two ways to go with this.
>>>>>>>>
>>>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>>>     network namespaces.  The table would need to be populated manually by
>>>>>>>>     the likes of ip netns add.
>>>>>>>>
>>>>>>>>     That flips the order of assignment and makes this idea solid.
>>>>> I have a preference for this solution, because it allows to have a full
>>>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>>>> it saves a lot of time to avoid another request to get all informations.
>>>>
>>>> My practical question is how often does it happen that we care?
>>> In fact, I don't think that scenarii with a lot of netns have a full mesh of
>>> x-netns interfaces. It will be more one "link" netns with the physical
>>> interface and all other with one interface with the link part in this "link"
>>> netns. Hence, only one nsid is needing in each netns.
>>
>> I will buy that a full mesh is unlikely.
>>
>> For people doing simulations anything physical has a limited number of
>> links.
>>
>> For people wanting all to all connectivity setting up an internal
>> macvlan (or the equivalent) is likely much simpler and more efficient
>> that a full mesh.
>>
>> So the question in my mind is how do we create these identifiers at need
>> (when we create the cross network namespace links) instead of at network
>> namespace creation time.  I don't see an answer to that in your patches,
>> and perhaps it obvious.
>>
>
> I wonder whether part of the problem is that we're thinking about
> scoping wrong.  What if we made the hierarchy more explicit?
>
> For example, we could give each netns an admin-assigned identifier
> (e.g. a 64-bit number, maybe required to be unique, maybe not)
> relative to its containing userns.  Then we could come up with a way
> to identify user namespaces (i.e. inode number relative to containing
> user ns, if that's well-defined).

If as suggested we only assign ids when a tunnel (or equivalent) is
created between two network namespaces the space cost is a non-issue.
The ids become at worst a constant factor addition to the cost of the
tunnel.

To keep things simple we may want to assign a free id (if one does not
exist) when we connect a tunnel to a network namespace.

> From user code's perspective, netnses that are in the requester's
> userns or its descendents are identified by a path through a (possibly
> zero-length) sequence of userns ids followed by a netns id.  Netnses
> outside the requester's userns hierarchy cannot be named at all.
>
> Would this make sense? 

Nope.  What happens if I migrate 2 of the 4 network namespaces in a user
namespace?  The migration potentially fails.  Application migration does
not require user namespace migration.

Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [RFC PATCH net-next v3 1/4] netns: add genl cmd to add and get peer netns ids
From: Eric W. Biederman @ 2014-10-02 19:33 UTC (permalink / raw)
  To: Nicolas Dichtel
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-kltTT9wpgjJwATOyAt5JVQ,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	cwang-xCSkyg8dI+0RB7SZvlqPiA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1412257690-31253-2-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:

> With this patch, a user can define an id for a peer netns by providing a FD or a
> PID. These ids are local to netns (ie valid only into one netns).
>
> This will be useful for netlink messages when a x-netns interface is
> dumped.

You have a "id -> struct net *" table but you don't have a 
"struct net * -> id" table which looks like it will impact the
performance of peernet2id at scale.

Eric

^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Andy Lutomirski @ 2014-10-02 19:31 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Andrew Morton, Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
	David S. Miller
In-Reply-To: <8761g2nurx.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

On Thu, Oct 2, 2014 at 12:20 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>
>>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>>
>>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>>> <ebiederm@xmission.com> wrote:
>>>>>>> I see two ways to go with this.
>>>>>>>
>>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>>     network namespaces.  The table would need to be populated manually by
>>>>>>>     the likes of ip netns add.
>>>>>>>
>>>>>>>     That flips the order of assignment and makes this idea solid.
>>>> I have a preference for this solution, because it allows to have a full
>>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>>> it saves a lot of time to avoid another request to get all informations.
>>>
>>> My practical question is how often does it happen that we care?
>> In fact, I don't think that scenarii with a lot of netns have a full mesh of
>> x-netns interfaces. It will be more one "link" netns with the physical
>> interface and all other with one interface with the link part in this "link"
>> netns. Hence, only one nsid is needing in each netns.
>
> I will buy that a full mesh is unlikely.
>
> For people doing simulations anything physical has a limited number of
> links.
>
> For people wanting all to all connectivity setting up an internal
> macvlan (or the equivalent) is likely much simpler and more efficient
> that a full mesh.
>
> So the question in my mind is how do we create these identifiers at need
> (when we create the cross network namespace links) instead of at network
> namespace creation time.  I don't see an answer to that in your patches,
> and perhaps it obvious.
>

I wonder whether part of the problem is that we're thinking about
scoping wrong.  What if we made the hierarchy more explicit?

For example, we could give each netns an admin-assigned identifier
(e.g. a 64-bit number, maybe required to be unique, maybe not)
relative to its containing userns.  Then we could come up with a way
to identify user namespaces (i.e. inode number relative to containing
user ns, if that's well-defined).

From user code's perspective, netnses that are in the requester's
userns or its descendents are identified by a path through a (possibly
zero-length) sequence of userns ids followed by a netns id.  Netnses
outside the requester's userns hierarchy cannot be named at all.

Would this make sense?  It should keep the asymptotic complexity of
everything under control and, for users of very large numbers of
network namespaces with complex routing, it doesn't require a
correspondingly large number of fds. It would have the added benefit
of allowing the same scheme to be used for all the other namespace
types, although it could be a bit odd for pid namespaces, which really
do have their own hierarchy.

--Andy
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-10-02 19:20 UTC (permalink / raw)
  To: nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w
  Cc: Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andy Lutomirski, Stephen Hemminger, Cong Wang, Linux API,
	Andrew Morton, David S. Miller
In-Reply-To: <542D5726.8070308-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:

> Le 29/09/2014 20:43, Eric W. Biederman a écrit :
>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>
>>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>>
>>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>>> <ebiederm@xmission.com> wrote:
>>>>>> I see two ways to go with this.
>>>>>>
>>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>>     network namespaces.  The table would need to be populated manually by
>>>>>>     the likes of ip netns add.
>>>>>>
>>>>>>     That flips the order of assignment and makes this idea solid.
>>> I have a preference for this solution, because it allows to have a full
>>> broadcast messages. When you have a lot of network interfaces (> 10k),
>>> it saves a lot of time to avoid another request to get all informations.
>>
>> My practical question is how often does it happen that we care?
> In fact, I don't think that scenarii with a lot of netns have a full mesh of
> x-netns interfaces. It will be more one "link" netns with the physical
> interface and all other with one interface with the link part in this "link"
> netns. Hence, only one nsid is needing in each netns.

I will buy that a full mesh is unlikely.  

For people doing simulations anything physical has a limited number of
links.

For people wanting all to all connectivity setting up an internal
macvlan (or the equivalent) is likely much simpler and more efficient
that a full mesh.

So the question in my mind is how do we create these identifiers at need
(when we create the cross network namespace links) instead of at network
namespace creation time.  I don't see an answer to that in your patches,
and perhaps it obvious.

Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH] fanotify: add a flag to allow setting O_CLOEXEC on event fd
From: Yann Droneaud @ 2014-10-02 14:44 UTC (permalink / raw)
  To: Pádraig Brady
  Cc: Andrew Morton, Heinrich Schuchardt, Eric Paris,
	Richard Guy Briggs, Al Viro, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	stable-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	Jan Kara, Lino Sanfilippo, Valdis Kletnieks,
	Michael Kerrisk-manpages, Yann Droneaud
In-Reply-To: <542D1726.3040801-V8g9lnOeT5ydJdNcDFJN0w@public.gmane.org>

Hi,

Le jeudi 02 octobre 2014 à 10:13 +0100, Pádraig Brady a écrit :
> On 10/02/2014 08:52 AM, Yann Droneaud wrote:
> > In order to not potentially break applications which were
> > requesting O_CLOEXEC on event file descriptors but which
> > actually need it to be not effective as the kernel currently
> > ignore the flag, so the file descriptor is inherited accross
> > exec regardless of O_CLOEXEC (please forgive me for the
> > wording), this patch introduces FAN_FD_CLOEXEC flag to
> > fanotify_init() so that application can request O_CLOEXEC
> > to be effective.
> > Newer application would use FAN_FD_CLOEXEC flag along
> > O_CLOEXEC to enable close on exec on newly created
> > file descriptor:
> > 
> >   fd = fanotify_init(FAN_CLOEXEC|FAN_NONBLOCK|FAN_FD_CLOEXEC,
> >                      O_RDONLY|O_LARGEFILE|O_CLOEXEC|O_NOATIME);
> 
> Ugh really?
> IMHO there should be widespread or at least known breakage with
> O_CLOEXEC before adding messiness like this.

You should have read the other part of my message:

> > While I believe fanotify_init() must enable close-on-exec
> > when requested by userspace to prevent unwelcomed security
> > issue, I understand your concerns regarding the possible
> > breakage on userspace application requesting O_CLOEXEC
> > but relying on it not being enable on file descriptor
> > created for the events.
> 
> > So with a new flag to fanotify_init(), we could allow
> > newer applications to really enable O_CLOEXEC.
> 
> > But I feel bad to have to force application to specify
> > twice they want close on exec:
> >  - are you sure ?
> >  - are you really sure ?
> >  - is this your final answer ?
> >  ...


I'm not really fond of this option.

> It seems surprising to me that apps that would depend on
> O_CLOEXEC being ineffective.
> 

We have seen userspace developers making mistakes, and those mistakes
were mistakenly ignored by the kernel until someone try to fix the
mistake on kernel side, which broke the existing userspace application.

> please reconsider this one.
> 

I'm not going to promote this patch as it's a quick and dirty hack to
demonstrate what would be the other option.

Regards.

-- 
Yann Droneaud
OPTEYA

^ permalink raw reply

* [RFC PATCH net-next v3 4/4] rtnl: allow to create device with IFLA_LINK_NETNSID set
From: Nicolas Dichtel @ 2014-10-02 13:48 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: cwang-xCSkyg8dI+0RB7SZvlqPiA, Nicolas Dichtel,
	luto-kltTT9wpgjJwATOyAt5JVQ,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1412257690-31253-1-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

This patch adds the ability to create a netdevice in a specified netns and
then move it into the final netns. In fact, it allows to have a symetry between
get and set rtnl messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1b9329512496..57959a85ed2c 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1211,6 +1211,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_NUM_RX_QUEUES]	= { .type = NLA_U32 },
 	[IFLA_PHYS_PORT_ID]	= { .type = NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
 	[IFLA_CARRIER_CHANGES]	= { .type = NLA_U32 },  /* ignored */
+	[IFLA_LINK_NETNSID]	= { .type = NLA_S32 },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1983,7 +1984,7 @@ replay:
 		struct nlattr *slave_attr[m_ops ? m_ops->slave_maxtype + 1 : 0];
 		struct nlattr **data = NULL;
 		struct nlattr **slave_data = NULL;
-		struct net *dest_net;
+		struct net *dest_net, *link_net = NULL;
 
 		if (ops) {
 			if (ops->maxtype && linkinfo[IFLA_INFO_DATA]) {
@@ -2089,7 +2090,18 @@ replay:
 		if (IS_ERR(dest_net))
 			return PTR_ERR(dest_net);
 
-		dev = rtnl_create_link(dest_net, ifname, name_assign_type, ops, tb);
+		if (tb[IFLA_LINK_NETNSID]) {
+			int id = nla_get_s32(tb[IFLA_LINK_NETNSID]);
+
+			link_net = get_net_ns_by_id(dest_net, id);
+			if (link_net == NULL) {
+				err =  -EINVAL;
+				goto out;
+			}
+		}
+
+		dev = rtnl_create_link(link_net ? : dest_net, ifname,
+				       name_assign_type, ops, tb);
 		if (IS_ERR(dev)) {
 			err = PTR_ERR(dev);
 			goto out;
@@ -2117,9 +2129,16 @@ replay:
 			}
 		}
 		err = rtnl_configure_link(dev, ifm);
-		if (err < 0)
+		if (err < 0) {
 			unregister_netdevice(dev);
+			goto out;
+		}
+
+		if (link_net)
+			err = dev_change_net_namespace(dev, dest_net, ifname);
 out:
+		if (link_net)
+			put_net(link_net);
 		put_net(dest_net);
 		return err;
 	}
-- 
2.1.0

^ permalink raw reply related

* [RFC PATCH net-next v3 3/4] iptunnels: advertise link netns via netlink
From: Nicolas Dichtel @ 2014-10-02 13:48 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: cwang-xCSkyg8dI+0RB7SZvlqPiA, Nicolas Dichtel,
	luto-kltTT9wpgjJwATOyAt5JVQ,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1412257690-31253-1-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Implement rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 include/net/ip6_tunnel.h | 1 +
 include/net/ip_tunnels.h | 1 +
 net/ipv4/ip_gre.c        | 2 ++
 net/ipv4/ip_tunnel.c     | 8 ++++++++
 net/ipv4/ip_vti.c        | 1 +
 net/ipv4/ipip.c          | 1 +
 net/ipv6/ip6_gre.c       | 1 +
 net/ipv6/ip6_tunnel.c    | 9 +++++++++
 net/ipv6/ip6_vti.c       | 1 +
 net/ipv6/sit.c           | 1 +
 10 files changed, 26 insertions(+)

diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index a5593dab6af7..8648519f4555 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -69,6 +69,7 @@ int ip6_tnl_xmit_ctl(struct ip6_tnl *t);
 __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw);
 __u32 ip6_tnl_get_cap(struct ip6_tnl *t, const struct in6_addr *laddr,
 			     const struct in6_addr *raddr);
+struct net *ip6_tnl_get_link_net(const struct net_device *dev);
 
 static inline void ip6tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
 {
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 7f538ba6e267..c92a99b5b77e 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -119,6 +119,7 @@ struct ip_tunnel_net {
 int ip_tunnel_init(struct net_device *dev);
 void ip_tunnel_uninit(struct net_device *dev);
 void  ip_tunnel_dellink(struct net_device *dev, struct list_head *head);
+struct net *ip_tunnel_get_link_net(const struct net_device *dev);
 int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 		       struct rtnl_link_ops *ops, char *devname);
 
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 0485ef18d254..c75974986053 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -827,6 +827,7 @@ static struct rtnl_link_ops ipgre_link_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipgre_get_size,
 	.fill_info	= ipgre_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct rtnl_link_ops ipgre_tap_ops __read_mostly = {
@@ -841,6 +842,7 @@ static struct rtnl_link_ops ipgre_tap_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipgre_get_size,
 	.fill_info	= ipgre_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static int __net_init ipgre_tap_init_net(struct net *net)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index b75b47b0a223..a8ab238d0df4 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -954,6 +954,14 @@ void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_dellink);
 
+struct net *ip_tunnel_get_link_net(const struct net_device *dev)
+{
+	struct ip_tunnel *tunnel = netdev_priv(dev);
+
+	return tunnel->net;
+}
+EXPORT_SYMBOL(ip_tunnel_get_link_net);
+
 int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 				  struct rtnl_link_ops *ops, char *devname)
 {
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index e453cb724a95..93862411669c 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -530,6 +530,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
 	.changelink	= vti_changelink,
 	.get_size	= vti_get_size,
 	.fill_info	= vti_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static int __init vti_init(void)
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index ea88ab3102a8..406910d04b1b 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -498,6 +498,7 @@ static struct rtnl_link_ops ipip_link_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipip_get_size,
 	.fill_info	= ipip_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct xfrm_tunnel ipip_handler __read_mostly = {
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 9a0a1aafe727..10981f568250 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1659,6 +1659,7 @@ static struct rtnl_link_ops ip6gre_link_ops __read_mostly = {
 	.dellink	= ip6gre_dellink,
 	.get_size	= ip6gre_get_size,
 	.fill_info	= ip6gre_fill_info,
+	.get_link_net	= ip6_tnl_get_link_net,
 };
 
 static struct rtnl_link_ops ip6gre_tap_ops __read_mostly = {
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index e01bd0399297..b86d9f4ea5ec 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1699,6 +1699,14 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
+struct net *ip6_tnl_get_link_net(const struct net_device *dev)
+{
+	struct ip6_tnl *tunnel = netdev_priv(dev);
+
+	return tunnel->net;
+}
+EXPORT_SYMBOL(ip6_tnl_get_link_net);
+
 static const struct nla_policy ip6_tnl_policy[IFLA_IPTUN_MAX + 1] = {
 	[IFLA_IPTUN_LINK]		= { .type = NLA_U32 },
 	[IFLA_IPTUN_LOCAL]		= { .len = sizeof(struct in6_addr) },
@@ -1722,6 +1730,7 @@ static struct rtnl_link_ops ip6_link_ops __read_mostly = {
 	.dellink	= ip6_tnl_dellink,
 	.get_size	= ip6_tnl_get_size,
 	.fill_info	= ip6_tnl_fill_info,
+	.get_link_net	= ip6_tnl_get_link_net,
 };
 
 static struct xfrm6_tunnel ip4ip6_handler __read_mostly = {
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 7f52fd9fa7b0..88e8aadcfac1 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -988,6 +988,7 @@ static struct rtnl_link_ops vti6_link_ops __read_mostly = {
 	.changelink	= vti6_changelink,
 	.get_size	= vti6_get_size,
 	.fill_info	= vti6_fill_info,
+	.get_link_net	= ip6_tnl_get_link_net,
 };
 
 static void __net_exit vti6_destroy_tunnels(struct vti6_net *ip6n)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 0d4e27466f82..02ef387811be 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1765,6 +1765,7 @@ static struct rtnl_link_ops sit_link_ops __read_mostly = {
 	.get_size	= ipip6_get_size,
 	.fill_info	= ipip6_fill_info,
 	.dellink	= ipip6_dellink,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct xfrm_tunnel sit_handler __read_mostly = {
-- 
2.1.0

^ permalink raw reply related

* [RFC PATCH net-next v3 2/4] rtnl: add link netns id to interface messages
From: Nicolas Dichtel @ 2014-10-02 13:48 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: cwang-xCSkyg8dI+0RB7SZvlqPiA, Nicolas Dichtel,
	luto-kltTT9wpgjJwATOyAt5JVQ,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1412257690-31253-1-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

This patch adds a new attribute (IFLA_LINK_NETNSID) which contains the 'link'
netns id when this netns is different from the netns where the interface
stands (for example for x-net interfaces like ip tunnels). When there is no id,
we put NETNSA_NSINDEX_UNKNOWN into this attribute to indicate to userland that
the link netns is different from the interface netns. Hence, userland knows that
some information like IFLA_LINK are not interpretable.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 include/net/rtnetlink.h      |  2 ++
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 13 +++++++++++++
 3 files changed, 16 insertions(+)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index e21b9f9653c0..6c6d5393fc34 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -46,6 +46,7 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
  *			    to create when creating a new device.
  *	@get_num_rx_queues: Function to determine number of receive queues
  *			    to create when creating a new device.
+ *	@get_link_net: Function to get the i/o netns of the device
  */
 struct rtnl_link_ops {
 	struct list_head	list;
@@ -93,6 +94,7 @@ struct rtnl_link_ops {
 	int			(*fill_slave_info)(struct sk_buff *skb,
 						   const struct net_device *dev,
 						   const struct net_device *slave_dev);
+	struct net		*(*get_link_net)(const struct net_device *dev);
 };
 
 int __rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 0bdb77e16875..938c0c02ed2e 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -145,6 +145,7 @@ enum {
 	IFLA_CARRIER,
 	IFLA_PHYS_PORT_ID,
 	IFLA_CARRIER_CHANGES,
+	IFLA_LINK_NETNSID,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a6882686ca3a..1b9329512496 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -862,6 +862,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(1) /* IFLA_OPERSTATE */
 	       + nla_total_size(1) /* IFLA_LINKMODE */
 	       + nla_total_size(4) /* IFLA_CARRIER_CHANGES */
+	       + nla_total_size(4) /* IFLA_LINK_NETNSID */
 	       + nla_total_size(ext_filter_mask
 			        & RTEXT_FILTER_VF ? 4 : 0) /* IFLA_NUM_VF */
 	       + rtnl_vfinfo_size(dev, ext_filter_mask) /* IFLA_VFINFO_LIST */
@@ -1134,6 +1135,18 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			goto nla_put_failure;
 	}
 
+	if (dev->rtnl_link_ops &&
+	    dev->rtnl_link_ops->get_link_net) {
+		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+		if (!net_eq(dev_net(dev), link_net)) {
+			int id = peernet2id(dev_net(dev), link_net);
+
+			if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+				goto nla_put_failure;
+		}
+	}
+
 	if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
 		goto nla_put_failure;
 
-- 
2.1.0

^ permalink raw reply related

* [RFC PATCH net-next v3 1/4] netns: add genl cmd to add and get peer netns ids
From: Nicolas Dichtel @ 2014-10-02 13:48 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	luto-kltTT9wpgjJwATOyAt5JVQ, cwang-xCSkyg8dI+0RB7SZvlqPiA,
	Nicolas Dichtel
In-Reply-To: <1412257690-31253-1-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

With this patch, a user can define an id for a peer netns by providing a FD or a
PID. These ids are local to netns (ie valid only into one netns).

This will be useful for netlink messages when a x-netns interface is dumped.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 MAINTAINERS                 |   1 +
 include/net/net_namespace.h |   5 ++
 include/uapi/linux/Kbuild   |   1 +
 include/uapi/linux/netns.h  |  31 +++++++
 net/core/net_namespace.c    | 195 ++++++++++++++++++++++++++++++++++++++++++++
 net/netlink/genetlink.c     |   4 +
 6 files changed, 237 insertions(+)
 create mode 100644 include/uapi/linux/netns.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f8db3c3acc67..8e7f5d668e6a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6278,6 +6278,7 @@ F:	include/linux/netdevice.h
 F:	include/uapi/linux/in.h
 F:	include/uapi/linux/net.h
 F:	include/uapi/linux/netdevice.h
+F:	include/uapi/linux/netns.h
 F:	tools/net/
 F:	tools/testing/selftests/net/
 F:	lib/random32.c
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 361d26077196..d8847d978b59 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -59,6 +59,7 @@ struct net {
 	struct list_head	exit_list;	/* Use only net_mutex */
 
 	struct user_namespace   *user_ns;	/* Owning user namespace */
+	struct idr		netns_ids;
 
 	unsigned int		proc_inum;
 
@@ -289,6 +290,10 @@ static inline struct net *read_pnet(struct net * const *pnet)
 #define __net_initconst	__initconst
 #endif
 
+int peernet2id(struct net *net, struct net *peer);
+struct net *get_net_ns_by_id(struct net *net, int id);
+int netns_genl_register(void);
+
 struct pernet_operations {
 	struct list_head list;
 	int (*init)(struct net *net);
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 70e150ebc6c9..33a0bbfe4736 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -276,6 +276,7 @@ header-y += netfilter_decnet.h
 header-y += netfilter_ipv4.h
 header-y += netfilter_ipv6.h
 header-y += netlink.h
+header-y += netns.h
 header-y += netrom.h
 header-y += nfc.h
 header-y += nfs.h
diff --git a/include/uapi/linux/netns.h b/include/uapi/linux/netns.h
new file mode 100644
index 000000000000..8ebb08885795
--- /dev/null
+++ b/include/uapi/linux/netns.h
@@ -0,0 +1,31 @@
+#ifndef _UAPI_LINUX_NETNS_H_
+#define _UAPI_LINUX_NETNS_H_
+
+/* Generic netlink messages */
+
+#define NETNS_GENL_NAME			"netns"
+#define NETNS_GENL_VERSION		0x1
+
+/* Commands */
+enum {
+	NETNS_CMD_UNSPEC,
+	NETNS_CMD_NEWID,
+	NETNS_CMD_GETID,
+	__NETNS_CMD_MAX,
+};
+
+#define NETNS_CMD_MAX		(__NETNS_CMD_MAX - 1)
+
+/* Attributes */
+enum {
+	NETNSA_NONE,
+#define NETNSA_NSINDEX_UNKNOWN	-1
+	NETNSA_NSID,
+	NETNSA_PID,
+	NETNSA_FD,
+	__NETNSA_MAX,
+};
+
+#define NETNSA_MAX		(__NETNSA_MAX - 1)
+
+#endif /* _UAPI_LINUX_NETNS_H_ */
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 7f155175bba8..4a5680ed42fb 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -15,6 +15,8 @@
 #include <linux/file.h>
 #include <linux/export.h>
 #include <linux/user_namespace.h>
+#include <linux/netns.h>
+#include <net/genetlink.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 
@@ -144,6 +146,50 @@ static void ops_free_list(const struct pernet_operations *ops,
 	}
 }
 
+/* This function is used by idr_for_each(). If net is equal to peer, the
+ * function returns the id so that idr_for_each() stops. Because we cannot
+ * returns the id 0 (idr_for_each() will not stop), we return the magic value
+ * -1 for it.
+ */
+static int net_eq_idr(int id, void *net, void *peer)
+{
+	if (net_eq(net, peer))
+		return id ? : -1;
+	return 0;
+}
+
+/* returns NETNSA_NSINDEX_UNKNOWN if not found */
+int peernet2id(struct net *net, struct net *peer)
+{
+	int id = idr_for_each(&net->netns_ids, net_eq_idr, peer);
+
+	ASSERT_RTNL();
+
+	/* Magic value for id 0. */
+	if (id == -1)
+		return 0;
+	if (id == 0)
+		return NETNSA_NSINDEX_UNKNOWN;
+
+	return id;
+}
+
+struct net *get_net_ns_by_id(struct net *net, int id)
+{
+	struct net *peer;
+
+	if (id < 0)
+		return NULL;
+
+	rcu_read_lock();
+	peer = idr_find(&net->netns_ids, id);
+	if (peer)
+		get_net(peer);
+	rcu_read_unlock();
+
+	return peer;
+}
+
 /*
  * setup_net runs the initializers for the network namespace object.
  */
@@ -158,6 +204,7 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
 	atomic_set(&net->passive, 1);
 	net->dev_base_seq = 1;
 	net->user_ns = user_ns;
+	idr_init(&net->netns_ids);
 
 #ifdef NETNS_REFCNT_DEBUG
 	atomic_set(&net->use_count, 0);
@@ -288,6 +335,14 @@ static void cleanup_net(struct work_struct *work)
 	list_for_each_entry(net, &net_kill_list, cleanup_list) {
 		list_del_rcu(&net->list);
 		list_add_tail(&net->exit_list, &net_exit_list);
+		for_each_net(tmp) {
+			int id = peernet2id(tmp, net);
+
+			if (id >= 0)
+				idr_remove(&tmp->netns_ids, id);
+		}
+		idr_destroy(&net->netns_ids);
+
 	}
 	rtnl_unlock();
 
@@ -399,6 +454,146 @@ static struct pernet_operations __net_initdata net_ns_ops = {
 	.exit = net_ns_net_exit,
 };
 
+static struct genl_family netns_genl_family = {
+	.id		= GENL_ID_GENERATE,
+	.name		= NETNS_GENL_NAME,
+	.version	= NETNS_GENL_VERSION,
+	.hdrsize	= 0,
+	.maxattr	= NETNSA_MAX,
+	.netnsok	= true,
+};
+
+static struct nla_policy netns_nl_policy[NETNSA_MAX + 1] = {
+	[NETNSA_NONE]		= { .type = NLA_UNSPEC },
+	[NETNSA_NSID]		= { .type = NLA_S32 },
+	[NETNSA_PID]		= { .type = NLA_U32 },
+	[NETNSA_FD]		= { .type = NLA_U32 },
+};
+
+static int netns_nl_cmd_newid(struct sk_buff *skb, struct genl_info *info)
+{
+	struct net *net = genl_info_net(info);
+	struct net *peer;
+	int nsid, err;
+
+	if (!info->attrs[NETNSA_NSID])
+		return -EINVAL;
+	nsid = nla_get_s32(info->attrs[NETNSA_NSID]);
+	if (nsid < 0)
+		return -EINVAL;
+
+	if (info->attrs[NETNSA_PID])
+		peer = get_net_ns_by_pid(nla_get_u32(info->attrs[NETNSA_PID]));
+	else if (info->attrs[NETNSA_FD])
+		peer = get_net_ns_by_fd(nla_get_u32(info->attrs[NETNSA_FD]));
+	else
+		return -EINVAL;
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	rtnl_lock();
+	if (peernet2id(net, peer) >= 0) {
+		err = -EEXIST;
+		goto out;
+	}
+
+	err = idr_alloc(&net->netns_ids, peer, nsid, nsid + 1, GFP_KERNEL);
+	if (err >= 0)
+		err = 0;
+out:
+	rtnl_unlock();
+	put_net(peer);
+	return err;
+}
+
+static int netns_nl_get_size(void)
+{
+	return nla_total_size(sizeof(s32)) /* NETNSA_NSID */
+	       ;
+}
+
+static int netns_nl_fill(struct sk_buff *skb, u32 portid, u32 seq, int flags,
+			 int cmd, struct net *net, struct net *peer)
+{
+	void *hdr;
+	int id;
+
+	hdr = genlmsg_put(skb, portid, seq, &netns_genl_family, flags, cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	rtnl_lock();
+	id = peernet2id(net, peer);
+	rtnl_unlock();
+	if (nla_put_s32(skb, NETNSA_NSID, id))
+		goto nla_put_failure;
+
+	return genlmsg_end(skb, hdr);
+
+nla_put_failure:
+	genlmsg_cancel(skb, hdr);
+	return -EMSGSIZE;
+}
+
+static int netns_nl_cmd_getid(struct sk_buff *skb, struct genl_info *info)
+{
+	struct net *net = genl_info_net(info);
+	struct sk_buff *msg;
+	int err = -ENOBUFS;
+	struct net *peer;
+
+	if (info->attrs[NETNSA_PID])
+		peer = get_net_ns_by_pid(nla_get_u32(info->attrs[NETNSA_PID]));
+	else if (info->attrs[NETNSA_FD])
+		peer = get_net_ns_by_fd(nla_get_u32(info->attrs[NETNSA_FD]));
+	else
+		return -EINVAL;
+
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	msg = genlmsg_new(netns_nl_get_size(), GFP_KERNEL);
+	if (!msg) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = netns_nl_fill(msg, info->snd_portid, info->snd_seq,
+			    NLM_F_ACK, NETNS_CMD_GETID, net, peer);
+	if (err < 0)
+		goto err_out;
+
+	err = genlmsg_unicast(net, msg, info->snd_portid);
+	goto out;
+
+err_out:
+	nlmsg_free(msg);
+out:
+	put_net(peer);
+	return err;
+}
+
+static struct genl_ops netns_genl_ops[] = {
+	{
+		.cmd = NETNS_CMD_NEWID,
+		.policy = netns_nl_policy,
+		.doit = netns_nl_cmd_newid,
+		.flags = GENL_ADMIN_PERM,
+	},
+	{
+		.cmd = NETNS_CMD_GETID,
+		.policy = netns_nl_policy,
+		.doit = netns_nl_cmd_getid,
+		.flags = GENL_ADMIN_PERM,
+	},
+};
+
+int netns_genl_register(void)
+{
+	return genl_register_family_with_ops(&netns_genl_family,
+					     netns_genl_ops);
+}
+
 static int __init net_ns_init(void)
 {
 	struct net_generic *ng;
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 76393f2f4b22..c6f39e40c9f3 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1029,6 +1029,10 @@ static int __init genl_init(void)
 	if (err)
 		goto problem;
 
+	err = netns_genl_register();
+	if (err < 0)
+		goto problem;
+
 	return 0;
 
 problem:
-- 
2.1.0

^ permalink raw reply related

* [RFC PATCH net-next v3 0/4] netns: allow to identify peer netns
From: Nicolas Dichtel @ 2014-10-02 13:48 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
  Cc: cwang-xCSkyg8dI+0RB7SZvlqPiA, luto-kltTT9wpgjJwATOyAt5JVQ,
	stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <542D5726.8070308-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

The goal of this serie is to be able to multicast netlink messages with an
attribute that identify a peer netns.
This is needed by the userland to interpret some informations contained in
netlink messages (like IFLA_LINK value, but also some other attributes in case
of x-netns netdevice (see also
http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).

Ids of peer netns are set by userland via a new genl messages. These ids are
stored per netns and are local (ie only valid in the netns where they are set).
To avoid allocating an int for each peer netns, I use idr_for_each() to retrieve
the id of a peer netns.

Patch 1/4 introduces the netlink API mechanism to set and get these ids.
Patch 2/4 and 3/4 shows an example of how to use these ids in rtnetlink
messages. And patch 4/4 shows that the netlink messages can be symetric between
a GET and a SET.

iproute2 patches are available, I can send them on demand.

Here is a small screenshot to show how it can be used by userland:
$ ip netns add foo
$ ip netns del foo
$ ip netns
$ touch /var/run/netns/init_net
$ mount --bind /proc/1/ns/net /var/run/netns/init_net
$ ip netns add foo
$ ip netns exec foo ip netns set init_net 0
$ ip netns
foo
init_net
$ ip netns exec foo ip netns
foo
init_net (id: 0)
$ ip netns exec foo ip link add ipip1 link-netnsid 0 type ipip remote 10.16.0.121 local 10.16.0.249
$ ip netns exec foo ip l ls ipip1
6: ipip1@NONE: <POINTOPOINT,NOARP> mtu 1480 qdisc noop state DOWN mode DEFAULT group default 
    link/ipip 10.16.0.249 peer 10.16.0.121 link-netnsid 0

The parameter link-netnsid shows us where the interface sends and receives
packets (and thus we know where encapsulated addresses are set).

RFCv2 -> RFCv3:
  ids are now defined by userland (via netlink). Ids are stored in each netns
  (and they are local to this netns).
  add get_link_net support for ip6 tunnels
  netnsid is now a s32 instead of a u32

RFCv1 -> RFCv2:
  remove useless ()
  ids are now stored in the user ns. It's possible to get an id for a peer netns
  only if the current netns and the peer netns have the same user ns parent.

 MAINTAINERS                  |   1 +
 include/net/ip6_tunnel.h     |   1 +
 include/net/ip_tunnels.h     |   1 +
 include/net/net_namespace.h  |   5 ++
 include/net/rtnetlink.h      |   2 +
 include/uapi/linux/Kbuild    |   1 +
 include/uapi/linux/if_link.h |   1 +
 include/uapi/linux/netns.h   |  31 +++++++
 net/core/net_namespace.c     | 195 +++++++++++++++++++++++++++++++++++++++++++
 net/core/rtnetlink.c         |  38 ++++++++-
 net/ipv4/ip_gre.c            |   2 +
 net/ipv4/ip_tunnel.c         |   8 ++
 net/ipv4/ip_vti.c            |   1 +
 net/ipv4/ipip.c              |   1 +
 net/ipv6/ip6_gre.c           |   1 +
 net/ipv6/ip6_tunnel.c        |   9 ++
 net/ipv6/ip6_vti.c           |   1 +
 net/ipv6/sit.c               |   1 +
 net/netlink/genetlink.c      |   4 +
 19 files changed, 301 insertions(+), 3 deletions(-)

Comments are welcome.

Regards,
Nicolas

^ permalink raw reply

* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Nicolas Dichtel @ 2014-10-02 13:46 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Network Development, Linux Containers,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Andy Lutomirski, Stephen Hemminger, Cong Wang, Linux API,
	Andrew Morton, David S. Miller
In-Reply-To: <87y4t2gtd0.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>

Le 29/09/2014 20:43, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> Le 26/09/2014 20:57, Eric W. Biederman a écrit :
>>> Andy Lutomirski <luto@amacapital.net> writes:
>>>
>>>> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
>>>> <ebiederm@xmission.com> wrote:
>>>>> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>>>>>
>>>>>> The goal of this serie is to be able to multicast netlink messages with an
>>>>>> attribute that identify a peer netns.
>>>>>> This is needed by the userland to interpret some informations contained in
>>>>>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>>>>>> of x-netns netdevice (see also
>>>>>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
>>>>>> http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).
>>>>>
>>>>> I want say that the problem addressed by patch 3/5 of this series is a
>>>>> fundamentally valid problem.  We have network objects spanning network
>>>>> namespaces and it would be very nice to be able to talk about them in
>>>>> netlink, and file descriptors are too local and argubably too heavy
>>>>> weight for netlink quires and especially for netlink broadcast messages.
>>>>>
>>>>> Furthermore the concept of ineternal concept of peernet2id seems valid.
>>>>>
>>>>> However what you do not address is a way for CRIU (aka process
>>>>> migration) to be able to restore these ids after process migration.
>>>>> Going farther it looks like you are actively breaking process migration
>>>>> at this time, making this set of patches a no-go.
>> Ok, I will look more deeply into CRIU.
>>
>>>>>
>>>>> When adding a new form of namespace id CRIU patches are just about
>>>>> as necessary as iproute patches.
>> Noted.
>
>
>
>>>>> That does not describe what you have actually implemented in the
>>>>> patches.
>>>>>
>>>>> I see two ways to go with this.
>>>>>
>>>>> - A per network namespace table to that you can store ids for ``peer''
>>>>>     network namespaces.  The table would need to be populated manually by
>>>>>     the likes of ip netns add.
>>>>>
>>>>>     That flips the order of assignment and makes this idea solid.
>> I have a preference for this solution, because it allows to have a full
>> broadcast messages. When you have a lot of network interfaces (> 10k),
>> it saves a lot of time to avoid another request to get all informations.
>
> My practical question is how often does it happen that we care?
In fact, I don't think that scenarii with a lot of netns have a full mesh of
x-netns interfaces. It will be more one "link" netns with the physical
interface and all other with one interface with the link part in this "link"
netns. Hence, only one nsid is needing in each netns.

>
>>>>>     Unfortunately in the case of a fully referencing mesh of N network
>>>>>     namespaces such a mesh winds up taking O(N^2) space, which seems
>>>>>     undesirable.
>> Memory consumption vs performances ;-)
>> In fact, when you have a lot of netns, you already should have some memory
>> available (at least N lo interfaces + N interfaces (veth or a x-netns
>> interface)). I'm not convinced that this is really an obstacle.
>
> I would have to see how it all fits together. O(N^2) grows a lot faster
> that N.  So after a point it isn't in the same ballpark of memory
> consumption.
>
>>> broadcast message business, and only care about the remote namespace for
>>> unicast messages.  Putting the work in an infrequently used slow path
>>> instead of a comparitively common path gives us much more freedom in
>>> the implementation.
>> I think it's better to have a full netlink messages, instead a partial one.
>> There is already a lot of attributes added for each rtnl interface messages to
>> be sure to describe all parameters of these interfaces.
>> And if the user don't care about ids (user has not set any id with iproute2),
>> we can just add the same attribute with id 0 (let's say it's a reserved id) to
>> indicate that the link part of this interface is in another netns.
>
> I imagine an id like that is something we would want ip netns add to
> set, and probably set in all existing network namespaces as well.
>
>> The great benefit of your first proposal is that the ids are set by the
>> userspace and thus it allows a high flexibility.
>>
>> Would you accept a patch that implements this first solution?
>
> I would not fundamentally reject it.  I would really like to make
> certain we think through how it will be used and what the practical
> benefits are.  Depending on how it is used the data structure could
> be a killer or it could be a case where we see how to manage it and
> simply don't care.
I will send a v3, so we can talk about it.


Thank you,
Nicolas
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox