netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Benc <jbenc@redhat.com>
To: Christian Brauner <christian.brauner@canonical.com>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd
Date: Tue, 23 Jan 2018 13:22:33 +0100	[thread overview]
Message-ID: <20180123132233.6d77040b@redhat.com> (raw)
In-Reply-To: <20180123114218.vsm5nu2jajrqjvko@gmail.com>

(Christian, I'm adding back the netdev list, there's no reason not to
have the discussion in open.)

On Tue, 23 Jan 2018 12:42:19 +0100, Christian Brauner wrote:
> Thanks for the comments and discussion. Sorry, for not going through the
> list but I just have a quick question that doesn't deserve to spam the
> whole netdev list. :) I have written another set of patches that would
> add support for IFLA_IF_NETNSID to RTM_{N,S}ETLINK and friends which I
> planned to send out after the pid and fd ones. Would you be interested
> in those if I sent them out over the next week?

Yes, please CC me. I will have limited time next week (whole day
meetings for the most of the week) but I'll try to review.

In general, I see value in adding netnsid support to setlink, newlink
and dellink. It's a bit of minefield, though. Look for and be sure to
resolve:

- Backwards compatibility with old kernels. Old kernels will ignore
  the IFLA_IF_NETNSID attribute that is unknown to them. And it's
  certainly undesirable for an application to attempt to set/create an
  interface in a given netns and instead the kernel sets/creates an
  interface in the current netns.

  In 79e1ad148c844, I put into place means to handle that: any kernel
  that understands IFLA_IF_NETNSID but does not support
  setlink/newlink/dellink will return -EOPNOTSUPP. This reduces the
  problem to detecting whether the kernel understands IFLA_IF_NETNSID.
  This can be done by invoking getlink with IFLA_IF_NETNSID and
  looking whether the reply contains IFLA_IF_NETNSID.

  The current getlink calls are also limited to CAP_NET_ADMIN in the
  *target* netns. If/when this is relaxed in the future, we still need
  to stay backwards compatible.

  So, the application needs to do this:
  1. call RTM_GETLINK with IFLA_IF_NETNSID on lo (or do a dump)
  2. if getting EACCESS, assume IFLA_IF_NETNSID is not supported =>
     fall back to other means
  2. if the reply does not contain IFLA_IF_NETNSID, the kernel does not
     support IFLA_IF_NETNSID => fall back to other means
  3. try RTM_SETLINK/NEWLINK/DELLINK and check for EOPNOTSUPP => fall
     back to other means

  What is important for your kernel patches is to not break this. If
  you implement only partial support for some of the operation, think
  about the future extensibility. Applications have to be able to
  reliably detect what is supported on the running kernel, regardless
  of the kernel version.

- Access rights. This is security sensitive stuff. I took the big
  hammer and limited everything to CAP_NET_ADMIN in the target netns.
  If you want to relax this, be sure to get review from people involved
  in name space security.

 Jiri

  parent reply	other threads:[~2018-01-23 12:22 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-18 20:21 [PATCH net-next 0/1] rtnetlink: request RTM_GETLINK by pid or fd Christian Brauner
2018-01-18 20:21 ` [PATCH net-next 1/1] " Christian Brauner
2018-01-18 20:29   ` Jiri Benc
2018-01-18 20:55     ` Christian Brauner
2018-01-22 21:00       ` Jiri Benc
2018-01-22 21:23         ` Christian Brauner
2018-01-22 22:06           ` Jiri Benc
2018-01-22 22:25             ` Christian Brauner
2018-01-23  9:30               ` Jiri Benc
2018-01-23 10:26                 ` Wolfgang Bumiller
2018-01-23 10:42                   ` Jiri Benc
     [not found]                     ` <20180123114218.vsm5nu2jajrqjvko@gmail.com>
2018-01-23 12:22                       ` Jiri Benc [this message]
2018-01-23 16:55                         ` Nicolas Dichtel
2018-01-23 18:05                           ` Christian Brauner
2018-01-24 11:32                         ` [PATCH net-next 0/3] rtnetlink: enable IFLA_IF_NETNSID for RTM_{DEL,SET}LINK Christian Brauner
2018-01-24 11:32                           ` [PATCH net-next 1/3] rtnetlink: enable IFLA_IF_NETNSID in do_setlink() Christian Brauner
2018-01-24 11:52                         ` [PATCH net-next 2/3] rtnetlink: enable IFLA_IF_NETNSID for RTM_SETLINK Christian Brauner
2018-01-24 11:53                         ` [PATCH net-next 3/3] rtnetlink: enable IFLA_IF_NETNSID for RTM_DELLINK Christian Brauner
2018-01-23 16:50                   ` [PATCH net-next 1/1] rtnetlink: request RTM_GETLINK by pid or fd Nicolas Dichtel
2018-01-23 16:37             ` Nicolas Dichtel
2018-01-23 17:08               ` Jiri Benc
2018-01-24 10:53                 ` Nicolas Dichtel
2018-01-24 11:03                   ` Jiri Benc

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=20180123132233.6d77040b@redhat.com \
    --to=jbenc@redhat.com \
    --cc=christian.brauner@canonical.com \
    --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 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).