From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0069.outbound.protection.outlook.com [104.47.1.69]) by dpdk.org (Postfix) with ESMTP id A16F558C4 for ; Fri, 26 Oct 2018 10:39:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=E1Js5kLdAUQKg5xpO5Jy0yOg5YiETLieF6UzKztRU+U=; b=tx34oJ2iPeze8BvtOJXOs+/OJ3gbRonYFoltIysB0IS/QavmMW7PPdiP6/cu86VYtFoyiKqv98O3xzyV3Z0vTCBiTG0ss4OpziRyq4dfWGDGZF/X5T+qU0pO7z3HPKG+nDvuFWVoa2iWr8XDTfyGiBE5SBLp+rrQJ4+jOQJqXK8= Received: from AM4PR05MB3265.eurprd05.prod.outlook.com (10.171.186.150) by AM4PR05MB3393.eurprd05.prod.outlook.com (10.171.187.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1273.19; Fri, 26 Oct 2018 08:39:38 +0000 Received: from AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::544b:a68d:e6a5:ba6e]) by AM4PR05MB3265.eurprd05.prod.outlook.com ([fe80::544b:a68d:e6a5:ba6e%2]) with mapi id 15.20.1273.025; Fri, 26 Oct 2018 08:39:38 +0000 From: Slava Ovsiienko To: Yongseok Koh CC: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH v2 2/7] net/mlx5: e-switch VXLAN flow validation routine Thread-Index: AQHUZJFYi3kRmdGok0OzngUkDwlPjaUspnMAgANTC8CAAO9tAIAAVSbQ Date: Fri, 26 Oct 2018 08:39:38 +0000 Message-ID: References: <1538461807-37507-1-git-send-email-viacheslavo@mellanox.com> <1539612815-47199-1-git-send-email-viacheslavo@mellanox.com> <1539612815-47199-3-git-send-email-viacheslavo@mellanox.com> <20181023100424.GB14792@mtidpdk.mti.labs.mlnx> <20181026030719.GB6434@mtidpdk.mti.labs.mlnx> In-Reply-To: <20181026030719.GB6434@mtidpdk.mti.labs.mlnx> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=viacheslavo@mellanox.com; x-originating-ip: [95.67.35.250] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR05MB3393; 6:1PIrCQzdtjOFLrQE2i0GwrO2d2TBpX6EIqg2POeDRbUclJ72+r/+B2B9f/OVyHA7sheDbWSVnsieY/NaBXZli5lYd4bNw2w99leaOKrcqOcnVLedeC8yq8WeOYKShyR9otlauU5mZTXBWlHTwQz3+3jgmIIySJJAeQ/+m0vcmogpe/imnjcE1JGEiIeJimx/wHYsYFCmlIEQfFkFsWy7639hJHtgAwRz16owaDxfM5ZT0i3s+qHdG8XSMotsC1VSYOiGD3qbKxyNX3grFYb2X+v2AFRFYTuyDyZhRSkO6KugpAiI5iuFAJdgB3Ps98yc7G0WuJJVBwNpaIyyX0aSmD9wb4RKna2c+shnDDu71mYy3y80Qlwy2ddtda1Q+pHSaf8gRKp5i6ByeHKTydpcesuZ8BeQVGuoErWPNWXPVErL/AHGvaUpnae66+ExHx19bPbNyCeH0LT7S0eEoH1PjA==; 5:iYaCrUIj6IU+VKvTEtjITXSbkjHvu4wqEs+3PLBVDXaG4rrMCXWaYA56L+Cy4z6a+N+xW4Gd6rrrs1V4ywKu9XnZaRTunPRqtKwWPDO9TpWZcibmi5Hgl9Z7Um5srtN4zmZjikcM+fPXG37tg0lCr/Zli2QWEfPvaFColTiHQhI=; 7:IZDyT+2PAcp33kGiu6OXXO8ATDmAZJ0JIPeVInUE9ZbdSvgsQiNirC3v8Iz0+JN8oU9mydj6Dom3rm7WOnsDmtY7AAEYUF1IbMrsD5BduprfNLO1KknydkkhrzpBxTkWIcxdU0AnBvAOygQ24PQxXg== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 3e6e18be-732c-40b6-d735-08d63b1e90b6 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM4PR05MB3393; x-ms-traffictypediagnostic: AM4PR05MB3393: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231355)(944501410)(52105095)(3002001)(93006095)(93001095)(10201501046)(6055026)(148016)(149066)(150057)(6041310)(20161123558120)(20161123560045)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:AM4PR05MB3393; BCL:0; PCL:0; RULEID:; SRVR:AM4PR05MB3393; x-forefront-prvs: 083751FCA6 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(376002)(346002)(39860400002)(366004)(396003)(13464003)(52314003)(199004)(189003)(2906002)(66066001)(2900100001)(5660300001)(446003)(86362001)(71200400001)(71190400001)(6862004)(305945005)(7736002)(81166006)(25786009)(81156014)(74316002)(478600001)(8676002)(6636002)(8936002)(229853002)(68736007)(4326008)(6506007)(53546011)(53936002)(102836004)(14444005)(256004)(6436002)(106356001)(14454004)(105586002)(186003)(76176011)(3846002)(476003)(6116002)(486006)(54906003)(99286004)(33656002)(5250100002)(11346002)(97736004)(6246003)(26005)(93886005)(55016002)(7696005)(9686003)(316002)(21314003); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR05MB3393; H:AM4PR05MB3265.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: Oc7n5KAAExQBQP2oNZt/V7Li6R964JJVNsLB121HMr4k9ANd60XYGl/+NOUWCy2fx2Z87BLsV7pGxLgZXzamKOEhtuBLP6iUwfyH5s7AV/UoEqkK1t4OZuMTLzbChAu5yul/DCnSx/B4hyklVh9XatuvSliAe/VEf0gYkhloHTF5h1f+PdVqfbWquRuzFGpFNs7fdqjlExbQgf0LOKm+0CAka2xPpAf5H/K00x/0mX7VcU8hihsm/LRat2BBjfPN0jhzbBZ5AF9unGEHVLIVtEv6DTSK0E35d2gZNs8NeXWvjNd3t5sKQlEzPnoyUs60Jp3j4MB4/6OxXhJIB1rS4LMAtf1HlEE3rVFMmcdR0Qs= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3e6e18be-732c-40b6-d735-08d63b1e90b6 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Oct 2018 08:39:38.1838 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR05MB3393 Subject: Re: [dpdk-dev] [PATCH v2 2/7] net/mlx5: e-switch VXLAN flow validation routine X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Oct 2018 08:39:39 -0000 > -----Original Message----- > From: Yongseok Koh > Sent: Friday, October 26, 2018 6:07 > To: Slava Ovsiienko > Cc: Shahaf Shuler ; dev@dpdk.org > Subject: Re: [PATCH v2 2/7] net/mlx5: e-switch VXLAN flow validation > routine >=20 > On Thu, Oct 25, 2018 at 06:53:11AM -0700, Slava Ovsiienko wrote: > > > -----Original Message----- > > > From: Yongseok Koh > > > Sent: Tuesday, October 23, 2018 13:05 > > > To: Slava Ovsiienko > > > Cc: Shahaf Shuler ; dev@dpdk.org > > > Subject: Re: [PATCH v2 2/7] net/mlx5: e-switch VXLAN flow validation > > > routine > > > > > > On Mon, Oct 15, 2018 at 02:13:30PM +0000, Viacheslav Ovsiienko wrote: > [...] > > > > @@ -1114,7 +1733,6 @@ struct pedit_parser { > > > > error); > > > > if (ret < 0) > > > > return ret; > > > > - item_flags |=3D MLX5_FLOW_LAYER_OUTER_L3_IPV4; > > > > mask.ipv4 =3D flow_tcf_item_mask > > > > (items, &rte_flow_item_ipv4_mask, > > > > &flow_tcf_mask_supported.ipv4, > > > > @@ -1135,13 +1753,22 @@ struct pedit_parser { > > > > next_protocol =3D > > > > ((const struct rte_flow_item_ipv4 *) > > > > (items->spec))->hdr.next_proto_id; > > > > + if (item_flags & > > > MLX5_FLOW_LAYER_OUTER_L3_IPV4) { > > > > + /* > > > > + * Multiple outer items are not allowed as > > > > + * tunnel parameters, will raise an error later. > > > > + */ > > > > + ipv4 =3D NULL; > > > > > > Can't it be inner then? > > AFAIK, no for tc rules, we can not specify multiple levels (inner + ou= ter) for > them. > > There is just no TCA_FLOWER_KEY_xxx attributes for specifying inner > items > > to match by flower. >=20 > When I briefly read the kernel code, I thought TCA_FLOWER_KEY_* are for > inner > header before decap. I mean TCA_FLOWER_KEY_IPV4_SRC is for inner L3 > and > TCA_FLOWER_KEY_ENC_IPV4_SRC is for outer tunnel header. Please do > some > experiments with tc-flower command. Hm. Interesting. I will check. > > It is quite unclear comment, not the best one, sorry. I did not like it= too, > > just forgot to rewrite. > > > > ipv4, ipv6 , udp variables gather the matching items during the item li= st > scanning, > > later variables are used for VXLAN decap action validation only. So, th= e > "outer" > > means that ipv4 variable contains the VXLAN decap outer addresses, and > > should be NULL-ed if multiple items are found in the items list. > > > > But we can generate an error here if we have valid action_flags > > (gathered by prepare function) and VXLAN decap is set. Raising > > an error looks more relevant and clear. >=20 > You can't use flags at this point. It is validate() so prepare() might no= t be > preceded. >=20 > > > flow create 1 ingress transfer > > > pattern eth src is 66:77:88:99:aa:bb > > > dst is 00:11:22:33:44:55 / ipv4 src is 2.2.2.2 dst is 1.1.1.1 / > > > udp src is 4789 dst is 4242 / vxlan vni is 0x112233 / > > > eth / ipv6 / tcp dst is 42 / end > > > actions vxlan_decap / port_id id 2 / end > > > > > > Is this flow supported by linux tcf? I took this example from Adrien'= s > patch - > > > "[8/8] net/mlx5: add VXLAN decap support to switch flow rules". If so= , > isn't it > > > possible to have inner L3 layer (MLX5_FLOW_LAYER_INNER_*)? If not, > you > > > should return error in this case. I don't see any code to check redun= dant > > > outer items. > > > Did I miss something? > > > > Interesting, besides rule has correct syntax, I'm not sure whether it c= an be > applied w/o errors. >=20 > Please try. You owns this patchset. However, you just can prohibit such f= lows > (tunneled item) and come up with follow-up patches to enable it later if = it is > support by tcf as this whole patchset itself is pretty huge enough and we > don't > have much time. >=20 > > At least our current flow_tcf_translate() implementation does not suppo= rt > any INNERs. > > But it seems the flow_tcf_validate() does, it's subject to recheck - we > should not allow > > unsupported items to pass the validation. I'll check and provide the > separate bugfix patch > > (if any). >=20 > Neither has tunnel support. It is the first time to add tunnel support to= TCF. > If it was needed, you should've added it, not skipping it. >=20 > You can check how MLX5_FLOW_LAYER_TUNNEL is used in Verbs/DV as a > reference. Yes. I understood your point. Will check and add tunnel support for TCF rul= es. Anyway, inner MAC addresses are supported for VXLAN decap, I think we shoul= d specify these ones in the rule as inners (after VNI item), definitely some tunnel support in validate/parse/translate should be added. >=20 > > > BTW, for the tunneled items, why don't you follow the code of > > > Verbs(mlx5_flow_verbs.c) and DV(mlx5_flow_dv.c)? For tcf, it is the f= irst > time > > For VXLAN it has some specifics (warning about ignored params, etc.) > > I've checked which of verbs/dv code could be reused and did not > discovered > > a lot. I'll recheck the latest code commits, possible it became more > appropriate > > for VXLAN. >=20 > Agreed. I'm not forcing you to do it because we run out of time but > mentioned it > because if there's any redundancy in our code, that usually causes bug la= ter. > Let's not waste too much time for that. Just grab low hanging fruits if a= ny. >=20 > > > to add tunneled item, but Verbs/DV already have validation code for > tunnel, > > > so you can reuse the existing code. In flow_tcf_validate_vxlan_decap(= ), > not > > > every validation is VXLAN-specific but some of them can be common > code. > > > > > > And if you need to know whether there's the VXLAN decap action prior = to > > > outer header item validation, you can relocate the code - action > validation > > > first and item validation next, as there's no dependency yet in the c= urrent > > > > We can not validate action first - we need items to be preliminary > gathered, > > to check them in action's specific fashion and to check action itself. > > I mean, if we see VXLAN decap action, we should check the presence of > > L2, L3, L4 and VNI items. I minimized the number of passes along the it= em > > and action lists. BTW, Adrien's approach performed two passes, mine doe= s > only. > > > > > code. Defining ipv4, ipv6, udp seems to make the code path more > complex. > > Yes, but it allows us to avoid the extra item list scanning and minimiz= es the > changes > > of existing code. > > In your approach we should: > > - scan actions, w/o full checking, just action_flags gathering and chec= king > > - scan items, performing variating check (depending on gathered action > flags) > > - scan actions again, performing full check with params (at least for n= ow > > check whether all params gathered) >=20 > Disagree. flow_tcf_validate_vxlan_encap() doesn't even need any info of > items > and flow_tcf_validate_vxlan_decap() needs item_flags to check whether > VXLAN > item is there or not and ipv4/ipv6/udp are all for item checks. Let me gi= ve > you > very detailed exmaple: >=20 > { > for (actions[]...) { > ... > case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP: > ... > flow_tcf_validate_vxlan_encap(); > ... > break; > case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP: > if (action_flags & (MLX5_ACTION_VXLAN_ENCAP > | MLX5_ACTION_VXLAN_DECAP)) > return rte_flow_error_set > (error, ENOTSUP, > RTE_FLOW_ERROR_TYPE_ACTION, > actions, > "can't have multiple vxlan actions"); > /* Don't call flow_tcf_validate_vxlan_decap(). */ > action_flags |=3D MLX5_ACTION_VXLAN_DECAP; > break; > } > for (items[]...) { > ... > case RTE_FLOW_ITEM_TYPE_IPV4: > /* Existing common validation. */ > ... > if (action_flags & MLX5_ACTION_VXLAN_DECAP) { > /* Do ipv4 validation in > * flow_tcf_validate_vxlan_decap()/ > } > break; > } > } >=20 > Curretly you are doing, >=20 > - validate items > - validate actions > - validate items again if decap. >=20 > But this can simply be >=20 > - validate actions How we could validate VXLAN decap at this stage?=20 As we do not have item_flags set yet? Do I miss something? > - validate items >=20 > Thanks, > Yongseok >=20 With best regards, Slava > > > > > > For example, you just can call vxlan decap item validation (by splitt= ing > > > flow_tcf_validate_vxlan_decap()) at this point like: > > > > > > if (action_flags & > > > MLX5_FLOW_ACTION_VXLAN_DECAP) > > > ret =3D > > > flow_tcf_validate_vxlan_decap_ipv4(...); > > > ... > > > > > > Same for other items. > > >