> Currently, it does so. The current code unconditionally calls 
> OvsClearTunTxCtx() on success and failure paths. Since the change you are 
> adds a new return point, we just need to add a call to OvsClearTunTxCtx() in 
> that case as well. Maybe a comment will also be great.

This logic has to  be revisited. OvsTunnelPortTx completes the NBL, However, 
its caller OvsOutputForwardingCtx completes the NBL if the caller fails before 
calling  OvsTunnelPortTx. It makes sense that the caller would always completes 
the NBL if the call fails (or the other way, not both).

Also, if OvsTunnelPortTx fails and clear the  tunKey through calling 
OvsClearTunTxCtx it will asserts once it returns:


        status = OvsTunnelPortTx(ovsFwdCtx);
        ASSERT(ovsFwdCtx->tunnelTxNic == NULL);
        ASSERT(ovsFwdCtx->tunKey.dst == 0);


At the least the caller should not asserts if OvsTunnelPortTx fails.

Thanks.
Eitan 

-----Original Message-----
From: Nithin Raju 
Sent: Thursday, July 31, 2014 5:30 PM
To: Eitan Eliahu
Cc: <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH] datapath-windows - fix crash when internal port 
is removed


On Jul 31, 2014, at 5:22 PM, Eitan Eliahu <elia...@vmware.com>
 wrote:

> 3. If clearing the context from will be  removed from 
> OvsCompleteNBLForwardingCtx then we need to call OvsClearTunCtx() on all 
> error paths of this function. Apply it only to this path does not buy 
> anything here.

Currently, it does so. The current code unconditionally calls 
OvsClearTunTxCtx() on success and failure paths. Since the change you are adds 
a new return point, we just need to add a call to OvsClearTunTxCtx() in that 
case as well. Maybe a comment will also be great.

> 4. This state is caused by misconfiguration. This should be blocked in user 
> mode so I don't see a lot of meaning for incrementing a counter.

OK.

> A better approach would be just to fail the driver Attach callback but I 
> would defer it for later. This change is just to prevent the BSOD.

Sure.

thanks,
-- Nithin
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to