On Thu, Jul 13, 2023 at 9:55 AM Eelco Chaudron <[email protected]> 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 <[email protected]> > >>> Reviewed-by: Simon Horman <[email protected]> > >>> Acked-by: Eelco Chaudron <[email protected]> > >>> > >>> --- > >>> 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. Thank you, M > > //Eelco > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
