netdev - Re: [PATCH v3,net-next] vlan: implement vlan id and protocol changes

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c39f97c6-7ae2-e66f-2226-acbe5c1352d2@cumulusnetworks.com>
Date:   Mon, 25 Jun 2018 14:45:24 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Chas Williams <3chas3@...il.com>, davem@...emloft.net
Cc:     netdev@...r.kernel.org, Roopa Prabhu <roopa@...ulusnetworks.com>,
        Ido Schimmel <idosch@...lanox.com>
Subject: Re: [PATCH v3,net-next] vlan: implement vlan id and protocol changes

On 6/25/18 4:30 AM, Chas Williams wrote:
> vlan_changelink silently ignores attempts to change the vlan id
> or protocol id of an existing vlan interface.  Implement by adding
> the new vlan id and protocol to the interface's vlan group and then
> removing the old vlan id and protocol from the vlan group.
> 
> Signed-off-by: Chas Williams <3chas3@...il.com>
> ---
>  include/linux/netdevice.h |  1 +
>  net/8021q/vlan.c          |  4 ++--
>  net/8021q/vlan.h          |  2 ++
>  net/8021q/vlan_netlink.c  | 38 ++++++++++++++++++++++++++++++++++++++
>  net/core/dev.c            |  1 +
>  5 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 3ec9850c7936..a95ae238addf 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2409,6 +2409,7 @@ enum netdev_cmd {
>  	NETDEV_CVLAN_FILTER_DROP_INFO,
>  	NETDEV_SVLAN_FILTER_PUSH_INFO,
>  	NETDEV_SVLAN_FILTER_DROP_INFO,
> +	NETDEV_CHANGEVLAN,
>  };
>  const char *netdev_cmd_to_name(enum netdev_cmd cmd);
>  

you add the new notifier, but do not add any hooks to catch and process it.

Personally, I think it is a bit sketchy to change the vlan id on an
existing device and I suspect it will cause latent errors.

What's your use case for trying to implement the change versus causing
it to generate an unsupported error?

If this patch does get accepted, I believe the mlxsw switchdev driver
will be impacted.


> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index 73a65789271b..b5e0ad1a581a 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -51,8 +51,8 @@ const char vlan_version[] = DRV_VERSION;
>  
>  /* End of global variables definitions. */
>  
> -static int vlan_group_prealloc_vid(struct vlan_group *vg,
> -				   __be16 vlan_proto, u16 vlan_id)
> +int vlan_group_prealloc_vid(struct vlan_group *vg,
> +			    __be16 vlan_proto, u16 vlan_id)
>  {
>  	struct net_device **array;
>  	unsigned int pidx, vidx;
> diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
> index 44df1c3df02d..c734dd21d70d 100644
> --- a/net/8021q/vlan.h
> +++ b/net/8021q/vlan.h
> @@ -116,6 +116,8 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack);
>  void unregister_vlan_dev(struct net_device *dev, struct list_head *head);
>  bool vlan_dev_inherit_address(struct net_device *dev,
>  			      struct net_device *real_dev);
> +int vlan_group_prealloc_vid(struct vlan_group *vg,
> +			    __be16 vlan_proto, u16 vlan_id);
>  
>  static inline u32 vlan_get_ingress_priority(struct net_device *dev,
>  					    u16 vlan_tci)
> diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
> index 9b60c1e399e2..ee27781939e0 100644
> --- a/net/8021q/vlan_netlink.c
> +++ b/net/8021q/vlan_netlink.c
> @@ -107,10 +107,48 @@ static int vlan_changelink(struct net_device *dev, struct nlattr *tb[],
>  			   struct nlattr *data[],
>  			   struct netlink_ext_ack *extack)
>  {
> +	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>  	struct ifla_vlan_flags *flags;
>  	struct ifla_vlan_qos_mapping *m;
>  	struct nlattr *attr;
>  	int rem;
> +	__be16 vlan_proto = vlan->vlan_proto;
> +	u16 vlan_id = vlan->vlan_id;
> +
> +	if (data[IFLA_VLAN_ID])
> +		vlan_id = nla_get_u16(data[IFLA_VLAN_ID]);
> +
> +	if (data[IFLA_VLAN_PROTOCOL])
> +		vlan_proto = nla_get_be16(data[IFLA_VLAN_PROTOCOL]);
> +
> +	if (vlan->vlan_id != vlan_id || vlan->vlan_proto != vlan_proto) {
> +		struct net_device *real_dev = vlan->real_dev;
> +		struct vlan_info *vlan_info;
> +		struct vlan_group *grp;
> +		__be16 old_vlan_proto = vlan->vlan_proto;
> +		u16 old_vlan_id = vlan->vlan_id;
> +		int err;
> +
> +		err = vlan_vid_add(real_dev, vlan_proto, vlan_id);
> +		if (err)
> +			return err;
> +		vlan_info = rtnl_dereference(real_dev->vlan_info);
> +		grp = &vlan_info->grp;
> +		err = vlan_group_prealloc_vid(grp, vlan_proto, vlan_id);
> +		if (err < 0) {
> +			vlan_vid_del(real_dev, vlan_proto, vlan_id);
> +			return err;
> +		}
> +		vlan_group_set_device(grp, vlan_proto, vlan_id, dev);
> +		vlan->vlan_proto = vlan_proto;
> +		vlan->vlan_id = vlan_id;
> +
> +		vlan_group_set_device(grp, old_vlan_proto, old_vlan_id, NULL);
> +		vlan_vid_del(real_dev, old_vlan_proto, old_vlan_id);
> +
> +		err = call_netdevice_notifiers(NETDEV_CHANGEVLAN, dev);
> +		notifier_to_errno(err);
> +	}
>  
>  	if (data[IFLA_VLAN_FLAGS]) {
>  		flags = nla_data(data[IFLA_VLAN_FLAGS]);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a5aa1c7444e6..4bda5d2e3c85 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1587,6 +1587,7 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
>  	N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
>  	N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
>  	N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
> +	N(CHANGEVLAN)
>  	}
>  #undef N
>  	return "UNKNOWN_NETDEV_EVENT";
>