From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-20.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85C7DC04FF3 for ; Mon, 24 May 2021 19:34:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4DE1D61402 for ; Mon, 24 May 2021 19:34:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233599AbhEXTgP (ORCPT ); Mon, 24 May 2021 15:36:15 -0400 Received: from mslow1.mail.gandi.net ([217.70.178.240]:60321 "EHLO mslow1.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233070AbhEXTgP (ORCPT ); Mon, 24 May 2021 15:36:15 -0400 Received: from relay6-d.mail.gandi.net (unknown [217.70.183.198]) by mslow1.mail.gandi.net (Postfix) with ESMTP id 93055C87F2 for ; Mon, 24 May 2021 19:32:21 +0000 (UTC) Received: (Authenticated sender: i.maximets@ovn.org) by relay6-d.mail.gandi.net (Postfix) with ESMTPSA id 72CA7C0002; Mon, 24 May 2021 19:31:58 +0000 (UTC) Subject: Re: [PATCH net-next v2] net: openvswitch: IPv6: Add IPv6 extension header support To: Cpp Code , Ilya Maximets Cc: netdev@vger.kernel.org, "pshelar@ovn.org" , "David S. Miller" , ovs dev , Jakub Kicinski , Ben Pfaff References: <20210517152051.35233-1-cpp.code.lv@gmail.com> <614d9840-cd9d-d8b1-0d88-ce07e409068d@ovn.org> From: Ilya Maximets Message-ID: <3b10f4e0-c660-a7ed-bcd5-64cc1a23f19a@ovn.org> Date: Mon, 24 May 2021 21:31:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 5/24/21 8:57 PM, Cpp Code wrote: > Yes, these changes only works together with changes in userspace. I > believe in any solution there should be corresponding changes in > userspace. If we would be able to easily run old version of userspace > with these changes in kernel without userspace complaining about > struct size, we could get in to a situation with hard to find bugs. You can't modify existing binaries and we can't expect that everyone will get new version of OVS along with the kernel update. Existing binaries should work fine with any version of underlying kernel. With this change applied, kernel will not be able to parse OVS_KEY_ATTR_IPV6 sent from userspace by the older OVS and this OVS will not be able to correctly parse netlink messages sent by the kernel. > > I don't agree with the solution of a new struct key as semantically > ipv6 extension headers are integral part of every ipv6 packet thus > expected to be in the struct along with label, for example. Correct if > I am missing something. Even though ipv6 extensions are part of ipv6, they never was part of the user interface here. I agree that original design of this structure was not perfect, but breaking of the user interface, i.e. breaking all the existing OVS binaries, is just not acceptable. > > On Wed, May 19, 2021 at 2:52 AM Ilya Maximets wrote: >> >> On 5/17/21 5:20 PM, Toms Atteka wrote: >>> IPv6 extension headers carry optional internet layer information >>> and are placed between the fixed header and the upper-layer >>> protocol header. >>> >>> This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and >>> packets can be filtered using ipv6_ext flag. >>> >>> Tested-at: https://github.com/TomCodeLV/ovs/actions/runs/504185214 >>> Signed-off-by: Toms Atteka >>> --- >>> include/uapi/linux/openvswitch.h | 1 + >>> net/openvswitch/flow.c | 141 +++++++++++++++++++++++++++++++ >>> net/openvswitch/flow.h | 14 +++ >>> net/openvswitch/flow_netlink.c | 5 +- >>> 4 files changed, 160 insertions(+), 1 deletion(-) >>> >>> >>> base-commit: 5d869070569a23aa909c6e7e9d010fc438a492ef >>> >>> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h >>> index 8d16744edc31..a19812b6631a 100644 >>> --- a/include/uapi/linux/openvswitch.h >>> +++ b/include/uapi/linux/openvswitch.h >>> @@ -420,6 +420,7 @@ struct ovs_key_ipv6 { >>> __u8 ipv6_tclass; >>> __u8 ipv6_hlimit; >>> __u8 ipv6_frag; /* One of OVS_FRAG_TYPE_*. */ >>> + __u16 ipv6_exthdr; >>> }; >> >> Wouldn't this break existing userspace? Curent OVS expects netlink >> message with attribute size equal to the old version of 'struct ovs_key_ipv6' >> and it will discard OVS_KEY_ATTR_IPV6 as malformed. >> >> This should likely be a completely new structure and a completely new >> OVS_KEY_ATTR. >> >> Best regards, Ilya Maximets.