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¸ 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. > > Well, not a huge task. If you (or FW/drv team) think this max value is likely > 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. > > > > + > > > +#define MLX5_TCF_FATE_ACTIONS \ [...] > > > > > > 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. > > Is this possible today w/o any code change? > > 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. This is an application error. Obviously the TCP rule will never hit. 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 > implementation. > > Will submit v3 with the change in Makefile and meson.build. But if you think > there's need to add additional features like I answered above, let me know > 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. It is needed because: 1. to save the user the overhead of creating the same pattern prefix for the rules in the groups 2. it fits better with OVS and openflow model. For example you will have one table for the outer header and one table for the inner header each contains only the outer/inner headers. 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 matched by previous rules. It is bigger steering entries with more cache misses and comparators. > > > Thanks, > Yongseok
