Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only when extension is enabled

2018-04-05 Thread Shashank Ram 




From: aserd...@ovn.org <aserd...@ovn.org>
Sent: Thursday, April 5, 2018 2:32 AM
To: Shashank Ram; d...@openvswitch.org
Subject: RE: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only 
when    extension is enabled



> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Shashank Ram
> Trimis: Wednesday, April 4, 2018 8:31 PM
> Către: Alin Gabriel Serdean <aserd...@ovn.org>; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device
> only when extension is enabled
>
>
>
>
>
> 
> From: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> on behalf of Alin Gabriel Serdean
> <aserd...@ovn.org>
> Sent: Tuesday, April 3, 2018 8:08 AM
> To: d...@openvswitch.org
> Cc: Alin Gabriel Serdean
> Subject: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only
> whenextension is enabled
>
> Until now the communication device between the kernel and userspace
> is created when the network driver is installed.
>
> Since we only do processing if the Hyper-V vSwitch extension is enabled by
> the user, it makes more sense to move the device creation only when that is
> true.
>
> Signed-off-by: Alin Gabriel Serdean <aserd...@ovn.org>
> ---
>  datapath-windows/ovsext/Datapath.c |  2 +-
>  datapath-windows/ovsext/Driver.c   | 10 --
>  datapath-windows/ovsext/Switch.c   |  7 +++
>  3 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-
> windows/ovsext/Datapath.c
> index 34ef2b40a..264cfd3a0 100644
> --- a/datapath-windows/ovsext/Datapath.c
> +++ b/datapath-windows/ovsext/Datapath.c
> @@ -352,7 +352,7 @@ _Dispatch_type_(IRP_MJ_DEVICE_CONTROL)
>  DRIVER_DISPATCH OvsDeviceControl;
>
>  #ifdef ALLOC_PRAGMA
> -#pragma alloc_text(INIT, OvsCreateDeviceObject)
> +#pragma alloc_text(PAGE, OvsCreateDeviceObject)
>  #pragma alloc_text(PAGE, OvsOpenCloseDevice)
>  #pragma alloc_text(PAGE, OvsCleanupDevice)
>  #pragma alloc_text(PAGE, OvsDeviceControl)
> diff --git a/datapath-windows/ovsext/Driver.c b/datapath-
> windows/ovsext/Driver.c
> index 50c9614e4..39e50c34b 100644
> --- a/datapath-windows/ovsext/Driver.c
> +++ b/datapath-windows/ovsext/Driver.c
> @@ -152,14 +152,6 @@ DriverEntry(PDRIVER_OBJECT driverObject,
>  goto cleanup;
>  }
>
> -/* Create the communication channel for userspace. */
> -status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> -if (status != NDIS_STATUS_SUCCESS) {
> -NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> -gOvsExtDriverHandle = NULL;
> -goto cleanup;
> -}
> -
>  cleanup:
>  if (status != NDIS_STATUS_SUCCESS){
>  OvsCleanup();
> @@ -179,8 +171,6 @@ OvsExtUnload(struct _DRIVER_OBJECT *driverObject)
>  {
>  UNREFERENCED_PARAMETER(driverObject);
>
> -OvsDeleteDeviceObject();
> -
>  NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
>
>  /* Release driver associated data structures. */
> diff --git a/datapath-windows/ovsext/Switch.c b/datapath-
> windows/ovsext/Switch.c
> index 1ac4fa77c..81b897ffe 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -73,6 +73,12 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
>  NDIS_STATUS status = NDIS_STATUS_FAILURE;
>  NDIS_FILTER_ATTRIBUTES ovsExtAttributes;
>  POVS_SWITCH_CONTEXT switchContext = NULL;
> +/* Create the communication channel for userspace. */
> +status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> +if (status != NDIS_STATUS_SUCCESS) {
> +status = NDIS_STATUS_INVALID_PARAMETER;
>
> Why change the status? It would be better to retain the status to whatever is
> returned by OvsCreateDeviceObject()
>
[Alin Serdean] Ooops. Copy paste error . I should also log the error using 
'OVS_LOG_TRACE'.
Does the change fix the issue you encountered?

I will test this patch and get back to you. I think the issue I saw was 
different, and this might not fix it. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only when extension is enabled

2018-04-05 Thread aserdean 


> -Mesaj original-
> De la: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> În numele Shashank Ram
> Trimis: Wednesday, April 4, 2018 8:31 PM
> Către: Alin Gabriel Serdean <aserd...@ovn.org>; d...@openvswitch.org
> Subiect: Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device
> only when extension is enabled
> 
> 
> 
> 
> 
> 
> From: ovs-dev-boun...@openvswitch.org  boun...@openvswitch.org> on behalf of Alin Gabriel Serdean
> <aserd...@ovn.org>
> Sent: Tuesday, April 3, 2018 8:08 AM
> To: d...@openvswitch.org
> Cc: Alin Gabriel Serdean
> Subject: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only
> whenextension is enabled
> 
> Until now the communication device between the kernel and userspace
> is created when the network driver is installed.
> 
> Since we only do processing if the Hyper-V vSwitch extension is enabled by
> the user, it makes more sense to move the device creation only when that is
> true.
> 
> Signed-off-by: Alin Gabriel Serdean <aserd...@ovn.org>
> ---
>  datapath-windows/ovsext/Datapath.c |  2 +-
>  datapath-windows/ovsext/Driver.c   | 10 --
>  datapath-windows/ovsext/Switch.c   |  7 +++
>  3 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/datapath-windows/ovsext/Datapath.c b/datapath-
> windows/ovsext/Datapath.c
> index 34ef2b40a..264cfd3a0 100644
> --- a/datapath-windows/ovsext/Datapath.c
> +++ b/datapath-windows/ovsext/Datapath.c
> @@ -352,7 +352,7 @@ _Dispatch_type_(IRP_MJ_DEVICE_CONTROL)
>  DRIVER_DISPATCH OvsDeviceControl;
> 
>  #ifdef ALLOC_PRAGMA
> -#pragma alloc_text(INIT, OvsCreateDeviceObject)
> +#pragma alloc_text(PAGE, OvsCreateDeviceObject)
>  #pragma alloc_text(PAGE, OvsOpenCloseDevice)
>  #pragma alloc_text(PAGE, OvsCleanupDevice)
>  #pragma alloc_text(PAGE, OvsDeviceControl)
> diff --git a/datapath-windows/ovsext/Driver.c b/datapath-
> windows/ovsext/Driver.c
> index 50c9614e4..39e50c34b 100644
> --- a/datapath-windows/ovsext/Driver.c
> +++ b/datapath-windows/ovsext/Driver.c
> @@ -152,14 +152,6 @@ DriverEntry(PDRIVER_OBJECT driverObject,
>  goto cleanup;
>  }
> 
> -/* Create the communication channel for userspace. */
> -status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> -if (status != NDIS_STATUS_SUCCESS) {
> -NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> -gOvsExtDriverHandle = NULL;
> -goto cleanup;
> -}
> -
>  cleanup:
>  if (status != NDIS_STATUS_SUCCESS){
>  OvsCleanup();
> @@ -179,8 +171,6 @@ OvsExtUnload(struct _DRIVER_OBJECT *driverObject)
>  {
>  UNREFERENCED_PARAMETER(driverObject);
> 
> -OvsDeleteDeviceObject();
> -
>  NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
> 
>  /* Release driver associated data structures. */
> diff --git a/datapath-windows/ovsext/Switch.c b/datapath-
> windows/ovsext/Switch.c
> index 1ac4fa77c..81b897ffe 100644
> --- a/datapath-windows/ovsext/Switch.c
> +++ b/datapath-windows/ovsext/Switch.c
> @@ -73,6 +73,12 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
>  NDIS_STATUS status = NDIS_STATUS_FAILURE;
>  NDIS_FILTER_ATTRIBUTES ovsExtAttributes;
>  POVS_SWITCH_CONTEXT switchContext = NULL;
> +/* Create the communication channel for userspace. */
> +status = OvsCreateDeviceObject(gOvsExtDriverHandle);
> +if (status != NDIS_STATUS_SUCCESS) {
> +status = NDIS_STATUS_INVALID_PARAMETER;
> 
> Why change the status? It would be better to retain the status to whatever is
> returned by OvsCreateDeviceObject()
> 
[Alin Serdean] Ooops. Copy paste error . I should also log the error using 
'OVS_LOG_TRACE'.
Does the change fix the issue you encountered?

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only when extension is enabled

2018-04-04 Thread Shashank Ram 





From: ovs-dev-boun...@openvswitch.org  on 
behalf of Alin Gabriel Serdean 
Sent: Tuesday, April 3, 2018 8:08 AM
To: d...@openvswitch.org
Cc: Alin Gabriel Serdean
Subject: [ovs-dev] [PATCH] RFC datapath-windows: Create com device only when
extension is enabled

Until now the communication device between the kernel and userspace
is created when the network driver is installed.

Since we only do processing if the Hyper-V vSwitch extension is enabled by
the user, it makes more sense to move the device creation only when that is 
true.

Signed-off-by: Alin Gabriel Serdean 
---
 datapath-windows/ovsext/Datapath.c |  2 +-
 datapath-windows/ovsext/Driver.c   | 10 --
 datapath-windows/ovsext/Switch.c   |  7 +++
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/datapath-windows/ovsext/Datapath.c 
b/datapath-windows/ovsext/Datapath.c
index 34ef2b40a..264cfd3a0 100644
--- a/datapath-windows/ovsext/Datapath.c
+++ b/datapath-windows/ovsext/Datapath.c
@@ -352,7 +352,7 @@ _Dispatch_type_(IRP_MJ_DEVICE_CONTROL)
 DRIVER_DISPATCH OvsDeviceControl;

 #ifdef ALLOC_PRAGMA
-#pragma alloc_text(INIT, OvsCreateDeviceObject)
+#pragma alloc_text(PAGE, OvsCreateDeviceObject)
 #pragma alloc_text(PAGE, OvsOpenCloseDevice)
 #pragma alloc_text(PAGE, OvsCleanupDevice)
 #pragma alloc_text(PAGE, OvsDeviceControl)
diff --git a/datapath-windows/ovsext/Driver.c b/datapath-windows/ovsext/Driver.c
index 50c9614e4..39e50c34b 100644
--- a/datapath-windows/ovsext/Driver.c
+++ b/datapath-windows/ovsext/Driver.c
@@ -152,14 +152,6 @@ DriverEntry(PDRIVER_OBJECT driverObject,
 goto cleanup;
 }

-/* Create the communication channel for userspace. */
-status = OvsCreateDeviceObject(gOvsExtDriverHandle);
-if (status != NDIS_STATUS_SUCCESS) {
-NdisFDeregisterFilterDriver(gOvsExtDriverHandle);
-gOvsExtDriverHandle = NULL;
-goto cleanup;
-}
-
 cleanup:
 if (status != NDIS_STATUS_SUCCESS){
 OvsCleanup();
@@ -179,8 +171,6 @@ OvsExtUnload(struct _DRIVER_OBJECT *driverObject)
 {
 UNREFERENCED_PARAMETER(driverObject);

-OvsDeleteDeviceObject();
-
 NdisFDeregisterFilterDriver(gOvsExtDriverHandle);

 /* Release driver associated data structures. */
diff --git a/datapath-windows/ovsext/Switch.c b/datapath-windows/ovsext/Switch.c
index 1ac4fa77c..81b897ffe 100644
--- a/datapath-windows/ovsext/Switch.c
+++ b/datapath-windows/ovsext/Switch.c
@@ -73,6 +73,12 @@ OvsExtAttach(NDIS_HANDLE ndisFilterHandle,
 NDIS_STATUS status = NDIS_STATUS_FAILURE;
 NDIS_FILTER_ATTRIBUTES ovsExtAttributes;
 POVS_SWITCH_CONTEXT switchContext = NULL;
+/* Create the communication channel for userspace. */
+status = OvsCreateDeviceObject(gOvsExtDriverHandle);
+if (status != NDIS_STATUS_SUCCESS) {
+status = NDIS_STATUS_INVALID_PARAMETER;

Why change the status? It would be better to retain the status to whatever is 
returned by OvsCreateDeviceObject()

+goto cleanup;
+}

 UNREFERENCED_PARAMETER(filterDriverContext);

@@ -266,6 +272,7 @@ OvsExtDetach(NDIS_HANDLE filterModuleContext)
 while(switchContext->pendingOidCount > 0) {
 NdisMSleep(1000);
 }
+OvsDeleteDeviceObject();
 OvsDeleteSwitch(switchContext);
 OvsCleanupIpHelper();
 OvsCleanupSttDefragmentation();
--
2.16.1.windows.1

___

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev