On 5/11/20 11:21 PM, Ilya Maximets wrote:
> 'lrp_networks' never destroyed but constantly overwritten in a loop that
> handles DHCPv6 replies.  In some cases this point leaks several MB per
> minute making ovn-northd to constantly growing its memory consumption:
> 
>  399,820,764 bytes in 1,885,947 blocks are definitely lost in loss record 182 
> of 182
>     at 0x4839748: malloc (vg_replace_malloc.c:308)
>     by 0x483BD63: realloc (vg_replace_malloc.c:836)
>     by 0x1E7BF8: xrealloc (util.c:149)
>     by 0x152723: add_ipv6_netaddr.isra.0 (ovn-util.c:55)
>     by 0x152F1C: extract_lrp_networks (ovn-util.c:275)
>     by 0x142EE2: build_lrouter_flows (ovn-northd.c:8607)
>     by 0x142EE2: build_lflows.isra.0 (ovn-northd.c:10296)
>     by 0x14E4F8: ovnnb_db_run (ovn-northd.c:11128)
>     by 0x14E4F8: ovn_db_run (ovn-northd.c:11672)
>     by 0x13304D: main (ovn-northd.c:12035)
> 
> Additionally fixed similar leak in case of a duplicate logical router
> port.
> 
> Reported-by: Joe Talerico <[email protected]>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1827769
> Fixes: 5c1d2d230773 ("northd: Add logical flows for dhcpv6 pfd parsing")
> Signed-off-by: Ilya Maximets <[email protected]>
> ---
>  northd/ovn-northd.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 76b4a14ee..8200af24b 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -2129,6 +2129,7 @@ join_logical_ports(struct northd_context *ctx,
>                              = VLOG_RATE_LIMIT_INIT(5, 1);
>                          VLOG_WARN_RL(&rl, "duplicate logical router port %s",
>                                       nbrp->name);
> +                        destroy_lport_addresses(&lrp_networks);
>                          continue;
>                      }
>                      ovn_port_set_nb(op, NULL, nbrp);
> @@ -8622,6 +8623,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>              ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 100,
>                            ds_cstr(&match), ds_cstr(&actions));
>          }
> +        destroy_lport_addresses(&lrp_networks);


This is basically a mechanical fix, but maybe we could do something a bit more 
clever.
I'm not sure why we're extracting 'lrp_networks' in this loop in the first 
place.
Can we just use 'op->lrp_networks' like all other loops in this function?  In 
this
case we will not need to allocate and destroy at all.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to