Hi Sorin, Although the patch by itself can't hurt but I would suggest that we would root cause the issue of not freeing WFP object on error conditions. Thank you, Eitan
-----Original Message----- From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] Sent: Monday, June 22, 2015 10:26 AM To: Eitan Eliahu Cc: Nithin Raju; dev@openvswitch.org Subject: RE: [PATCH v2] datapath-windows: Return success for already existing WFP objects Hi Eitan/Nithin, In normal execution flow all WFP objects are released after the extension is disabled. The patch was meant to address the cases when the extension was not properly disabled due to some unforeseen error, which does not deletes all WFP objects like the OVS sublayer or callout. Many times I have seen an improper activation of the OVS extension because the sublayer or callout already existed. However I have not been able to reproduce the problem with the latest code from the master branch. The extension is much more stable with the latest patches and I cannot provide reproduction steps for this issue. -Sorin -----Original Message----- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Saturday, 20 June, 2015 15:35 To: Sorin Vinturis; dev@openvswitch.org Subject: RE: [PATCH v2] datapath-windows: Return success for already existing WFP objects Hi Sorin, Could you please let us know what are the cases where these object are already existed? I just want to be sure that we free all these objects when stopping the extension. Thanks, Eitan -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Thursday, June 18, 2015 11:37 AM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH v2] datapath-windows: Return success for already existing WFP objects There are cases when the WFP callout or sublayer, being persistent objects, already exists when we try to register the OVS callout. In this cases, when trying to add again these WFP objects the return code is STATUS_FWP_ALREADY_EXISTS, which we are interpreting as an error. This is incorrect and this patch changes that. Signed-off-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> Reported-by: Sorin Vinturis <svintu...@cloudbasesolutions.com> Reported-at: https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_84&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=2m9yV1dqLYwW4Lrbg7_G7J56pKu7tJ_TE125ybtwKTA&s=bUUf7r2FXP4oMOFNjo2zKOSwbuJHfFjLSq2Pe8yz0sM&e= --- datapath-windows/ovsext/TunnelFilter.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/datapath-windows/ovsext/TunnelFilter.c b/datapath-windows/ovsext/TunnelFilter.c index 08cc13f..231750e 100644 --- a/datapath-windows/ovsext/TunnelFilter.c +++ b/datapath-windows/ovsext/TunnelFilter.c @@ -414,7 +414,12 @@ OvsTunnelRegisterDatagramDataCallouts(const GUID *layerKey, status = FwpmCalloutAdd(gEngineHandle, &mCallout, NULL, NULL); if (!NT_SUCCESS(status)) { - goto Exit; + if (STATUS_FWP_ALREADY_EXISTS != status) { + OVS_LOG_ERROR("Failed to add WFP callout, status: %x.", + status); + goto Exit; + } + status = STATUS_SUCCESS; } Exit: @@ -459,7 +464,11 @@ OvsTunnelRegisterCallouts(VOID *deviceObject) status = FwpmSubLayerAdd(gEngineHandle, &OvsTunnelSubLayer, NULL); if (!NT_SUCCESS(status)) { - goto Exit; + if (STATUS_FWP_ALREADY_EXISTS != status) { + OVS_LOG_ERROR("Failed to add WFP sublayer, status: %x.", + status); + goto Exit; + } } /* In order to use this callout a socket must be opened. */ -- 1.9.0.msysgit.0 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=2m9yV1dqLYwW4Lrbg7_G7J56pKu7tJ_TE125ybtwKTA&s=1h_FXA4x7kLJqZuawSWu_SwiMnBCMlk29lqJ5ldr1Tk&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
