From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01on0060.outbound.protection.outlook.com [104.47.0.60]) by dpdk.org (Postfix) with ESMTP id 561D71B45A for ; Wed, 10 Oct 2018 04:47:45 +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=2igi1SIpNihkP2rMukhcxYxa6eb1lrlM6mD0JTjcNpE=; b=Ja6lClPQW+p6AXmAmnBv36aspBCWwqgiouIM1OBckY9cavp0Of/kYHrWVgcvFy+H7MKefx5vPgkXR/SV3Veatu7eDxq4D7NdY1eQGb/SVApIiVuGUxFVAzB8HoXJmWyL2AebTvb3IwSRV19RVdW6MTLbYFL2tQn4iy6nwNVTuyU= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB3945.eurprd05.prod.outlook.com (52.134.65.155) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1228.21; Wed, 10 Oct 2018 02:47:41 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::1cb0:661b:ecab:6045]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::1cb0:661b:ecab:6045%2]) with mapi id 15.20.1207.029; Wed, 10 Oct 2018 02:47:41 +0000 From: Yongseok Koh To: Shahaf Shuler CC: "dev@dpdk.org" Thread-Topic: [PATCH v2] net/mlx5: support multiple groups and jump action Thread-Index: AQHUXzGiKl69OujvvkSaaq/Zudo7xaUW3u4AgADp3oA= Date: Wed, 10 Oct 2018 02:47:41 +0000 Message-ID: <20181010024731.GI9031@mtidpdk.mti.labs.mlnx> References: <20181003015640.36306-1-yskoh@mellanox.com> <20181008180552.39671-1-yskoh@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: SN6PR0102CA0010.prod.exchangelabs.com (2603:10b6:805:1::23) To DB3PR0502MB3980.eurprd05.prod.outlook.com (2603:10a6:8:10::27) authentication-results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB3945; 6:fPo7kO3LYbu7ZtuYa80wVt28fNLi4/dIXMb2X51fa4xdykZ5UPlcD+P0uoiK4sukC923V+Ki9xTGTk87PwHHUrtjg4qXZlmu0MkR8b3HvqjkOmChviQZnvOFs+LMukbRcuirg2Ie7ozvgT2g4XBDJUIJUg17JgE9iobWNmL4BZiaeSnnJIxzhwMOp/1yZ+z/Xx1iJFd5zH1cFo9jWClKspgU+K6jsdE4uPqcxwhSmQ7uG/belQW70ccUWNWNUnIoEkkDJDMuNoGFhX1rQEJ3Jauo3kRsFdkl0z22oywNRypeJ5DXkZd7ceBqS/ysLLgkqqppIMT5aVL+FdTN4OXOH0UVgVBnAxUKseLDq51teVePfkbIv6AGQwRtv0rsZf8I66qYT7jnssKBK7HOf6zCTDcDiPbdX8T4rd+1dzpaAEHAaQDIstFsV3qcVxJX63uVEvg86YYnK4IS9fEUMtdYYQ==; 5:M761MZPF5cIGeOxFJUnk4TKolltZIRWmGlZTEiZczmY0bI4rDWLb8V0FecK6Beh+TaH3MIGn/Yr8qCsICW6wkdLwRdT1sJ8wYxu60T0P350kBAgtvHZMYatW2lN3L7cqp+p21InSvjuHdP/pRKYv91D/wCixSZI97KXfQIiNGWg=; 7:Cx/TxpL0n0BmoDJvEI2C3XsytzrO//xUqErWy7Oy62HS1874tE8KgmELxHSoJeT3GQDDgv4icZR5FxutVgAp+2f+C6WUpjJ77LPmuynG1/6+3hjmJkb53yU9luzE/cKCgeJSchbQjx2UGLE5DaAL/zWHnusWjeMWAKihxS0WTpjkMlfe0e6kxaeY1a/iu7dBHmOOaZXGbB0vmG3UG+KmySuNTTprVO6ajlihAbTFhGbQTBbs2DeKVe4R1RNFs2lf x-ms-office365-filtering-correlation-id: a15ec618-adae-4cb5-d196-08d62e5abeea 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:DB3PR0502MB3945; x-ms-traffictypediagnostic: DB3PR0502MB3945: 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)(8121501046)(5005006)(3002001)(3231355)(944501410)(52105095)(10201501046)(93006095)(93001095)(6055026)(149066)(150057)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123560045)(20161123562045)(20161123558120)(201708071742011)(7699051); SRVR:DB3PR0502MB3945; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB3945; x-forefront-prvs: 08213D42D3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(396003)(346002)(136003)(366004)(376002)(189003)(199004)(476003)(97736004)(316002)(11346002)(71200400001)(71190400001)(5250100002)(446003)(33656002)(6246003)(9686003)(6512007)(6862004)(102836004)(6116002)(6506007)(386003)(1076002)(6486002)(3846002)(186003)(6436002)(26005)(4326008)(25786009)(66066001)(53936002)(33896004)(76176011)(99286004)(229853002)(52116002)(2906002)(6636002)(2900100001)(7736002)(305945005)(5660300001)(68736007)(86362001)(8936002)(105586002)(8676002)(478600001)(81166006)(486006)(256004)(81156014)(14454004)(14444005)(575784001)(106356001); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3945; H:DB3PR0502MB3980.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: ZEBrZQolHShkJB6gsU2EdAmnurGXE5eyDj4hFbk3MWnpyviFZVFb+F/WZNjbsis3LkiGM5UYOviVMb2EdUwacaSrrqCYpKqeKJNvBwN5h3QEhzggUvrFIN3PL8wOW3Pi//bD9gB3sRhubrzUEK387AMpqhJU6QPxPtdODvDKizIW/t5C875B/vxFViMNx5tU4FjZLHdKJ8gDCvcf/IyNDEsoBFOF6yvyUVfVHd0bUCoHjragnOmgQBacYbPrf/cc1atlXCGr9s0kLDkRN7INQYs1DQgQNEc4hMT3nSQufzl/p6FOvYBdePbOahat1kAf9MgyLzkmyvPqF3xegic2nN6NZt7wL2MItrtrqqomynM= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-ID: <15F8C16D88B2C247BAD6B049074B7135@eurprd05.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: a15ec618-adae-4cb5-d196-08d62e5abeea X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Oct 2018 02:47:41.1733 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB3945 Subject: Re: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action 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: Wed, 10 Oct 2018 02:47:45 -0000 On Tue, Oct 09, 2018 at 05:50:30AM -0700, Shahaf Shuler wrote: > Hi Koh, > Few comments.=20 >=20 > Monday, October 8, 2018 9:06 PM=B8 Yongseok Koh: > > Subject: [PATCH v2] net/mlx5: support multiple groups and jump action > >=20 > > rte_flow has 'group' attribute and 'jump' action in order to support mu= ltiple > > groups. This feature is known as multi-table support ('chain' in linux = TC > > flower) in general because a group means a table of flows. Example > > commands are: > >=20 > > flow create 0 transfer priority 1 ingress > > pattern eth / vlan vid is 100 / end > > actions jump group 1 / end > >=20 > > flow create 0 transfer priority 1 ingress > > pattern eth / vlan vid is 200 / end > > actions jump group 2 / end > >=20 > > flow create 0 transfer group 1 priority 2 ingress > > pattern eth / vlan vid is 100 / > > ipv4 dst spec 192.168.40.0 dst prefix 24 / end > > actions drop / end > >=20 > > flow create 0 transfer group 1 priority 2 ingress > > pattern end > > actions of_pop_vlan / port_id id 1 / end > >=20 > > flow create 0 transfer group 2 priority 2 ingress > > pattern eth / vlan vid is 200 / > > ipv4 dst spec 192.168.40.0 dst prefix 24 / end > > actions of_pop_vlan / port_id id 2 / end > >=20 > > flow create 0 transfer group 2 priority 2 ingress > > pattern end > > actions port_id id 2 / end > >=20 > > With theses flows, if a packet having vlan 200 and src_ip as 192.168.40= .1, this > > packet will firstly hit the 1st flow. Then it will hit the 5th flow bec= ause of the > > 'jump' action. As a result, the packet will be forwarded to port 2 (VF > > representor) with vlan tag being stripped off. If the packet had vlan 1= 00 > > instead, it would be dropped by the 3rd flow. > >=20 > > Signed-off-by: Yongseok Koh > > --- > >=20 > > v2: > > * drop ethdev patch as it had already been fixed by Adrien's patch > > * move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c > >=20 > > drivers/net/mlx5/Makefile | 10 +++++ > > drivers/net/mlx5/meson.build | 4 ++ > > drivers/net/mlx5/mlx5_flow.h | 1 + > > drivers/net/mlx5/mlx5_flow_tcf.c | 82 > > +++++++++++++++++++++++++++++++++++----- > > 4 files changed, 88 insertions(+), 9 deletions(-) > >=20 > > diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile inde= x > > ca1de9f21..92bae9dfc 100644 > > --- a/drivers/net/mlx5/Makefile > > +++ b/drivers/net/mlx5/Makefile > > @@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto- > > config-h.sh > > enum TCA_VLAN_PUSH_VLAN_PRIORITY \ > > $(AUTOCONF_OUTPUT) > > $Q sh -- '$<' '$@' \ > > + HAVE_TCA_CHAIN \ > > + linux/rtnetlink.h \ > > + enum TCA_CHAIN \ > > + $(AUTOCONF_OUTPUT) > > + $Q sh -- '$<' '$@' \ > > + HAVE_TC_ACT_GOTO_CHAIN \ > > + linux/pkt_cls.h \ > > + define TC_ACT_GOTO_CHAIN \ > > + $(AUTOCONF_OUTPUT) > > + $Q sh -- '$<' '$@' \ > > HAVE_SUPPORTED_40000baseKR4_Full \ > > /usr/include/linux/ethtool.h \ > > define SUPPORTED_40000baseKR4_Full \ > > diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.buil= d > > index fd93ac162..696624838 100644 > > --- a/drivers/net/mlx5/meson.build > > +++ b/drivers/net/mlx5/meson.build > > @@ -182,6 +182,10 @@ if build > > 'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ], > > [ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h', > > 'TCA_VLAN_PUSH_VLAN_PRIORITY' ], > > + [ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h', > > + 'TCA_CHAIN' ], > > + [ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h', > > + 'TC_ACT_GOTO_CHAIN' ], >=20 > Please keep the dictionary order according to the linux header for the se= arch.=20 Okay. > > [ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h', > > 'RDMA_NL_NLDEV' ], > > [ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h', > > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.= h > > index 12de841e8..3ed0ddc58 100644 > > --- a/drivers/net/mlx5/mlx5_flow.h > > +++ b/drivers/net/mlx5/mlx5_flow.h > > @@ -78,6 +78,7 @@ > > #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8) #define > > MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9) #define > > MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10) > > +#define MLX5_FLOW_ACTION_JUMP (1u << 11) > >=20 > > #define MLX5_FLOW_FATE_ACTIONS \ > > (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | > > MLX5_FLOW_ACTION_RSS) diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c > > b/drivers/net/mlx5/mlx5_flow_tcf.c > > index 91f6ef678..fbc4c2bb7 100644 > > --- a/drivers/net/mlx5/mlx5_flow_tcf.c > > +++ b/drivers/net/mlx5/mlx5_flow_tcf.c > > @@ -148,6 +148,12 @@ struct tc_vlan { > > #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE #define > > TCA_FLOWER_KEY_VLAN_ETH_TYPE 25 #endif > > +#ifndef HAVE_TCA_CHAIN > > +#define TCA_CHAIN 11 > > +#endif > > +#ifndef HAVE_TC_ACT_GOTO_CHAIN > > +#define TC_ACT_GOTO_CHAIN 0x20000000 > > +#endif > >=20 > > #ifndef IPV6_ADDR_LEN > > #define IPV6_ADDR_LEN 16 > > @@ -225,7 +231,13 @@ struct flow_tcf_ptoi { > > unsigned int ifindex; /**< Network interface index. */ }; > >=20 > > -#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | > > MLX5_FLOW_ACTION_PORT_ID) > > +/* Due to a limitation on driver/FW. */ #define > > MLX5_TCF_GROUP_ID_MAX 3 > > +#define MLX5_TCF_GROUP_PRIORITY_MAX 14 >=20 > I guess there is no way to query those and trial and error is overkill fo= r this first implementation.=20 Well, not a huge task. If you (or FW/drv team) think this max value is like= ly increased in the near future (before the next LTS version), then it isn't a= bad idea to add such code now. Let me know. > > + > > +#define MLX5_TCF_FATE_ACTIONS \ > > + (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \ > > + MLX5_FLOW_ACTION_JUMP) > > #define MLX5_TCF_VLAN_ACTIONS \ > > (MLX5_FLOW_ACTION_OF_POP_VLAN | > > MLX5_FLOW_ACTION_OF_PUSH_VLAN | \ > > MLX5_FLOW_ACTION_OF_SET_VLAN_VID | > > MLX5_FLOW_ACTION_OF_SET_VLAN_PCP) @@ -370,14 +382,25 @@ > > flow_tcf_validate_attributes(const struct rte_flow_attr *attr, > > struct rte_flow_error *error) > > { > > /* > > - * Supported attributes: no groups, some priorities and ingress only. > > - * Don't care about transfer as it is the caller's problem. > > + * Supported attributes: groups, some priorities and ingress only. > > + * group is supported only if kernel supports chain. Don't care about > > + * transfer as it is the caller's problem. > > */ > > - if (attr->group) > > + if (attr->group > MLX5_TCF_GROUP_ID_MAX) > > return rte_flow_error_set(error, ENOTSUP, > >=20 > > RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr, > > - "groups are not supported"); > > - if (attr->priority > 0xfffe) > > + "group ID larger than " > > + > > RTE_STR(MLX5_TCF_GROUP_ID_MAX) > > + " isn't supported"); > > + else if (attr->group > 0 && > > + attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX) > > + return rte_flow_error_set(error, ENOTSUP, > > + > > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > > + attr, > > + "lowest priority level is " >=20 > Lowest or highest? Lowest it is. Consistent with the error message of no-multi-group case belo= w. > > + > > RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX) > > + " when group is configured"); > > + else if (attr->priority > 0xfffe) > > return rte_flow_error_set(error, ENOTSUP, > >=20 > > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > > attr, >=20 > [... ] >=20 > > flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow, > > mnl_attr_nest_end(nlh, na_act); > > mnl_attr_nest_end(nlh, na_act_index); > > break; > > + case RTE_FLOW_ACTION_TYPE_JUMP: > > + conf.jump =3D actions->conf; > > + na_act_index =3D > > + mnl_attr_nest_start(nlh, > > na_act_index_cur++); > > + assert(na_act_index); > > + mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact"); > > + na_act =3D mnl_attr_nest_start(nlh, > > TCA_ACT_OPTIONS); > > + assert(na_act); > > + mnl_attr_put(nlh, TCA_GACT_PARMS, > > + sizeof(struct tc_gact), > > + &(struct tc_gact){ > > + .action =3D TC_ACT_GOTO_CHAIN | > > + conf.jump->group, > > + }); > > + mnl_attr_nest_end(nlh, na_act); > > + mnl_attr_nest_end(nlh, na_act_index); > > + break; > > case RTE_FLOW_ACTION_TYPE_DROP: > > na_act_index =3D > > mnl_attr_nest_start(nlh, > > na_act_index_cur++); >=20 >=20 > We also spoke about that for group > 0 the flow items can start from the = middle. E.g. on table 0 we have flow rule that redirect all eth/ip to group= 1, and on group 1 the rules can start with udp or tcp.=20 > Is this possible today w/o any code change?=20 Not possible. It needs more changes. Complexity of the additional code depe= nds on a set of limitations we make. In the simplest way, we can unconditionall= y allow such flows if TCF allows it. But if we need smarter way, we would hav= e to add much more validation code. For example, in a case where grp 0 has "item eth/ip proto is udp / aciton goto grp 1" and grp 1 has "item tcp ...", we s= hould decide whether this is a violation or not. IIRC, that's why we decided to n= ot allow such flows during the design review. Users have to specify full items starting from 'eth' with the current implementation. Will submit v3 with the change in Makefile and meson.build. But if you thin= k there's need to add additional features like I answered above, let me know = so that I can submit v4. Thanks, Yongseok