diff mbox series

[net-next,1/4] vlan: support binding link state to vlan member bridge ports

Message ID 20190402153543.6277-2-mmanning@vyatta.att-mail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series net: support binding vlan dev link state to vlan member bridge ports | expand

Commit Message

Mike Manning April 2, 2019, 3:35 p.m. UTC
In the case of vlan filtering on bridges, the bridge may also have the
corresponding vlan devices as upper devices. Currently the link state
of vlan devices is transferred from the lower device. So this is up if
the bridge is in admin up state and there is at least one bridge port
that is up, regardless of the vlan that the port is a member of.

The link state of the vlan device may need to track only the state of
the subset of ports that are also members of the corresponding vlan,
rather than that of all ports.

Add a flag to specify a vlan bridge binding mode, by which the link
state is no longer automatically transferred from the lower device,
but is instead determined by the bridge ports that are members of the
vlan.

Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
---
 include/uapi/linux/if_vlan.h | 9 +++++----
 net/8021q/vlan_dev.c         | 3 ++-
 net/8021q/vlan_netlink.c     | 3 ++-
 3 files changed, 9 insertions(+), 6 deletions(-)

Comments

Nikolay Aleksandrov April 2, 2019, 7:20 p.m. UTC | #1
On 02/04/2019 18:35, Mike Manning wrote:
> In the case of vlan filtering on bridges, the bridge may also have the
> corresponding vlan devices as upper devices. Currently the link state
> of vlan devices is transferred from the lower device. So this is up if
> the bridge is in admin up state and there is at least one bridge port
> that is up, regardless of the vlan that the port is a member of.
> 
> The link state of the vlan device may need to track only the state of
> the subset of ports that are also members of the corresponding vlan,
> rather than that of all ports.
> 
> Add a flag to specify a vlan bridge binding mode, by which the link
> state is no longer automatically transferred from the lower device,
> but is instead determined by the bridge ports that are members of the
> vlan.
> 
> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> ---
>  include/uapi/linux/if_vlan.h | 9 +++++----
>  net/8021q/vlan_dev.c         | 3 ++-
>  net/8021q/vlan_netlink.c     | 3 ++-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 

What if the following happens:
1. add binding vlan X on bridge
2. no up vlan ports for X
3. set no carrier on vlan X
4. remove binding flag on X 

Would vlan X remain in no carrier state ?

> diff --git a/include/uapi/linux/if_vlan.h b/include/uapi/linux/if_vlan.h
> index 7a0e8bd65b6b..601931ac8002 100644
> --- a/include/uapi/linux/if_vlan.h
> +++ b/include/uapi/linux/if_vlan.h
> @@ -32,10 +32,11 @@ enum vlan_ioctl_cmds {
>  };
>  
>  enum vlan_flags {
> -	VLAN_FLAG_REORDER_HDR	= 0x1,
> -	VLAN_FLAG_GVRP		= 0x2,
> -	VLAN_FLAG_LOOSE_BINDING	= 0x4,
> -	VLAN_FLAG_MVRP		= 0x8,
> +	VLAN_FLAG_REORDER_HDR		= 0x1,
> +	VLAN_FLAG_GVRP			= 0x2,
> +	VLAN_FLAG_LOOSE_BINDING		= 0x4,
> +	VLAN_FLAG_MVRP			= 0x8,
> +	VLAN_FLAG_BRIDGE_BINDING	= 0x16,
>  };
>  
>  enum vlan_name_types {
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 15293c2a5dd8..86b38bb87f9a 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -223,7 +223,8 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
>  	u32 old_flags = vlan->flags;
>  
>  	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
> -		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP))
> +		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
> +		     VLAN_FLAG_BRIDGE_BINDING))
>  		return -EINVAL;
>  
>  	vlan->flags = (old_flags & ~mask) | (flags & mask);
> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index 9b60c1e399e2..a624dccf68fd 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -84,7 +84,8 @@ static int vlan_validate(struct nlattr *tb[], struct nlattr *data[],
>  		flags = nla_data(data[IFLA_VLAN_FLAGS]);
>  		if ((flags->flags & flags->mask) &
>  		    ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
> -		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP)) {
> +		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
> +		      VLAN_FLAG_BRIDGE_BINDING)) {
>  			NL_SET_ERR_MSG_MOD(extack, "Invalid VLAN flags");
>  			return -EINVAL;
>  		}
>
Nikolay Aleksandrov April 2, 2019, 8:13 p.m. UTC | #2
On 02/04/2019 22:20, Nikolay Aleksandrov wrote:
> On 02/04/2019 18:35, Mike Manning wrote:
>> In the case of vlan filtering on bridges, the bridge may also have the
>> corresponding vlan devices as upper devices. Currently the link state
>> of vlan devices is transferred from the lower device. So this is up if
>> the bridge is in admin up state and there is at least one bridge port
>> that is up, regardless of the vlan that the port is a member of.
>>
>> The link state of the vlan device may need to track only the state of
>> the subset of ports that are also members of the corresponding vlan,
>> rather than that of all ports.
>>
>> Add a flag to specify a vlan bridge binding mode, by which the link
>> state is no longer automatically transferred from the lower device,
>> but is instead determined by the bridge ports that are members of the
>> vlan.
>>
>> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
>> ---
>>  include/uapi/linux/if_vlan.h | 9 +++++----
>>  net/8021q/vlan_dev.c         | 3 ++-
>>  net/8021q/vlan_netlink.c     | 3 ++-
>>  3 files changed, 9 insertions(+), 6 deletions(-)
>>
> 
> What if the following happens:
> 1. add binding vlan X on bridge
> 2. no up vlan ports for X
> 3. set no carrier on vlan X
> 4. remove binding flag on X 
> 
> Would vlan X remain in no carrier state ?
> 

Ah patch 02 arrived much later, I think this should be fine. :)

>> diff --git a/include/uapi/linux/if_vlan.h b/include/uapi/linux/if_vlan.h
>> index 7a0e8bd65b6b..601931ac8002 100644
>> --- a/include/uapi/linux/if_vlan.h
>> +++ b/include/uapi/linux/if_vlan.h
>> @@ -32,10 +32,11 @@ enum vlan_ioctl_cmds {
>>  };
>>  
>>  enum vlan_flags {
>> -	VLAN_FLAG_REORDER_HDR	= 0x1,
>> -	VLAN_FLAG_GVRP		= 0x2,
>> -	VLAN_FLAG_LOOSE_BINDING	= 0x4,
>> -	VLAN_FLAG_MVRP		= 0x8,
>> +	VLAN_FLAG_REORDER_HDR		= 0x1,
>> +	VLAN_FLAG_GVRP			= 0x2,
>> +	VLAN_FLAG_LOOSE_BINDING		= 0x4,
>> +	VLAN_FLAG_MVRP			= 0x8,
>> +	VLAN_FLAG_BRIDGE_BINDING	= 0x16,
>>  };
>>  
>>  enum vlan_name_types {
>> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>> index 15293c2a5dd8..86b38bb87f9a 100644
>> --- a/net/8021q/vlan_dev.c
>> +++ b/net/8021q/vlan_dev.c
>> @@ -223,7 +223,8 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
>>  	u32 old_flags = vlan->flags;
>>  
>>  	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
>> -		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP))
>> +		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
>> +		     VLAN_FLAG_BRIDGE_BINDING))
>>  		return -EINVAL;
>>  
>>  	vlan->flags = (old_flags & ~mask) | (flags & mask);
>> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
>> index 9b60c1e399e2..a624dccf68fd 100644
>> --- a/net/8021q/vlan_netlink.c
>> +++ b/net/8021q/vlan_netlink.c
>> @@ -84,7 +84,8 @@ static int vlan_validate(struct nlattr *tb[], struct nlattr *data[],
>>  		flags = nla_data(data[IFLA_VLAN_FLAGS]);
>>  		if ((flags->flags & flags->mask) &
>>  		    ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
>> -		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP)) {
>> +		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
>> +		      VLAN_FLAG_BRIDGE_BINDING)) {
>>  			NL_SET_ERR_MSG_MOD(extack, "Invalid VLAN flags");
>>  			return -EINVAL;
>>  		}
>>
>
diff mbox series

Patch

diff --git a/include/uapi/linux/if_vlan.h b/include/uapi/linux/if_vlan.h
index 7a0e8bd65b6b..601931ac8002 100644
--- a/include/uapi/linux/if_vlan.h
+++ b/include/uapi/linux/if_vlan.h
@@ -32,10 +32,11 @@  enum vlan_ioctl_cmds {
 };
 
 enum vlan_flags {
-	VLAN_FLAG_REORDER_HDR	= 0x1,
-	VLAN_FLAG_GVRP		= 0x2,
-	VLAN_FLAG_LOOSE_BINDING	= 0x4,
-	VLAN_FLAG_MVRP		= 0x8,
+	VLAN_FLAG_REORDER_HDR		= 0x1,
+	VLAN_FLAG_GVRP			= 0x2,
+	VLAN_FLAG_LOOSE_BINDING		= 0x4,
+	VLAN_FLAG_MVRP			= 0x8,
+	VLAN_FLAG_BRIDGE_BINDING	= 0x16,
 };
 
 enum vlan_name_types {
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 15293c2a5dd8..86b38bb87f9a 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -223,7 +223,8 @@  int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
 	u32 old_flags = vlan->flags;
 
 	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
-		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP))
+		     VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
+		     VLAN_FLAG_BRIDGE_BINDING))
 		return -EINVAL;
 
 	vlan->flags = (old_flags & ~mask) | (flags & mask);
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 9b60c1e399e2..a624dccf68fd 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -84,7 +84,8 @@  static int vlan_validate(struct nlattr *tb[], struct nlattr *data[],
 		flags = nla_data(data[IFLA_VLAN_FLAGS]);
 		if ((flags->flags & flags->mask) &
 		    ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
-		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP)) {
+		      VLAN_FLAG_LOOSE_BINDING | VLAN_FLAG_MVRP |
+		      VLAN_FLAG_BRIDGE_BINDING)) {
 			NL_SET_ERR_MSG_MOD(extack, "Invalid VLAN flags");
 			return -EINVAL;
 		}