From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Benc Subject: Re: [PATCHv2 net-next 2/2] openvswitch: add erspan version II support Date: Wed, 10 Jan 2018 22:35:14 +0100 Message-ID: <20180110223514.1e5030cd@redhat.com> References: <1515549082-4141-1-git-send-email-u9012063@gmail.com> <1515549082-4141-3-git-send-email-u9012063@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, Pravin B Shelar To: William Tu Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43607 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753944AbeAJVfR (ORCPT ); Wed, 10 Jan 2018 16:35:17 -0500 In-Reply-To: <1515549082-4141-3-git-send-email-u9012063@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 9 Jan 2018 17:51:22 -0800, William Tu wrote: > - [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = sizeof(u32) }, > + [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV1] = { .len = sizeof(u32) }, > + [OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS] = { .len = OVS_ATTR_NESTED, > + .next = ovs_erspan_opt_lens }, Ouch. It's actually much worse than that, you're redefining the meaning of the field. That's complete no-go. > + case OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTSV1: > + OVS_NLERR(log, "ERSPAN attribute %d is deprecated.", > + type); > + return -EINVAL; As is this. > @@ -906,8 +1017,8 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb, > vxlan_opt_to_nlattr(skb, tun_opts, swkey_tun_opts_len)) > return -EMSGSIZE; > else if (output->tun_flags & TUNNEL_ERSPAN_OPT && > - nla_put_be32(skb, OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS, > - ((struct erspan_metadata *)tun_opts)->u.index)) > + erspan_opt_to_nlattr(skb, tun_opts, > + swkey_tun_opts_len)) And this. The existing field must continue to work in the same way as before. It must be accepted and *returned* by the kernel. You may add an additional field but the existing behavior must be 100% preserved, both uABI and uAPI wise. Jiri