netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Cong Wang <cwang@twopensource.com>
To: Alexander Holler <holler@ahsoftware.de>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	netdev <netdev@vger.kernel.org>, Thomas Graf <tgraf@suug.ch>,
	David Miller <davem@davemloft.net>
Subject: Re: [PATCH net-next v2 0/7] netns: ease netlink use with a lot of netns
Date: Fri, 22 May 2015 14:04:29 -0700	[thread overview]
Message-ID: <CAHA+R7NCjKn86R+OLX+4bkB0kv3WcXGB5NPui+v_X=CKqQtPLA@mail.gmail.com> (raw)
In-Reply-To: <555F969B.3090706@ahsoftware.de>

On Fri, May 22, 2015 at 1:50 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 08.05.2015 um 14:02 schrieb Eric W. Biederman:
>>
>>
>> So I am dense.  I have read through the patches and I don't see where
>> you tag packets from other network namespaces with a network namespace
>> id.
>
>
> Me too,
>
> I've recently written a little tool called snetmanmon (source is
> available at github) to monitor and handle network related events
> by using rtnetlink.
>
> Having seen this patch series (thanks!), I've played with it.
>
> I've applied the patch series to v4.1-rc4.
>
> Maybe I'm using or holding it wrong, but I've some comments.
>
> First I think if NETLINK_LISTEN_ALL_NSID is enabled, a dump
> of the interfaces through RTM_GETLINK together with NLM_F_DUMP and
> NLM_F_REQUEST should return all interfaces of all reachable namespaces.
>
> Next, if NETLINK_LISTEN_ALL_NSID is enabled, I receive RTM_NEWLINK
> but without any indication of the namespace. E.g. if I do
>         ip netns add netns1
>         ip netns exec netns1 brctl addbr br0
> the RTM_NEWLINK for br0 (received in the root ns, not netns1) doesn't
> have the attribute IFLA_LINK_NETNSID.


Bridge doesn't have an underlying link, so no LINK_NETNSID. LINK_NETNSID
is only added when its underlying link is in a different netns.


>
> Same for the RTM_DELLINK msg if I call
>         ip netns exec netns1 brctl delbr br0
> afterwards. So both netlink messages are looking like br0 was
> created in the root ns.
>
> Another problem seems to be with veth devices. E.g. if I do
>         ip link add veth0 type veth peer name veth1
>         ip link set veth1 netns netns1
> I receive
>         RTM_NEWLINK for veth0 (no nsid)
>         RTM_NEWLINK for veth1 (no nsid)
>         RTM_DELLINK for veth1 (no nsid)
>         RTM_NEWLINK for veth1 (with nsid 0)
> That looks ok, except the missing RTM_NEWLINK for lo in netns1, which
> was created together with the namespace. But if I now request a dump,
> I get
>         RTM_NEWLINK for veth0 (with nsid 0)
> which looks like veth0 is part of nsid 0, and I get nothing for veth1.
> Of course, that vlan device might be part of nsid 0 too (as veth1),
> but its part named veth0 is not part of that namespace. So the
> IFLA_LINK_NETNSID attribute received with the RTM_NEWLINK for veth0 through
> the dump is misleading.

That is because the code tries to do "lazy" allocation for netnsid,
it defers it util the dumping, veth case is special here given how they pair,
I noticed the same "problem" (it doesn't have to be a bug) when I reviewed
the code, nobody cared. ;-/

  reply	other threads:[~2015-05-22 21:04 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-06  9:58 [PATCH net-next 0/6] netns: ease netlink use with a lot of netns Nicolas Dichtel
2015-05-06  9:58 ` [PATCH net-next 1/6] netns: returns always an id in __peernet2id() Nicolas Dichtel
2015-05-06 11:19   ` Thomas Graf
2015-05-06  9:58 ` [PATCH net-next 2/6] netns: always provide the id to rtnl_net_fill() Nicolas Dichtel
2015-05-06 11:25   ` Thomas Graf
2015-05-06  9:58 ` [PATCH net-next 3/6] netns: rename peernet2id() to peernet2id_alloc() Nicolas Dichtel
2015-05-06 11:27   ` Thomas Graf
2015-05-06  9:58 ` [PATCH net-next 4/6] netns: notify new nsid outside __peernet2id() Nicolas Dichtel
2015-05-06 11:48   ` Thomas Graf
2015-05-06 13:39     ` Nicolas Dichtel
2015-05-06  9:58 ` [PATCH net-next 5/6] netns: use a spin_lock to protect nsid management Nicolas Dichtel
2015-05-06 12:23   ` Thomas Graf
2015-05-06 13:40     ` Nicolas Dichtel
2015-05-06 14:05       ` Thomas Graf
2015-05-06  9:58 ` [PATCH net-next 6/6] netlink: allow to listen "all" netns Nicolas Dichtel
2015-05-06 12:10   ` Thomas Graf
2015-05-06 13:42     ` Nicolas Dichtel
2015-05-07  9:02 ` [PATCH net-next v2 0/7] netns: ease netlink use with a lot of netns Nicolas Dichtel
2015-05-07  9:02   ` [PATCH net-next v2 1/7] netns: returns always an id in __peernet2id() Nicolas Dichtel
2015-05-07  9:02   ` [PATCH net-next v2 2/7] netns: always provide the id to rtnl_net_fill() Nicolas Dichtel
2015-05-07  9:02   ` [PATCH net-next v2 3/7] netns: rename peernet2id() to peernet2id_alloc() Nicolas Dichtel
2015-05-07  9:02   ` [PATCH net-next v2 4/7] netns: notify new nsid outside __peernet2id() Nicolas Dichtel
2015-05-07 11:47     ` Thomas Graf
2015-05-07  9:02   ` [PATCH net-next v2 5/7] netns: use a spin_lock to protect nsid management Nicolas Dichtel
2015-05-07  9:02   ` [PATCH net-next v2 6/7] netlink: rename private flags and states Nicolas Dichtel
2015-05-07 11:49     ` Thomas Graf
2015-05-07  9:02   ` [PATCH net-next v2 7/7] netlink: allow to listen "all" netns Nicolas Dichtel
2015-05-07 11:55     ` Thomas Graf
2015-05-08 12:02   ` [PATCH net-next v2 0/7] netns: ease netlink use with a lot of netns Eric W. Biederman
2015-05-09 21:07     ` Nicolas Dichtel
2015-05-22 20:50     ` Alexander Holler
2015-05-22 21:04       ` Cong Wang [this message]
2015-05-22 21:12         ` Alexander Holler
2015-05-22 21:29           ` Cong Wang
2015-05-22 21:46             ` Alexander Holler
2015-05-22 21:19       ` Eric W. Biederman
2015-05-22 21:30         ` Alexander Holler
2015-05-25  7:45       ` Nicolas Dichtel
2015-05-25 10:55         ` Alexander Holler
2015-05-25 13:09           ` Nicolas Dichtel
2015-05-26 10:53             ` Alexander Holler
2015-05-26 12:10               ` Nicolas Dichtel
2015-05-26 14:36                 ` Alexander Holler
2015-05-29  5:57                   ` Alexander Holler
2015-05-10  2:15   ` David Miller
2015-05-20 14:19     ` [PATCH iproute2-next 0/6] Allow to monitor 'all-nsid' with ip and ip xfrm Nicolas Dichtel
2015-05-20 14:19       ` [PATCH iproute2-next 1/6] include: update linux/netlink.h Nicolas Dichtel
2015-05-20 14:19       ` [PATCH iproute2-next 2/6] man: update ip monitor page Nicolas Dichtel
2015-05-20 14:19       ` [PATCH iproute2-next 3/6] libnetlink: introduce rtnl_listen_filter_t Nicolas Dichtel
2015-05-20 14:19       ` [PATCH iproute2-next 4/6] ipmonitor: introduce print_headers Nicolas Dichtel
2015-05-20 14:20       ` [PATCH iproute2-next 5/6] ipmonitor: allows to monitor in several netns Nicolas Dichtel
2015-05-20 14:20       ` [PATCH iproute2-next 6/6] xfrmmonitor: " Nicolas Dichtel

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='CAHA+R7NCjKn86R+OLX+4bkB0kv3WcXGB5NPui+v_X=CKqQtPLA@mail.gmail.com' \
    --to=cwang@twopensource.com \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=holler@ahsoftware.de \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.dichtel@6wind.com \
    --cc=tgraf@suug.ch \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).