Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix ukey leak on udpif destroy.
On 2/17/21 7:42 PM, William Tu wrote:
> On Wed, Feb 17, 2021 at 8:40 AM Ilya Maximets wrote:
>>
>> On 1/18/21 5:12 PM, Ilya Maximets wrote:
>>> Since commit 79eadafeb1b4 udpif_stop_threads() doesn't delete datapath
>>> flows while called from udpif_destroy(). This means that ukeys are
>>> not cleaned up either. So, hash maps in udpif->ukeys[] might still
>>> contain valid pointers to ukeys that should be destroyed before
>>> destroying the hash map itself:
>>>
>>> ==2783089==ERROR: LeakSanitizer: detected memory leaks
>>>
>>> Direct leak of 1560 byte(s) in 1 object(s) allocated from:
>>> # 0 0x7f8a57eae667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
>>> # 1 0x8411f6 in xmalloc lib/util.c:138
>>> # 2 0x4d8a52 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1682
>>> # 3 0x4d99e3 in ukey_create_from_upcall
>>> ofproto/ofproto-dpif-upcall.c:1751
>>> # 4 0x4d517d in upcall_xlate ofproto/ofproto-dpif-upcall.c:1242
>>> # 5 0x4d63d2 in process_upcall ofproto/ofproto-dpif-upcall.c:1414
>>> # 6 0x4d29f3 in recv_upcalls ofproto/ofproto-dpif-upcall.c:833
>>> # 7 0x4d1ee1 in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:750
>>> # 8 0x795aa2 in ovsthread_wrapper lib/ovs-thread.c:383
>>> # 9 0x7f8a57a59431 in start_thread (/lib64/libpthread.so.0+0x9431)
>>>
>>> Fixes: 79eadafeb1b4 ("ofproto: Do not delete datapath flows on exit by
>>> default.")
>>> Reported-by: Dumitru Ceara
>>> Signed-off-by: Ilya Maximets
>>> ---
>
> LGTM. LeakSanitizer is pretty useful.
> Acked-by: William Tu
>
Thanks!
Applied to master and backported down to 2.14.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix ukey leak on udpif destroy.
On Wed, Feb 17, 2021 at 8:40 AM Ilya Maximets wrote:
>
> On 1/18/21 5:12 PM, Ilya Maximets wrote:
> > Since commit 79eadafeb1b4 udpif_stop_threads() doesn't delete datapath
> > flows while called from udpif_destroy(). This means that ukeys are
> > not cleaned up either. So, hash maps in udpif->ukeys[] might still
> > contain valid pointers to ukeys that should be destroyed before
> > destroying the hash map itself:
> >
> > ==2783089==ERROR: LeakSanitizer: detected memory leaks
> >
> > Direct leak of 1560 byte(s) in 1 object(s) allocated from:
> > # 0 0x7f8a57eae667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
> > # 1 0x8411f6 in xmalloc lib/util.c:138
> > # 2 0x4d8a52 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1682
> > # 3 0x4d99e3 in ukey_create_from_upcall
> > ofproto/ofproto-dpif-upcall.c:1751
> > # 4 0x4d517d in upcall_xlate ofproto/ofproto-dpif-upcall.c:1242
> > # 5 0x4d63d2 in process_upcall ofproto/ofproto-dpif-upcall.c:1414
> > # 6 0x4d29f3 in recv_upcalls ofproto/ofproto-dpif-upcall.c:833
> > # 7 0x4d1ee1 in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:750
> > # 8 0x795aa2 in ovsthread_wrapper lib/ovs-thread.c:383
> > # 9 0x7f8a57a59431 in start_thread (/lib64/libpthread.so.0+0x9431)
> >
> > Fixes: 79eadafeb1b4 ("ofproto: Do not delete datapath flows on exit by
> > default.")
> > Reported-by: Dumitru Ceara
> > Signed-off-by: Ilya Maximets
> > ---
LGTM. LeakSanitizer is pretty useful.
Acked-by: William Tu
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Fix ukey leak on udpif destroy.
On 1/18/21 5:12 PM, Ilya Maximets wrote:
> Since commit 79eadafeb1b4 udpif_stop_threads() doesn't delete datapath
> flows while called from udpif_destroy(). This means that ukeys are
> not cleaned up either. So, hash maps in udpif->ukeys[] might still
> contain valid pointers to ukeys that should be destroyed before
> destroying the hash map itself:
>
> ==2783089==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 1560 byte(s) in 1 object(s) allocated from:
> # 0 0x7f8a57eae667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
> # 1 0x8411f6 in xmalloc lib/util.c:138
> # 2 0x4d8a52 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1682
> # 3 0x4d99e3 in ukey_create_from_upcall ofproto/ofproto-dpif-upcall.c:1751
> # 4 0x4d517d in upcall_xlate ofproto/ofproto-dpif-upcall.c:1242
> # 5 0x4d63d2 in process_upcall ofproto/ofproto-dpif-upcall.c:1414
> # 6 0x4d29f3 in recv_upcalls ofproto/ofproto-dpif-upcall.c:833
> # 7 0x4d1ee1 in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:750
> # 8 0x795aa2 in ovsthread_wrapper lib/ovs-thread.c:383
> # 9 0x7f8a57a59431 in start_thread (/lib64/libpthread.so.0+0x9431)
>
> Fixes: 79eadafeb1b4 ("ofproto: Do not delete datapath flows on exit by
> default.")
> Reported-by: Dumitru Ceara
> Signed-off-by: Ilya Maximets
> ---
> ofproto/ofproto-dpif-upcall.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 5fae46adf..ccf97266c 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -491,6 +491,11 @@ udpif_destroy(struct udpif *udpif)
> dpif_register_upcall_cb(udpif->dpif, NULL, udpif);
>
> for (int i = 0; i < N_UMAPS; i++) {
> +struct udpif_key *ukey;
> +
> +CMAP_FOR_EACH (ukey, cmap_node, &udpif->ukeys[i].cmap) {
> +ukey_delete__(ukey);
> +}
> cmap_destroy(&udpif->ukeys[i].cmap);
> ovs_mutex_destroy(&udpif->ukeys[i].mutex);
> }
>
William, if you have some time, could you, please, take a look at this patch?
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix ukey leak on udpif destroy.
Since commit 79eadafeb1b4 udpif_stop_threads() doesn't delete datapath
flows while called from udpif_destroy(). This means that ukeys are
not cleaned up either. So, hash maps in udpif->ukeys[] might still
contain valid pointers to ukeys that should be destroyed before
destroying the hash map itself:
==2783089==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 1560 byte(s) in 1 object(s) allocated from:
# 0 0x7f8a57eae667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667)
# 1 0x8411f6 in xmalloc lib/util.c:138
# 2 0x4d8a52 in ukey_create__ ofproto/ofproto-dpif-upcall.c:1682
# 3 0x4d99e3 in ukey_create_from_upcall ofproto/ofproto-dpif-upcall.c:1751
# 4 0x4d517d in upcall_xlate ofproto/ofproto-dpif-upcall.c:1242
# 5 0x4d63d2 in process_upcall ofproto/ofproto-dpif-upcall.c:1414
# 6 0x4d29f3 in recv_upcalls ofproto/ofproto-dpif-upcall.c:833
# 7 0x4d1ee1 in udpif_upcall_handler ofproto/ofproto-dpif-upcall.c:750
# 8 0x795aa2 in ovsthread_wrapper lib/ovs-thread.c:383
# 9 0x7f8a57a59431 in start_thread (/lib64/libpthread.so.0+0x9431)
Fixes: 79eadafeb1b4 ("ofproto: Do not delete datapath flows on exit by
default.")
Reported-by: Dumitru Ceara
Signed-off-by: Ilya Maximets
---
ofproto/ofproto-dpif-upcall.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 5fae46adf..ccf97266c 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -491,6 +491,11 @@ udpif_destroy(struct udpif *udpif)
dpif_register_upcall_cb(udpif->dpif, NULL, udpif);
for (int i = 0; i < N_UMAPS; i++) {
+struct udpif_key *ukey;
+
+CMAP_FOR_EACH (ukey, cmap_node, &udpif->ukeys[i].cmap) {
+ukey_delete__(ukey);
+}
cmap_destroy(&udpif->ukeys[i].cmap);
ovs_mutex_destroy(&udpif->ukeys[i].mutex);
}
--
2.25.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
