diff mbox

[ovs-dev] ovs-numa: fixed cmask parse with 0x prefix

Message ID FC7607CF-1C09-443E-9418-F2B142915491@intel.com
State Superseded
Headers show

Commit Message

Wei Shen July 28, 2016, 6:22 a.m. UTC
Thanks for the reply. The INSTALL.DPDK.md has those “0x” prefix used as example

212      * dpdk-lcore-mask
213      Specifies the CPU cores on which dpdk lcore threads should be spawned and
214      expects hex string (eg '0x123').

so I think either we make those documents compliant or make the parsing be able to accept both form as long as they are base 16 regardless of the presence of “0x”.

Also thanks for the styling reminder… I haven’t gone through those in much detail. Let me send another patch that complies with those.

--
Best,
Wei Shen.

From: Daniele Di Proietto <diproiettod@ovn.org>

Date: Wednesday, July 27, 2016 at 2:28 PM
To: Wei1 Shen <wei1.shen@intel.com>
Cc: "dev@openvswitch.org" <dev@openvswitch.org>
Subject: Re: [ovs-dev] [PATCH] ovs-numa: fixed cmask parse with 0x prefix

Thanks for the patch.
We never accepted the 0x prefix for pmd-cpu-mask, but I guess there's no harm in doing it and it might make user's life easier.
We always use braces, even for single statement, please read CodingStyle.md

https://github.com/openvswitch/ovs/blob/master/CodingStyle.md#statements
I cannot merge this unless you provide a signoff, the details and the meaning is explained here:

https://github.com/openvswitch/ovs/blob/master/CONTRIBUTING.md#developers-certificate-of-origin
Thanks,
Daniele

2016-07-26 14:56 GMT-07:00 Wei Shen <wei1.shen@intel.com<mailto:wei1.shen@intel.com>>:
Fixed a minor bug that would print out a confusing warning about core mask,
"ovs_numa|WARN|Invalid cpu mask: x", when dpdl-lcore-mask has 0x prefix, e.g.
0x123, which is the convention used in INSTALL.DPDK.md<http://INSTALL.DPDK.md>.
---
 lib/ovs-numa.c | 4 ++++
 1 file changed, 4 insertions(+)

--
2.5.5

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

Comments

Daniele Di Proietto July 29, 2016, 10:17 p.m. UTC | #1
2016-07-27 23:22 GMT-07:00 Shen, Wei1 <wei1.shen@intel.com>:

> Thanks for the reply. The INSTALL.DPDK.md has those “0x” prefix used as
> example
>
>
>
> 212      * dpdk-lcore-mask
>
> 213      Specifies the CPU cores on which dpdk lcore threads should be
> spawned and
>
> 214      expects hex string (eg '0x123').
>
>
>
> so I think either we make those documents compliant or make the parsing be
> able to accept both form as long as they are base 16 regardless of the
> presence of “0x”.
>

OVS already has no problem accepting 0x prefixes as part of
"dpdk-lcore-mask".

With your patch, OVS accepts the 0x prefix also as part of "pmd-cpu-mask",
which I think is an enhancement.  If this is the intended effect, please
update the commit message and submit another version.

Thanks,

Daniele


>
>
> Also thanks for the styling reminder… I haven’t gone through those in much
> detail. Let me send another patch that complies with those.
>
>
>
> --
>
> Best,
>
> Wei Shen.
>
>
>
> *From: *Daniele Di Proietto <diproiettod@ovn.org>
> *Date: *Wednesday, July 27, 2016 at 2:28 PM
> *To: *Wei1 Shen <wei1.shen@intel.com>
> *Cc: *"dev@openvswitch.org" <dev@openvswitch.org>
> *Subject: *Re: [ovs-dev] [PATCH] ovs-numa: fixed cmask parse with 0x
> prefix
>
>
>
> Thanks for the patch.
>
> We never accepted the 0x prefix for pmd-cpu-mask, but I guess there's no
> harm in doing it and it might make user's life easier.
>
> We always use braces, even for single statement, please read CodingStyle.md
>
> https://github.com/openvswitch/ovs/blob/master/CodingStyle.md#statements
>
> I cannot merge this unless you provide a signoff, the details and the
> meaning is explained here:
>
>
> https://github.com/openvswitch/ovs/blob/master/CONTRIBUTING.md#developers-certificate-of-origin
>
> Thanks,
>
> Daniele
>
>
>
> 2016-07-26 14:56 GMT-07:00 Wei Shen <wei1.shen@intel.com>:
>
> Fixed a minor bug that would print out a confusing warning about core mask,
> "ovs_numa|WARN|Invalid cpu mask: x", when dpdl-lcore-mask has 0x prefix,
> e.g.
> 0x123, which is the convention used in INSTALL.DPDK.md.
> ---
>  lib/ovs-numa.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
> index c8173e0..c1938eb 100644
> --- a/lib/ovs-numa.c
> +++ b/lib/ovs-numa.c
> @@ -551,6 +551,10 @@ ovs_numa_set_cpu_mask(const char *cmask)
>          return;
>      }
>
> +    /* Skip 0x if supplied in the cmask */
> +    if (!strncmp(cmask, "0x", 2))
> +        cmask += 2;
> +
>      for (i = strlen(cmask) - 1; i >= 0; i--) {
>          char hex = toupper((unsigned char)cmask[i]);
>          int bin, j;
> --
> 2.5.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
>
>
diff mbox

Patch

diff --git a/lib/ovs-numa.c b/lib/ovs-numa.c
index c8173e0..c1938eb 100644
--- a/lib/ovs-numa.c
+++ b/lib/ovs-numa.c
@@ -551,6 +551,10 @@  ovs_numa_set_cpu_mask(const char *cmask)
         return;
     }

+    /* Skip 0x if supplied in the cmask */
+    if (!strncmp(cmask, "0x", 2))
+        cmask += 2;
+
     for (i = strlen(cmask) - 1; i >= 0; i--) {
         char hex = toupper((unsigned char)cmask[i]);
         int bin, j;