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

Roi Dayan roid at mellanox.com
Mon Oct 28 08:35:33 UTC 2019



On 2019-10-28 10:15 AM, Simon Horman wrote:
> On Fri, Oct 25, 2019 at 12:19:22PM +0200, Simon Horman wrote:
>> On Thu, Oct 24, 2019 at 10:43:00AM +0200, Simon Horman wrote:
>>> 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.
>>
>> I have now pushed this patch to branch-2.11.
>>
>>>
>>> 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.
>>
>> I have proposed a fix for this problem.
>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fpipermail%2Fovs-dev%2F2019-October%2F363868.html&amp;data=02%7C01%7Croid%40mellanox.com%7Caca4498333aa467cae6c08d75b7ef3d1%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637078473125610665&amp;sdata=p4oai3sC8F4FHfFrvfv7q0XAzk6%2BY%2Bs9cD736XgLvdg%3D&amp;reserved=0
> 
> That fix was merged by Ben and now that branch-2.10 is travis-ci clean
> I have gone ahead and applied this patch there.
> 

thanks!


More information about the dev mailing list