diff mbox series

[net] netns: export get_net_ns_by_id()

Message ID 20210512212956.4727-1-ryazanov.s.a@gmail.com
State New
Headers show
Series [net] netns: export get_net_ns_by_id() | expand

Commit Message

Sergey Ryazanov May 12, 2021, 9:29 p.m. UTC
No one loadable module is able to obtain netns by id since the
corresponding function has not been exported. Export it to be able to
use netns id API in loadable modules too as already done for
peernet2id_alloc().

CC: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
---
 net/core/net_namespace.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Jakub Kicinski May 14, 2021, 7:14 p.m. UTC | #1
On Thu, 13 May 2021 00:29:56 +0300 Sergey Ryazanov wrote:
> No one loadable module is able to obtain netns by id since the

> corresponding function has not been exported. Export it to be able to

> use netns id API in loadable modules too as already done for

> peernet2id_alloc().


peernet2id_alloc() is used by OvS, what's the user for get_net_ns_by_id()?
Sergey Ryazanov May 14, 2021, 8:52 p.m. UTC | #2
On Fri, May 14, 2021 at 10:14 PM Jakub Kicinski <kuba@kernel.org> wrote:
> On Thu, 13 May 2021 00:29:56 +0300 Sergey Ryazanov wrote:

> > No one loadable module is able to obtain netns by id since the

> > corresponding function has not been exported. Export it to be able to

> > use netns id API in loadable modules too as already done for

> > peernet2id_alloc().

>

> peernet2id_alloc() is used by OvS, what's the user for get_net_ns_by_id()?


There are currently no active users of get_net_ns_by_id(), that is why
I did not add a "Fix" tag. Missed function export does not break
existing code in any way.

On the other hand netns id API is incomplete without this export. You
have no way to write and test a code that uses netns id API without
manual kernel patching and rebuilding. This is annoying, but could be
trivially fixed.

Accounting for the fact that this change is trivial, closer to a fix
than to a new functionality, I prefered net tree over the net-next
tree. This way I expect the new API to reach mainstream distros faster
than over 5 years :)

-- 
Sergey
Leon Romanovsky May 16, 2021, 11:05 a.m. UTC | #3
On Fri, May 14, 2021 at 11:52:51PM +0300, Sergey Ryazanov wrote:
> On Fri, May 14, 2021 at 10:14 PM Jakub Kicinski <kuba@kernel.org> wrote:

> > On Thu, 13 May 2021 00:29:56 +0300 Sergey Ryazanov wrote:

> > > No one loadable module is able to obtain netns by id since the

> > > corresponding function has not been exported. Export it to be able to

> > > use netns id API in loadable modules too as already done for

> > > peernet2id_alloc().

> >

> > peernet2id_alloc() is used by OvS, what's the user for get_net_ns_by_id()?

> 

> There are currently no active users of get_net_ns_by_id(), that is why

> I did not add a "Fix" tag. Missed function export does not break

> existing code in any way.


It is against kernel rule to do not expose APIs, even internal to the kernel,
without real users. There are many patches every cycle that remove such EXPORT_*()s.

EXPORT_*() creates extra entries in Module.symvers and can be seen as unnecessary
namespace pollution.

Thanks
Sergey Ryazanov May 16, 2021, 1:47 p.m. UTC | #4
Hello Leon,

On Sun, May 16, 2021 at 2:05 PM Leon Romanovsky <leon@kernel.org> wrote:
> On Fri, May 14, 2021 at 11:52:51PM +0300, Sergey Ryazanov wrote:

> > On Fri, May 14, 2021 at 10:14 PM Jakub Kicinski <kuba@kernel.org> wrote:

> > > On Thu, 13 May 2021 00:29:56 +0300 Sergey Ryazanov wrote:

> > > > No one loadable module is able to obtain netns by id since the

> > > > corresponding function has not been exported. Export it to be able to

> > > > use netns id API in loadable modules too as already done for

> > > > peernet2id_alloc().

> > >

> > > peernet2id_alloc() is used by OvS, what's the user for get_net_ns_by_id()?

> >

> > There are currently no active users of get_net_ns_by_id(), that is why

> > I did not add a "Fix" tag. Missed function export does not break

> > existing code in any way.

>

> It is against kernel rule to do not expose APIs, even internal to the kernel,

> without real users. There are many patches every cycle that remove such EXPORT_*()s.

>

> EXPORT_*() creates extra entries in Module.symvers and can be seen as unnecessary

> namespace pollution.


Ok, I got it. Maintainers do not like uncontrollable API experiments
:) I have no more arguments and I give up. Jakub, please drop this
patch.

BTW, for those who might be interested in experimenting with netnsid.
I found another way to search netns by its id without the kernel
rebuild. get_net_ns_by_id() is a simple container for the idr_found()
invocation, which is wrapped with the RCU lock. So it is no big deal
to implement this function locally to a module.

--
Sergey
Nicolas Dichtel May 17, 2021, 7:18 a.m. UTC | #5
Le 16/05/2021 à 13:05, Leon Romanovsky a écrit :
> On Fri, May 14, 2021 at 11:52:51PM +0300, Sergey Ryazanov wrote:

>> On Fri, May 14, 2021 at 10:14 PM Jakub Kicinski <kuba@kernel.org> wrote:

>>> On Thu, 13 May 2021 00:29:56 +0300 Sergey Ryazanov wrote:

>>>> No one loadable module is able to obtain netns by id since the

>>>> corresponding function has not been exported. Export it to be able to

>>>> use netns id API in loadable modules too as already done for

>>>> peernet2id_alloc().

>>>

>>> peernet2id_alloc() is used by OvS, what's the user for get_net_ns_by_id()?

>>

>> There are currently no active users of get_net_ns_by_id(), that is why

>> I did not add a "Fix" tag. Missed function export does not break

>> existing code in any way.

> 

> It is against kernel rule to do not expose APIs, even internal to the kernel,

Yep, there was no internal user, it's why I didn't add this EXPORT_*() at the
beginning.


Regards,
Nicolas
diff mbox series

Patch

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 43b6ac4c4439..0414406ed664 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -306,6 +306,7 @@  struct net *get_net_ns_by_id(const struct net *net, int id)
 
 	return peer;
 }
+EXPORT_SYMBOL_GPL(get_net_ns_by_id);
 
 /*
  * setup_net runs the initializers for the network namespace object.