From: Xiao Liang <shaw.leon@xxxxxxxxx>
Date: Mon, 10 Feb 2025 21:29:53 +0800
> There are 4 net namespaces involved when creating links:
>
> - source netns - where the netlink socket resides,
> - target netns - where to put the device being created,
> - link netns - netns associated with the device (backend),
> - peer netns - netns of peer device.
>
> Currently, two nets are passed to newlink() callback - "src_net"
> parameter and "dev_net" (implicitly in net_device). They are set as
> follows, depending on netlink attributes in the request.
>
> +------------+-------------------+---------+---------+
> | peer netns | IFLA_LINK_NETNSID | src_net | dev_net |
> +------------+-------------------+---------+---------+
> | | absent | source | target |
> | absent +-------------------+---------+---------+
> | | present | link | link |
> +------------+-------------------+---------+---------+
> | | absent | peer | target |
> | present +-------------------+---------+---------+
> | | present | peer | link |
> +------------+-------------------+---------+---------+
>
> When IFLA_LINK_NETNSID is present, the device is created in link netns
> first and then moved to target netns. This has some side effects,
> including extra ifindex allocation, ifname validation and link events.
> These could be avoided if we create it in target netns from
> the beginning.
>
> On the other hand, the meaning of src_net parameter is ambiguous. It
> varies depending on how parameters are passed. It is the effective
> link (or peer netns) by design, but some drivers ignore it and use
> dev_net instead.
>
> This patch packs existing newlink() parameters, along with the source
> netns, link netns and peer netns, into a struct. The old "src_net"
> is renamed to "net" to avoid confusion with real source netns, and
> will be deprecated later. The use of src_net are converted to
> params->net trivially.
>
> To make the semantics more clear, two helper functions -
> rtnl_newlink_link_net() and rtnl_newlink_peer_net() - are provided
> for netns fallback logic. Peer netns falls back to link netns, and
> link netns falls back to source netns.
>
> In following patches, to prepare for creating link in target netns
> directly:
>
> - For device drivers that are aware of the old "src_net", the use of
> it are replace with one of the two helper functions.
> - And for those that takes dev_net() as link netns, we try
> params->link_net and then dev_net(), in order to maintain
> compatibility with the old behavior.
>
> Signed-off-by: Xiao Liang <shaw.leon@xxxxxxxxx>
Reviewed-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
I left small comments below.
[...]
> diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
> index 523025106a64..0f7281e3e448 100644
> --- a/include/linux/if_macvlan.h
> +++ b/include/linux/if_macvlan.h
> @@ -59,8 +59,10 @@ static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
>
> extern void macvlan_common_setup(struct net_device *dev);
>
> -extern int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> - struct nlattr *tb[], struct nlattr *data[],
> +struct rtnl_newlink_params;
You can just include <net/rtnetlink.h> and remove it from .c
files, then this forward declaration will be unnecessary.
> +
> +extern int macvlan_common_newlink(struct net_device *dev,
> + struct rtnl_newlink_params *params,
> struct netlink_ext_ack *extack);
>
> extern void macvlan_dellink(struct net_device *dev, struct list_head *head);
[...]
> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
> index bc0069a8b6ea..00c086ca0c11 100644
> --- a/include/net/rtnetlink.h
> +++ b/include/net/rtnetlink.h
> @@ -69,6 +69,42 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
> return AF_UNSPEC;
> }
>
> +/**
> + * struct rtnl_newlink_params - parameters of rtnl_link_ops::newlink()
The '\t' after '*' should be single '\s'.
Same for lines below.
> + *
> + * @net: Netns of interest
> + * @src_net: Source netns of rtnetlink socket
> + * @link_net: Link netns by IFLA_LINK_NETNSID, NULL if not specified
> + * @peer_net: Peer netns
> + * @tb: IFLA_* attributes
> + * @data: IFLA_INFO_DATA attributes
> + */
> +struct rtnl_newlink_params {
[...]
> +/* Get effective link netns from newlink params. Generally, this is link_net
> + * and falls back to src_net. But for compatibility, a driver may * choose to
> + * use dev_net(dev) instead.
> + */
> +static inline struct net *rtnl_newlink_link_net(struct rtnl_newlink_params *p)
> +{
> + return p->link_net ? : p->src_net;
> +}
> +
> +/* Get peer netns from newlink params. Fallback to link netns if peer netns is
> + * not specified explicitly.
> + */
> +static inline struct net *rtnl_newlink_peer_net(struct rtnl_newlink_params *p)
> +{
> + return p->peer_net ? : rtnl_newlink_link_net(p);
> +}
These helpers should belong to patch 2 ?
