Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling

2015-03-23 Thread Sorin Vinturis
Hi Eitan,

Thanks for reviewing my patch.
[1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it 
receives before closing it.
[2] In order to retrieve the current state of the filter engine I am using the 
FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout 
driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason 
I am registering the callback no mater of the state of the filter engine. 
Only if the engine is running the provider is added by opening the engine 
handle first and closing it after the provider is added, then unsubscribes the 
callback.
If the engine is not running the callback is called and the provider is added 
only for the service running notification.

Hope this answers your comments.

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com] 
Sent: Monday, 23 March, 2015 02:32
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling


Hi Sorin,
Thank you for posting this change.
Here are few comments:
[1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. 
(you might want to add an assertion if it zero).
[2] I'm somewhat confused about the registration for engine state change. As I 
understand you check the state of the engine and if it is running you open the 
engine handle (in the context of the foreground thread). However, you subscribe 
to state change anyway and you open the handle to the engine from the callback 
function too.
Did I miss anything?
Thanks,
Eitan


-Original Message-

From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Monday, 16 March, 2015 20:08

To: dev@openvswitch.org

Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



If the Base Filtering Engine (BFE) is not started, the WFP system provider 
failed to be added because 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 
WFP system provider is added.



Signed-off-by: Sorin Vinturis 

Reported-by: Sorin Vinturis 

Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e=
 

---

 datapath-windows/ovsext/Datapath.c | 20 ++-

 datapath-windows/ovsext/TunnelFilter.c | 99 --

 datapath-windows/ovsext/TunnelIntf.h   |  8 +--

 3 files changed, 100 insertions(+), 27 deletions(-)



diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c

index 8eb13f1..c6fe89e 100644

--- a/datapath-windows/ovsext/Datapath.c

+++ b/datapath-windows/ovsext/Datapath.c

@@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock;  VOID

 OvsInit()

 {

-HANDLE handle = NULL;

-

 gOvsCtrlLock = &ovsCtrlLockObj;

 NdisAllocateSpinLock(gOvsCtrlLock);

 OvsInitEventQueue();

-

-OvsTunnelEngineOpen(&handle);

-if (handle) {

-OvsTunnelAddSystemProvider(handle);

-}

-OvsTunnelEngineClose(&handle);

 }

 

 VOID

 OvsCleanup()

 {

-HANDLE handle = NULL;

-

 OvsCleanupEventQueue();

 if (gOvsCtrlLock) {

 NdisFreeSpinLock(gOvsCtrlLock);

 gOvsCtrlLock = NULL;

 }

-

-OvsTunnelEngineOpen(&handle);

-if (handle) {

-OvsTunnelRemoveSystemProvider(handle);

-}

-OvsTunnelEngineClose(&handle);

 }

 

 VOID

@@ -448,6 +432,8 @@ OvsCreateDeviceObject(NDIS_HANDLE ovsExtDriverHandle)

 if (ovsExt) {

 ovsExt->numberOpenInstance = 0;

 }

+} else {

+OvsRegisterSystemProvider((PVOID)gOvsDeviceObject);

 }

 

 OVS_LOG_TRACE("DeviceObject: %p", gOvsDeviceObject); @@ -471,6 +457,8 @@ 
OvsDeleteDeviceObject()

 NdisDeregisterDeviceEx(gOvsDeviceHandle);

 gOvsDeviceHandle = NULL;

 gOvsDeviceObject = NULL;

+

+OvsUnregisterSystemProvider();

 }

 }

 

diff --git a/datapath-windows/ovsext/TunnelFilter.c 
b/datapath-windows/ovsext/TunnelFilter.c

index e0adc37..4b879c0 100644

--- a/datapath-windows/ovsext/TunnelFilter.c

+++ b/datapath-windows/ovsext/TunnelFilter.c

@@ -111,6 +111,7 @@ DEFINE_GUID(

 PDEVICE_OBJECT gDeviceObject;

 

 HANDLE gEngineHandle = NULL;

+HANDLE gBfeSubscriptionHandle = NULL;

 UINT32 gCalloutIdV4;

 

 

@@ -173,17 +174,20 @@ OvsTunnelAddSystemProvider(HANDLE handle)

 provider.displayData.name = OVS_TUNNEL_PROVIDER_NAME;

 provider.displayData.description = OVS_TUNNEL_PROVIDER_DESC;

 /*

-* Since we always want the provider to be present, it's easiest to add

-* it as persistent object during d

Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration

2015-03-23 Thread Gray, Mark D
> 
> > On 19 Mar 2015, at 18:34, Gray, Mark D  wrote:
> >
> >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Gray,
> >> Mark D
> >> Sent: Thursday, March 19, 2015 6:19 PM
> >> To: Traynor, Kevin; Daniele Di Proietto
> >> Subject: Re: [ovs-dev] [PATCH 0/6] DPDK: simplify configuration
> >>
> >> [snip]
> >
> > Hi, I've reviewed this patchset - few comments/questions on it…
> 
>  Hi Kevin, thanks
> 
> > I haven't tested yet - but I'm wondering what is the impact to the
> > dpdk -c parameter. Is it no longer used for OVS?
> >
> 
>  Yes, that’s correct. It should have no impact on OVS. I think we
>  should also provide a default, i.e. generate the parameters passed
>  to dpdk_eal_init at some point.
> >>
> >> +1 to removing the -c parameter as it seems to be the same as
> >> 'other_config:n-pmd-cores' and is a little confusing as to how they
> interact.
> >> Maybe 'other_config:n-pmd-cores' could be a mandatory option?
> >
> > I used the wrong config option in above. Let me rephrase it.
> > +1 to removing the -c parameter as it seems to be the same as
> > 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask' and is a
> little confusing as to how they interact.
> > Maybe 'other_config:pmd-cpu-mask + other_config:nonpmd-cpu-mask'
> could be a mandatory option?
> 
> The idea behind this change is to let advanced users configure their masks,
> and provide a simple configuration parameter (n-pmd-cores) for users who
> do not want or need to deal with cpu masks. What do you think?

Is the -c option needed in either of these cases?  In both cases, it could be
derived and would simplify the command line.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH] datapath-windows: Removed gOvsCtrlLock global spinlock

2015-03-23 Thread Sorin Vinturis
There is no need to use gOvsCtrlLock spinlock to guard the switch
context, as there is now the switch context's reference count used
for this purpose.

Now the gOvsCtrlLock spinlock guards only one shared resource, the
OVS_OPEN_INSTANCE global instance array.

Signed-off-by: Sorin Vinturis 
---
 datapath-windows/ovsext/Datapath.c | 15 ---
 datapath-windows/ovsext/Flow.c | 39 +-
 datapath-windows/ovsext/Switch.c   |  5 -
 datapath-windows/ovsext/User.c | 22 -
 datapath-windows/ovsext/Vport.c| 22 ++---
 5 files changed, 19 insertions(+), 84 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 8b561a3..6a76a12 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -918,14 +918,11 @@ ValidateNetlinkCmd(UINT32 devOp,
 
 /* Validate the DP for commands that require a DP. */
 if (nlFamilyOps->cmds[i].validateDpIndex == TRUE) {
-OvsAcquireCtrlLock();
 if (ovsMsg->ovsHdr.dp_ifindex !=
   (INT)gOvsSwitchContext->dpNo) {
 status = STATUS_INVALID_PARAMETER;
-OvsReleaseCtrlLock();
 goto done;
 }
-OvsReleaseCtrlLock();
 }
 
 /* Validate the PID. */
@@ -1012,7 +1009,6 @@ OvsGetPidCmdHandler(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
  * --
  * Utility function to fill up information about the datapath in a reply to
  * userspace.
- * Assumes that 'gOvsCtrlLock' lock is acquired.
  * --
  */
 static NTSTATUS
@@ -1212,9 +1208,7 @@ HandleGetDpDump(POVS_USER_PARAMS_CONTEXT usrParamsCtx,
 NlBufInit(&nlBuf, usrParamsCtx->outputBuffer,
   usrParamsCtx->outputLength);
 
-OvsAcquireCtrlLock();
 status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
-OvsReleaseCtrlLock();
 
 if (status != STATUS_SUCCESS) {
 *replyLen = 0;
@@ -1301,11 +1295,9 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 
 NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
 
-OvsAcquireCtrlLock();
 if (dpAttrs[OVS_DP_ATTR_NAME] != NULL) {
 if (!OvsCompareString(NlAttrGet(dpAttrs[OVS_DP_ATTR_NAME]),
   OVS_SYSTEM_DP_NAME)) {
-OvsReleaseCtrlLock();
 
 /* Creation of new datapaths is not supported. */
 if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_SET) {
@@ -1317,19 +1309,16 @@ HandleDpTransactionCommon(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 goto cleanup;
 }
 } else if ((UINT32)msgIn->ovsHdr.dp_ifindex != gOvsSwitchContext->dpNo) {
-OvsReleaseCtrlLock();
 nlError = NL_ERROR_NODEV;
 goto cleanup;
 }
 
 if (usrParamsCtx->ovsMsg->genlMsg.cmd == OVS_DP_CMD_NEW) {
-OvsReleaseCtrlLock();
 nlError = NL_ERROR_EXIST;
 goto cleanup;
 }
 
 status = OvsDpFillInfo(gOvsSwitchContext, msgIn, &nlBuf);
-OvsReleaseCtrlLock();
 
 *replyLen = NlBufSize(&nlBuf);
 
@@ -1411,7 +1400,6 @@ MapIrpOutputBuffer(PIRP irp,
  * --
  * Utility function to fill up information about the state of a port in a reply
  * to* userspace.
- * Assumes that 'gOvsCtrlLock' lock is acquired.
  * --
  */
 static NTSTATUS
@@ -1515,8 +1503,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 
 NlBufInit(&nlBuf, usrParamsCtx->outputBuffer, usrParamsCtx->outputLength);
 
-OvsAcquireCtrlLock();
-
 /* remove an event entry from the event queue */
 status = OvsRemoveEventEntry(usrParamsCtx->ovsInstance, &eventEntry);
 if (status != STATUS_SUCCESS) {
@@ -1532,7 +1518,6 @@ OvsReadEventCmdHandler(POVS_USER_PARAMS_CONTEXT 
usrParamsCtx,
 }
 
 cleanup:
-OvsReleaseCtrlLock();
 return status;
 }
 
diff --git a/datapath-windows/ovsext/Flow.c b/datapath-windows/ovsext/Flow.c
index d3de8cc..1f1bd2f 100644
--- a/datapath-windows/ovsext/Flow.c
+++ b/datapath-windows/ovsext/Flow.c
@@ -31,7 +31,6 @@
 #pragma warning( push )
 #pragma warning( disable:4127 )
 
-extern PNDIS_SPIN_LOCK gOvsCtrlLock;
 extern POVS_SWITCH_CONTEXT gOvsSwitchContext;
 extern UINT64 ovsTimeIncrementPerTick;
 
@@ -1995,25 +1994,23 @@ OvsDoDumpFlows(OvsFlowDumpInput *dumpInput,
 BOOLEAN findNextNonEmpty = FALSE;
 
 dpNo = dumpInput->dpNo;
-NdisAcquireSpinLock(gOvsCtrlLock);
 if (gOvsSwitchContext->dpNo != dpNo) {
 status = STATUS_INVALID_PARAMETER;
-goto unlock;
+goto exit;
 }
 
 rowIndex = dum

Re: [ovs-dev] [PATCH v4] ovn: add ovn-nbctl

2015-03-23 Thread Ben Pfaff
On Sun, Mar 22, 2015 at 04:38:50PM -0400, Russell Bryant wrote:
> ovn-nbctl is intended to be a utility for manually interacting with
> the OVN northbound db.  A real consumer of OVN, such as OpenStack,
> should be using ovsdb directly.  However, a utility like this is
> useful during development and testing of both ovn itself as well as
> the OpenStack Neutron plugin.  It could also be used when debugging
> the state of an ovn deployment.
> 
> The commands related to logical switches and ports have been
> implemented.  You can add, delete, list, and get/set external:ids.
> On ports you can also get/set MAC addresses.
> 
> Signed-off-by: Russell Bryant 
> ---
> 
> 
> v1
>  - first cut of lswitch-* commands
> v2
>  - fix handling of multiple switches with the same name
> v3
>  - add -d/--database option
>  - add lport-* commands
> v4
>  - add --database option to support a non-default or remote db
>  - set up and use vlog
>  - add tls options
>  - other minor cleanups
>  - (I think I'm actually done for now this time ...)

Thanks a lot!

One thing we'll have to think about here is clean transactions.  The
code as-is does not use any "verify" operations to make sure that the
overall transaction is atomic.  This might not be a big deal for
ovn-nbctl, and it definitely is not yet, but it is worth thinking about
(especially since OpenStack will want atomic transactions so if we catch
any issues soon then we'll be better prepared for OpenStack's real use
case).

I noticed that some of the VLOG invocations put a new-line at the end.
That's not necessary, although it's harmless, and our convention is to
leave it out, so I removed them.

I noticed a few long lines, so I wrapped them.

The manpage needed some font adjustments, so I did that.

I also changed the subject line of the commit message to start with a
capital letter and end with a period, because for whatever reason that's
my preferred convention in OVS.

With those changes I applied this to the ovn branch.  Thanks again!

diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml
index dad5e9a..54f9620 100644
--- a/ovn/ovn-nbctl.8.xml
+++ b/ovn/ovn-nbctl.8.xml
@@ -4,42 +4,42 @@
 ovn-nbctl -- Open Virtual Network northbound db management utility
 
 Synopsys
-ovn-nbctl [OPTIONS] COMMAND [ARG...]
+ovn-nbctl [options] command 
[arg...]
 
 Description
 This utility can be used to manage the OVN northbound database.
 
 Logical Switch Commands
-lswitch-add [name]
-lswitch-del 
-lswitch-list
-lswitch-set-external-id   [value]
-lswitch-get-external-id  [key]
+lswitch-add [name]
+lswitch-del lswitch
+lswitch-list
+lswitch-set-external-id lswitch key 
[value]
+lswitch-get-external-id lswitch 
[key]
 
 Logical Port Commands
-lport-add  
-lport-del 
-lport-list 
-lport-set-external-id   [value]
-lport-get-external-id  [key]
-lport-set-macs  [MAC] [MAC] [...]
-lport-get-macs 
+lport-add name lswitch
+lport-del lport
+lport-list lswitch
+lport-set-external-id lport key 
[value]
+lport-get-external-id lport [key]
+lport-set-macs lport [mac] 
[mac] [...]
+lport-get-macs lport
 
 Options
--d DATABASE | --database DATABASE
--h | --help
--o | --options
--V | --version
+-d database | --database 
database
+-h | --help
+-o | --options
+-V | --version
 
 Logging options
--vSPEC, --verbose=SPEC
--v, --verbose
---log-file[=FILE]
---syslog-target=HOST:PORT
+-vspec, 
--verbose=spec
+-v, --verbose
+--log-file[=file]
+
--syslog-target=host:port
 
 PKI configuration (required to use SSL)
--p, --private-key=FILE  file with private key
--c, --certificate=FILE  file with certificate for private key
--C, --ca-cert=FILE  file with peer CA certificate
+-p, --private-key=file  file with 
private key
+-c, --certificate=file  file with 
certificate for private key
+-C, --ca-cert=file  file with 
peer CA certificate
 
 
diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c
index 3dc41e9..eade156 100644
--- a/ovn/ovn-nbctl.c
+++ b/ovn/ovn-nbctl.c
@@ -89,7 +89,8 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
 
 if (uuid_from_string(&lswitch_uuid, id)) {
 is_uuid = true;
-lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl, 
&lswitch_uuid);
+lswitch = nbrec_logical_switch_get_for_uuid(nb_ctx->idl,
+&lswitch_uuid);
 } else {
 const struct nbrec_logical_switch *iter;
 
@@ -99,7 +100,7 @@ lswitch_by_name_or_uuid(struct nbctl_context *nb_ctx, const 
char *id)
 }
 if (lswitch) {
 VLOG_WARN("There is more than one logical switch named '%s'. "
-"Use a UUID.\n", id);
+"Use a UUID.", i

Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling

2015-03-23 Thread Eitan Eliahu
Sorin,
Ok on [1]. Thanks.

On [2], I can see a scenario when the provider can added twice from 
OvsRegisterSystemProvider() as well as from the callback

if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) {
 >  Callback is called from a different thread context !
 OvsTunnelEngineOpen(&handle);


How do you prevent it?

Also, can you please explain which issue you ran into which triggered this 
change?
Thank you,
Eitan

-Original Message-
From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] 
Sent: Monday, March 23, 2015 1:37 AM
To: Eitan Eliahu
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Hi Eitan,

Thanks for reviewing my patch.
[1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it 
receives before closing it.
[2] In order to retrieve the current state of the filter engine I am using the 
FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout 
driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason 
I am registering the callback no mater of the state of the filter engine. 
Only if the engine is running the provider is added by opening the engine 
handle first and closing it after the provider is added, then unsubscribes the 
callback.
If the engine is not running the callback is called and the provider is added 
only for the service running notification.

Hope this answers your comments.

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com] 
Sent: Monday, 23 March, 2015 02:32
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling


Hi Sorin,
Thank you for posting this change.
Here are few comments:
[1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. 
(you might want to add an assertion if it zero).
[2] I'm somewhat confused about the registration for engine state change. As I 
understand you check the state of the engine and if it is running you open the 
engine handle (in the context of the foreground thread). However, you subscribe 
to state change anyway and you open the handle to the engine from the callback 
function too.
Did I miss anything?
Thanks,
Eitan


-Original Message-

From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Monday, 16 March, 2015 20:08

To: dev@openvswitch.org

Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



If the Base Filtering Engine (BFE) is not started, the WFP system provider 
failed to be added because 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 
WFP system provider is added.



Signed-off-by: Sorin Vinturis 

Reported-by: Sorin Vinturis 

Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e=
 

---

 datapath-windows/ovsext/Datapath.c | 20 ++-

 datapath-windows/ovsext/TunnelFilter.c | 99 --

 datapath-windows/ovsext/TunnelIntf.h   |  8 +--

 3 files changed, 100 insertions(+), 27 deletions(-)



diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c

index 8eb13f1..c6fe89e 100644

--- a/datapath-windows/ovsext/Datapath.c

+++ b/datapath-windows/ovsext/Datapath.c

@@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock;  VOID

 OvsInit()

 {

-HANDLE handle = NULL;

-

 gOvsCtrlLock = &ovsCtrlLockObj;

 NdisAllocateSpinLock(gOvsCtrlLock);

 OvsInitEventQueue();

-

-OvsTunnelEngineOpen(&handle);

-if (handle) {

-OvsTunnelAddSystemProvider(handle);

-}

-OvsTunnelEngineClose(&handle);

 }

 

 VOID

 OvsCleanup()

 {

-HANDLE handle = NULL;

-

 OvsCleanupEventQueue();

 if (gOvsCtrlLock) {

 NdisFreeSpinLock(gOvsCtrlLock);

 gOvsCtrlLock = NULL;

 }

-

-OvsTunnelEngineOpen(&handle);

-if (handle) {

-OvsTunnelRemoveSystemProvider(handle);

-}

-OvsTunnelEngineClose(&handle);

 }

 

 VOID

@@ -448,6 +432,8 @@ OvsCreateDeviceObject(NDIS_HANDLE ovsExtDriverHandle)

 if (ovsExt) {

 ovsExt->numberOpenInstance = 0;

 }

+} else {

+OvsRegisterSystemProvider((PVOID)gOvsDeviceObject);

 }

 

 OVS_LOG_TRACE("DeviceObject: %p", gOvsDeviceObject); @@ -471,6 +457,8 @@ 
OvsDeleteDeviceObject()

 NdisDeregisterDeviceEx(gOvsDeviceHandle);

 gOvsDeviceHandle = NULL;

 gOvsDeviceObject = NULL;

+

+OvsUnregisterSystemProvider();

 }

 }

 

diff --git a/datapath-wi

[ovs-dev] [PATCH] INSTALL.DPDK.md: Terminate code section.

2015-03-23 Thread Russell Bryant
Add a missing terminator for a code section.  Without this, the
rendering on github at least shows the rest of the file as a code
block.

Signed-off-by: Russell Bryant 
---
 INSTALL.DPDK.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md
index 5b61272..60889d0 100644
--- a/INSTALL.DPDK.md
+++ b/INSTALL.DPDK.md
@@ -430,6 +430,7 @@ qemu-wrap.py -cpu host -boot c -hda  -m 4096 
-smp 4
   --enable-kvm -nographic -vnc none -net none -netdev tap,id=net1,
   script=no,downscript=no,ifname=if1,vhost=on -device virtio-net-pci,
   netdev=net1,mac=00:00:00:00:00:01
+```
 
 DPDK vhost VM configuration with libvirt:
 -
-- 
2.1.0

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


[ovs-dev] Cloud Expo Europe 2015

2015-03-23 Thread Jennifer Hallgreen
Hi,

 

As a database provider, I was trying to check if you would be interested in
owning a list available of companies with virtualized environments that can
be used for your marketing campaigns? These lists can be customized based on
geography, titles and more.

 

Some of the lists are categorized into:

 

* VMware users

* Citrix Users

* MS Hyper V users

* RedHat users

* DELL users

* IBM users and more

 

We have some authentic data of other Virtualization, CRM, ERP, PLM, BI
software users too; we can help you if you have a similar niche market to
target.  Let me know if you would be interested in seeing a sample and
getting more info on our authentic lists. 

 

Do you need anything custom to reach your prospects? We can help you
virtually with anything on the data front, be it industry specific, title
specific and any such parameters.  Reach out with your specific requirement
and get a set of free samples.

 

If you are not the right person to discuss this please forward this email to
the right person in your organization. 

 

I look forward to hearing from you.

 

Kind Regards,

 

Jennifer Hall

Business Coordinator

Target Biz Solutions

75 Fifth Street NW

Suite 1100

Atlanta, GA 30308

 

 

We respect your privacy. If you don't wish to receive an email from us,
please send a reply with an email subject line as "Leave out".

 

 

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


Re: [ovs-dev] [PATCH] INSTALL.DPDK.md: Terminate code section.

2015-03-23 Thread Ben Pfaff
On Mon, Mar 23, 2015 at 01:49:10PM -0400, Russell Bryant wrote:
> Add a missing terminator for a code section.  Without this, the
> rendering on github at least shows the rest of the file as a code
> block.
> 
> Signed-off-by: Russell Bryant 

Thanks!  Applied to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH 2/2] datapath: Add Stateless TCP Tunneling protocol.

2015-03-23 Thread Jesse Gross
On Mon, Mar 9, 2015 at 3:12 PM, Pravin B Shelar  wrote:
>  openvswitch_headers = \
> diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore
> index 69d6658..1d1aae7 100644
> --- a/datapath/linux/.gitignore
> +++ b/datapath/linux/.gitignore
> @@ -50,6 +50,7 @@
>  /vport-lisp.c
>  /vport-netdev.c
>  /vport-patch.c
> +/vport-stt.c
>  /vport-vxlan.c
>  /vport.c
>  /vxlan.c

Should we add stt.c as well?

> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
> new file mode 100644
> index 000..4cb4ea6
> --- /dev/null
> +++ b/datapath/linux/compat/stt.c
> +static int skb_list_linearize(struct sk_buff *head, gfp_t gfp_mask)
> +{
> +   struct sk_buff *skb;
> +   int tlen = 0;
> +   int err;
> +
> +   err = skb_linearize(head);
> +   if (err)
> +   return err;
> +
> +   skb = head->next;
> +   while (skb) {
> +   tlen += skb->len;
> +   skb = skb->next;
> +   }
> +   err = pskb_expand_head(head, 0, tlen, gfp_mask);
> +   if (err)
> +   return err;

Were you going to try to avoid linearizing and then expanding as Alex
had mentioned in his comments?

> +static int __build_segments(struct sk_buff **headp, bool ipv4, int l4_offset)
> +{
> +   struct sk_buff *head = *headp;
> +   struct sk_buff *nskb = NULL;
> +   struct sk_buff *rskb, *skb;
> +   struct tcphdr *tcph;
> +   int seg_len = 0;
> +   int hdr_len;
> +   int tcp_len;
> +   u32 seq;
> +
> +   /* GRO can produce skbs with only the headers, which we've
> +* already pulled off.  We can just dump them.
> +*/
> +   while (head->len == 0) {
> +   nskb = head->next;
> +   copy_skb_metadata(nskb, head);
> +   consume_skb(head);
> +   head = nskb;
> +   }
> +   *headp = head;

> +   tcph = (struct tcphdr *)(head->data + l4_offset);
> +   tcp_len = tcph->doff * 4;
> +   hdr_len = l4_offset + tcp_len;

I think there is something wrong with the ordering of the calls in
stt_rcv(). reassemble() will just produce a linked list of skbs that
are expected to be joined here but we've already called functions that
do pskb_may_pull(), which won't traverse that list. In particular, it
doesn't make sense to both remove zero length skbs above and expect
that there is a TCP header available below.

> +
> +   if (unlikely((tcp_len < sizeof(struct tcphdr)) ||
> +(head->len < hdr_len)))
> +   return -EINVAL;
> +
> +   if (unlikely(!pskb_may_pull(head, hdr_len)))
> +   return -ENOMEM;

It seems risky to me to combine skb coalescing with TCP header
updating as we would need to very carefully check boundary conditions.
I feel like there are two halves of the loop anyways, so it seems
better to just break them apart.

> +static int __segment_skb(struct sk_buff **headp, bool ipv4, bool tcp, bool 
> csum_partial, int l4_offset)
> +{
> +   int err;
> +
> +   err = straighten_frag_list(headp);
> +   if (unlikely(err))
> +   return err;
> +
> +   if (__linearize(*headp, ipv4, tcp, csum_partial)) {
> +   return skb_list_linearize(*headp, GFP_ATOMIC);
> +   }
> +
> +   if ((*headp)->next) {

I think we can push this check up above linearization. If there is no
list of skbs, we don't need to linearize either.

> +static int segment_skb(struct sk_buff **headp)
> +{

Isn't there a check for frag_list for the receive path missing
somewhere? It seems like linearization happens somewhat
unconditionally.

> +static int skb_list_xmit(struct rtable *rt, struct sk_buff *skb, __be32 src,
> +__be32 dst, __u8 tos, __u8 ttl, __be16 df)
> +{
> +   int len = 0;
> +
> +   while (skb) {
> +   struct sk_buff *next = skb->next;
> +
> +   if (next)
> +   dst_clone(&rt->dst);
> +
> +   skb->next = NULL;
> +   len += iptunnel_xmit(NULL, rt, skb, src, dst, IPPROTO_TCP,
> +tos, ttl, df, false);

I think there may be a problem in our version of ip_local_out() if it
thinks that fix_segment is set in the CB, so we need to find a way to
make sure that it is cleared.

> +int stt_xmit_skb(struct sk_buff *skb, struct rtable *rt,
> +__be32 src, __be32 dst, __u8 tos,
> +__u8 ttl, __be16 df, __be16 src_port, __be16 dst_port,
> +__be64 tun_id)
[...]
> +   while (skb) {
> +   struct sk_buff *next_skb = skb->next;
> +
> +   skb->next = NULL;
> +
> +   if (next_skb)
> +   dst_clone(&rt->dst);
> +
> +   /* Push STT and TCP header. */
> +   skb = push_stt_header(skb, tun_id, src_port, dst_port, src,
> + dst, inner_h_proto, inner_nw_proto,
> + dst_mtu(&rt->dst));
> +

Re: [ovs-dev] question about revalidation

2015-03-23 Thread Ben Pfaff
On Wed, Mar 18, 2015 at 06:30:17PM +, Gray, Mark D wrote:
> In udpif_revalidator(), the leader thread seems to adjust the datapath 
> flow_limit
> in response to duration (which is the time taken to revalidate). It does this 
> with
> some heuristic calculations that I have reproduced below. Not sure exactly 
> why it
> is doing this or how the heuristics work as I would have assumed the 
> flow_limit would
> be independent of revalidation time. Can anyone shed some light on that for 
> me?
> 
> if (duration > 2000) {
> flow_limit /= duration / 1000;
> } else if (duration > 1300) {
> flow_limit = flow_limit * 3 / 4;
> } else if (duration < 1000 && n_flows > 2000
>&& flow_limit < n_flows * 1000 / duration) {
> flow_limit += 1000;
> }

The number of flows is limited so that OVS can be confident that it
can revalidate every flow within a bounded amount of time (I think the
goal is 1 second).
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] auto-attach: Support latest version of auto-attach LLDP TLVs

2015-03-23 Thread Ben Pfaff
On Wed, Mar 18, 2015 at 02:47:14PM -0400, drfl...@avaya.com wrote:
> From: Dennis Flynn 
> 
> The following enhancements to the auto-attach feature are provided
> 
> - Support recent modifications to the AA element discovery TLV
> - Support recent Avaya Organizationally Unique ID (OUI) change.
>   (This will change to IEEE assigned OUI once AA standard has been ratified)
> - Remove some Avaya specific #defines
> 
> The primary purpose of this commit is to catch up with the latest changes made
> to the auto attach TLVs as the Auto Attach feature progresses through the
> 802.1Q IEEE standards committee. Most notably this includes some minor rework
> of the AA element discovery TLV and a recent change to the Avaya OUI value.
> 
> Signed-off-by: Dennis Flynn 

Thank you for the patch!  I am very happy to see this code being
maintained.  I applied this to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] netdev-linux: Support for SFQ, FQ_CoDel and CoDel qdiscs.

2015-03-23 Thread Ben Pfaff
On Wed, Mar 18, 2015 at 05:13:01PM +0100, Jonathan Vestin wrote:
> This patch adds support for SFQ, CoDel and FQ_CoDel classless qdiscs to Open 
> vSwitch. It also removes the requirement for a QoS to have at least one Queue 
> (as this makes no sense when using classless qdiscs). I have also not 
> implemented class_{get,set,delete,get_stats,dump_stats} because they are 
> meant for qdiscs with classes. 
> 
> Signed-off-by: Jonathan Vestin 

Thanks for the contribution!  I applied this to master.  I folded in
the following incremental changes.  Most of them are purely stylistic
(including a lot of removal of trailing white space) but you might
want to notice the nl_msg_put_u32() and nl_attr_get_u32() functions
for next time.

Thanks again!

diff --git a/AUTHORS b/AUTHORS
index db4520f..8fba915 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -85,6 +85,7 @@ Jesse Gross je...@nicira.com
 Jing Ai ji...@google.com
 Joe Perches j...@perches.com
 Joe Stringerjoestrin...@nicira.com
+Jonathan Vestin jonav...@kau.se
 Jun Nakajimajun.nakaj...@intel.com
 Justin Pettit   jpet...@nicira.com
 Keith Amidonke...@nicira.com
@@ -268,7 +269,6 @@ Joan Cirer  j...@ev0.net
 John Darrington j...@darrington.wattle.id.au
 John Galgay j...@galgay.net
 John Hurley john.hur...@netronome.com
-Jonathan Vestin jonav...@kau.se
 K ???k940...@hotmail.com
 Kevin Mancuso   kevin.manc...@rackspace.com
 Kiran Shanbhog  ki...@vmware.com
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 65f2555..8253dfb 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2856,7 +2856,7 @@ codel_get__(const struct netdev *netdev_)
 }
 
 static void
-codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit, 
+codel_install__(struct netdev *netdev_, uint32_t target, uint32_t limit,
 uint32_t interval)
 {
 struct netdev_linux *netdev = netdev_linux_cast(netdev_);
@@ -2872,7 +2872,7 @@ codel_install__(struct netdev *netdev_, uint32_t target, 
uint32_t limit,
 }
 
 static int
-codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit, 
+codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
 uint32_t interval)
 {
 size_t opt_offset;
@@ -2897,18 +2897,17 @@ codel_setup_qdisc__(struct netdev *netdev, uint32_t 
target, uint32_t limit,
 
 nl_msg_put_string(&request, TCA_KIND, "codel");
 opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS);
-nl_msg_put_unspec(&request, TCA_CODEL_TARGET, &otarget, sizeof otarget);
-nl_msg_put_unspec(&request, TCA_CODEL_LIMIT, &olimit, sizeof olimit);
-nl_msg_put_unspec(&request, TCA_CODEL_INTERVAL, &ointerval, 
-  sizeof ointerval);
+nl_msg_put_u32(&request, TCA_CODEL_TARGET, otarget);
+nl_msg_put_u32(&request, TCA_CODEL_LIMIT, olimit);
+nl_msg_put_u32(&request, TCA_CODEL_INTERVAL, ointerval);
 nl_msg_end_nested(&request, opt_offset);
 
 error = tc_transact(&request, NULL);
 if (error) {
 VLOG_WARN_RL(&rl, "failed to replace %s qdisc, "
-"target %u, limit %u, interval %u error %d(%s)", 
+"target %u, limit %u, interval %u error %d(%s)",
 netdev_get_name(netdev),
-otarget, olimit, ointerval, 
+otarget, olimit, ointerval,
 error, ovs_strerror(error));
 }
 return error;
@@ -2930,9 +2929,15 @@ codel_parse_qdisc_details__(struct netdev *netdev 
OVS_UNUSED,
 codel->limit = limit_s ? strtoull(limit_s, NULL, 10) : 0;
 codel->interval = interval_s ? strtoull(interval_s, NULL, 10) : 0;
 
-if (!codel->target) codel->target = 5000;
-if (!codel->limit) codel->limit = 10240;
-if (!codel->interval) codel->interval = 10;
+if (!codel->target) {
+codel->target = 5000;
+}
+if (!codel->limit) {
+codel->limit = 10240;
+}
+if (!codel->interval) {
+codel->interval = 10;
+}
 }
 
 static int
@@ -2942,7 +2947,7 @@ codel_tc_install(struct netdev *netdev, const struct smap 
*details)
 struct codel codel;
 
 codel_parse_qdisc_details__(netdev, details, &codel);
-error = codel_setup_qdisc__(netdev, codel.target, codel.limit, 
+error = codel_setup_qdisc__(netdev, codel.target, codel.limit,
 codel.interval);
 if (!error) {
 codel_install__(netdev, codel.target, codel.limit, codel.interval);
@@ -2967,9 +2972,9 @@ codel_parse_tca_options__(struct nlattr *nl_options, 
struct codel *codel)
 return EPROTO;
 }
 
-codel->target = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_TARGET]));
-codel->limit = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_LIMIT]));
-codel->interval = *((uint32_t *)nl_attr_get(attrs[TCA_CODEL_INTERVAL]));
+codel->target = nl_attr_get_u32(attrs[TCA_CODEL_TARGET]);
+codel->limit = nl_attr_get_

[ovs-dev] OVN - OVN Northbound DB and ovn-nbd question

2015-03-23 Thread Millward, Scott T (HP Networking / CEB- Roseville)


I apologize if the 'ASCII Draw' does not appear correctly in your email client. 
 Outlook seems to be changing it..



Hello All,



I wonder if you could help me understand the OVN architecture and specifically, 
how we might implement this in OpenStack or any other CMS..



The XML document has the following ASCII-Draw picture:



The implication in the dotted box below the CMS is that the CMS Plugin, OVN 
NorthBound DB, and the ovn-nbd are all part of the same group or module.  In 
other words, for OpenStack, this suggests the solution would contain an 
OpenStack specific Plugin (of course), OVN Northbound DB and ovn-ndb (perhaps 
an agent??).







 CMS

   |

   |

   +---|---+

   |   |   |

   | OVN/CMS Plugin|

   |   |   |

   |   |   |

   |   OVN Northbound DB   |

   |   |   |

   |   |   |

   |ovn-nbd|

   |   |   |

   +---|---+

   |

   |

+--+

|OVN DB|

+--+

   |

   |

+--+--+

|  |  |

HV 1   |  |HV n  |

+---|---+  .  +---|---+

|   |   |  .  |   |   |

|ovn-controller |  .  |ovn-controller |

| |  |  |  .  | |  |  |

| |  |  | | |  |  |

|  ovs-vswitchd   ovsdb-server  | |  ovs-vswitchd   ovsdb-server  |

|   | |   |

+---+ +---+



However, the OVN Northbound DB is not the CMS DB ( Neutron DB in the case of 
OpenStack), it is a Schema defined by OVN.  This suggests that the OVN 
Northbound DB and ovn-nbd is CMS independent and should be outside the CMS 
implementation.



The OVN Northbound DB and the Central OVN DB could be in the same 'System' (or 
cluster or systems).







  CMS

   |

   |

   +---|---+

   |   |   |

   | OVN/CMS Plugin|

   +---|---+

  |

   |

  |

   +---|---+

   |   |   |

   |   |   |

   |   OVN Northbound DB   |

   |   |   |

   |   |   |

   |ovn-nbd|

   |   |   |

   +---|---+

:  |  :

:  |  :

+--+

|OVN DB|

+--+

|

   |

+--+--+

|  |  |

HV 1|  |HV n  |

+---|---+  .  +---|---+

|   |   |  .  |   |   |

|ovn-controller |  .  |ovn-controller |

| |  |  |  .  | |  |  |

| |  |  | | |  |  |

|  ovs-vswitchd   ovsdb-server  | |  ovs-vswitchd   ovsdb-server  |

|   | |   |

+---+ +---+



Above is the split that I am talking about.  The OVN DB could still be separate 
as it will likely have different HA requirements than the OVN Northbound 
(assuming the OVN Northbound could be recreated from the CMS DB).



The above starts to look a bit more like a 'Controller' architecture where 
something like ODL or another SDN Controller could host the OVN N

[ovs-dev] [PATCH] Updated spec file - fixed RPM build errors: Installed (but unpackaged) file(s) found.

2015-03-23 Thread Oleg Gashev
On exec rpmbuild -ba openvswitch-fedora.spec displayed:

error: Installed (but unpackaged) file(s) found:
   /etc/bash_completion.d/ovs-appctl-bashcomp.bash
   /etc/bash_completion.d/ovs-vsctl-bashcomp.bash

RPM build errors:
Installed (but unpackaged) file(s) found:
   /etc/bash_completion.d/ovs-appctl-bashcomp.bash
   /etc/bash_completion.d/ovs-vsctl-bashcomp.bash

/etc/bash_completion.d/ovs-appctl-bashcomp.bash and
/etc/bash_completion.d/ovs-vsctl-bashcomp.bash added to files list.

Signed-off-by: Oleg Gashev 
Acked-by: Alex Wang 
---
 rhel/openvswitch-fedora.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 5a3af4a..4889321 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -200,6 +200,8 @@ rm -rf $RPM_BUILD_ROOT
 
 %files
 %defattr(-,root,root)
+%{_sysconfdir}/bash_completion.d/ovs-appctl-bashcomp.bash
+%{_sysconfdir}/bash_completion.d/ovs-vsctl-bashcomp.bash
 %dir %{_sysconfdir}/openvswitch
 %config %ghost %{_sysconfdir}/openvswitch/conf.db
 %config %ghost %{_sysconfdir}/openvswitch/system-id.conf
-- 
1.9.3

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


Re: [ovs-dev] OVN - OVN Northbound DB and ovn-nbd question

2015-03-23 Thread Russell Bryant
I snipped the ascii diagrams, as they were pretty badly munged when I
tried to reply ...

On 03/23/2015 05:22 PM, Millward, Scott T (HP Networking / CEB-
Roseville) wrote:
> I wonder if you could help me understand the OVN architecture and
> specifically, how we might implement this in OpenStack or any other
> CMS..
> 
> 
> 
> The XML document has the following ASCII-Draw picture:
> 
> 
> 
> The implication in the dotted box below the CMS is that the CMS
> Plugin, OVN NorthBound DB, and the ovn-nbd are all part of the same
> group or module.  In other words, for OpenStack, this suggests the
> solution would contain an OpenStack specific Plugin (of course), OVN
> Northbound DB and ovn-ndb (perhaps an agent??).
> 

> 
> However, the OVN Northbound DB is not the CMS DB ( Neutron DB in the
> case of OpenStack), it is a Schema defined by OVN.  This suggests
> that the OVN Northbound DB and ovn-nbd is CMS independent and should
> be outside the CMS implementation.
> 
> 
> 
> The OVN Northbound DB and the Central OVN DB could be in the same
> 'System' (or cluster or systems).
> 


> 
> Above is the split that I am talking about.  The OVN DB could still
> be separate as it will likely have different HA requirements than the
> OVN Northbound (assuming the OVN Northbound could be recreated from
> the CMS DB).
> 
> 
> 
> The above starts to look a bit more like a 'Controller' architecture
> where something like ODL or another SDN Controller could host the OVN
> Northbound DB and the ovn-nbd.  This also means that assuming the
> interface between the CMS (and its plugin architecture) is OVSDB
> protocol to the OVN Northbound DB, only the CMS and its plugin would
> need to be CMS Specific.

What you suggest here is the intention.  The OpenStack plugin only has
to implement communication to the ovn northbound DB.  The rest is
general OVN code.  The OVN northbound db is just a db served by an
instance of ovsdb-server, and ovn-nbd is a new daemon to be implemented
for OVN.

The Neutron plugin has been started here:

http://git.openstack.org/cgit/stackforge/networking-ovn/tree/

Right now the code has started using the 'ovn-nbctl' utility to talk to
the northbound DB.  That's only intended to be the "make it work ASAP"
approach.  We'd like to make it talk to ovsdb natively.  Some very
similar work was just finished in Neutron, so we'll be taking advantage
of it.

-- 
Russell Bryant
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling

2015-03-23 Thread Sorin Vinturis
Hi Eitan,

In this case the second call will enter this code path:

 status = FwpmProviderAdd(handle,
  &provider,
  NULL);
 if (!NT_SUCCESS(status)) {
-OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status);
-break;
+if (STATUS_FWP_ALREADY_EXISTS != status) {
+OVS_LOG_ERROR("Failed to add WFP provider, status: %x.",
+  status);
+break;
+}
 }

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com] 
Sent: Monday, 23 March, 2015 18:13
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Sorin,
Ok on [1]. Thanks.

On [2], I can see a scenario when the provider can added twice from 
OvsRegisterSystemProvider() as well as from the callback

if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) {  >  Callback is 
called from a different thread context !
 OvsTunnelEngineOpen(&handle);


How do you prevent it?

Also, can you please explain which issue you ran into which triggered this 
change?
Thank you,
Eitan

-Original Message-
From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com]
Sent: Monday, March 23, 2015 1:37 AM
To: Eitan Eliahu
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Hi Eitan,

Thanks for reviewing my patch.
[1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it 
receives before closing it.
[2] In order to retrieve the current state of the filter engine I am using the 
FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout 
driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason 
I am registering the callback no mater of the state of the filter engine. 
Only if the engine is running the provider is added by opening the engine 
handle first and closing it after the provider is added, then unsubscribes the 
callback.
If the engine is not running the callback is called and the provider is added 
only for the service running notification.

Hope this answers your comments.

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com]
Sent: Monday, 23 March, 2015 02:32
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling


Hi Sorin,
Thank you for posting this change.
Here are few comments:
[1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. 
(you might want to add an assertion if it zero).
[2] I'm somewhat confused about the registration for engine state change. As I 
understand you check the state of the engine and if it is running you open the 
engine handle (in the context of the foreground thread). However, you subscribe 
to state change anyway and you open the handle to the engine from the callback 
function too.
Did I miss anything?
Thanks,
Eitan


-Original Message-

From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Monday, 16 March, 2015 20:08

To: dev@openvswitch.org

Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



If the Base Filtering Engine (BFE) is not started, the WFP system provider 
failed to be added because 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 
WFP system provider is added.



Signed-off-by: Sorin Vinturis 

Reported-by: Sorin Vinturis 

Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e=
 

---

 datapath-windows/ovsext/Datapath.c | 20 ++-

 datapath-windows/ovsext/TunnelFilter.c | 99 --

 datapath-windows/ovsext/TunnelIntf.h   |  8 +--

 3 files changed, 100 insertions(+), 27 deletions(-)



diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c

index 8eb13f1..c6fe89e 100644

--- a/datapath-windows/ovsext/Datapath.c

+++ b/datapath-windows/ovsext/Datapath.c

@@ -353,35 +353,19 @@ PNDIS_SPIN_LOCK gOvsCtrlLock;  VOID

 OvsInit()

 {

-HANDLE handle = NULL;

-

 gOvsCtrlLock = &ovsCtrlLockObj;

 NdisAllocateSpinLock(gOvsCtrlLock);

 OvsInitEventQueue();

-

-OvsTunnelEngineOpen(&handle);

-if (handle) {

-OvsTunnelAddSystemProvider(handle);

-}

-OvsTunnelEngineClose(&handle);

 }

 

 VOID

 OvsCleanup()

 {

-HANDLE handle = NULL;

-

 OvsCleanupEventQueue();

 if (gOvsCtrlLock) 

Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling

2015-03-23 Thread Sorin Vinturis
As I explained in the patch description, I have added this change because, at 
driver initialization phase, the system provider failed to be added due to the 
fact that the Base Filtering Engine (BFE) is not started and no session to the 
engine could be acquired, i.e. OvsTunnelEngineOpen() fails.

-Original Message-
From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis
Sent: Tuesday, 24 March, 2015 01:01
To: Eitan Eliahu
Cc: dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Hi Eitan,

In this case the second call will enter this code path:

 status = FwpmProviderAdd(handle,
  &provider,
  NULL);
 if (!NT_SUCCESS(status)) {
-OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status);
-break;
+if (STATUS_FWP_ALREADY_EXISTS != status) {
+OVS_LOG_ERROR("Failed to add WFP provider, status: %x.",
+  status);
+break;
+}
 }

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com] 
Sent: Monday, 23 March, 2015 18:13
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Sorin,
Ok on [1]. Thanks.

On [2], I can see a scenario when the provider can added twice from 
OvsRegisterSystemProvider() as well as from the callback

if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) {  >  Callback is 
called from a different thread context !
 OvsTunnelEngineOpen(&handle);


How do you prevent it?

Also, can you please explain which issue you ran into which triggered this 
change?
Thank you,
Eitan

-Original Message-
From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com]
Sent: Monday, March 23, 2015 1:37 AM
To: Eitan Eliahu
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

Hi Eitan,

Thanks for reviewing my patch.
[1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it 
receives before closing it.
[2] In order to retrieve the current state of the filter engine I am using the 
FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout 
driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason 
I am registering the callback no mater of the state of the filter engine. 
Only if the engine is running the provider is added by opening the engine 
handle first and closing it after the provider is added, then unsubscribes the 
callback.
If the engine is not running the callback is called and the provider is added 
only for the service running notification.

Hope this answers your comments.

Thanks,
Sorin

-Original Message-
From: Eitan Eliahu [mailto:elia...@vmware.com]
Sent: Monday, 23 March, 2015 02:32
To: Sorin Vinturis
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling


Hi Sorin,
Thank you for posting this change.
Here are few comments:
[1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. 
(you might want to add an assertion if it zero).
[2] I'm somewhat confused about the registration for engine state change. As I 
understand you check the state of the engine and if it is running you open the 
engine handle (in the context of the foreground thread). However, you subscribe 
to state change anyway and you open the handle to the engine from the callback 
function too.
Did I miss anything?
Thanks,
Eitan


-Original Message-

From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Monday, 16 March, 2015 20:08

To: dev@openvswitch.org

Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



If the Base Filtering Engine (BFE) is not started, the WFP system provider 
failed to be added because 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 
WFP system provider is added.



Signed-off-by: Sorin Vinturis 

Reported-by: Sorin Vinturis 

Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=CWsgHUxi6ExLXY798tmo3LJ4e3geGYp56lkcH-5cLCY&m=MVriWGIsZGTQ5f_haLS0X5H8R3YdfikbqnWKRpKksKU&s=1iWqFul95i3o_mD41jKjZNdLnK-df_QWRzSY1614RTw&e=
 

---

 datapath-windows/ovsext/Datapath.c | 20 ++-

 datapath-windows/ovsext/TunnelFilter.c | 99 --

 datapath-windows/ovsext/TunnelIntf.h   |  8 +--

 3 files changed, 100 insertions(+), 27 deletions(-)



diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c

index 8eb13f1..c6fe89e 10064

[ovs-dev] [PATCH] datapath-windows Release lock on HVUpdateNIC

2015-03-23 Thread Alin Serdean
Quick fix to release the dispatch lock when the vport could not be found.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Vport.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/datapath-windows/ovsext/Vport.c b/datapath-windows/ovsext/Vport.c
index c9dfaea..70bef3d 100644
--- a/datapath-windows/ovsext/Vport.c
+++ b/datapath-windows/ovsext/Vport.c
@@ -405,6 +405,7 @@ HvUpdateNic(POVS_SWITCH_CONTEXT switchContext,
 nicParam->NicIndex);
 if (vport == NULL) {
 OVS_LOG_WARN("Vport search failed.");
+NdisReleaseRWLock(switchContext->dispatchLock, &lockState);
 goto update_nic_done;
 }
 switch (nicParam->NicType) {
-- 
1.9.5.msysgit.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider handling

2015-03-23 Thread Eitan Eliahu
Thanks Sorin,
What would be the case where BFE is not started before the driver initializes?
When BFE is not started does the driver continue with WFP rule setiings?
Eitan



-Original Message-
From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com] 
Sent: Monday, March 23, 2015 4:40 PM
To: Sorin Vinturis; Eitan Eliahu
Cc: dev@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling

As I explained in the patch description, I have added this change because, at 
driver initialization phase, the system provider failed to be added due to the 
fact that the Base Filtering Engine (BFE) is not started and no session to the 
engine could be acquired, i.e. OvsTunnelEngineOpen() fails.



-Original Message-

From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis

Sent: Tuesday, 24 March, 2015 01:01

To: Eitan Eliahu

Cc: dev@openvswitch.org

Subject: Re: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



Hi Eitan,



In this case the second call will enter this code path:



 status = FwpmProviderAdd(handle,

  &provider,

  NULL);

 if (!NT_SUCCESS(status)) {

-OVS_LOG_ERROR("Fail to add WFP provider, status: %x.", status);

-break;

+if (STATUS_FWP_ALREADY_EXISTS != status) {

+OVS_LOG_ERROR("Failed to add WFP provider, status: %x.",

+  status);

+break;

+}

 }



Thanks,

Sorin



-Original Message-

From: Eitan Eliahu [mailto:elia...@vmware.com] 

Sent: Monday, 23 March, 2015 18:13

To: Sorin Vinturis

Cc: dev@openvswitch.org

Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



Sorin,

Ok on [1]. Thanks.



On [2], I can see a scenario when the provider can added twice from 
OvsRegisterSystemProvider() as well as from the callback



if (FWPM_SERVICE_RUNNING == FwpmBfeStateGet()) {  >  Callback is 
called from a different thread context !

 OvsTunnelEngineOpen(&handle);





How do you prevent it?



Also, can you please explain which issue you ran into which triggered this 
change?

Thank you,

Eitan



-Original Message-

From: Sorin Vinturis [mailto:svintu...@cloudbasesolutions.com]

Sent: Monday, March 23, 2015 1:37 AM

To: Eitan Eliahu

Cc: dev@openvswitch.org

Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling



Hi Eitan,



Thanks for reviewing my patch.

[1] The OvsTunnelEngineClose(HANDLE *handle) is verifying the handle it 
receives before closing it.

[2] In order to retrieve the current state of the filter engine I am using the 
FwpmBfeStateGet function. But before calling FwpmBfeStateGet, the callout 
driver MUST call the FwpmBfeStateSubscribeChanges function. That is the reason 
I am registering the callback no mater of the state of the filter engine. 

Only if the engine is running the provider is added by opening the engine 
handle first and closing it after the provider is added, then unsubscribes the 
callback.

If the engine is not running the callback is called and the provider is added 
only for the service running notification.



Hope this answers your comments.



Thanks,

Sorin



-Original Message-

From: Eitan Eliahu [mailto:elia...@vmware.com]

Sent: Monday, 23 March, 2015 02:32

To: Sorin Vinturis

Cc: dev@openvswitch.org

Subject: RE: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling





Hi Sorin,

Thank you for posting this change.

Here are few comments:

[1] Please call OvsTunnelEngineClose(&handle) only if the handle is not zero. 
(you might want to add an assertion if it zero).

[2] I'm somewhat confused about the registration for engine state change. As I 
understand you check the state of the engine and if it is running you open the 
engine handle (in the context of the foreground thread). However, you subscribe 
to state change anyway and you open the handle to the engine from the callback 
function too.

Did I miss anything?

Thanks,

Eitan





-Original Message-



From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Sorin Vinturis



Sent: Monday, 16 March, 2015 20:08



To: dev@openvswitch.org



Subject: [ovs-dev] [PATCH] datapath-windows: Updated WFP system provider 
handling







If the Base Filtering Engine (BFE) is not started, the WFP system provider 
failed to be added because 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 
WFP system provider is added.







Signed-off-by: Sorin Vinturis 



Reported-by: Sorin Vinturis 



Reported-at: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_65

[ovs-dev] Returned mail: Data format error

2015-03-23 Thread Bounced mail
Dear user of openvswitch.org,

Your e-mail account was used to send a large amount of junk e-mail messages 
during this week.
Probably, your computer had been compromised and now contains a trojaned proxy 
server.

Please follow our instructions in order to keep your computer safe.

Have a nice day,
openvswitch.org support team.

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