[PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Toke Høiland-Jørgensen]

Eric suggested[0] allowing user-settable values for dev->perm_addr at
device creation time, instead of mucking about with netdevsim to get a
virtual device with a permanent address set.

The original use case for this was easing testing for network management
daemons that use the permanent address to match against. However, having
the ability to set a permanent identifier for a virtual device at
creation time seems generally useful, so decided to go for this approach
instead.

[0] https://lore.kernel.org/r/CANn89iK8YpzNhJv4R+x80hcq794bh_ykS-O-2UHziBXixNhzyA@mail.gmail.com

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 net/core/rtnetlink.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index cb7fad8d1f95ff287810229c341de6a6d20a9c07..38dec2ae1a19f91daf9200744c7497088ecc037e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2224,7 +2224,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_PROP_LIST]	= { .type = NLA_NESTED },
 	[IFLA_ALT_IFNAME]	= { .type = NLA_STRING,
 				    .len = ALTIFNAMSIZ - 1 },
-	[IFLA_PERM_ADDRESS]	= { .type = NLA_REJECT },
+	[IFLA_PERM_ADDRESS]	= { .type = NLA_BINARY, .len = MAX_ADDR_LEN },
 	[IFLA_PROTO_DOWN_REASON] = { .type = NLA_NESTED },
 	[IFLA_NEW_IFINDEX]	= NLA_POLICY_MIN(NLA_S32, 1),
 	[IFLA_PARENT_DEV_NAME]	= { .type = NLA_NUL_STRING },
@@ -2647,7 +2647,7 @@ static	int rtnl_set_vf_rate(struct net_device *dev, int vf, int min_tx_rate,
 }
 
 static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[],
-			    struct netlink_ext_ack *extack)
+			    struct netlink_ext_ack *extack, bool create)
 {
 	if (tb[IFLA_ADDRESS] &&
 	    nla_len(tb[IFLA_ADDRESS]) < dev->addr_len)
@@ -2657,6 +2657,17 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[],
 	    nla_len(tb[IFLA_BROADCAST]) < dev->addr_len)
 		return -EINVAL;
 
+	if (tb[IFLA_PERM_ADDRESS]) {
+		if (!create) {
+			NL_SET_ERR_MSG(extack,
+				       "can't change permanent address");
+			return -EINVAL;
+		}
+
+		if (nla_len(tb[IFLA_PERM_ADDRESS]) < dev->addr_len)
+			return -EINVAL;
+	}
+
 	if (tb[IFLA_GSO_MAX_SIZE] &&
 	    nla_get_u32(tb[IFLA_GSO_MAX_SIZE]) > dev->tso_max_size) {
 		NL_SET_ERR_MSG(extack, "too big gso_max_size");
@@ -3010,7 +3021,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
 	char ifname[IFNAMSIZ];
 	int err;
 
-	err = validate_linkmsg(dev, tb, extack);
+	err = validate_linkmsg(dev, tb, extack, false);
 	if (err < 0)
 		goto errout;
 
@@ -3614,7 +3625,7 @@ struct net_device *rtnl_create_link(struct net *net, const char *ifname,
 	if (!dev)
 		return ERR_PTR(-ENOMEM);
 
-	err = validate_linkmsg(dev, tb, extack);
+	err = validate_linkmsg(dev, tb, extack, true);
 	if (err < 0) {
 		free_netdev(dev);
 		return ERR_PTR(err);
@@ -3642,6 +3653,9 @@ struct net_device *rtnl_create_link(struct net *net, const char *ifname,
 	if (tb[IFLA_BROADCAST])
 		memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
 				nla_len(tb[IFLA_BROADCAST]));
+	if (tb[IFLA_PERM_ADDRESS])
+		memcpy(dev->perm_addr, nla_data(tb[IFLA_PERM_ADDRESS]),
+		       nla_len(tb[IFLA_PERM_ADDRESS]));
 	if (tb[IFLA_TXQLEN])
 		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
 	if (tb[IFLA_OPERSTATE])

---
base-commit: 7aca0d8a727da503a8adeb6866a136ded5bea4b1
change-id: 20250213-virt-dev-permaddr-7d7dca7c6cb3

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Jakub Kicinski]

On Thu, 13 Feb 2025 14:45:22 +0100 Toke Høiland-Jørgensen wrote:
> Eric suggested[0] allowing user-settable values for dev->perm_addr at
> device creation time, instead of mucking about with netdevsim to get a
> virtual device with a permanent address set.

I vote no. Complicating the core so that its easier for someone 
to write a unit test is the wrong engineering trade off.
Use a VM or netdevsim, that's what they are for.

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Toke Høiland-Jørgensen]

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 13 Feb 2025 14:45:22 +0100 Toke Høiland-Jørgensen wrote:
>> Eric suggested[0] allowing user-settable values for dev->perm_addr at
>> device creation time, instead of mucking about with netdevsim to get a
>> virtual device with a permanent address set.
>
> I vote no. Complicating the core so that its easier for someone 
> to write a unit test is the wrong engineering trade off.
> Use a VM or netdevsim, that's what they are for.

Hmm, and you don't see any value in being able to specify a permanent
identifier for virtual devices? That bit was not just motivated
reasoning on my part... :)

-Toke

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Jakub Kicinski]

On Thu, 13 Feb 2025 17:13:53 +0100 Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <kuba@kernel.org> writes:
> 
> > On Thu, 13 Feb 2025 14:45:22 +0100 Toke Høiland-Jørgensen wrote:  
> >> Eric suggested[0] allowing user-settable values for dev->perm_addr at
> >> device creation time, instead of mucking about with netdevsim to get a
> >> virtual device with a permanent address set.  
> >
> > I vote no. Complicating the core so that its easier for someone 
> > to write a unit test is the wrong engineering trade off.
> > Use a VM or netdevsim, that's what they are for.  
> 
> Hmm, and you don't see any value in being able to specify a permanent
> identifier for virtual devices? That bit was not just motivated
> reasoning on my part... :)

I can't think of any :( Specifying an address is already possible.
Permanent address is a property of the hardware platform.
Virtual devices OTOH are primarily used by containers, 
which are ephemeral by design. At least that's my mental model.

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Toke Høiland-Jørgensen]

Jakub Kicinski <kuba@kernel.org> writes:

> On Thu, 13 Feb 2025 17:13:53 +0100 Toke Høiland-Jørgensen wrote:
>> Jakub Kicinski <kuba@kernel.org> writes:
>> 
>> > On Thu, 13 Feb 2025 14:45:22 +0100 Toke Høiland-Jørgensen wrote:  
>> >> Eric suggested[0] allowing user-settable values for dev->perm_addr at
>> >> device creation time, instead of mucking about with netdevsim to get a
>> >> virtual device with a permanent address set.  
>> >
>> > I vote no. Complicating the core so that its easier for someone 
>> > to write a unit test is the wrong engineering trade off.
>> > Use a VM or netdevsim, that's what they are for.  
>> 
>> Hmm, and you don't see any value in being able to specify a permanent
>> identifier for virtual devices? That bit was not just motivated
>> reasoning on my part... :)
>
> I can't think of any :( Specifying an address is already possible.

Right, but the address can be changed later. Setting the perm_addr makes
it possible for a management daemon to set a unique identifier at device
creation time which is guaranteed to persist through any renames and
address changes that other utilities may perform. That seems like a
useful robustness feature that comes at a relatively low cost (the patch
is fairly small and uncomplicated)?

> Permanent address is a property of the hardware platform.
> Virtual devices OTOH are primarily used by containers, 
> which are ephemeral by design. At least that's my mental model.

Sure, any device feature that comes from hardware is only going to fit
virtual devices by analogy. But I don't think the analogy here is super
far fetched (cf the above)? :)

-Toke

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Jakub Kicinski]

On Tue, 18 Feb 2025 13:51:42 +0100 Toke Høiland-Jørgensen wrote:
> >> Hmm, and you don't see any value in being able to specify a permanent
> >> identifier for virtual devices? That bit was not just motivated
> >> reasoning on my part... :)  
> >
> > I can't think of any :( Specifying an address is already possible.  
> 
> Right, but the address can be changed later. Setting the perm_addr makes
> it possible for a management daemon to set a unique identifier at device
> creation time which is guaranteed to persist through any renames and
> address changes that other utilities may perform. That seems like a
> useful robustness feature that comes at a relatively low cost (the patch
> is fairly small and uncomplicated)?
> 
> > Permanent address is a property of the hardware platform.
> > Virtual devices OTOH are primarily used by containers, 
> > which are ephemeral by design. At least that's my mental model.  
> 
> Sure, any device feature that comes from hardware is only going to fit
> virtual devices by analogy. But I don't think the analogy here is super
> far fetched (cf the above)? :)

I'm not sure how to answer this. It all sounds really speculative
and disconnected with how I see virtual devices being used.

Re: [PATCH net-next] rtnetlink: Allow setting IFLA_PERM_ADDRESS at device creation time

[Toke Høiland-Jørgensen]

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 18 Feb 2025 13:51:42 +0100 Toke Høiland-Jørgensen wrote:
>> >> Hmm, and you don't see any value in being able to specify a permanent
>> >> identifier for virtual devices? That bit was not just motivated
>> >> reasoning on my part... :)  
>> >
>> > I can't think of any :( Specifying an address is already possible.  
>> 
>> Right, but the address can be changed later. Setting the perm_addr makes
>> it possible for a management daemon to set a unique identifier at device
>> creation time which is guaranteed to persist through any renames and
>> address changes that other utilities may perform. That seems like a
>> useful robustness feature that comes at a relatively low cost (the patch
>> is fairly small and uncomplicated)?
>> 
>> > Permanent address is a property of the hardware platform.
>> > Virtual devices OTOH are primarily used by containers, 
>> > which are ephemeral by design. At least that's my mental model.  
>> 
>> Sure, any device feature that comes from hardware is only going to fit
>> virtual devices by analogy. But I don't think the analogy here is super
>> far fetched (cf the above)? :)
>
> I'm not sure how to answer this. It all sounds really speculative
> and disconnected with how I see virtual devices being used.


Alright fine - I'll respin the netdevsim patch instead...

-Toke