| Message ID | 20210512212956.4727-1-ryazanov.s.a@gmail.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] netns: export get_net_ns_by_id() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | fail | Series targets non-next tree, but doesn't contain any Fixes tags |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 6 maintainers not CCed: ktkhai@virtuozzo.com daniel@iogearbox.net ast@kernel.org ap420073@gmail.com christian.brauner@ubuntu.com edumazet@google.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 1 this patch: 1 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 1 this patch: 1 |
| netdev/header_inline | success | Link |
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()?
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 :)
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
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
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 --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.
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(+)