1

Merge branch 'rtnetlink-handle-error-of-rtnl_register_module'

Kuniyuki Iwashima says:

====================
rtnetlink: Handle error of rtnl_register_module().

While converting phonet to per-netns RTNL, I found a weird comment

  /* Further rtnl_register_module() cannot fail */

that was true but no longer true after commit addf9b90de ("net:
rtnetlink: use rcu to free rtnl message handlers").

Many callers of rtnl_register_module() just ignore the returned
value but should handle them properly.

This series introduces two helpers, rtnl_register_many() and
rtnl_unregister_many(), to do that easily and fix such callers.

All rtnl_register() and rtnl_register_module() will be converted
to _many() variant and some rtnl_lock() will be saved in _many()
later in net-next.

Changes:
  v4:
    * Add more context in changelog of each patch

  v3: https://lore.kernel.org/all/20241007124459.5727-1-kuniyu@amazon.com/
    * Move module *owner to struct rtnl_msg_handler
    * Make struct rtnl_msg_handler args/vars const
    * Update mctp goto labels

  v2: https://lore.kernel.org/netdev/20241004222358.79129-1-kuniyu@amazon.com/
    * Remove __exit from mctp_neigh_exit().

  v1: https://lore.kernel.org/netdev/20241003205725.5612-1-kuniyu@amazon.com/
====================

Link: https://patch.msgid.link/20241008184737.9619-1-kuniyu@amazon.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni 2024-10-10 15:39:37 +02:00
commit ffc8fa91be
15 changed files with 176 additions and 89 deletions

View File

@ -4913,9 +4913,13 @@ static int __init vxlan_init_module(void)
if (rc)
goto out4;
vxlan_vnifilter_init();
rc = vxlan_vnifilter_init();
if (rc)
goto out5;
return 0;
out5:
rtnl_link_unregister(&vxlan_link_ops);
out4:
unregister_switchdev_notifier(&vxlan_switchdev_notifier_block);
out3:

View File

@ -202,7 +202,7 @@ int vxlan_vni_in_use(struct net *src_net, struct vxlan_dev *vxlan,
int vxlan_vnigroup_init(struct vxlan_dev *vxlan);
void vxlan_vnigroup_uninit(struct vxlan_dev *vxlan);
void vxlan_vnifilter_init(void);
int vxlan_vnifilter_init(void);
void vxlan_vnifilter_uninit(void);
void vxlan_vnifilter_count(struct vxlan_dev *vxlan, __be32 vni,
struct vxlan_vni_node *vninode,

View File

@ -992,19 +992,18 @@ static int vxlan_vnifilter_process(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
void vxlan_vnifilter_init(void)
static const struct rtnl_msg_handler vxlan_vnifilter_rtnl_msg_handlers[] = {
{THIS_MODULE, PF_BRIDGE, RTM_GETTUNNEL, NULL, vxlan_vnifilter_dump, 0},
{THIS_MODULE, PF_BRIDGE, RTM_NEWTUNNEL, vxlan_vnifilter_process, NULL, 0},
{THIS_MODULE, PF_BRIDGE, RTM_DELTUNNEL, vxlan_vnifilter_process, NULL, 0},
};
int vxlan_vnifilter_init(void)
{
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETTUNNEL, NULL,
vxlan_vnifilter_dump, 0);
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWTUNNEL,
vxlan_vnifilter_process, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELTUNNEL,
vxlan_vnifilter_process, NULL, 0);
return rtnl_register_many(vxlan_vnifilter_rtnl_msg_handlers);
}
void vxlan_vnifilter_uninit(void)
{
rtnl_unregister(PF_BRIDGE, RTM_GETTUNNEL);
rtnl_unregister(PF_BRIDGE, RTM_NEWTUNNEL);
rtnl_unregister(PF_BRIDGE, RTM_DELTUNNEL);
rtnl_unregister_many(vxlan_vnifilter_rtnl_msg_handlers);
}

View File

@ -295,7 +295,7 @@ void mctp_neigh_remove_dev(struct mctp_dev *mdev);
int mctp_routes_init(void);
void mctp_routes_exit(void);
void mctp_device_init(void);
int mctp_device_init(void);
void mctp_device_exit(void);
#endif /* __NET_MCTP_H */

View File

@ -29,6 +29,15 @@ static inline enum rtnl_kinds rtnl_msgtype_kind(int msgtype)
return msgtype & RTNL_KIND_MASK;
}
struct rtnl_msg_handler {
struct module *owner;
int protocol;
int msgtype;
rtnl_doit_func doit;
rtnl_dumpit_func dumpit;
int flags;
};
void rtnl_register(int protocol, int msgtype,
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
int rtnl_register_module(struct module *owner, int protocol, int msgtype,
@ -36,6 +45,14 @@ int rtnl_register_module(struct module *owner, int protocol, int msgtype,
int rtnl_unregister(int protocol, int msgtype);
void rtnl_unregister_all(int protocol);
int __rtnl_register_many(const struct rtnl_msg_handler *handlers, int n);
void __rtnl_unregister_many(const struct rtnl_msg_handler *handlers, int n);
#define rtnl_register_many(handlers) \
__rtnl_register_many(handlers, ARRAY_SIZE(handlers))
#define rtnl_unregister_many(handlers) \
__rtnl_unregister_many(handlers, ARRAY_SIZE(handlers))
static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
{
if (nlmsg_len(nlh) >= sizeof(struct rtgenmsg))

View File

@ -1920,7 +1920,10 @@ int __init br_netlink_init(void)
{
int err;
br_vlan_rtnl_init();
err = br_vlan_rtnl_init();
if (err)
goto out;
rtnl_af_register(&br_af_ops);
err = rtnl_link_register(&br_link_ops);
@ -1931,6 +1934,7 @@ int __init br_netlink_init(void)
out_af:
rtnl_af_unregister(&br_af_ops);
out:
return err;
}

View File

@ -1571,7 +1571,7 @@ void br_vlan_get_stats(const struct net_bridge_vlan *v,
void br_vlan_port_event(struct net_bridge_port *p, unsigned long event);
int br_vlan_bridge_event(struct net_device *dev, unsigned long event,
void *ptr);
void br_vlan_rtnl_init(void);
int br_vlan_rtnl_init(void);
void br_vlan_rtnl_uninit(void);
void br_vlan_notify(const struct net_bridge *br,
const struct net_bridge_port *p,
@ -1802,8 +1802,9 @@ static inline int br_vlan_bridge_event(struct net_device *dev,
return 0;
}
static inline void br_vlan_rtnl_init(void)
static inline int br_vlan_rtnl_init(void)
{
return 0;
}
static inline void br_vlan_rtnl_uninit(void)

View File

@ -2296,19 +2296,18 @@ static int br_vlan_rtm_process(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
void br_vlan_rtnl_init(void)
static const struct rtnl_msg_handler br_vlan_rtnl_msg_handlers[] = {
{THIS_MODULE, PF_BRIDGE, RTM_NEWVLAN, br_vlan_rtm_process, NULL, 0},
{THIS_MODULE, PF_BRIDGE, RTM_DELVLAN, br_vlan_rtm_process, NULL, 0},
{THIS_MODULE, PF_BRIDGE, RTM_GETVLAN, NULL, br_vlan_rtm_dump, 0},
};
int br_vlan_rtnl_init(void)
{
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETVLAN, NULL,
br_vlan_rtm_dump, 0);
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWVLAN,
br_vlan_rtm_process, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELVLAN,
br_vlan_rtm_process, NULL, 0);
return rtnl_register_many(br_vlan_rtnl_msg_handlers);
}
void br_vlan_rtnl_uninit(void)
{
rtnl_unregister(PF_BRIDGE, RTM_GETVLAN);
rtnl_unregister(PF_BRIDGE, RTM_NEWVLAN);
rtnl_unregister(PF_BRIDGE, RTM_DELVLAN);
rtnl_unregister_many(br_vlan_rtnl_msg_handlers);
}

View File

@ -384,6 +384,35 @@ void rtnl_unregister_all(int protocol)
}
EXPORT_SYMBOL_GPL(rtnl_unregister_all);
int __rtnl_register_many(const struct rtnl_msg_handler *handlers, int n)
{
const struct rtnl_msg_handler *handler;
int i, err;
for (i = 0, handler = handlers; i < n; i++, handler++) {
err = rtnl_register_internal(handler->owner, handler->protocol,
handler->msgtype, handler->doit,
handler->dumpit, handler->flags);
if (err) {
__rtnl_unregister_many(handlers, i);
break;
}
}
return err;
}
EXPORT_SYMBOL_GPL(__rtnl_register_many);
void __rtnl_unregister_many(const struct rtnl_msg_handler *handlers, int n)
{
const struct rtnl_msg_handler *handler;
int i;
for (i = n - 1, handler = handlers + n - 1; i >= 0; i--, handler--)
rtnl_unregister(handler->protocol, handler->msgtype);
}
EXPORT_SYMBOL_GPL(__rtnl_unregister_many);
static LIST_HEAD(link_ops);
static const struct rtnl_link_ops *rtnl_link_ops_get(const char *kind)

View File

@ -756,10 +756,14 @@ static __init int mctp_init(void)
if (rc)
goto err_unreg_routes;
mctp_device_init();
rc = mctp_device_init();
if (rc)
goto err_unreg_neigh;
return 0;
err_unreg_neigh:
mctp_neigh_exit();
err_unreg_routes:
mctp_routes_exit();
err_unreg_proto:

View File

@ -524,25 +524,31 @@ static struct notifier_block mctp_dev_nb = {
.priority = ADDRCONF_NOTIFY_PRIORITY,
};
void __init mctp_device_init(void)
{
register_netdevice_notifier(&mctp_dev_nb);
static const struct rtnl_msg_handler mctp_device_rtnl_msg_handlers[] = {
{THIS_MODULE, PF_MCTP, RTM_NEWADDR, mctp_rtm_newaddr, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_DELADDR, mctp_rtm_deladdr, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_GETADDR, NULL, mctp_dump_addrinfo, 0},
};
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETADDR,
NULL, mctp_dump_addrinfo, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWADDR,
mctp_rtm_newaddr, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELADDR,
mctp_rtm_deladdr, NULL, 0);
int __init mctp_device_init(void)
{
int err;
register_netdevice_notifier(&mctp_dev_nb);
rtnl_af_register(&mctp_af_ops);
err = rtnl_register_many(mctp_device_rtnl_msg_handlers);
if (err) {
rtnl_af_unregister(&mctp_af_ops);
unregister_netdevice_notifier(&mctp_dev_nb);
}
return err;
}
void __exit mctp_device_exit(void)
{
rtnl_unregister_many(mctp_device_rtnl_msg_handlers);
rtnl_af_unregister(&mctp_af_ops);
rtnl_unregister(PF_MCTP, RTM_DELADDR);
rtnl_unregister(PF_MCTP, RTM_NEWADDR);
rtnl_unregister(PF_MCTP, RTM_GETADDR);
unregister_netdevice_notifier(&mctp_dev_nb);
}

View File

@ -322,22 +322,29 @@ static struct pernet_operations mctp_net_ops = {
.exit = mctp_neigh_net_exit,
};
static const struct rtnl_msg_handler mctp_neigh_rtnl_msg_handlers[] = {
{THIS_MODULE, PF_MCTP, RTM_NEWNEIGH, mctp_rtm_newneigh, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_DELNEIGH, mctp_rtm_delneigh, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_GETNEIGH, NULL, mctp_rtm_getneigh, 0},
};
int __init mctp_neigh_init(void)
{
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWNEIGH,
mctp_rtm_newneigh, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELNEIGH,
mctp_rtm_delneigh, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETNEIGH,
NULL, mctp_rtm_getneigh, 0);
int err;
return register_pernet_subsys(&mctp_net_ops);
err = register_pernet_subsys(&mctp_net_ops);
if (err)
return err;
err = rtnl_register_many(mctp_neigh_rtnl_msg_handlers);
if (err)
unregister_pernet_subsys(&mctp_net_ops);
return err;
}
void __exit mctp_neigh_exit(void)
void mctp_neigh_exit(void)
{
rtnl_unregister_many(mctp_neigh_rtnl_msg_handlers);
unregister_pernet_subsys(&mctp_net_ops);
rtnl_unregister(PF_MCTP, RTM_GETNEIGH);
rtnl_unregister(PF_MCTP, RTM_DELNEIGH);
rtnl_unregister(PF_MCTP, RTM_NEWNEIGH);
}

View File

@ -1474,26 +1474,39 @@ static struct pernet_operations mctp_net_ops = {
.exit = mctp_routes_net_exit,
};
static const struct rtnl_msg_handler mctp_route_rtnl_msg_handlers[] = {
{THIS_MODULE, PF_MCTP, RTM_NEWROUTE, mctp_newroute, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_DELROUTE, mctp_delroute, NULL, 0},
{THIS_MODULE, PF_MCTP, RTM_GETROUTE, NULL, mctp_dump_rtinfo, 0},
};
int __init mctp_routes_init(void)
{
int err;
dev_add_pack(&mctp_packet_type);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_GETROUTE,
NULL, mctp_dump_rtinfo, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_NEWROUTE,
mctp_newroute, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MCTP, RTM_DELROUTE,
mctp_delroute, NULL, 0);
err = register_pernet_subsys(&mctp_net_ops);
if (err)
goto err_pernet;
return register_pernet_subsys(&mctp_net_ops);
err = rtnl_register_many(mctp_route_rtnl_msg_handlers);
if (err)
goto err_rtnl;
return 0;
err_rtnl:
unregister_pernet_subsys(&mctp_net_ops);
err_pernet:
dev_remove_pack(&mctp_packet_type);
return err;
}
void mctp_routes_exit(void)
{
rtnl_unregister_many(mctp_route_rtnl_msg_handlers);
unregister_pernet_subsys(&mctp_net_ops);
rtnl_unregister(PF_MCTP, RTM_DELROUTE);
rtnl_unregister(PF_MCTP, RTM_NEWROUTE);
rtnl_unregister(PF_MCTP, RTM_GETROUTE);
dev_remove_pack(&mctp_packet_type);
}

View File

@ -2728,6 +2728,15 @@ static struct rtnl_af_ops mpls_af_ops __read_mostly = {
.get_stats_af_size = mpls_get_stats_af_size,
};
static const struct rtnl_msg_handler mpls_rtnl_msg_handlers[] __initdata_or_module = {
{THIS_MODULE, PF_MPLS, RTM_NEWROUTE, mpls_rtm_newroute, NULL, 0},
{THIS_MODULE, PF_MPLS, RTM_DELROUTE, mpls_rtm_delroute, NULL, 0},
{THIS_MODULE, PF_MPLS, RTM_GETROUTE, mpls_getroute, mpls_dump_routes, 0},
{THIS_MODULE, PF_MPLS, RTM_GETNETCONF,
mpls_netconf_get_devconf, mpls_netconf_dump_devconf,
RTNL_FLAG_DUMP_UNLOCKED},
};
static int __init mpls_init(void)
{
int err;
@ -2746,24 +2755,25 @@ static int __init mpls_init(void)
rtnl_af_register(&mpls_af_ops);
rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_NEWROUTE,
mpls_rtm_newroute, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_DELROUTE,
mpls_rtm_delroute, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETROUTE,
mpls_getroute, mpls_dump_routes, 0);
rtnl_register_module(THIS_MODULE, PF_MPLS, RTM_GETNETCONF,
mpls_netconf_get_devconf,
mpls_netconf_dump_devconf,
RTNL_FLAG_DUMP_UNLOCKED);
err = ipgre_tunnel_encap_add_mpls_ops();
err = rtnl_register_many(mpls_rtnl_msg_handlers);
if (err)
goto out_unregister_rtnl_af;
err = ipgre_tunnel_encap_add_mpls_ops();
if (err) {
pr_err("Can't add mpls over gre tunnel ops\n");
goto out_unregister_rtnl;
}
err = 0;
out:
return err;
out_unregister_rtnl:
rtnl_unregister_many(mpls_rtnl_msg_handlers);
out_unregister_rtnl_af:
rtnl_af_unregister(&mpls_af_ops);
dev_remove_pack(&mpls_packet_type);
out_unregister_pernet:
unregister_pernet_subsys(&mpls_net_ops);
goto out;

View File

@ -285,23 +285,17 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
return err;
}
static const struct rtnl_msg_handler phonet_rtnl_msg_handlers[] __initdata_or_module = {
{THIS_MODULE, PF_PHONET, RTM_NEWADDR, addr_doit, NULL, 0},
{THIS_MODULE, PF_PHONET, RTM_DELADDR, addr_doit, NULL, 0},
{THIS_MODULE, PF_PHONET, RTM_GETADDR, NULL, getaddr_dumpit, 0},
{THIS_MODULE, PF_PHONET, RTM_NEWROUTE, route_doit, NULL, 0},
{THIS_MODULE, PF_PHONET, RTM_DELROUTE, route_doit, NULL, 0},
{THIS_MODULE, PF_PHONET, RTM_GETROUTE, NULL, route_dumpit,
RTNL_FLAG_DUMP_UNLOCKED},
};
int __init phonet_netlink_register(void)
{
int err = rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWADDR,
addr_doit, NULL, 0);
if (err)
return err;
/* Further rtnl_register_module() cannot fail */
rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELADDR,
addr_doit, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETADDR,
NULL, getaddr_dumpit, 0);
rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_NEWROUTE,
route_doit, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_DELROUTE,
route_doit, NULL, 0);
rtnl_register_module(THIS_MODULE, PF_PHONET, RTM_GETROUTE,
NULL, route_dumpit, RTNL_FLAG_DUMP_UNLOCKED);
return 0;
return rtnl_register_many(phonet_rtnl_msg_handlers);
}