| Message ID | 20241105020514.41963-4-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | rtnetlink: Convert rtnl_newlink() to per-netns RTNL. | expand |
On Tue, Nov 5, 2024 at 3:06 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > For veth, vxcan, and netkit, we need to prefetch the peer device's > netns in rtnl_newlink() for per-netns RTNL. > > All of the three get the netns in the same way peer netlink attr tb: > > 1. Call rtnl_nla_parse_ifinfomsg() > 2. Call ops->validate() (vxcan doesn't have) > 3. Call rtnl_link_get_net_tb() > > Let's add a new field peer_type to struct rtnl_link_ops and fetch > netns in peer attrbutes to add it to rtnl_nets in rtnl_newlink(). > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> Reviewed-by: Eric Dumazet <edumazet@google.com>
On Mon, 4 Nov 2024 18:05:09 -0800 Kuniyuki Iwashima wrote:
> + const unsigned char peer_type;
technically netlink attr types are 14b wide or some such
On Tue, 5 Nov 2024 16:39:11 -0800 Jakub Kicinski wrote: > On Mon, 4 Nov 2024 18:05:09 -0800 Kuniyuki Iwashima wrote: > > + const unsigned char peer_type; > > technically netlink attr types are 14b wide or some such I guess compiler will warn if someone tries to use < 255
From: Jakub Kicinski <kuba@kernel.org> Date: Tue, 5 Nov 2024 16:39:57 -0800 > On Tue, 5 Nov 2024 16:39:11 -0800 Jakub Kicinski wrote: > > On Mon, 4 Nov 2024 18:05:09 -0800 Kuniyuki Iwashima wrote: > > > + const unsigned char peer_type; > > > > technically netlink attr types are 14b wide or some such > > I guess compiler will warn if someone tries to use < 255 I chose 1 just because all of the three peer attr types were 1. Should peer_type be u16 or extend when a future device use >255 for peer ifla ? VETH_INFO_PEER VXCAN_INFO_PEER IFLA_NETKIT_PEER_INFO
From: Kuniyuki Iwashima <kuniyu@amazon.com> Date: Tue, 5 Nov 2024 16:52:37 -0800 > From: Jakub Kicinski <kuba@kernel.org> > Date: Tue, 5 Nov 2024 16:39:57 -0800 > > On Tue, 5 Nov 2024 16:39:11 -0800 Jakub Kicinski wrote: > > > On Mon, 4 Nov 2024 18:05:09 -0800 Kuniyuki Iwashima wrote: > > > > + const unsigned char peer_type; > > > > > > technically netlink attr types are 14b wide or some such > > > > I guess compiler will warn if someone tries to use < 255 > > I chose 1 just because all of the three peer attr types were 1. s/chose 1/chose u8/ :) > Should peer_type be u16 or extend when a future device use >255 for > peer ifla ? > > VETH_INFO_PEER > VXCAN_INFO_PEER > IFLA_NETKIT_PEER_INFO
On Tue, 5 Nov 2024 16:58:25 -0800 Kuniyuki Iwashima wrote: > > > I guess compiler will warn if someone tries to use < 255 > > > > I chose 1 just because all of the three peer attr types were 1. > > s/chose 1/chose u8/ :) > > > > Should peer_type be u16 or extend when a future device use >255 for > > peer ifla ? I think we can extend in the future if you're doing this for packing reasons. Barely any family has more attrs than 256 and as I replied to myself we will assign a constant so compiler will warn us.
From: Jakub Kicinski <kuba@kernel.org> Date: Tue, 5 Nov 2024 17:04:47 -0800 > On Tue, 5 Nov 2024 16:58:25 -0800 Kuniyuki Iwashima wrote: > > > > I guess compiler will warn if someone tries to use < 255 > > > > > > I chose 1 just because all of the three peer attr types were 1. > > > > s/chose 1/chose u8/ :) > > > > > > > Should peer_type be u16 or extend when a future device use >255 for > > > peer ifla ? > > I think we can extend in the future if you're doing this for packing > reasons. Barely any family has more attrs than 256 and as I replied to > myself we will assign a constant so compiler will warn us. I have no strong preference so will use u16. Also, there was a 3-bytes hole. Thanks!
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h index b9ed44b2d056..c3548da95ffa 100644 --- a/include/net/rtnetlink.h +++ b/include/net/rtnetlink.h @@ -85,6 +85,7 @@ void rtnl_nets_add(struct rtnl_nets *rtnl_nets, struct net *net); * @srcu: Used internally * @kind: Identifier * @netns_refund: Physical device, move to init_net on netns exit + * @peer_type: Peer device specific netlink attribute number (e.g. VETH_INFO_PEER) * @maxtype: Highest device specific netlink attribute number * @policy: Netlink policy for device specific attribute validation * @validate: Optional validation function for netlink/changelink parameters @@ -126,6 +127,7 @@ struct rtnl_link_ops { void (*setup)(struct net_device *dev); bool netns_refund; + const unsigned char peer_type; unsigned int maxtype; const struct nla_policy *policy; int (*validate)(struct nlattr *tb[], diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 1bc8afcefc1e..48bd9e062550 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3798,6 +3798,37 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm, goto out; } +static int rtnl_add_peer_net(struct rtnl_nets *rtnl_nets, + const struct rtnl_link_ops *ops, + struct nlattr *data[], + struct netlink_ext_ack *extack) +{ + struct nlattr *tb[IFLA_MAX + 1]; + struct net *net; + int err; + + if (!data || !data[ops->peer_type]) + return 0; + + err = rtnl_nla_parse_ifinfomsg(tb, data[ops->peer_type], extack); + if (err < 0) + return err; + + if (ops->validate) { + err = ops->validate(tb, NULL, extack); + if (err < 0) + return err; + } + + net = rtnl_link_get_net_tb(tb); + if (IS_ERR(net)) + return PTR_ERR(net); + if (net) + rtnl_nets_add(rtnl_nets, net); + + return 0; +} + static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, const struct rtnl_link_ops *ops, struct net *tgt_net, struct net *link_net, @@ -3926,12 +3957,18 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, if (ret < 0) goto put_ops; } + + if (ops->peer_type) { + ret = rtnl_add_peer_net(&rtnl_nets, ops, data, extack); + if (ret < 0) + goto put_ops; + } } tgt_net = rtnl_link_get_net_capable(skb, sock_net(skb->sk), tb, CAP_NET_ADMIN); if (IS_ERR(tgt_net)) { ret = PTR_ERR(tgt_net); - goto put_ops; + goto put_net; } rtnl_nets_add(&rtnl_nets, tgt_net);
For veth, vxcan, and netkit, we need to prefetch the peer device's netns in rtnl_newlink() for per-netns RTNL. All of the three get the netns in the same way peer netlink attr tb: 1. Call rtnl_nla_parse_ifinfomsg() 2. Call ops->validate() (vxcan doesn't have) 3. Call rtnl_link_get_net_tb() Let's add a new field peer_type to struct rtnl_link_ops and fetch netns in peer attrbutes to add it to rtnl_nets in rtnl_newlink(). Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> --- include/net/rtnetlink.h | 2 ++ net/core/rtnetlink.c | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-)