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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Apr 2019 23:13:14 +0300
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Mike Manning <mmanning@...tta.att-mail.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/4] vlan: support binding link state to vlan
 member bridge ports

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@...tta.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;
>>  		}
>>
>