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

[Daniele Di Proietto]

2016-07-27 23:22 GMT-07:00 Shen, Wei1 <wei1.s...@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 <diproiet...@ovn.org>
> *Date: *Wednesday, July 27, 2016 at 2:28 PM
> *To: *Wei1 Shen <wei1.s...@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.s...@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
>
>
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

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

[Shen, Wei1]

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 <diproiet...@ovn.org>
Date: Wednesday, July 27, 2016 at 2:28 PM
To: Wei1 Shen <wei1.s...@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.s...@intel.com<mailto:wei1.s...@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(+)

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<mailto:dev@openvswitch.org>
http://openvswitch.org/mailman/listinfo/dev

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

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

[Daniele Di Proietto]

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 :

> 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
>
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev