On 12/9/24 15:01, Xiao Liang wrote:
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.
+------------+-------------------+---------+---------+
| 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. This has some side effects, including extra ifindex allocation,
ifname validation and link notifications. There's also an extra step to
move the device to target netns. These could be avoided if we create it
in target netns at the beginning.
On the other hand, the meaning of src_net 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 refactors netns handling by packing newlink() parameters into
a struct, and passing source, link and peer netns as is through this
struct. Fallback logic is implemented in helper functions -
rtnl_newlink_link_net() and rtnl_newlink_peer_net(). If is not set, peer
netns falls back to link netns, and link netns falls back to source netns.
rtnl_newlink_create() now creates devices in target netns directly,
so dev_net is always target netns.
For drivers that use dev_net as fallback of link_netns, current behavior
is kept for compatibility.
Signed-off-by: Xiao Liang <shaw.leon(a)gmail.com>
I must admit this patch is way too huge for me to allow any reasonable
review except that this has the potential of breaking a lot of things.
I think you should be splitted to make it more palatable; i.e.
- a patch just add the params struct with no semantic changes.
- a patch making the dev_change_net_namespace() conditional on net !=
tge_net[1]
- many per-device patches creating directly the device in the target
namespace.
- a patch reverting [1]
Other may have different opinions, I'd love to hear them.
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 98c6205ed19f..2f7bf50e05d2 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -3161,14 +3161,17 @@ static int amt_validate(struct nlattr *tb[], struct nlattr
*data[],
return 0;
}
-static int amt_newlink(struct net *net, struct net_device *dev,
- struct nlattr *tb[], struct nlattr *data[],
- struct netlink_ext_ack *extack)
+static int amt_newlink(struct rtnl_newlink_params *params)
{
+ struct net_device *dev = params->dev;
+ struct nlattr **tb = params->tb;
+ struct nlattr **data = params->data;
+ struct netlink_ext_ack *extack = params->extack;
+ struct net *link_net = rtnl_newlink_link_net(params);
struct amt_dev *amt = netdev_priv(dev);
int err = -EINVAL;
Minor nit: here and and many other places, please respect the reverse
xmas tree order.
Thanks,
Paolo