From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ido Schimmel Subject: Re: [PATCH v3,net-next] vlan: implement vlan id and protocol changes Date: Tue, 26 Jun 2018 13:32:16 +0300 Message-ID: <20180626103216.GA11722@splinter.mtl.com> References: <20180610231912.1543-1-3chas3@gmail.com> <20180625103043.25384-1-3chas3@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Chas Williams <3chas3@gmail.com>, davem@davemloft.net, netdev@vger.kernel.org, Roopa Prabhu , Ido Schimmel To: David Ahern Return-path: Received: from out2-smtp.messagingengine.com ([66.111.4.26]:47745 "EHLO out2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933509AbeFZKcT (ORCPT ); Tue, 26 Jun 2018 06:32:19 -0400 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Mon, Jun 25, 2018 at 02:45:24PM -0600, David Ahern wrote: > 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@gmail.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. +1 > > 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. Yes, at minimum we need to return an error for NETDEV_CHANGEVLAN, but looking at the code it seems that there's no proper rollback. Thanks for the Cc, David.