Hi Wilson, Apologies for the delay and thank you for your patch.
Its really hard for me to wrap my head around how this issue occurred. Would it be possible to reproduce the issue and run the !locks command inside WinDbg and provide the output? Thank you, Alin. ________________________________ From: dev <[email protected]> on behalf of Wilson Peng via dev <[email protected]> Sent: Thursday, June 6, 2024 3:47 PM To: Simon Horman <[email protected]>; [email protected] <[email protected]> Subject: Re: [ovs-dev] [PATCH v2 1/1] datapath-windows : Avoid a deadlock when processing TFTP conntrack. Hi, Simon, Below list why it would go into deadlock, 1). 1st place OvsCtExecute_()->OvsProcessConntrackEntry(). entry->parent = parentEntry;(here parentEntry == entry) 2). After it return back from OvsProcessConntrackEntry(), it goes to the lines below,(Still on function OvsCtExecute_()) 1263 if (entry == NULL) { 1264 return status; 1265 } 1266 1267 /* 1268 * Note that natInfo is not the same as entry->natInfo here. natInfo 1269 * is decided by action in the openflow rule, entry->natInfo is decided 1270 * when the entry is created. In the reverse NAT case, natInfo is 1271 * NAT_ACTION_REVERSE, yet entry->natInfo is NAT_ACTION_SRC or 1272 * NAT_ACTION_DST without NAT_ACTION_REVERSE 1273 */ 1274 KIRQL irql = KeGetCurrentIrql(); 1275 OVS_ACQUIRE_SPIN_LOCK(&(entry->lock), irql); ------>SPIN_LOCK 1st time 1276 if (natInfo->natAction != NAT_ACTION_NONE) { 1277 OvsNatPacket(fwdCtx, entry, entry->natInfo.natAction, 1278 key, ctx.reply); 1279 } 3, Then it will goto the next same SPIN_LOCK for then lines below(Still on function OvsCtExecute_()) 1340 /* Add original tuple information to flow Key */ 1341 if (entry->key.dl_type == ntohs(ETH_TYPE_IPV4)) { 1342 if (parent != NULL) { 1343 OVS_ACQUIRE_SPIN_LOCK(&(parent->lock), irql); --->>SPIN_LOCK 2nd time causing deadlock 1344 OvsCtUpdateTuple(key, &parent->key); 1345 OVS_RELEASE_SPIN_LOCK(&(parent->lock), irql); 1346 } else { 1347 OvsCtUpdateTuple(key, &entry->key); 1348 } ... 1365 OVS_RELEASE_SPIN_LOCK(&(entry->lock), irql); --->this is the SPIN_LOCK release place. Regards Wilson On Thu, Jun 6, 2024 at 8:56 PM Simon Horman <[email protected]> wrote: > On Wed, Jun 05, 2024 at 09:26:02PM +0800, Wilson Peng wrote: > > Hi, Simon, > > In this case, for tftp packet processing, it does have a child-parent > > processing logic just like ftp in tcp. > > Tftp packet1 from port1 to 69 and it will create one new conntrack entry > > and create one related conntrack > > entry also with src-port 0 and dst-port port1. Original idea is if the > > tftp reply packet is from port2 to port1 and > > then it will create one new conntrack entry then it could set this > > conntrack entry(port2->port1) parent be equal > > to conntrack_entry(port1-->69). > > > > In the processing part, if the tftp reply packet is from 69 to port1 > > then it would hit the original conntrack entry > > (port1-->69) and lead to conntrack_entry parent be equal to itself. > > > > Only ftp/tftp traffic processing will have parent-child logic. > > Hi Wilson, > > Thanks for the explanation. > > I understand that this patch will prevent OVS from crashing (good!). But > I'm unclear how flows that would have caused a crash will be handled by > conntrack with this patch present. Could you shed some light on that too? > -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. _______________________________________________ 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
