[PATCH] net: allow interface to be set into VRF if VLAN interface in same VRF

[PATCH] net: allow interface to be set into VRF if VLAN interface in same VRF

[Mike Manning]

Setting an interface into a VRF fails with 'RTNETLINK answers: File
exists' if one of its VLAN interfaces is already in the same VRF.
As the VRF is an upper device of the VLAN interface, it is also showing
up as an upper device of the interface itself. The solution is to
restrict this check to devices other than master. As only one master
device can be linked to a device, the check in this case is that the
upper device (VRF) being linked to is not the same as the master device
instead of it not being any one of the upper devices.

The following example shows an interface ens12 (with a VLAN interface
ens12.10) being set into VRF green, which behaves as expected:

  # ip link add link ens12 ens12.10 type vlan id 10
  # ip link set dev ens12 master vrfgreen
  # ip link show dev ens12
    3: ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
       master vrfgreen state UP mode DEFAULT group default qlen 1000
       link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff

But if the VLAN interface has previously been set into the same VRF,
then setting the interface into the VRF fails:

  # ip link set dev ens12 nomaster
  # ip link set dev ens12.10 master vrfgreen
  # ip link show dev ens12.10
    39: ens12.10@ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
    qdisc noqueue master vrfgreen state UP mode DEFAULT group default
    qlen 1000 link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff
  # ip link set dev ens12 master vrfgreen
    RTNETLINK answers: File exists

The workaround is to move the VLAN interface back into the default VRF
beforehand, but it has to be shut first so as to avoid the risk of
traffic leaking from the VRF. This fix avoids needing this workaround.

Signed-off-by: Mike Manning <mmanning@att.com>
---
 net/core/dev.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index d4362be..2cedf52 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6396,6 +6396,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 		.linking = true,
 		.upper_info = upper_info,
 	};
+	struct net_device *master_dev;
 	int ret = 0;
 
 	ASSERT_RTNL();
@@ -6407,11 +6408,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 	if (netdev_has_upper_dev(upper_dev, dev))
 		return -EBUSY;
 
-	if (netdev_has_upper_dev(dev, upper_dev))
-		return -EEXIST;
-
-	if (master && netdev_master_upper_dev_get(dev))
-		return -EBUSY;
+	if (!master) {
+		if (netdev_has_upper_dev(dev, upper_dev))
+			return -EEXIST;
+	} else {
+		master_dev = netdev_master_upper_dev_get(dev);
+		if (master_dev)
+			return master_dev == upper_dev ? -EEXIST : -EBUSY;
+	}
 
 	ret = call_netdevice_notifiers_info(NETDEV_PRECHANGEUPPER,
 					    &changeupper_info.info);
-- 
2.1.4

Re: [PATCH] net: allow interface to be set into VRF if VLAN interface in same VRF

[David Ahern]

On 2/26/18 4:49 PM, Mike Manning wrote:
> Setting an interface into a VRF fails with 'RTNETLINK answers: File
> exists' if one of its VLAN interfaces is already in the same VRF.
> As the VRF is an upper device of the VLAN interface, it is also showing
> up as an upper device of the interface itself. The solution is to
> restrict this check to devices other than master. As only one master
> device can be linked to a device, the check in this case is that the
> upper device (VRF) being linked to is not the same as the master device
> instead of it not being any one of the upper devices.
> 
> The following example shows an interface ens12 (with a VLAN interface
> ens12.10) being set into VRF green, which behaves as expected:
> 
>   # ip link add link ens12 ens12.10 type vlan id 10
>   # ip link set dev ens12 master vrfgreen
>   # ip link show dev ens12
>     3: ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
>        master vrfgreen state UP mode DEFAULT group default qlen 1000
>        link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff
> 
> But if the VLAN interface has previously been set into the same VRF,
> then setting the interface into the VRF fails:
> 
>   # ip link set dev ens12 nomaster
>   # ip link set dev ens12.10 master vrfgreen
>   # ip link show dev ens12.10
>     39: ens12.10@ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>     qdisc noqueue master vrfgreen state UP mode DEFAULT group default
>     qlen 1000 link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff
>   # ip link set dev ens12 master vrfgreen
>     RTNETLINK answers: File exists
> 
> The workaround is to move the VLAN interface back into the default VRF
> beforehand, but it has to be shut first so as to avoid the risk of
> traffic leaking from the VRF. This fix avoids needing this workaround.
> 
> Signed-off-by: Mike Manning <mmanning@att.com>
> ---
>  net/core/dev.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d4362be..2cedf52 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6396,6 +6396,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
>  		.linking = true,
>  		.upper_info = upper_info,
>  	};
> +	struct net_device *master_dev;
>  	int ret = 0;
>  
>  	ASSERT_RTNL();
> @@ -6407,11 +6408,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
>  	if (netdev_has_upper_dev(upper_dev, dev))
>  		return -EBUSY;
>  
> -	if (netdev_has_upper_dev(dev, upper_dev))
> -		return -EEXIST;
> -
> -	if (master && netdev_master_upper_dev_get(dev))
> -		return -EBUSY;
> +	if (!master) {
> +		if (netdev_has_upper_dev(dev, upper_dev))
> +			return -EEXIST;
> +	} else {
> +		master_dev = netdev_master_upper_dev_get(dev);
> +		if (master_dev)
> +			return master_dev == upper_dev ? -EEXIST : -EBUSY;
> +	}
>  
>  	ret = call_netdevice_notifiers_info(NETDEV_PRECHANGEUPPER,
>  					    &changeupper_info.info);
> 

This looks ok to me. Adding a few others to double check.

Re: [PATCH] net: allow interface to be set into VRF if VLAN interface in same VRF

[David Miller]

From: Mike Manning <mmanning@vyatta.mail-att.com>
Date: Mon, 26 Feb 2018 23:49:30 +0000

> Setting an interface into a VRF fails with 'RTNETLINK answers: File
> exists' if one of its VLAN interfaces is already in the same VRF.
> As the VRF is an upper device of the VLAN interface, it is also showing
> up as an upper device of the interface itself. The solution is to
> restrict this check to devices other than master. As only one master
> device can be linked to a device, the check in this case is that the
> upper device (VRF) being linked to is not the same as the master device
> instead of it not being any one of the upper devices.
> 
> The following example shows an interface ens12 (with a VLAN interface
> ens12.10) being set into VRF green, which behaves as expected:
> 
>   # ip link add link ens12 ens12.10 type vlan id 10
>   # ip link set dev ens12 master vrfgreen
>   # ip link show dev ens12
>     3: ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
>        master vrfgreen state UP mode DEFAULT group default qlen 1000
>        link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff
> 
> But if the VLAN interface has previously been set into the same VRF,
> then setting the interface into the VRF fails:
> 
>   # ip link set dev ens12 nomaster
>   # ip link set dev ens12.10 master vrfgreen
>   # ip link show dev ens12.10
>     39: ens12.10@ens12: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500
>     qdisc noqueue master vrfgreen state UP mode DEFAULT group default
>     qlen 1000 link/ether 52:54:00:4c:a0:45 brd ff:ff:ff:ff:ff:ff
>   # ip link set dev ens12 master vrfgreen
>     RTNETLINK answers: File exists
> 
> The workaround is to move the VLAN interface back into the default VRF
> beforehand, but it has to be shut first so as to avoid the risk of
> traffic leaking from the VRF. This fix avoids needing this workaround.
> 
> Signed-off-by: Mike Manning <mmanning@att.com>

Applied, thanks Mike.