On 8/9/23 15:48, Mike Pattrick wrote: > On Thu, Jul 13, 2023 at 9:55 AM Eelco Chaudron <echau...@redhat.com> wrote: >> >> >> >> 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. > > Thanks everyone, > > I've sent in backports of this patch down to 2.17. I would like to > backport this because currently users can easily segfault OVS with a > normal looking OVN configuration unless they also increase the default > stack size for the process before starting it.
Makes sense. I'll take a look at backports. Thanks! Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
