Re: [PATCH net-next v8 06/11] net: ipv6: Use link netns in newlink() of rtnl_link_ops

Kuniyuki Iwashima
2025-01-14T04:49:59Z

From: Xiao Liang <shaw.leon@gmail.com>
Date: Mon, 13 Jan 2025 22:37:14 +0800
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 2a6a424806aa..ac5e402c34bc 100644
> --- a/drivers/net/bonding/bond_netlink.c
> +++ b/drivers/net/bonding/bond_netlink.c
> @@ -564,10 +564,12 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> return 0;
> }
>
> -static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
> - struct nlattr *tb[], struct nlattr *data[],
> +static int bond_newlink(struct net_device *bond_dev,
> + struct rtnl_newlink_params *params,
> struct netlink_ext_ack *extack)
> {
> + struct nlattr **data = params->data;
> + struct nlattr **tb = params->tb;
> int err;
>
> err = bond_changelink(bond_dev, tb, data, extack);

Note that IFLA_BOND_ACTIVE_SLAVE uses dev_net(dev) for
__dev_get_by_index().

[...]
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index fed4fe2a4748..0c496aa1f706 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -1565,11 +1565,12 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> }
> EXPORT_SYMBOL_GPL(macvlan_common_newlink);
>
> -static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> - struct nlattr *tb[], struct nlattr *data[],
> +static int macvlan_newlink(struct net_device *dev,
> + struct rtnl_newlink_params *params,
> struct netlink_ext_ack *extack)
> {
> - return macvlan_common_newlink(src_net, dev, tb, data, extack);
> + return macvlan_common_newlink(params->net, dev, params->tb,
> + params->data, extack);

Pass params as is as you did for ipvlan_link_new().

Same for macvtap_newlink().

[...]
> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
> index 1e1b00756be7..1e9eadc77da2 100644
> --- a/drivers/net/netkit.c
> +++ b/drivers/net/netkit.c
> @@ -327,10 +327,13 @@ static int netkit_validate(struct nlattr *tb[], struct nlattr *data[],
>
> static struct rtnl_link_ops netkit_link_ops;
>
> -static int netkit_new_link(struct net *peer_net, struct net_device *dev,
> - struct nlattr *tb[], struct nlattr *data[],
> +static int netkit_new_link(struct net_device *dev,
> + struct rtnl_newlink_params *params,
> struct netlink_ext_ack *extack)
> {
> + struct nlattr **data = params->data;
> + struct net *peer_net = params->net;
> + struct nlattr **tb = params->tb;

nit: please keep the reverse xmas tree order.

> struct nlattr *peer_tb[IFLA_MAX + 1], **tbp = tb, *attr;

you can define *tbp here and initialise it later.

struct nlattr *peer_tb[IFLA_MAX + 1], **tbp, *attr;

> enum netkit_action policy_prim = NETKIT_PASS;
> enum netkit_action policy_peer = NETKIT_PASS;

[...]
> @@ -1064,6 +1067,11 @@ static void wwan_create_default_link(struct wwan_device *wwandev,
> struct net_device *dev;
> struct nlmsghdr *nlh;
> struct sk_buff *msg;
> + struct rtnl_newlink_params params = {
> + .net = &init_net,
> + .tb = tb,
> + .data = data,
> + };

nit: Reverse xmas tree order

[...]
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index ec98349b9620..7ff5e96f6ba7 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -3766,6 +3766,14 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
> struct net_device *dev;
> char ifname[IFNAMSIZ];
> int err;
> + struct rtnl_newlink_params params = {

nit: Reverse xmas tree order

> + .net = net,

Use sock_net(skb->sk) directly here and remove net defined above,
which is no longer used in this function.

---8<---
unsigned char name_assign_type = NET_NAME_USER;
struct rtnl_newlink_params params = {
.net = sock_net(skb->sk),
.src_net = net,
.link_net = link_net,
.peer_net = peer_net,
.tb = tb,
.data = data,
};
u32 portid = NETLINK_CB(skb).portid;
---8<---

[...]
> @@ -1698,6 +1702,10 @@ struct net_device *gretap_fb_dev_create(struct net *net, const char *name,
> LIST_HEAD(list_kill);
> struct ip_tunnel *t;
> int err;
> + struct rtnl_newlink_params params = {
> + .net = net,
> + .tb = tb,
> + };
>
> memset(&tb, 0, sizeof(tb));

nit: Reverse xmas tree