On 23 Oct 2025, at 4:24, LIU Yulong wrote: > Maybe we should use the patch I uploaded 19 month ago: > https://mail.openvswitch.org/pipermail/ovs-dev/2024-March/412491.html > > > It did solve the issue for about 2 years.
First of all, sorry for my previous patch, it doesn’t actually fix anything; I misinterpreted some of the code. :( As mentioned in the comment, that 2 year old patch might not address the root cause of the problem. I’ve created some debug patches that you might want to try out to see if they provide any clues about what’s really going on. Note that this is based on a general debug library I’ve been using for a while. It does use a mutex to lock the circular buffer, but I hope that’s not enough to make the problem go away entirely. Anyway, it will log quite a bit of information, and you can use the GDB macros to dump the buffer once it has crashed. Make sure to build OVS with libunwind, otherwise, the callback trace won’t be useful. You can find the code here: https://github.com/chaudron/ovs/tree/refs/heads/dev/ec_dbg Here’s an example of how to dump the buffer from GDB: (gdb) source ovs_gdb.py (gdb) ovs_dump_ec_debug Dumping EC_DEBUG_BUFFER (Total Size: 536870912, Used: 258324): 3373100819104|handler11|||: time: 2025-10-27 10:59:11.457 3373100819104|handler11|ofproto/ofproto-dpif-upcall.c:1875|ukey_create__()[4462481]<-process_upcall()[4464430]<-recv_upcalls()[4469457]<-udpif_upcall_handler()[4470863]:ukey[0x7fd11c0055a0][1c2a11dd-9300-4dec-8f67-d85a70b5695f]create 3373100819881|handler11|ofproto/ofproto-dpif-upcall.c:2040|ukey_install__()[4467798]<-recv_upcalls()[4469906]<-udpif_upcall_handler()[4470863]<-ovsthread_wrapper()[5327791]:ukey[0x7fd11c0055a0][1c2a11dd-9300-4dec-8f67-d85a70b5695f]install/insert 3373100820426|handler11|ofproto/ofproto-dpif-upcall.c:2055|transition_ukey_at()[4465347]<-ukey_install__()[4467832]<-recv_upcalls()[4469906]<-udpif_upcall_handler()[4470863]:ukey[0x7fd11c0055a0][1c2a11dd-9300-4dec-8f67-d85a70b5695f]state 0 -> 1 3373100820972|handler11|ofproto/ofproto-dpif-upcall.c:2055|transition_ukey_at()[4465347]<-recv_upcalls()[4470738]<-udpif_upcall_handler()[4470863]<-ovsthread_wrapper()[5327791]:ukey[0x7fd11c0055a0][1c2a11dd-9300-4dec-8f67-d85a70b5695f]state 1 -> 2 3373100823070|handler6|ofproto/ofproto-dpif-upcall.c:1875|ukey_create__()[4462481]<-process_upcall()[4464430]<-recv_upcalls()[4469457]<-udpif_upcall_handler()[4470863]:ukey[0x7fd130004e30][5483ed11-6aef-4efd-a1f2-6dacfa1b3bda]create 3373100823061|handler11|ofproto/ofproto-dpif-upcall.c:1875|ukey_create__()[4462481]<-process_upcall()[4464430]<-recv_upcalls()[4469457]<-udpif_upcall_handler()[4470863]:ukey[0x7fd11c005e20][2b2a41bb-942a-4674-a995-94b210a0d09d]create Cheers, Eelco > > > ------------------ Original ------------------ > From: "LIU Yulong"<[email protected]>; > Date: Wed, Oct 22, 2025 06:03 PM > To: "liuyulong"<[email protected]>; "Eelco > Chaudron"<[email protected]>; > Cc: "dev"<[email protected]>; > Subject: Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Add ovsrcu_postpone > for ukey_delete__. > > > Updates: > 2. The change of function `upcall_receive` did not solve the issuse, > ovs-vswitchd still get cored: > #0 0x00007f7bf21ca337 in __GI_raise (sig=sig@entry=6) at > ../nptl/sysdeps/unix/sysv/linux/raise.c:55 > #1 0x00007f7bf21cba28 in __GI_abort () at abort.c:90 > #2 0x000055811c97cc6e in ovs_abort_valist (err_no=<optimized out>, > format=<optimized out>, args=args@entry=0x7f7bdfffa360) at lib/util.c:499 > #3 0x000055811c97cd04 in ovs_abort (err_no=err_no@entry=0, > format=format@entry=0x55811cddaec0 "%s: %s() passed uninitialized ovs_mutex") > at lib/util.c:491 > #4 0x000055811c947a81 in ovs_mutex_trylock_at > (l_=l_@entry=0x7f7bc9a751f8, where=where@entry=0x55811cdb7e78 > "ofproto/ofproto-dpif-upcall.c:3027") at lib/ovs-thread.c:106 > #5 0x000055811c86f4f1 in revalidator_sweep__ > (revalidator=revalidator@entry=0x558120f6df00, purge=purge@entry=false) at > ofproto/ofproto-dpif-upcall.c:3027 > #6 0x000055811c873516 in revalidator_sweep (revalidator=0x558120f6df00) > at ofproto/ofproto-dpif-upcall.c:3085 > #7 udpif_revalidator (arg=0x558120f6df00) at > ofproto/ofproto-dpif-upcall.c:1093 > #8 0x000055811c94863f in ovsthread_wrapper (aux_=<optimized out>) at > lib/ovs-thread.c:422 > #9 0x00007f7bf4321e65 in start_thread (arg=0x7f7bdffff700) at > pthread_create.c:307 > #10 0x00007f7bf229288d in clone () at > ../sysdeps/unix/sysv/linux/x86_64/clone.S:111 > > > ------------------ Original ------------------ > From: "LIU Yulong"<[email protected]>; > Date: Wed, Oct 22, 2025 01:54 PM > To: "Eelco Chaudron"<[email protected]>; > Cc: "dev"<[email protected]>; > Subject: Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Add ovsrcu_postpone for > ukey_delete__. > > > Updates: > 1. The change of function `upcall_uninit` did not solve the issue, > ovs-vswitchd can still run cored from call `ukey_delete(umap, ukey);` in the > `revalidator_sweep__`. > 2. The change of function `upcall_receive` was applied to another host, and > we do not see core issue for 24h. We need to run it for a longer period of > time to verify. > > > > > ------------------ Original ------------------ > From: "Eelco Chaudron"<[email protected]>; > Date: Mon, Oct 20, 2025 07:04 PM > To: "LIU Yulong"<[email protected]>; > Cc: "dev"<[email protected]>; > Subject: Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Add ovsrcu_postpone > for ukey_delete__. > > > On 20 Oct 2025, at 12:35, LIU Yulong wrote: > > > Thank you Eelco. > > > > > > Code search shows we have `recv_upcalls` and `upcall_cb` which will call > `upcall_uninit`. > > And dp_netdev_upcall will call the dp->upcall_cb. > > So we have call stacks like this: > > i) > handle_packet_upcall->dp_netdev_upcall->upcall_cb->upcall_uninit > > ii) > dp_execute_userspace_action->dp_netdev_upcall->upcall_cb->upcall_uninit > > > > Cloud you confirm these calls? > > From the top of my head, this is correct. However, the new ukey structure is > never inserted, so we do not need the RCU-delayed remove. > > > For your change, I'll run tests with recoreded packets to verify. > > Thanks, and let me know the results. > > //Eelco > > > > > Regards, > > > > > > LIU Yulong > > > > > > ------------------ Original ------------------ > > From: "Eelco Chaudron"<[email protected]>; > > Date: Fri, Oct 17, 2025 08:08 PM > > To: "LIU Yulong"<[email protected]>; > > Cc: "dev"<[email protected]>; > > Subject: Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: Add > ovsrcu_postpone for ukey_delete__. > > > > > > Hi Liu, > > > > I looked at the change; however, upcall_uninit() is only called for > newly created (never inserted) ukeys, so the ovs_postpone() call is not > needed. > > > > However, I did find an issue in upcall_receive(), where, in an error > path, it could use an uninitialized upcall structure — causing a ukey to be > freed that should not have been. > > > > Can you try out the diff below to see if it fixes your problem? > > > > Cheers, > > > > Eelco > > > > diff --git a/ofproto/ofproto-dpif-upcall.c > b/ofproto/ofproto-dpif-upcall.c > > index b3b4b2d2f..53b906a16 100644 > > --- a/ofproto/ofproto-dpif-upcall.c > > +++ b/ofproto/ofproto-dpif-upcall.c > > @@ -1230,6 +1230,17 @@ upcall_receive(struct upcall *upcall, const > struct dpif_backer *backer, > > { > > int error; > > > > + /* Initialize the minimal required fields in the upcall > structure to ensure > > + * upcall_uninit() does not operate on invalid data. > */ > > + upcall->have_recirc_ref = false; > > + upcall->xout_initialized = false; > > + upcall->ukey_persists = false; > > + upcall->ukey = NULL; > > + ofpbuf_use_stub(&upcall->odp_actions, > upcall->odp_actions_stub, > > > + > sizeof upcall->odp_actions_stub); > > + ofpbuf_init(&upcall->put_actions, 0); > > + > > + > > upcall->type = classify_upcall(type, userdata, > &upcall->cookie); > > if (upcall->type == BAD_UPCALL) { > > return EAGAIN; > > @@ -1258,19 +1269,11 @@ upcall_receive(struct upcall *upcall, const > struct dpif_backer *backer, > > } > > > > upcall->recirc = NULL; > > - upcall->have_recirc_ref = false; > > upcall->flow = flow; > > upcall->packet = packet; > > upcall->ufid = ufid; > > upcall->pmd_id = pmd_id; > > - ofpbuf_use_stub(&upcall->odp_actions, > upcall->odp_actions_stub, > > > - > sizeof upcall->odp_actions_stub); > > - ofpbuf_init(&upcall->put_actions, 0); > > > > - upcall->xout_initialized = false; > > - upcall->ukey_persists = false; > > - > > - upcall->ukey = NULL; > > upcall->key = NULL; > > upcall->key_len = 0; > > upcall->mru = mru; > > > > > > On 16 Oct 2025, at 3:12, LIU Yulong wrote: > > > > > We have such call stack of coredump: > > > *0 0x00007f7f197ae337 in raise () from /lib64/libc.so.6 > > > *1 0x00007f7f197afa28 in abort () from /lib64/libc.so.6 > > > *2 0x000055934ca4f4ee in ovs_abort_valist (err_no=<optimized > out>, format=<optimized out>, args=args@entry=0x7f7f07530360) at > lib/util.c:499 > > > *3 0x000055934ca4f584 in ovs_abort (err_no=err_no@entry=0, > format=format@entry=0x55934ccedd18 "%s: %s() passed uninitialized ovs_mutex") > at lib/util.c:491 > > > *4 0x000055934ca1a4a1 in ovs_mutex_trylock_at > (l_=l_@entry=0x7f7ed4a43e58, where=where@entry=0x55934cccb318 > "ofproto/ofproto-dpif-upcall.c:3014") at lib/ovs-thread.c:106 > > > *5 0x000055934c943181 in revalidator_sweep__ > (revalidator=revalidator@entry=0x5593518c1720, purge=purge@entry=false) at > ofproto/ofproto-dpif-upcall.c:3014 > > > *6 0x000055934c9471a6 in revalidator_sweep > (revalidator=0x5593518c1720) at ofproto/ofproto-dpif-upcall.c:3072 > > > *7 udpif_revalidator (arg=0x5593518c1720) at > ofproto/ofproto-dpif-upcall.c:1086 > > > *8 0x000055934ca1b05f in ovsthread_wrapper (aux_=<optimized > out>) at lib/ovs-thread.c:422 > > > *9 0x00007f7f1b6ece65 in start_thread () from > /lib64/libpthread.so.0 > > > *10 0x00007f7f1987688d in clone () from /lib64/libc.so.6 > > > > > > When calling ovs_mutex_trylock() on ukey->mutex, > ovs_mutex_trylock_at > > > sees that the input is an "uninitialized ovs_mutex" (l->where is > NULL), > > > and aborts. > > > > > > This state can only occur after the mutex has not been initialized > or > > > has been destroyed. The mutex is definitely initialized in > ukey_create__ > > > by ukey, so the "uninitialized" state is almost certainly > "destroyed". > > > Destruction occurs in ukey_delete__, which calls > ovs_mutex_destroy(&ukey->mutex) > > > and sets where to NULL. > > > > > > When revalidator_sweep__ is traversing cmap and trying to lock > (&ukey->mutex), > > > it encounters a ukey that has been directly destroyed by > ukey_delete__, > > > However, the ukey is still visible to the revalidator (either still > in > > > the cmap or has not yet passed the RCU grace period), resulting in > an > > > abort. That is to say, there is a path where ukey_delete__ is > directly > > > called during concurrent traversal of ukey, bypassing the > > > cmap_remove + ovsrcu_postpone semantics of ukey_delete. > > > > > > Modify upcall_uninit() to change direct ukey_delete__ to RCU > deferred > > > release to avoid concurrent traversal conflicts with revalidator. > > > This ensures that ukey_delete__ is not executed until after the > > > global grace period, and that the CMAP_FOR_EACH within > > > revalidator_sweep__ will not encounter a destroyed mutex before > > > the end of running cycle. > > > > > > Some earlier email discussions: > > > [1] > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052973.html > > > [2] > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-February/052949.html > > > [3] > https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052993.html > > > > > > Signed-off-by: LIU Yulong <[email protected]> > > > --- > > > ofproto/ofproto-dpif-upcall.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/ofproto/ofproto-dpif-upcall.c > b/ofproto/ofproto-dpif-upcall.c > > > index 9dfa52d82..b3b4b2d2f 100644 > > > --- a/ofproto/ofproto-dpif-upcall.c > > > +++ b/ofproto/ofproto-dpif-upcall.c > > > @@ -1386,7 +1386,7 @@ upcall_uninit(struct upcall *upcall) > > > > ofpbuf_uninit(&upcall->put_actions); > > > if > (upcall->ukey) { > > > > if (!upcall->ukey_persists) { > > > > - > ukey_delete__(upcall->ukey); > > > > + > ovsrcu_postpone(ukey_delete__, upcall->ukey); > > > > } > > > } else if > (upcall->have_recirc_ref) { > > > > /* The reference was transferred to the ukey if one was created. */ > > > -- > > > 2.50.1 (Apple Git-155) > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
