On 13 Jul 2023, at 15:15, Eelco Chaudron wrote:

> On 13 Jul 2023, at 15:10, Ilya Maximets wrote:
>
>> On 7/12/23 15:37, Mike Pattrick wrote:
>>> Several xlate actions used in recursive translation currently store a
>>> large amount of information on the stack. This can result in handler
>>> threads quickly running out of stack space despite before
>>> xlate_resubmit_resource_check() is able to terminate translation. This
>>> patch reduces stack usage by over 3kb from several translation actions.
>>>
>>> This patch also moves some trace function from do_xlate_actions into its
>>> own function.
>>>
>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2104779
>>> Signed-off-by: Mike Pattrick <m...@redhat.com>
>>> Reviewed-by: Simon Horman <simon.hor...@corigine.com>
>>> Acked-by: Eelco Chaudron <echau...@redhat.com>
>>>
>>> ---
>>> Since v1:
>>>  - Refactored code into specialized functions and renamed variables for
>>>  improved readability.
>>> Since v2:
>>>  - Removed inline keywords
>>> Since v3:
>>>  - Improved formatting.
>>> Since v4:
>>>  - v4 patch was an incorrect revision
>>> Since v5:
>>>  - Moved trace portmap to heap
>>>  - Moved additional flow_tnl mf_subvalue structs to heap
>>> Since v5.b:
>>>  - Reordered variables in xlate_sample
>>> ---
>>>  ofproto/ofproto-dpif-xlate.c | 259 ++++++++++++++++++++++-------------
>>>  1 file changed, 164 insertions(+), 95 deletions(-)
>>
>> <snip>
>>
>>> @@ -6343,8 +6407,8 @@ compose_conntrack_action(struct xlate_ctx *ctx, 
>>> struct ofpact_conntrack *ofc,
>>>  {
>>>      uint16_t zone;
>>>      if (ofc->zone_src.field) {
>>> -        union mf_subvalue value;
>>> -        memset(&value, 0xff, sizeof(value));
>>> +        union mf_subvalue *value = xmalloc(sizeof *value);
>>> +        memset(value, 0xff, sizeof(*value));
>>
>> Nit: If we can remove extra parenthesis in sizeof, that would be great.
>>      I suppose, can be done on commit.
>>
>> I will not Ack, because I didn't dive into the code review of this version.
>> But I can confirm that I see benefits now with all compilers I tested with.
>> Thanks!
>
> Thanks for the quick check! Will remove the extra parenthesis, and commit 
> this.

Thanks Mike,

Pushed this patch to master.

//Eelco

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to