Set the perm_addr of veth devices to whatever MAC has been assigned to
the device. Otherwise, it remains all zero, with the consequence that
ipv6_generate_stable_address() (which is used if the sysctl
net.ipv6.conf.DEV.addr_gen_mode is set to 2 or 3) assigns every veth
interface on a host the same link-local address.
The new behaviour matches that of several other virtual interface types
(such as gre), and as far as I can tell, perm_addr isn't used by any
other code sites that are relevant to veth.
Signed-off-by: Mira Ressel <[email protected]>
---
drivers/net/veth.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index e56cd562a664..af1a7cda6205 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -1342,6 +1342,8 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
if (!ifmp || !tbp[IFLA_ADDRESS])
eth_hw_addr_random(peer);
+ memcpy(peer->perm_addr, peer->dev_addr, peer->addr_len);
+
if (ifmp && (dev->ifindex != 0))
peer->ifindex = ifmp->ifi_index;
@@ -1370,6 +1372,8 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
if (tb[IFLA_ADDRESS] == NULL)
eth_hw_addr_random(dev);
+ memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);
+
if (tb[IFLA_IFNAME])
nla_strlcpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ);
else
--
2.25.4
Set the perm_addr of vlan devices to that of their parent device.
Otherwise, it remains all zero, with the consequence that
ipv6_generate_stable_address() (which is used if the sysctl
net.ipv6.conf.DEV.addr_gen_mode is set to 2 or 3) assigns every vlan
interface on a host the same link-local address.
This has the added benefit of giving vlan devices the same link-local
address as their parent device, which is common practice, and indeed
precisely what happens automatically if the default eui64-based address
generation is used.
Signed-off-by: Mira Ressel <[email protected]>
---
net/8021q/vlan_netlink.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 0db85aeb119b..8c60d92b7717 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -182,6 +182,8 @@ static int vlan_newlink(struct net *src_net, struct net_device *dev,
else if (dev->mtu > max_mtu)
return -EINVAL;
+ memcpy(dev->perm_addr, real_dev->perm_addr, real_dev->addr_len);
+
err = vlan_changelink(dev, tb, data, extack);
if (!err)
err = register_vlan_dev(dev, extack);
--
2.25.4
From: Mira Ressel <[email protected]>
Date: Mon, 24 Aug 2020 14:38:26 +0000
> Set the perm_addr of veth devices to whatever MAC has been assigned to
> the device. Otherwise, it remains all zero, with the consequence that
> ipv6_generate_stable_address() (which is used if the sysctl
> net.ipv6.conf.DEV.addr_gen_mode is set to 2 or 3) assigns every veth
> interface on a host the same link-local address.
>
> The new behaviour matches that of several other virtual interface types
> (such as gre), and as far as I can tell, perm_addr isn't used by any
> other code sites that are relevant to veth.
>
> Signed-off-by: Mira Ressel <[email protected]>
...
> @@ -1342,6 +1342,8 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
> if (!ifmp || !tbp[IFLA_ADDRESS])
> eth_hw_addr_random(peer);
>
> + memcpy(peer->perm_addr, peer->dev_addr, peer->addr_len);
Semantically don't you want to copy over the peer->perm_addr?
Otherwise this loses the entire point of the permanent address, and
what the ipv6 address generation facility expects.
On Mon, Aug 24, 2020 at 10:25:45AM -0700, David Miller wrote:
> From: Mira Ressel <[email protected]>
> Date: Mon, 24 Aug 2020 14:38:26 +0000
>
> > Set the perm_addr of veth devices to whatever MAC has been assigned to
> > the device. Otherwise, it remains all zero, with the consequence that
> > ipv6_generate_stable_address() (which is used if the sysctl
> > net.ipv6.conf.DEV.addr_gen_mode is set to 2 or 3) assigns every veth
> > interface on a host the same link-local address.
> >
> > The new behaviour matches that of several other virtual interface types
> > (such as gre), and as far as I can tell, perm_addr isn't used by any
> > other code sites that are relevant to veth.
> >
> > Signed-off-by: Mira Ressel <[email protected]>
> ...
> > @@ -1342,6 +1342,8 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
> > if (!ifmp || !tbp[IFLA_ADDRESS])
> > eth_hw_addr_random(peer);
> >
> > + memcpy(peer->perm_addr, peer->dev_addr, peer->addr_len);
>
> Semantically don't you want to copy over the peer->perm_addr?
>
> Otherwise this loses the entire point of the permanent address, and
> what the ipv6 address generation facility expects.
I'm confused. Am I misinterpreting what you're saying here, or did you
make a typo?
I'm setting the peer->perm_addr, which would otherwise be zero, to its
dev_addr, which has been either generated randomly by the kernel or
provided by userland in a netlink attribute.
--
Regards,
Mira
From: Mira Ressel <[email protected]>
Date: Wed, 26 Aug 2020 15:20:00 +0000
> I'm setting the peer->perm_addr, which would otherwise be zero, to its
> dev_addr, which has been either generated randomly by the kernel or
> provided by userland in a netlink attribute.
Which by definition makes it not necessarily a "permanent address" and
therefore is subject to being different across boots, which is exactly
what you don't want to happen for automatic address generation.
On Wed, Aug 26, 2020 at 08:28:57AM -0700, David Miller wrote:
> From: Mira Ressel <[email protected]>
> Date: Wed, 26 Aug 2020 15:20:00 +0000
>
> > I'm setting the peer->perm_addr, which would otherwise be zero, to its
> > dev_addr, which has been either generated randomly by the kernel or
> > provided by userland in a netlink attribute.
>
> Which by definition makes it not necessarily a "permanent address" and
> therefore is subject to being different across boots, which is exactly
> what you don't want to happen for automatic address generation.
That's true, but since veth devices aren't backed by any hardware, I
unfortunately don't have a good source for a permanent address. The only
inherently permanent thing about them is their name.
People who use the default eui64-based address generation don't get
persistent link-local addresses for their veth devices out of the box
either -- the EUI64 is derived from the device's dev_addr, which is
randomized by default.
If that presents a problem for anyone, they can configure their userland
to set the dev_addr to a static value, which handily fixes this problem
for both address generation algorithms.
I'm admittedly glancing over one problem here -- I'm only setting the
perm_addr during device creation, whereas userland can change the
dev_addr at any time. I'm not sure if it'd make sense here to update the
perm_addr if the dev_addr is changed later on?
--
Regards,
Mira
From: Mira Ressel <[email protected]>
Date: Wed, 26 Aug 2020 16:29:01 +0000
> On Wed, Aug 26, 2020 at 08:28:57AM -0700, David Miller wrote:
>> From: Mira Ressel <[email protected]>
>> Date: Wed, 26 Aug 2020 15:20:00 +0000
>>
>> > I'm setting the peer->perm_addr, which would otherwise be zero, to its
>> > dev_addr, which has been either generated randomly by the kernel or
>> > provided by userland in a netlink attribute.
>>
>> Which by definition makes it not necessarily a "permanent address" and
>> therefore is subject to being different across boots, which is exactly
>> what you don't want to happen for automatic address generation.
>
> That's true, but since veth devices aren't backed by any hardware, I
> unfortunately don't have a good source for a permanent address. The only
> inherently permanent thing about them is their name.
>
> People who use the default eui64-based address generation don't get
> persistent link-local addresses for their veth devices out of the box
> either -- the EUI64 is derived from the device's dev_addr, which is
> randomized by default.
>
> If that presents a problem for anyone, they can configure their userland
> to set the dev_addr to a static value, which handily fixes this problem
> for both address generation algorithms.
>
> I'm admittedly glancing over one problem here -- I'm only setting the
> perm_addr during device creation, whereas userland can change the
> dev_addr at any time. I'm not sure if it'd make sense here to update the
> perm_addr if the dev_addr is changed later on?
We are talking about which parent device address to inherit from, you
have choosen to use dev_addr and I am saying you should use perm_addr.
Can you explain why this isn't clear?
On Wed, Aug 26, 2020 at 09:33:29AM -0700, David Miller wrote:
> From: Mira Ressel <[email protected]>
> Date: Wed, 26 Aug 2020 16:29:01 +0000
>
> > On Wed, Aug 26, 2020 at 08:28:57AM -0700, David Miller wrote:
> >> From: Mira Ressel <[email protected]>
> >> Date: Wed, 26 Aug 2020 15:20:00 +0000
> >>
> >> > I'm setting the peer->perm_addr, which would otherwise be zero, to its
> >> > dev_addr, which has been either generated randomly by the kernel or
> >> > provided by userland in a netlink attribute.
> >>
> >> Which by definition makes it not necessarily a "permanent address" and
> >> therefore is subject to being different across boots, which is exactly
> >> what you don't want to happen for automatic address generation.
> >
> > That's true, but since veth devices aren't backed by any hardware, I
> > unfortunately don't have a good source for a permanent address. The only
> > inherently permanent thing about them is their name.
> >
> > People who use the default eui64-based address generation don't get
> > persistent link-local addresses for their veth devices out of the box
> > either -- the EUI64 is derived from the device's dev_addr, which is
> > randomized by default.
> >
> > If that presents a problem for anyone, they can configure their userland
> > to set the dev_addr to a static value, which handily fixes this problem
> > for both address generation algorithms.
> >
> > I'm admittedly glancing over one problem here -- I'm only setting the
> > perm_addr during device creation, whereas userland can change the
> > dev_addr at any time. I'm not sure if it'd make sense here to update the
> > perm_addr if the dev_addr is changed later on?
>
> We are talking about which parent device address to inherit from, you
> have choosen to use dev_addr and I am saying you should use perm_addr.
>
> Can you explain why this isn't clear?
Which parent device? This is the veth patch, not the vlan patch. I'm
setting the perm_addrs of both sides of the veth pair to their
respective dev_addrs that they were assigned by userland or through
random generation. If I were to give both of them the same perm_addr,
we'd again have the problem that they'd get conflicting link-local
addresses and trigger DAD.
My apologies if I'm misunderstanding something here!
Regards,
Mira