From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shahaf Shuler Subject: Re: [PATCH v2] net/mlx5: support multiple groups and jump action Date: Wed, 10 Oct 2018 05:35:35 +0000 Message-ID: References: <20181003015640.36306-1-yskoh@mellanox.com> <20181008180552.39671-1-yskoh@mellanox.com> <20181010024731.GI9031@mtidpdk.mti.labs.mlnx> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Yongseok Koh Return-path: Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80089.outbound.protection.outlook.com [40.107.8.89]) by dpdk.org (Postfix) with ESMTP id ED9F31B206 for ; Wed, 10 Oct 2018 07:35:37 +0200 (CEST) In-Reply-To: <20181010024731.GI9031@mtidpdk.mti.labs.mlnx> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Wednesday, October 10, 2018 5:48 AM, Yongseok Koh: > Subject: Re: [PATCH v2] net/mlx5: support multiple groups and jump action > On Tue, Oct 09, 2018 at 05:50:30AM -0700, Shahaf Shuler wrote: > > Hi Koh, > > Few comments. > > > > Monday, October 8, 2018 9:06 PM=B8 Yongseok Koh: > > > Subject: [PATCH v2] net/mlx5: support multiple groups and jump > > > action > > > [...] > > > -#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 > > > > I guess there is no way to query those and trial and error is overkill = for this > first implementation. >=20 > Well, not a huge task. If you (or FW/drv team) think this max value is li= kely > 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. I don't think it is needed now. Even if it is increased we can stay with 3 = tables till we will have a use case for more.=20 >=20 > > > + > > > +#define MLX5_TCF_FATE_ACTIONS \ [...] > > > > > > We also spoke about that for group > 0 the flow items can start from th= e > middle. E.g. on table 0 we have flow rule that redirect all eth/ip to gro= up 1, > and on group 1 the rules can start with udp or tcp. > > Is this possible today w/o any code change? >=20 > Not possible. It needs more changes. Complexity of the additional code > depends on a set of limitations we make. In the simplest way, we can > unconditionally allow such flows if TCF allows it. But if we need smarter= way, > we would have 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 should decide whether this is a violation or not.=20 This is an application error. Obviously the TCP rule will never hit. =20 IIRC, that's > why we decided to not allow such flows during the design review. Users > have to specify full items starting from 'eth' with the current implement= ation. >=20 > Will submit v3 with the change in Makefile and meson.build. But if you th= ink > there's need to add additional features like I answered above, let me kno= w > so that I can submit v4. I think we need to allow items in the groups to start from arbitrary header= and not always eth. It can be in the most simple way like you mentioned.=20 It is needed because: 1. to save the user the overhead of creating the same pattern prefix for th= e rules in the groups 2. it fits better with OVS and openflow model. For example you will have on= e table for the outer header and one table for the inner header each contai= ns only the outer/inner headers.=20 3. not sure how kernel inserts the rule but it is an overhead to create the= full steering entry from the outer eth in case it was already being matche= d by previous rules. It is bigger steering entries with more cache misses a= nd comparators.=20 >=20 >=20 > Thanks, > Yongseok