[ovs-dev] [PATCH] tc: Limit the max action number to 16

[Simon Horman]

On Mon, Oct 21, 2019 at 07:01:38AM +0000, Roi Dayan wrote:
> 
> 
> On 2019-10-18 1:00 PM, Simon Horman wrote:
> > On Wed, Oct 16, 2019 at 11:53:52AM +0000, Roi Dayan wrote:
> >>
> >>
> >> On 2019-10-16 2:40 PM, Simon Horman wrote:
> >>> On Wed, Oct 16, 2019 at 11:37:14AM +0300, Roi Dayan wrote:
> >>>> From: Chris Mi <chrism at mellanox.com>
> >>>>
> >>>> Currently, ovs supports to offload max TCA_ACT_MAX_PRIO(32) actions.
> >>>> But net sched api has a limit of 4K message size which is not enough
> >>>> for 32 actions when echo flag is set.
> >>>>
> >>>> After a lot of testing, we find that 16 actions is a reasonable number.
> >>>> So in this commit, we introduced a new define to limit the max actions.
> >>>>
> >>>> Fixes: 0c70132cd288("tc: Make the actions order consistent")
> >>>> Signed-off-by: Chris Mi <chrism at mellanox.com>
> >>>> Reviewed-by: Roi Dayan <roid at mellanox.com>
> >>>
> >>> Hi Chris,
> >>>
> >>> I'm unclear on what problem is this patch solving.
> >>
> >> Hi Simon,
> >>
> >> I can help with the answer here.
> >> When ovs send netlink msg to tc to add a filter we also add
> >> the echo flag to get a reply data. this reply can be big as
> >> it contains everything from the request and if it pass the 4K
> >> size then the kernel will just not fill/send it but the return
> >> status of the tc command is still 0 for success.
> >>
> >> In ovs we use that reply to get back the handle id on success but
> >> for this case will fail.
> >> To make sure this ack reply always exists for successful tc calls
> >> we limit the number of actions here to avoid reaching the 4K size limit.
> > 
> > Thanks,
> > 
> > It sounds like it would be good to develop a mechanism where
> > the information required can be retrieved in a less verbose manner,
> > thus allowing a higher limit.
> > 
> > But I do agree that this resolves a problem and I have applied it to
> > master. Let me know if you think it is also appropriate for older branches.
> > 
> > Kind regards,
> > Simon
> > 
> 
> thanks Simon.
> this patch can be applied also to branches 2.10, 2.11, 2.12.

Thanks,

I have pushed a backport to branch-2.12.
And I intend to do likewise for branch-2.11 if travis-ci passes.

https://travis-ci.org/horms2/ovs/builds/602211621

I am however holding back on branch-2.10 as it seems broken
with respect to travis-ci. And I do not like to add patches
to broken branches.

I am trying to investigate the cause of that breakage.
I would also welcome any help in this area.

Kind regards,
Simon