Re: [ovs-dev] [PATCH 12/13] dpdk: New module with some code from netdev-dpdk.

2016-10-12 Thread Daniele Di Proietto 





On 12/10/2016 13:59, "Aaron Conole"  wrote:

>Daniele Di Proietto  writes:
>
>> There's a lot of code in netdev-dpdk which is not at all related to the
>> netdev interface, mostly the library initialization code.
>>
>> This commit moves it to a new 'dpdk' module, to simplify 'netdev-dpdk'.
>>
>> Also a new module 'dpdk-stub' is introduced to implement some functions
>> when DPDK is not available.  This replaces the old 'netdev-nodpdk'
>> module.
>>
>> Some redundant includes are removed or reorganized as a consequence.
>>
>> No functional change.
>>
>> CC: Aaron Conole 
>> Signed-off-by: Daniele Di Proietto 
>> ---
>
>First - thanks for this.  I like everything about 1-12/13.
>
>You have my ACK for the first 12 of the series (and my Tested-by: as
>well - though I have only tested --with-dpdk).

Thanks for the review, I added your acks and tested-bys and applied this
to master!

>
>Just one thing...
>
>> diff --git a/lib/netdev-nodpdk.c b/lib/dpdk-stub.c
>> similarity index 84%
>> rename from lib/netdev-nodpdk.c
>> rename to lib/dpdk-stub.c
>> index 45564d2..42196c4 100644
>> --- a/lib/netdev-nodpdk.c
>> +++ b/lib/dpdk-stub.c
>> @@ -1,4 +1,5 @@
>>  /*
>> + * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
>>   * Copyright (c) 2016 Red Hat, Inc.
>>   *
>>   * Licensed under the Apache License, Version 2.0 (the "License");
>> @@ -15,7 +16,8 @@
>>   */
>>  
>>  #include 
>> -#include "netdev-dpdk.h"
>> +#include "dpdk.h"
>> +
>>  #include "smap.h"
>>  #include "ovs-thread.h"
>>  #include "openvswitch/vlog.h"
>> @@ -34,3 +36,16 @@ dpdk_init(const struct smap *ovs_other_config)
>>  ovsthread_once_done();
>>  }
>>  }
>> +
>> +void
>> +dpdk_set_lcore_id(unsigned cpu OVS_UNUSED)
>> +{
>> +/* Nothing */
>> +}
>> +
>> +const char *
>> +dpdk_get_vhost_sock_dir(void)
>> +{
>> +return NULL;
>> +}
>> +
>
>When applying, I get one warning from git here, for introducing this
>newline.
>
>   Applying: dpdk: New module with some code from netdev-dpdk.
>   .git/rebase-apply/patch:132: new blank line at EOF.
>   +
>   warning: 1 line adds whitespace errors.
>
>Just thought I'd point it out.  ;-)  It can probably be either ignored
>or fixed when applying to master.

after fixing this, thanks for noticing!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 12/13] dpdk: New module with some code from netdev-dpdk.

2016-10-12 Thread Aaron Conole 
Daniele Di Proietto  writes:

> There's a lot of code in netdev-dpdk which is not at all related to the
> netdev interface, mostly the library initialization code.
>
> This commit moves it to a new 'dpdk' module, to simplify 'netdev-dpdk'.
>
> Also a new module 'dpdk-stub' is introduced to implement some functions
> when DPDK is not available.  This replaces the old 'netdev-nodpdk'
> module.
>
> Some redundant includes are removed or reorganized as a consequence.
>
> No functional change.
>
> CC: Aaron Conole 
> Signed-off-by: Daniele Di Proietto 
> ---

First - thanks for this.  I like everything about 1-12/13.

You have my ACK for the first 12 of the series (and my Tested-by: as
well - though I have only tested --with-dpdk).

Just one thing...

> diff --git a/lib/netdev-nodpdk.c b/lib/dpdk-stub.c
> similarity index 84%
> rename from lib/netdev-nodpdk.c
> rename to lib/dpdk-stub.c
> index 45564d2..42196c4 100644
> --- a/lib/netdev-nodpdk.c
> +++ b/lib/dpdk-stub.c
> @@ -1,4 +1,5 @@
>  /*
> + * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
>   * Copyright (c) 2016 Red Hat, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
> @@ -15,7 +16,8 @@
>   */
>  
>  #include 
> -#include "netdev-dpdk.h"
> +#include "dpdk.h"
> +
>  #include "smap.h"
>  #include "ovs-thread.h"
>  #include "openvswitch/vlog.h"
> @@ -34,3 +36,16 @@ dpdk_init(const struct smap *ovs_other_config)
>  ovsthread_once_done();
>  }
>  }
> +
> +void
> +dpdk_set_lcore_id(unsigned cpu OVS_UNUSED)
> +{
> +/* Nothing */
> +}
> +
> +const char *
> +dpdk_get_vhost_sock_dir(void)
> +{
> +return NULL;
> +}
> +

When applying, I get one warning from git here, for introducing this
newline.

   Applying: dpdk: New module with some code from netdev-dpdk.
   .git/rebase-apply/patch:132: new blank line at EOF.
   +
   warning: 1 line adds whitespace errors.

Just thought I'd point it out.  ;-)  It can probably be either ignored
or fixed when applying to master.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 12/13] dpdk: New module with some code from netdev-dpdk.

2016-10-04 Thread Daniele Di Proietto 
There's a lot of code in netdev-dpdk which is not at all related to the
netdev interface, mostly the library initialization code.

This commit moves it to a new 'dpdk' module, to simplify 'netdev-dpdk'.

Also a new module 'dpdk-stub' is introduced to implement some functions
when DPDK is not available.  This replaces the old 'netdev-nodpdk'
module.

Some redundant includes are removed or reorganized as a consequence.

No functional change.

CC: Aaron Conole 
Signed-off-by: Daniele Di Proietto 
---
 lib/automake.mk  |   9 +-
 lib/dp-packet.c  |   5 +-
 lib/dp-packet.h  |   8 +-
 lib/{netdev-nodpdk.c => dpdk-stub.c} |  17 +-
 lib/dpdk.c   | 432 +++
 lib/dpdk.h   |  39 
 lib/dpif-netdev.c|   5 +-
 lib/dpif.h   |   2 +
 lib/netdev-dpdk.c| 429 ++
 lib/netdev-dpdk.h|  36 +--
 vswitchd/bridge.c|   2 +-
 11 files changed, 529 insertions(+), 455 deletions(-)
 rename lib/{netdev-nodpdk.c => dpdk-stub.c} (84%)
 create mode 100644 lib/dpdk.c
 create mode 100644 lib/dpdk.h

diff --git a/lib/automake.mk b/lib/automake.mk
index e9d508a..81d5097 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -77,6 +77,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/dpctl.h \
lib/dp-packet.h \
lib/dp-packet.c \
+   lib/dpdk.h \
lib/dpif-netdev.c \
lib/dpif-netdev.h \
lib/dpif-provider.h \
@@ -128,6 +129,7 @@ lib_libopenvswitch_la_SOURCES = \
lib/meta-flow.c \
lib/multipath.c \
lib/multipath.h \
+   lib/netdev-dpdk.h \
lib/netdev-dummy.c \
lib/netdev-provider.h \
lib/netdev-vport.c \
@@ -368,12 +370,11 @@ endif
 
 if DPDK_NETDEV
 lib_libopenvswitch_la_SOURCES += \
-   lib/netdev-dpdk.c \
-   lib/netdev-dpdk.h
+   lib/dpdk.c \
+   lib/netdev-dpdk.c
 else
 lib_libopenvswitch_la_SOURCES += \
-   lib/netdev-nodpdk.c \
-   lib/netdev-dpdk.h
+   lib/dpdk-stub.c
 endif
 
 if WIN32
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 8e7defc..793b54f 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -17,9 +17,10 @@
 #include 
 #include 
 #include 
-#include "openvswitch/dynamic-string.h"
-#include "netdev-dpdk.h"
+
 #include "dp-packet.h"
+#include "netdev-dpdk.h"
+#include "openvswitch/dynamic-string.h"
 #include "util.h"
 
 static void
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 7c1e637..1469864 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -19,10 +19,16 @@
 
 #include 
 #include 
+
+#ifdef DPDK_NETDEV
+#include 
+#include 
+#endif
+
+#include "netdev-dpdk.h"
 #include "openvswitch/list.h"
 #include "packets.h"
 #include "util.h"
-#include "netdev-dpdk.h"
 
 #ifdef  __cplusplus
 extern "C" {
diff --git a/lib/netdev-nodpdk.c b/lib/dpdk-stub.c
similarity index 84%
rename from lib/netdev-nodpdk.c
rename to lib/dpdk-stub.c
index 45564d2..42196c4 100644
--- a/lib/netdev-nodpdk.c
+++ b/lib/dpdk-stub.c
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
  * Copyright (c) 2016 Red Hat, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -15,7 +16,8 @@
  */
 
 #include 
-#include "netdev-dpdk.h"
+#include "dpdk.h"
+
 #include "smap.h"
 #include "ovs-thread.h"
 #include "openvswitch/vlog.h"
@@ -34,3 +36,16 @@ dpdk_init(const struct smap *ovs_other_config)
 ovsthread_once_done();
 }
 }
+
+void
+dpdk_set_lcore_id(unsigned cpu OVS_UNUSED)
+{
+/* Nothing */
+}
+
+const char *
+dpdk_get_vhost_sock_dir(void)
+{
+return NULL;
+}
+
diff --git a/lib/dpdk.c b/lib/dpdk.c
new file mode 100644
index 000..caea0f4
--- /dev/null
+++ b/lib/dpdk.c
@@ -0,0 +1,432 @@
+/*
+ * Copyright (c) 2014, 2015, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+#include "dpdk.h"
+
+#include 
+#include 
+#include 
+
+#include 
+
+#include "dirs.h"
+#include "netdev-dpdk.h"
+#include "openvswitch/dynamic-string.h"
+#include "openvswitch/vlog.h"
+#include "smap.h"
+
+VLOG_DEFINE_THIS_MODULE(dpdk);
+
+static char *vhost_sock_dir = NULL;   /* Location of vhost-user sockets */
+
+static int
+process_vhost_flags(char *flag, char *default_val, int size,
+const struct smap *ovs_other_config,