On 5/12/20 1:52 AM, Mark Michelson wrote: > On 5/11/20 5:31 PM, Ilya Maximets wrote: >> 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. > > I agree. The sections below and above this one in build_router_lflows() use > op->lrp_networks. There's no reason I can see to extract nbrp->lrp_networks.
OK. I'll prepare v2 with this change. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
