| Message ID | 20240309183458.3014713-2-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | genetlink: remove linux/genetlink.h | expand |
On 2024-03-09 10:34, Jakub Kicinski wrote: > There are things in linux/genetlink.h which are only used > under net/netlink/. Move them to a new local header. > A new header with just 2 externs isn't great, but alternative > would be to include af_netlink.h in genetlink.c which feels > even worse. Why is including af_netlink.h worse? > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > CC: kuniyu@amazon.com > CC: jiri@resnulli.us > --- > include/linux/genetlink.h | 5 ----- > net/netlink/af_netlink.c | 2 +- > net/netlink/genetlink.c | 2 ++ > net/netlink/genetlink.h | 11 +++++++++++ > 4 files changed, 14 insertions(+), 6 deletions(-) > create mode 100644 net/netlink/genetlink.h > > diff --git a/include/linux/genetlink.h b/include/linux/genetlink.h > index c285968e437a..9dbd7ba9b858 100644 > --- a/include/linux/genetlink.h > +++ b/include/linux/genetlink.h > @@ -4,15 +4,10 @@ > > #include <uapi/linux/genetlink.h> > > - > /* All generic netlink requests are serialized by a global lock. */ > extern void genl_lock(void); > extern void genl_unlock(void); > > -/* for synchronisation between af_netlink and genetlink */ > -extern atomic_t genl_sk_destructing_cnt; > -extern wait_queue_head_t genl_sk_destructing_waitq; Checked these are only used in net/netlink/af_netlink.c and net/netlink/genetlink.c > - > #define MODULE_ALIAS_GENL_FAMILY(family)\ > MODULE_ALIAS_NET_PF_PROTO_NAME(PF_NETLINK, NETLINK_GENERIC, "-family-" family) > > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c > index da846212fb9b..621ef3d7f044 100644 > --- a/net/netlink/af_netlink.c > +++ b/net/netlink/af_netlink.c > @@ -59,7 +59,6 @@ > #include <linux/rhashtable.h> > #include <asm/cacheflush.h> > #include <linux/hash.h> > -#include <linux/genetlink.h> > #include <linux/net_namespace.h> > #include <linux/nospec.h> > #include <linux/btf_ids.h> > @@ -73,6 +72,7 @@ > #include <trace/events/netlink.h> > > #include "af_netlink.h" > +#include "genetlink.h" > > struct listeners { > struct rcu_head rcu; > diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c > index 3b7666944b11..feb54c63a116 100644 > --- a/net/netlink/genetlink.c > +++ b/net/netlink/genetlink.c > @@ -22,6 +22,8 @@ > #include <net/sock.h> > #include <net/genetlink.h> > > +#include "genetlink.h" > + > static DEFINE_MUTEX(genl_mutex); /* serialization of message processing */ > static DECLARE_RWSEM(cb_lock); > > diff --git a/net/netlink/genetlink.h b/net/netlink/genetlink.h > new file mode 100644 > index 000000000000..89bd9d2631c3 > --- /dev/null > +++ b/net/netlink/genetlink.h > @@ -0,0 +1,11 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __NET_GENETLINK_H > +#define __NET_GENETLINK_H > + > +#include <linux/wait.h> > + > +/* for synchronisation between af_netlink and genetlink */ > +extern atomic_t genl_sk_destructing_cnt; > +extern wait_queue_head_t genl_sk_destructing_waitq; > + > +#endif /* __LINUX_GENERIC_NETLINK_H */
On Sat, 9 Mar 2024 14:35:30 -0800 David Wei wrote: > On 2024-03-09 10:34, Jakub Kicinski wrote: > > There are things in linux/genetlink.h which are only used > > under net/netlink/. Move them to a new local header. > > A new header with just 2 externs isn't great, but alternative > > would be to include af_netlink.h in genetlink.c which feels > > even worse. > > Why is including af_netlink.h worse? It exposes the internals of the lower layer of the protocol stack. genetlink.c doesn't really need to know those details.
diff --git a/include/linux/genetlink.h b/include/linux/genetlink.h index c285968e437a..9dbd7ba9b858 100644 --- a/include/linux/genetlink.h +++ b/include/linux/genetlink.h @@ -4,15 +4,10 @@ #include <uapi/linux/genetlink.h> - /* All generic netlink requests are serialized by a global lock. */ extern void genl_lock(void); extern void genl_unlock(void); -/* for synchronisation between af_netlink and genetlink */ -extern atomic_t genl_sk_destructing_cnt; -extern wait_queue_head_t genl_sk_destructing_waitq; - #define MODULE_ALIAS_GENL_FAMILY(family)\ MODULE_ALIAS_NET_PF_PROTO_NAME(PF_NETLINK, NETLINK_GENERIC, "-family-" family) diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index da846212fb9b..621ef3d7f044 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -59,7 +59,6 @@ #include <linux/rhashtable.h> #include <asm/cacheflush.h> #include <linux/hash.h> -#include <linux/genetlink.h> #include <linux/net_namespace.h> #include <linux/nospec.h> #include <linux/btf_ids.h> @@ -73,6 +72,7 @@ #include <trace/events/netlink.h> #include "af_netlink.h" +#include "genetlink.h" struct listeners { struct rcu_head rcu; diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 3b7666944b11..feb54c63a116 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -22,6 +22,8 @@ #include <net/sock.h> #include <net/genetlink.h> +#include "genetlink.h" + static DEFINE_MUTEX(genl_mutex); /* serialization of message processing */ static DECLARE_RWSEM(cb_lock); diff --git a/net/netlink/genetlink.h b/net/netlink/genetlink.h new file mode 100644 index 000000000000..89bd9d2631c3 --- /dev/null +++ b/net/netlink/genetlink.h @@ -0,0 +1,11 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __NET_GENETLINK_H +#define __NET_GENETLINK_H + +#include <linux/wait.h> + +/* for synchronisation between af_netlink and genetlink */ +extern atomic_t genl_sk_destructing_cnt; +extern wait_queue_head_t genl_sk_destructing_waitq; + +#endif /* __LINUX_GENERIC_NETLINK_H */
There are things in linux/genetlink.h which are only used under net/netlink/. Move them to a new local header. A new header with just 2 externs isn't great, but alternative would be to include af_netlink.h in genetlink.c which feels even worse. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- CC: kuniyu@amazon.com CC: jiri@resnulli.us --- include/linux/genetlink.h | 5 ----- net/netlink/af_netlink.c | 2 +- net/netlink/genetlink.c | 2 ++ net/netlink/genetlink.h | 11 +++++++++++ 4 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 net/netlink/genetlink.h