Hi Eitan, This change and the other one that you have mentioned were required because the base filtering engine (BFE) is not running at the time the OVS extension is initialized, and DriverEntry is called, nor at FilterAttach. At driver initialization phase the system provider is registered and at filter attach phase the tunnel filter is initialized, meaning the tunnel callout is added and registered. Because the BFE is not running when the two previous actions takes place, I registered callback functions that are called whenever there is a change to the state of the filter engine, using the 'FwpmBfeStateSubscribeChanges' function. The callback functions are called independently and in the order that were registered. Both callbacks are successfully executed and no errors happen. A log message is added if any error appears.
This patch just logs the errors that appear when tunnel filter is initialize. I haven't worked out a way to communicate the eventual errors to the driver and put the driver in a failure state. But the patch solves the reported issue and we can let the error reporting issue for a subsequent patch. What do you think? Thanks, Sorin -----Original Message----- From: Eitan Eliahu [mailto:elia...@vmware.com] Sent: Tuesday, 14 April, 2015 00:33 To: Sorin Vinturis; dev@openvswitch.org Subject: RE: [PATCH] datapath-windows: extension fails to be enabled Hi Sorin, I am somewhat confused about this change. Have you concluded that the filter could not be initialized unless a provider is already registered? I am trying to understand the relationships between this change and the change you check in before (also, to address the issue that the BFE had not started yet). On another issue: I don't see how the filter failure to initialize is communicated to the driver. I think we need to log a message in the system log and also to put the driver in a failure state. I'm concerned that a callback will be registered twice (On Driverentry as weel on Attach). Please let us know which issue you are trying to address with this change. Thanks, Eitan -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis Sent: Thursday, April 09, 2015 5:20 AM To: dev@openvswitch.org Subject: [ovs-dev] [PATCH] datapath-windows: extension fails to be enabled The extension failed to be activated during booting due to the failure to initialize tunnel filter. This happened because the Base Filtering Engine (BFE) is not started and no session to the engine could be acquired. The solution for this was to registered a BFE notification callback that is called whenever the BFE's state changes. Only if the BFE's state is running the tunnel filter is initialized. Also, I have removed an inappropriate assert from the FilterNetPnPEvent routine, OvsExtNetPnPEvent. When NDIS calls the FilterNetPnPEvent routine, the extension is in paused state and, obviously, the switch is not active. The switch becomes active after FilterRestart routine is called and the restart is successfully complete. 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_77&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=dBFLkhnGUaFy8lz8z0hjyZlLKcbd5GfUsaJezXD6AYw&s=Z1vNsPvCGNTMUH4_qkVTuNa-Xt23vnYCHeLHJ--o44o&e= --- datapath-windows/ovsext/Switch.c | 12 ++-- datapath-windows/ovsext/TunnelFilter.c | 117 ++++++++++++++++++++++++++++----- datapath-windows/ovsext/TunnelIntf.h | 4 +- 3 files changed, 105 insertions(+), 28 deletions(-) diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c index 61a4531..cfbb5a0 100644 --- a/datapath-windows/ovsext/Switch.c +++ b/datapath-windows/ovsext/Switch.c @@ -41,6 +41,7 @@ UINT64 ovsTimeIncrementPerTick; extern PNDIS_SPIN_LOCK gOvsCtrlLock; extern NDIS_HANDLE gOvsExtDriverHandle; extern NDIS_HANDLE gOvsExtDriverObject; +extern PDEVICE_OBJECT gOvsDeviceObject; static NDIS_STATUS OvsCreateSwitch(NDIS_HANDLE ndisFilterHandle, POVS_SWITCH_CONTEXT *switchContextOut); @@ -202,12 +203,8 @@ OvsCreateSwitch(NDIS_HANDLE ndisFilterHandle, goto create_switch_done; } - status = OvsTunnelFilterInitialize(gOvsExtDriverObject); - if (status != NDIS_STATUS_SUCCESS) { - OvsUninitSwitchContext(switchContext); - OvsFreeMemoryWithTag(switchContext, OVS_SWITCH_POOL_TAG); - goto create_switch_done; - } + OvsInitTunnelFilter(gOvsExtDriverObject, gOvsDeviceObject); + *switchContextOut = switchContext; create_switch_done: @@ -261,7 +258,7 @@ OvsDeleteSwitch(POVS_SWITCH_CONTEXT switchContext) if (switchContext) { dpNo = switchContext->dpNo; - OvsTunnelFilterUninitialize(gOvsExtDriverObject); + OvsUninitTunnelFilter(gOvsExtDriverObject); OvsClearAllSwitchVports(switchContext); OvsUninitSwitchContext(switchContext); OvsFreeMemoryWithTag(switchContext, OVS_SWITCH_POOL_TAG); @@ -526,7 +523,6 @@ OvsExtNetPnPEvent(NDIS_HANDLE filterModuleContext, switchContext->isActivateFailed = TRUE; } else { ASSERT(switchContext->isActivated == FALSE); - ASSERT(switchActive == TRUE); if (switchContext->isActivated == FALSE && switchActive == TRUE) { status = OvsActivateSwitch(switchContext); OVS_LOG_TRACE("OvsExtNetPnPEvent: activated switch: %p " diff --git a/datapath-windows/ovsext/TunnelFilter.c b/datapath-windows/ovsext/TunnelFilter.c index 4b879c0..e54ecff 100644 --- a/datapath-windows/ovsext/TunnelFilter.c +++ b/datapath-windows/ovsext/TunnelFilter.c @@ -111,7 +111,8 @@ DEFINE_GUID( PDEVICE_OBJECT gDeviceObject; HANDLE gEngineHandle = NULL; -HANDLE gBfeSubscriptionHandle = NULL; +HANDLE gTunnelProviderBfeHandle = NULL; HANDLE gTunnelInitBfeHandle = +NULL; UINT32 gCalloutIdV4; @@ -547,8 +548,8 @@ Exit: } VOID NTAPI -OvsBfeStateChangeCallback(PVOID context, - FWPM_SERVICE_STATE bfeState) +OvsTunnelProviderBfeCallback(PVOID context, + FWPM_SERVICE_STATE bfeState) { HANDLE handle = NULL; @@ -564,19 +565,19 @@ OvsBfeStateChangeCallback(PVOID context, } NTSTATUS -OvsSubscribeBfeStateChanges(PVOID deviceObject) +OvsSubscribeTunnelProviderBfeStateChanges(PVOID deviceObject) { NTSTATUS status = STATUS_SUCCESS; - if (!gBfeSubscriptionHandle) { + if (!gTunnelProviderBfeHandle) { status = FwpmBfeStateSubscribeChanges(deviceObject, - OvsBfeStateChangeCallback, + + OvsTunnelProviderBfeCallback, NULL, - &gBfeSubscriptionHandle); + + &gTunnelProviderBfeHandle); if (!NT_SUCCESS(status)) { OVS_LOG_ERROR( - "Failed to open subscribe BFE state change callback, status: %x.", - status); + "Failed to subscribe BFE tunnel provider callback,\ + status: %x.", status); } } @@ -584,18 +585,18 @@ OvsSubscribeBfeStateChanges(PVOID deviceObject) } VOID -OvsUnsubscribeBfeStateChanges() +OvsUnsubscribeTunnelProviderBfeStateChanges() { NTSTATUS status = STATUS_SUCCESS; - if (gBfeSubscriptionHandle) { - status = FwpmBfeStateUnsubscribeChanges(gBfeSubscriptionHandle); + if (gTunnelProviderBfeHandle) { + status = + FwpmBfeStateUnsubscribeChanges(gTunnelProviderBfeHandle); if (!NT_SUCCESS(status)) { OVS_LOG_ERROR( - "Failed to open unsubscribe BFE state change callback, status: %x.", - status); + "Failed to unsubscribe BFE tunnel provider callback,\ + status: %x.", status); } - gBfeSubscriptionHandle = NULL; + gTunnelProviderBfeHandle = NULL; } } @@ -604,7 +605,7 @@ VOID OvsRegisterSystemProvider(PVOID deviceObject) NTSTATUS status = STATUS_SUCCESS; HANDLE handle = NULL; - status = OvsSubscribeBfeStateChanges(deviceObject); + status = OvsSubscribeTunnelProviderBfeStateChanges(deviceObject); if (NT_SUCCESS(status)) { if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { OvsTunnelEngineOpen(&handle); @@ -613,7 +614,7 @@ VOID OvsRegisterSystemProvider(PVOID deviceObject) } OvsTunnelEngineClose(&handle); - OvsUnsubscribeBfeStateChanges(); + OvsUnsubscribeTunnelProviderBfeStateChanges(); } } } @@ -628,5 +629,85 @@ VOID OvsUnregisterSystemProvider() } OvsTunnelEngineClose(&handle); - OvsUnsubscribeBfeStateChanges(); + OvsUnsubscribeTunnelProviderBfeStateChanges(); +} + +VOID NTAPI +OvsTunnelInitBfeCallback(PVOID context, + FWPM_SERVICE_STATE bfeState) { + NTSTATUS status = STATUS_SUCCESS; + PDRIVER_OBJECT driverObject = (PDRIVER_OBJECT) context; + + if (FWPM_SERVICE_RUNNING == bfeState) { + status = OvsTunnelFilterInitialize(driverObject); + if (!NT_SUCCESS(status)) { + OVS_LOG_ERROR( + "Failed to initialize tunnel filter, status: %x.", + status); + } + } +} + +NTSTATUS +OvsSubscribeTunnelInitBfeStateChanges(PVOID deviceObject, + PDRIVER_OBJECT driverObject) { + NTSTATUS status = STATUS_SUCCESS; + + if (!gTunnelInitBfeHandle) { + status = FwpmBfeStateSubscribeChanges(deviceObject, + OvsTunnelInitBfeCallback, + driverObject, + &gTunnelInitBfeHandle); + if (!NT_SUCCESS(status)) { + OVS_LOG_ERROR( + "Failed to subscribe BFE tunnel init callback,\ + status: %x.", status); + } + } + + return status; +} + +VOID +OvsUnsubscribeTunnelInitBfeStateChanges() +{ + NTSTATUS status = STATUS_SUCCESS; + + if (gTunnelInitBfeHandle) { + status = FwpmBfeStateUnsubscribeChanges(gTunnelInitBfeHandle); + if (!NT_SUCCESS(status)) { + OVS_LOG_ERROR( + "Failed to unsubscribe BFE tunnel init callback,\ + status: %x.", status); + } + gTunnelInitBfeHandle = NULL; + } +} + +VOID OvsInitTunnelFilter(PDRIVER_OBJECT driverObject, PVOID +deviceObject) { + NTSTATUS status = STATUS_SUCCESS; + + status = OvsSubscribeTunnelInitBfeStateChanges(deviceObject, driverObject); + if (NT_SUCCESS(status)) { + if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) { + status = OvsTunnelFilterInitialize(driverObject); + if (!NT_SUCCESS(status)) { + /* XXX: We need to decide what actions to take in case of + * failure to initialize tunnel filter. */ + OVS_LOG_ERROR( + "Failed to initialize tunnel filter, status: %x.", + status); + } + OvsUnsubscribeTunnelInitBfeStateChanges(); + } + } +} + +VOID OvsUninitTunnelFilter(PDRIVER_OBJECT driverObject) { + OvsTunnelFilterUninitialize(driverObject); + OvsUnsubscribeTunnelInitBfeStateChanges(); } diff --git a/datapath-windows/ovsext/TunnelIntf.h b/datapath-windows/ovsext/TunnelIntf.h index 728a53f..346307e 100644 --- a/datapath-windows/ovsext/TunnelIntf.h +++ b/datapath-windows/ovsext/TunnelIntf.h @@ -18,9 +18,9 @@ #define __TUNNEL_INTF_H_ 1 /* Tunnel callout driver load/unload functions */ -NTSTATUS OvsTunnelFilterInitialize(PDRIVER_OBJECT driverObject); +VOID OvsInitTunnelFilter(PDRIVER_OBJECT driverObject, PVOID +deviceObject); -VOID OvsTunnelFilterUninitialize(PDRIVER_OBJECT driverObject); +VOID OvsUninitTunnelFilter(PDRIVER_OBJECT driverObject); VOID OvsRegisterSystemProvider(PVOID deviceObject); -- 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=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=dBFLkhnGUaFy8lz8z0hjyZlLKcbd5GfUsaJezXD6AYw&s=fsFLbMFU7sNv9bFvgkpQX7SiTS4bXfaFynSHj2j3MbQ&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
