Merge branch 'phonet-convert-all-doit-and-dumpit-to-rcu'

Kuniyuki Iwashima says:

====================
phonet: Convert all doit() and dumpit() to RCU.

addr_doit() and route_doit() access only phonet_device_list(dev_net(dev))
and phonet_pernet(dev_net(dev))->routes, respectively.

Each per-netns struct has its dedicated mutex, and RTNL also protects
the structs.  __dev_change_net_namespace() has synchronize_net(), so
we have two options to convert addr_doit() and route_doit().

  1. Use per-netns RTNL
  2. Use RCU and convert each struct mutex to spinlock_t

As RCU is preferable, this series converts all PF_PHONET's doit()
and dumpit() to RCU.

4 doit()s and 1 dumpit() are now converted to RCU, 70 doit()s and
28 dumpit()s are still under RTNL.
====================

Link: https://patch.msgid.link/20241017183140.43028-1-kuniyu@amazon.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Paolo Abeni
2024-10-24 16:04:27 +02:00
3 changed files with 120 additions and 72 deletions

View File

@@ -11,13 +11,13 @@
#define PN_DEV_H
#include <linux/list.h>
#include <linux/mutex.h>
#include <linux/spinlock.h>
struct net;
struct phonet_device_list {
struct list_head list;
struct mutex lock;
spinlock_t lock;
};
struct phonet_device_list *phonet_device_list(struct net *net);
@@ -38,11 +38,11 @@ int phonet_address_add(struct net_device *dev, u8 addr);
int phonet_address_del(struct net_device *dev, u8 addr);
u8 phonet_address_get(struct net_device *dev, u8 addr);
int phonet_address_lookup(struct net *net, u8 addr);
void phonet_address_notify(int event, struct net_device *dev, u8 addr);
void phonet_address_notify(struct net *net, int event, u32 ifindex, u8 addr);
int phonet_route_add(struct net_device *dev, u8 daddr);
int phonet_route_del(struct net_device *dev, u8 daddr);
void rtm_phonet_notify(int event, struct net_device *dev, u8 dst);
void rtm_phonet_notify(struct net *net, int event, u32 ifindex, u8 dst);
struct net_device *phonet_route_get_rcu(struct net *net, u8 daddr);
struct net_device *phonet_route_output(struct net *net, u8 daddr);

View File

@@ -22,7 +22,7 @@
#include <net/phonet/pn_dev.h>
struct phonet_routes {
struct mutex lock;
spinlock_t lock;
struct net_device __rcu *table[64];
};
@@ -54,7 +54,7 @@ static struct phonet_device *__phonet_device_alloc(struct net_device *dev)
pnd->netdev = dev;
bitmap_zero(pnd->addrs, 64);
BUG_ON(!mutex_is_locked(&pndevs->lock));
lockdep_assert_held(&pndevs->lock);
list_add_rcu(&pnd->list, &pndevs->list);
return pnd;
}
@@ -64,7 +64,8 @@ static struct phonet_device *__phonet_get(struct net_device *dev)
struct phonet_device_list *pndevs = phonet_device_list(dev_net(dev));
struct phonet_device *pnd;
BUG_ON(!mutex_is_locked(&pndevs->lock));
lockdep_assert_held(&pndevs->lock);
list_for_each_entry(pnd, &pndevs->list, list) {
if (pnd->netdev == dev)
return pnd;
@@ -91,17 +92,22 @@ static void phonet_device_destroy(struct net_device *dev)
ASSERT_RTNL();
mutex_lock(&pndevs->lock);
spin_lock(&pndevs->lock);
pnd = __phonet_get(dev);
if (pnd)
list_del_rcu(&pnd->list);
mutex_unlock(&pndevs->lock);
spin_unlock(&pndevs->lock);
if (pnd) {
struct net *net = dev_net(dev);
u32 ifindex = dev->ifindex;
u8 addr;
for_each_set_bit(addr, pnd->addrs, 64)
phonet_address_notify(RTM_DELADDR, dev, addr);
phonet_address_notify(net, RTM_DELADDR, ifindex, addr);
kfree(pnd);
}
}
@@ -133,7 +139,8 @@ int phonet_address_add(struct net_device *dev, u8 addr)
struct phonet_device *pnd;
int err = 0;
mutex_lock(&pndevs->lock);
spin_lock(&pndevs->lock);
/* Find or create Phonet-specific device data */
pnd = __phonet_get(dev);
if (pnd == NULL)
@@ -142,7 +149,9 @@ int phonet_address_add(struct net_device *dev, u8 addr)
err = -ENOMEM;
else if (test_and_set_bit(addr >> 2, pnd->addrs))
err = -EEXIST;
mutex_unlock(&pndevs->lock);
spin_unlock(&pndevs->lock);
return err;
}
@@ -152,7 +161,8 @@ int phonet_address_del(struct net_device *dev, u8 addr)
struct phonet_device *pnd;
int err = 0;
mutex_lock(&pndevs->lock);
spin_lock(&pndevs->lock);
pnd = __phonet_get(dev);
if (!pnd || !test_and_clear_bit(addr >> 2, pnd->addrs)) {
err = -EADDRNOTAVAIL;
@@ -161,7 +171,8 @@ int phonet_address_del(struct net_device *dev, u8 addr)
list_del_rcu(&pnd->list);
else
pnd = NULL;
mutex_unlock(&pndevs->lock);
spin_unlock(&pndevs->lock);
if (pnd)
kfree_rcu(pnd, rcu);
@@ -244,32 +255,39 @@ static int phonet_device_autoconf(struct net_device *dev)
ret = phonet_address_add(dev, req.ifr_phonet_autoconf.device);
if (ret)
return ret;
phonet_address_notify(RTM_NEWADDR, dev,
req.ifr_phonet_autoconf.device);
phonet_address_notify(dev_net(dev), RTM_NEWADDR, dev->ifindex,
req.ifr_phonet_autoconf.device);
return 0;
}
static void phonet_route_autodel(struct net_device *dev)
{
struct phonet_net *pnn = phonet_pernet(dev_net(dev));
unsigned int i;
struct net *net = dev_net(dev);
DECLARE_BITMAP(deleted, 64);
u32 ifindex = dev->ifindex;
struct phonet_net *pnn;
unsigned int i;
pnn = phonet_pernet(net);
/* Remove left-over Phonet routes */
bitmap_zero(deleted, 64);
mutex_lock(&pnn->routes.lock);
for (i = 0; i < 64; i++)
spin_lock(&pnn->routes.lock);
for (i = 0; i < 64; i++) {
if (rcu_access_pointer(pnn->routes.table[i]) == dev) {
RCU_INIT_POINTER(pnn->routes.table[i], NULL);
set_bit(i, deleted);
}
mutex_unlock(&pnn->routes.lock);
}
spin_unlock(&pnn->routes.lock);
if (bitmap_empty(deleted, 64))
return; /* short-circuit RCU */
synchronize_rcu();
for_each_set_bit(i, deleted, 64) {
rtm_phonet_notify(RTM_DELROUTE, dev, i);
rtm_phonet_notify(net, RTM_DELROUTE, ifindex, i);
dev_put(dev);
}
}
@@ -309,8 +327,8 @@ static int __net_init phonet_init_net(struct net *net)
return -ENOMEM;
INIT_LIST_HEAD(&pnn->pndevs.list);
mutex_init(&pnn->pndevs.lock);
mutex_init(&pnn->routes.lock);
spin_lock_init(&pnn->pndevs.lock);
spin_lock_init(&pnn->routes.lock);
return 0;
}
@@ -360,13 +378,15 @@ int phonet_route_add(struct net_device *dev, u8 daddr)
int err = -EEXIST;
daddr = daddr >> 2;
mutex_lock(&routes->lock);
spin_lock(&routes->lock);
if (routes->table[daddr] == NULL) {
rcu_assign_pointer(routes->table[daddr], dev);
dev_hold(dev);
err = 0;
}
mutex_unlock(&routes->lock);
spin_unlock(&routes->lock);
return err;
}
@@ -376,12 +396,13 @@ int phonet_route_del(struct net_device *dev, u8 daddr)
struct phonet_routes *routes = &pnn->routes;
daddr = daddr >> 2;
mutex_lock(&routes->lock);
spin_lock(&routes->lock);
if (rcu_access_pointer(routes->table[daddr]) == dev)
RCU_INIT_POINTER(routes->table[daddr], NULL);
else
dev = NULL;
mutex_unlock(&routes->lock);
spin_unlock(&routes->lock);
if (!dev)
return -ENOENT;

View File

@@ -19,10 +19,10 @@
/* Device address handling */
static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr,
static int fill_addr(struct sk_buff *skb, u32 ifindex, u8 addr,
u32 portid, u32 seq, int event);
void phonet_address_notify(int event, struct net_device *dev, u8 addr)
void phonet_address_notify(struct net *net, int event, u32 ifindex, u8 addr)
{
struct sk_buff *skb;
int err = -ENOBUFS;
@@ -31,17 +31,18 @@ void phonet_address_notify(int event, struct net_device *dev, u8 addr)
nla_total_size(1), GFP_KERNEL);
if (skb == NULL)
goto errout;
err = fill_addr(skb, dev, addr, 0, 0, event);
err = fill_addr(skb, ifindex, addr, 0, 0, event);
if (err < 0) {
WARN_ON(err == -EMSGSIZE);
kfree_skb(skb);
goto errout;
}
rtnl_notify(skb, dev_net(dev), 0,
RTNLGRP_PHONET_IFADDR, NULL, GFP_KERNEL);
rtnl_notify(skb, net, 0, RTNLGRP_PHONET_IFADDR, NULL, GFP_KERNEL);
return;
errout:
rtnl_set_sk_err(dev_net(dev), RTNLGRP_PHONET_IFADDR, err);
rtnl_set_sk_err(net, RTNLGRP_PHONET_IFADDR, err);
}
static const struct nla_policy ifa_phonet_policy[IFA_MAX+1] = {
@@ -64,8 +65,6 @@ static int addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!netlink_capable(skb, CAP_SYS_ADMIN))
return -EPERM;
ASSERT_RTNL();
err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFA_MAX,
ifa_phonet_policy, extack);
if (err < 0)
@@ -79,21 +78,29 @@ static int addr_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
/* Phonet addresses only have 6 high-order bits */
return -EINVAL;
dev = __dev_get_by_index(net, ifm->ifa_index);
if (dev == NULL)
rcu_read_lock();
dev = dev_get_by_index_rcu(net, ifm->ifa_index);
if (!dev) {
rcu_read_unlock();
return -ENODEV;
}
if (nlh->nlmsg_type == RTM_NEWADDR)
err = phonet_address_add(dev, pnaddr);
else
err = phonet_address_del(dev, pnaddr);
rcu_read_unlock();
if (!err)
phonet_address_notify(nlh->nlmsg_type, dev, pnaddr);
phonet_address_notify(net, nlh->nlmsg_type, ifm->ifa_index, pnaddr);
return err;
}
static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr,
u32 portid, u32 seq, int event)
static int fill_addr(struct sk_buff *skb, u32 ifindex, u8 addr,
u32 portid, u32 seq, int event)
{
struct ifaddrmsg *ifm;
struct nlmsghdr *nlh;
@@ -107,7 +114,7 @@ static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr,
ifm->ifa_prefixlen = 0;
ifm->ifa_flags = IFA_F_PERMANENT;
ifm->ifa_scope = RT_SCOPE_LINK;
ifm->ifa_index = dev->ifindex;
ifm->ifa_index = ifindex;
if (nla_put_u8(skb, IFA_LOCAL, addr))
goto nla_put_failure;
nlmsg_end(skb, nlh);
@@ -120,14 +127,17 @@ static int fill_addr(struct sk_buff *skb, struct net_device *dev, u8 addr,
static int getaddr_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
{
int addr_idx = 0, addr_start_idx = cb->args[1];
int dev_idx = 0, dev_start_idx = cb->args[0];
struct phonet_device_list *pndevs;
struct phonet_device *pnd;
int dev_idx = 0, dev_start_idx = cb->args[0];
int addr_idx = 0, addr_start_idx = cb->args[1];
int err = 0;
pndevs = phonet_device_list(sock_net(skb->sk));
rcu_read_lock();
list_for_each_entry_rcu(pnd, &pndevs->list, list) {
DECLARE_BITMAP(addrs, 64);
u8 addr;
if (dev_idx > dev_start_idx)
@@ -136,29 +146,32 @@ static int getaddr_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
continue;
addr_idx = 0;
for_each_set_bit(addr, pnd->addrs, 64) {
memcpy(addrs, pnd->addrs, sizeof(pnd->addrs));
for_each_set_bit(addr, addrs, 64) {
if (addr_idx++ < addr_start_idx)
continue;
if (fill_addr(skb, pnd->netdev, addr << 2,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, RTM_NEWADDR) < 0)
err = fill_addr(skb, READ_ONCE(pnd->netdev->ifindex),
addr << 2, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, RTM_NEWADDR);
if (err < 0)
goto out;
}
}
out:
rcu_read_unlock();
cb->args[0] = dev_idx;
cb->args[1] = addr_idx;
return skb->len;
return err;
}
/* Routes handling */
static int fill_route(struct sk_buff *skb, struct net_device *dev, u8 dst,
u32 portid, u32 seq, int event)
static int fill_route(struct sk_buff *skb, u32 ifindex, u8 dst,
u32 portid, u32 seq, int event)
{
struct rtmsg *rtm;
struct nlmsghdr *nlh;
@@ -177,8 +190,7 @@ static int fill_route(struct sk_buff *skb, struct net_device *dev, u8 dst,
rtm->rtm_scope = RT_SCOPE_UNIVERSE;
rtm->rtm_type = RTN_UNICAST;
rtm->rtm_flags = 0;
if (nla_put_u8(skb, RTA_DST, dst) ||
nla_put_u32(skb, RTA_OIF, READ_ONCE(dev->ifindex)))
if (nla_put_u8(skb, RTA_DST, dst) || nla_put_u32(skb, RTA_OIF, ifindex))
goto nla_put_failure;
nlmsg_end(skb, nlh);
return 0;
@@ -188,7 +200,7 @@ static int fill_route(struct sk_buff *skb, struct net_device *dev, u8 dst,
return -EMSGSIZE;
}
void rtm_phonet_notify(int event, struct net_device *dev, u8 dst)
void rtm_phonet_notify(struct net *net, int event, u32 ifindex, u8 dst)
{
struct sk_buff *skb;
int err = -ENOBUFS;
@@ -197,17 +209,18 @@ void rtm_phonet_notify(int event, struct net_device *dev, u8 dst)
nla_total_size(1) + nla_total_size(4), GFP_KERNEL);
if (skb == NULL)
goto errout;
err = fill_route(skb, dev, dst, 0, 0, event);
err = fill_route(skb, ifindex, dst, 0, 0, event);
if (err < 0) {
WARN_ON(err == -EMSGSIZE);
kfree_skb(skb);
goto errout;
}
rtnl_notify(skb, dev_net(dev), 0,
RTNLGRP_PHONET_ROUTE, NULL, GFP_KERNEL);
rtnl_notify(skb, net, 0, RTNLGRP_PHONET_ROUTE, NULL, GFP_KERNEL);
return;
errout:
rtnl_set_sk_err(dev_net(dev), RTNLGRP_PHONET_ROUTE, err);
rtnl_set_sk_err(net, RTNLGRP_PHONET_ROUTE, err);
}
static const struct nla_policy rtm_phonet_policy[RTA_MAX+1] = {
@@ -222,6 +235,7 @@ static int route_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
struct nlattr *tb[RTA_MAX+1];
struct net_device *dev;
struct rtmsg *rtm;
u32 ifindex;
int err;
u8 dst;
@@ -231,8 +245,6 @@ static int route_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!netlink_capable(skb, CAP_SYS_ADMIN))
return -EPERM;
ASSERT_RTNL();
err = nlmsg_parse_deprecated(nlh, sizeof(*rtm), tb, RTA_MAX,
rtm_phonet_policy, extack);
if (err < 0)
@@ -247,16 +259,26 @@ static int route_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
if (dst & 3) /* Phonet addresses only have 6 high-order bits */
return -EINVAL;
dev = __dev_get_by_index(net, nla_get_u32(tb[RTA_OIF]));
if (dev == NULL)
ifindex = nla_get_u32(tb[RTA_OIF]);
rcu_read_lock();
dev = dev_get_by_index_rcu(net, ifindex);
if (!dev) {
rcu_read_unlock();
return -ENODEV;
}
if (nlh->nlmsg_type == RTM_NEWROUTE)
err = phonet_route_add(dev, dst);
else
err = phonet_route_del(dev, dst);
rcu_read_unlock();
if (!err)
rtm_phonet_notify(nlh->nlmsg_type, dev, dst);
rtm_phonet_notify(net, nlh->nlmsg_type, ifindex, dst);
return err;
}
@@ -273,7 +295,7 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
if (!dev)
continue;
err = fill_route(skb, dev, addr << 2,
err = fill_route(skb, READ_ONCE(dev->ifindex), addr << 2,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, RTM_NEWROUTE);
if (err < 0)
@@ -286,13 +308,18 @@ static int route_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
}
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},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_NEWADDR,
.doit = addr_doit, .flags = RTNL_FLAG_DOIT_UNLOCKED},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_DELADDR,
.doit = addr_doit, .flags = RTNL_FLAG_DOIT_UNLOCKED},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_GETADDR,
.dumpit = getaddr_dumpit, .flags = RTNL_FLAG_DUMP_UNLOCKED},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_NEWROUTE,
.doit = route_doit, .flags = RTNL_FLAG_DOIT_UNLOCKED},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_DELROUTE,
.doit = route_doit, .flags = RTNL_FLAG_DOIT_UNLOCKED},
{.owner = THIS_MODULE, .protocol = PF_PHONET, .msgtype = RTM_GETROUTE,
.dumpit = route_dumpit, .flags = RTNL_FLAG_DUMP_UNLOCKED},
};
int __init phonet_netlink_register(void)