Hi Nithin,

Please see my answers inline.

Thanks,
Sorin

-----Original Message-----
From: Nithin Raju [mailto:nit...@vmware.com] 
Sent: Monday, 13 April, 2015 20:55
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] datapath-windows: extension fails to be enabled

> On Apr 9, 2015, at 5:20 AM, Sorin Vinturis <svintu...@cloudbasesolutions.com> 
> wrote:
> 
> 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.

Sorin,
Would you mind splitting up the patch into two parts since they are not related 
to each other.
SV: I'll do that.

> @@ -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 
> “

I look at the change in FilterNetPnPEvent callback. Your change looks good, but 
I am also wondering if we should remove the ‘switchActive == TRUE’ check in the 
‘if()’ condition.
SV: The FilterNetPnPEvent callback is called for a "NetEventSwitchActivate" 
NetPnpEvent notification just after FilterAttach routine call and before 
FilterRestart. Looking at the "Module States of a Filter Driver" picture, 
https://msdn.microsoft.com/en-us/library/windows/hardware/ff560558(v=vs.85).aspx,
 we can conclude that when NetPnpEvent notification arrives the module is in 
paused state. Thus I think that we cannot remove the switchActive check from 
the condition you have specified.

The ASSERT() is active only on debug builds anyway.

I looked at the documentation for “FilterNetPnPEvent”:
https://msdn.microsoft.com/en-us/library/ff549962(v=vs.85).aspx

And couldn’t figure out why the switch mini port would call the 
‘FilterNetPnPEvent’ function. I set a breakpoint on the function and activated 
the switch, and didn’t see the breakpoint hit. Only the Attach and Restart 
breakpoints hit but not the FilterNetPnPEvent.

1. Did you see it hit?
SV: Yes, I saw when the ‘FilterNetPnPEvent’ function is called. Boot in 
Hyper-V, enable the extension and restart the machine. Then in the booting 
phase the extension is loaded, DriverEntry routine is called, then 
FilterAttach, FilterNetPnPEvent and then FilterRestart. This is the sequence of 
the calls.

2. If it was hit, was it hit after the “Restart” function? I don’t know if the 
current logic would work if it was hit after “Restart”. I looked the sample 
code provided by Microsoft, and they also have call to “init switch” without 
much checks for whether the “init switch” has already been called or not.
SV: No, the FilterRestart is called after the FilterNetPnPEvent call as I 
explained in the previous answer.

-- Nithin

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to