* [dpdk-stable] [PATCH] app/testpmd: fix support of hex string parser for flow API
@ 2019-03-15 8:43 Wei Zhao
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Wei Zhao @ 2019-03-15 8:43 UTC (permalink / raw)
To: dev; +Cc: stable, qi.z.zhang, adrien.mazarguil, Wei Zhao
There is need for users to set configuration of HEX number for RSS
key. The key byte should be pass down as hex number not as char
string. This patch enable cmdline flow parse HEX number,
in order to not using string which pass ASIC number.
Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
Cc: stable@dpdk.org
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
app/test-pmd/cmdline_flow.c | 141 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 140 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 36659a6..2445731 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -35,6 +35,7 @@ enum index {
PREFIX,
BOOLEAN,
STRING,
+ HEX,
MAC_ADDR,
IPV4_ADDR,
IPV6_ADDR,
@@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const struct token *,
static int parse_string(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size);
static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
.call = parse_string,
.comp = comp_none,
},
+ [HEX] = {
+ .name = "{hex}",
+ .type = "HEX",
+ .help = "fixed string",
+ .call = parse_hex,
+ .comp = comp_none,
+ },
[MAC_ADDR] = {
.name = "{MAC address}",
.type = "MAC-48",
@@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
.name = "pattern",
.help = "byte string to look for",
.next = NEXT(item_raw,
- NEXT_ENTRY(STRING),
+ NEXT_ENTRY(HEX),
NEXT_ENTRY(ITEM_PARAM_IS,
ITEM_PARAM_SPEC,
ITEM_PARAM_MASK)),
@@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const struct token *token,
return -1;
}
+static uint32_t
+get_hex_val(char c)
+{
+ switch (c) {
+ case '0': case '1': case '2': case '3': case '4': case '5':
+ case '6': case '7': case '8': case '9':
+ return c - '0';
+ case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
+ return c - 'A' + 10;
+ case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
+ return c - 'a' + 10;
+ default:
+ return 0;
+ }
+}
+
+static int
+parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
+{
+ const char *c;
+ uint32_t i;
+
+ /* Check input parameters */
+ if ((src == NULL) ||
+ (dst == NULL) ||
+ (size == NULL) ||
+ (*size == 0))
+ return -1;
+ if ((*size & 1) != 0)
+ return -1;
+
+ for (c = src, i = 0; i < *size; c++, i++) {
+ if ((((*c) >= '0') && ((*c) <= '9')) ||
+ (((*c) >= 'A') && ((*c) <= 'F')) ||
+ (((*c) >= 'a') && ((*c) <= 'f')))
+ continue;
+
+ return -1;
+ }
+ *size = *size / 2;
+
+ /* Convert chars to bytes */
+ for (i = 0; i < *size; i++)
+ dst[i] = get_hex_val(src[2 * i]) * 16 +
+ get_hex_val(src[2 * i + 1]);
+
+ return 0;
+}
+
+static int
+parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg_data = pop_args(ctx);
+ const struct arg *arg_len = pop_args(ctx);
+ const struct arg *arg_addr = pop_args(ctx);
+ char tmp[16]; /* Ought to be enough. */
+ int ret;
+ unsigned int hexlen = len;
+ uint8_t hex_tmp[16];
+
+ /* Arguments are expected. */
+ if (!arg_data)
+ return -1;
+ if (!arg_len) {
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ if (!arg_addr) {
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ size = arg_data->size;
+ /* Bit-mask fill is not supported. */
+ if (arg_data->mask || size < len)
+ goto error;
+ if (!ctx->object)
+ return len;
+
+ /* translate bytes string to array. */
+ if (str[0] == '0' && ((str[1] == 'x') ||
+ (str[1] == 'X'))) {
+ str += 2;
+ hexlen -= 2;
+ }
+ parse_hex_string(str, hex_tmp, &hexlen);
+ /* Let parse_int() fill length information first. */
+ ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
+ if (ret < 0)
+ goto error;
+ push_args(ctx, arg_len);
+ ret = parse_int(ctx, token, tmp, ret, NULL, 0);
+ if (ret < 0) {
+ pop_args(ctx);
+ goto error;
+ }
+ buf = (uint8_t *)ctx->object + arg_data->offset;
+ /* Output buffer is not necessarily NUL-terminated. */
+ memcpy(buf, hex_tmp, hexlen);
+ memset((uint8_t *)buf + len, 0x00, size - hexlen);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg_data->offset,
+ 0xff, hexlen);
+ /* Save address if requested. */
+ if (arg_addr->size) {
+ memcpy((uint8_t *)ctx->object + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->object + arg_data->offset
+ },
+ arg_addr->size);
+ if (ctx->objmask)
+ memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->objmask + arg_data->offset
+ },
+ arg_addr->size);
+ }
+ return len;
+error:
+ push_args(ctx, arg_addr);
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+
+}
+
/**
* Parse a MAC address.
*
--
2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-15 8:43 [dpdk-stable] [PATCH] app/testpmd: fix support of hex string parser for flow API Wei Zhao
@ 2019-03-15 9:44 ` Ananyev, Konstantin
2019-03-18 1:36 ` Zhao1, Wei
2019-03-18 8:49 ` Zhao1, Wei
2019-03-18 8:16 ` [dpdk-stable] [PATCH v2] " Wei Zhao
2019-03-18 15:27 ` [dpdk-stable] [dpdk-dev] [PATCH] " Stephen Hemminger
2 siblings, 2 replies; 24+ messages in thread
From: Ananyev, Konstantin @ 2019-03-15 9:44 UTC (permalink / raw)
To: Zhao1, Wei, dev; +Cc: stable, Zhang, Qi Z, adrien.mazarguil, Zhao1, Wei
Hi Wei,
>
> There is need for users to set configuration of HEX number for RSS
> key. The key byte should be pass down as hex number not as char
> string. This patch enable cmdline flow parse HEX number,
> in order to not using string which pass ASIC number.
>
> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> ---
> app/test-pmd/cmdline_flow.c | 141 +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 140 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 36659a6..2445731 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -35,6 +35,7 @@ enum index {
> PREFIX,
> BOOLEAN,
> STRING,
> + HEX,
> MAC_ADDR,
> IPV4_ADDR,
> IPV6_ADDR,
> @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const struct token *,
> static int parse_string(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> +static int parse_hex(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size);
> static int parse_mac_addr(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> .call = parse_string,
> .comp = comp_none,
> },
> + [HEX] = {
> + .name = "{hex}",
> + .type = "HEX",
> + .help = "fixed string",
> + .call = parse_hex,
> + .comp = comp_none,
> + },
> [MAC_ADDR] = {
> .name = "{MAC address}",
> .type = "MAC-48",
> @@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
> .name = "pattern",
> .help = "byte string to look for",
> .next = NEXT(item_raw,
> - NEXT_ENTRY(STRING),
> + NEXT_ENTRY(HEX),
> NEXT_ENTRY(ITEM_PARAM_IS,
> ITEM_PARAM_SPEC,
> ITEM_PARAM_MASK)),
> @@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const struct token *token,
> return -1;
> }
>
> +static uint32_t
> +get_hex_val(char c)
> +{
> + switch (c) {
> + case '0': case '1': case '2': case '3': case '4': case '5':
> + case '6': case '7': case '8': case '9':
> + return c - '0';
> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> + return c - 'A' + 10;
> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> + return c - 'a' + 10;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
> +{
> + const char *c;
> + uint32_t i;
> +
> + /* Check input parameters */
> + if ((src == NULL) ||
> + (dst == NULL) ||
> + (size == NULL) ||
> + (*size == 0))
> + return -1;
> + if ((*size & 1) != 0)
> + return -1;
> +
> + for (c = src, i = 0; i < *size; c++, i++) {
> + if ((((*c) >= '0') && ((*c) <= '9')) ||
> + (((*c) >= 'A') && ((*c) <= 'F')) ||
> + (((*c) >= 'a') && ((*c) <= 'f')))
> + continue;
> +
> + return -1;
> + }
> + *size = *size / 2;
> +
> + /* Convert chars to bytes */
> + for (i = 0; i < *size; i++)
> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> + get_hex_val(src[2 * i + 1]);
> +
> + return 0;
> +}
What's wrong with strtoul() and friends?
Why you need to write your own version?
Konstantin
> +
> +static int
> +parse_hex(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size)
> +{
> + const struct arg *arg_data = pop_args(ctx);
> + const struct arg *arg_len = pop_args(ctx);
> + const struct arg *arg_addr = pop_args(ctx);
> + char tmp[16]; /* Ought to be enough. */
> + int ret;
> + unsigned int hexlen = len;
> + uint8_t hex_tmp[16];
> +
> + /* Arguments are expected. */
> + if (!arg_data)
> + return -1;
> + if (!arg_len) {
> + push_args(ctx, arg_data);
> + return -1;
> + }
> + if (!arg_addr) {
> + push_args(ctx, arg_len);
> + push_args(ctx, arg_data);
> + return -1;
> + }
> + size = arg_data->size;
> + /* Bit-mask fill is not supported. */
> + if (arg_data->mask || size < len)
> + goto error;
> + if (!ctx->object)
> + return len;
> +
> + /* translate bytes string to array. */
> + if (str[0] == '0' && ((str[1] == 'x') ||
> + (str[1] == 'X'))) {
> + str += 2;
> + hexlen -= 2;
> + }
> + parse_hex_string(str, hex_tmp, &hexlen);
> + /* Let parse_int() fill length information first. */
> + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> + if (ret < 0)
> + goto error;
> + push_args(ctx, arg_len);
> + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> + if (ret < 0) {
> + pop_args(ctx);
> + goto error;
> + }
> + buf = (uint8_t *)ctx->object + arg_data->offset;
> + /* Output buffer is not necessarily NUL-terminated. */
> + memcpy(buf, hex_tmp, hexlen);
> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> + if (ctx->objmask)
> + memset((uint8_t *)ctx->objmask + arg_data->offset,
> + 0xff, hexlen);
> + /* Save address if requested. */
> + if (arg_addr->size) {
> + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> + (void *[]){
> + (uint8_t *)ctx->object + arg_data->offset
> + },
> + arg_addr->size);
> + if (ctx->objmask)
> + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> + (void *[]){
> + (uint8_t *)ctx->objmask + arg_data->offset
> + },
> + arg_addr->size);
> + }
> + return len;
> +error:
> + push_args(ctx, arg_addr);
> + push_args(ctx, arg_len);
> + push_args(ctx, arg_data);
> + return -1;
> +
> +}
> +
> /**
> * Parse a MAC address.
> *
> --
> 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
@ 2019-03-18 1:36 ` Zhao1, Wei
2019-03-18 8:49 ` Zhao1, Wei
1 sibling, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-18 1:36 UTC (permalink / raw)
To: Ananyev, Konstantin, dev; +Cc: stable, Zhang, Qi Z, adrien.mazarguil
Hi, Konstantin
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, March 15, 2019 5:45 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> adrien.mazarguil@6wind.com; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix support of hex string
> parser for flow API
>
> Hi Wei,
>
> >
> > There is need for users to set configuration of HEX number for RSS
> > key. The key byte should be pass down as hex number not as char
> > string. This patch enable cmdline flow parse HEX number, in order to
> > not using string which pass ASIC number.
> >
> > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > action")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 141
> > +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 36659a6..2445731 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -35,6 +35,7 @@ enum index {
> > PREFIX,
> > BOOLEAN,
> > STRING,
> > + HEX,
> > MAC_ADDR,
> > IPV4_ADDR,
> > IPV6_ADDR,
> > @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const
> > struct token *, static int parse_string(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > +static int parse_hex(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size);
> > static int parse_mac_addr(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> > .call = parse_string,
> > .comp = comp_none,
> > },
> > + [HEX] = {
> > + .name = "{hex}",
> > + .type = "HEX",
> > + .help = "fixed string",
> > + .call = parse_hex,
> > + .comp = comp_none,
> > + },
> > [MAC_ADDR] = {
> > .name = "{MAC address}",
> > .type = "MAC-48",
> > @@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
> > .name = "pattern",
> > .help = "byte string to look for",
> > .next = NEXT(item_raw,
> > - NEXT_ENTRY(STRING),
> > + NEXT_ENTRY(HEX),
> > NEXT_ENTRY(ITEM_PARAM_IS,
> > ITEM_PARAM_SPEC,
> > ITEM_PARAM_MASK)),
> > @@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const struct
> token *token,
> > return -1;
> > }
> >
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > + switch (c) {
> > + case '0': case '1': case '2': case '3': case '4': case '5':
> > + case '6': case '7': case '8': case '9':
> > + return c - '0';
> > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > + return c - 'A' + 10;
> > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > + return c - 'a' + 10;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int
> > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > + const char *c;
> > + uint32_t i;
> > +
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if ((((*c) >= '0') && ((*c) <= '9')) ||
> > + (((*c) >= 'A') && ((*c) <= 'F')) ||
> > + (((*c) >= 'a') && ((*c) <= 'f')))
> > + continue;
> > +
> > + return -1;
> > + }
> > + *size = *size / 2;
> > +
> > + /* Convert chars to bytes */
> > + for (i = 0; i < *size; i++)
> > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > + get_hex_val(src[2 * i + 1]);
> > +
> > + return 0;
> > +}
>
> What's wrong with strtoul() and friends?
> Why you need to write your own version?
> Konstantin
>
Good idea, I WILL replace get_hex_val() with strtoul() in v2.
> > +
> > +static int
> > +parse_hex(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size)
> > +{
> > + const struct arg *arg_data = pop_args(ctx);
> > + const struct arg *arg_len = pop_args(ctx);
> > + const struct arg *arg_addr = pop_args(ctx);
> > + char tmp[16]; /* Ought to be enough. */
> > + int ret;
> > + unsigned int hexlen = len;
> > + uint8_t hex_tmp[16];
> > +
> > + /* Arguments are expected. */
> > + if (!arg_data)
> > + return -1;
> > + if (!arg_len) {
> > + push_args(ctx, arg_data);
> > + return -1;
> > + }
> > + if (!arg_addr) {
> > + push_args(ctx, arg_len);
> > + push_args(ctx, arg_data);
> > + return -1;
> > + }
> > + size = arg_data->size;
> > + /* Bit-mask fill is not supported. */
> > + if (arg_data->mask || size < len)
> > + goto error;
> > + if (!ctx->object)
> > + return len;
> > +
> > + /* translate bytes string to array. */
> > + if (str[0] == '0' && ((str[1] == 'x') ||
> > + (str[1] == 'X'))) {
> > + str += 2;
> > + hexlen -= 2;
> > + }
> > + parse_hex_string(str, hex_tmp, &hexlen);
> > + /* Let parse_int() fill length information first. */
> > + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> > + if (ret < 0)
> > + goto error;
> > + push_args(ctx, arg_len);
> > + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> > + if (ret < 0) {
> > + pop_args(ctx);
> > + goto error;
> > + }
> > + buf = (uint8_t *)ctx->object + arg_data->offset;
> > + /* Output buffer is not necessarily NUL-terminated. */
> > + memcpy(buf, hex_tmp, hexlen);
> > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> > + if (ctx->objmask)
> > + memset((uint8_t *)ctx->objmask + arg_data->offset,
> > + 0xff, hexlen);
> > + /* Save address if requested. */
> > + if (arg_addr->size) {
> > + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > + (void *[]){
> > + (uint8_t *)ctx->object + arg_data->offset
> > + },
> > + arg_addr->size);
> > + if (ctx->objmask)
> > + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > + (void *[]){
> > + (uint8_t *)ctx->objmask + arg_data->offset
> > + },
> > + arg_addr->size);
> > + }
> > + return len;
> > +error:
> > + push_args(ctx, arg_addr);
> > + push_args(ctx, arg_len);
> > + push_args(ctx, arg_data);
> > + return -1;
> > +
> > +}
> > +
> > /**
> > * Parse a MAC address.
> > *
> > --
> > 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
2019-03-18 1:36 ` Zhao1, Wei
@ 2019-03-18 8:49 ` Zhao1, Wei
2019-03-21 14:03 ` Ananyev, Konstantin
1 sibling, 1 reply; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-18 8:49 UTC (permalink / raw)
To: Ananyev, Konstantin, dev; +Cc: stable, Zhang, Qi Z, adrien.mazarguil
Hi, Konstantin
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, March 15, 2019 5:45 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> adrien.mazarguil@6wind.com; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix support of hex string
> parser for flow API
>
> Hi Wei,
>
> >
> > There is need for users to set configuration of HEX number for RSS
> > key. The key byte should be pass down as hex number not as char
> > string. This patch enable cmdline flow parse HEX number, in order to
> > not using string which pass ASIC number.
> >
> > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > action")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > ---
> > app/test-pmd/cmdline_flow.c | 141
> > +++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > index 36659a6..2445731 100644
> > --- a/app/test-pmd/cmdline_flow.c
> > +++ b/app/test-pmd/cmdline_flow.c
> > @@ -35,6 +35,7 @@ enum index {
> > PREFIX,
> > BOOLEAN,
> > STRING,
> > + HEX,
> > MAC_ADDR,
> > IPV4_ADDR,
> > IPV6_ADDR,
> > @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const
> > struct token *, static int parse_string(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > +static int parse_hex(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size);
> > static int parse_mac_addr(struct context *, const struct token *,
> > const char *, unsigned int,
> > void *, unsigned int);
> > @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> > .call = parse_string,
> > .comp = comp_none,
> > },
> > + [HEX] = {
> > + .name = "{hex}",
> > + .type = "HEX",
> > + .help = "fixed string",
> > + .call = parse_hex,
> > + .comp = comp_none,
> > + },
> > [MAC_ADDR] = {
> > .name = "{MAC address}",
> > .type = "MAC-48",
> > @@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
> > .name = "pattern",
> > .help = "byte string to look for",
> > .next = NEXT(item_raw,
> > - NEXT_ENTRY(STRING),
> > + NEXT_ENTRY(HEX),
> > NEXT_ENTRY(ITEM_PARAM_IS,
> > ITEM_PARAM_SPEC,
> > ITEM_PARAM_MASK)),
> > @@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const struct
> token *token,
> > return -1;
> > }
> >
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > + switch (c) {
> > + case '0': case '1': case '2': case '3': case '4': case '5':
> > + case '6': case '7': case '8': case '9':
> > + return c - '0';
> > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > + return c - 'A' + 10;
> > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > + return c - 'a' + 10;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int
> > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > + const char *c;
> > + uint32_t i;
> > +
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if ((((*c) >= '0') && ((*c) <= '9')) ||
> > + (((*c) >= 'A') && ((*c) <= 'F')) ||
> > + (((*c) >= 'a') && ((*c) <= 'f')))
> > + continue;
> > +
> > + return -1;
> > + }
> > + *size = *size / 2;
> > +
> > + /* Convert chars to bytes */
> > + for (i = 0; i < *size; i++)
> > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > + get_hex_val(src[2 * i + 1]);
> > +
> > + return 0;
> > +}
>
> What's wrong with strtoul() and friends?
> Why you need to write your own version?
> Konstantin
I have try to use strtoul(), but it do not work, because
I want to pars char byte on bye one, not the long string by one time, and also
RSS KEY can be as long as 100 byte long for i40e, but strtoul() return only 64 bit long.
app\test-eventdev and examples\ip_pipeline , drivers\net\softnic also use get_hex_val(), so I will it also.
>
> > +
> > +static int
> > +parse_hex(struct context *ctx, const struct token *token,
> > + const char *str, unsigned int len,
> > + void *buf, unsigned int size)
> > +{
> > + const struct arg *arg_data = pop_args(ctx);
> > + const struct arg *arg_len = pop_args(ctx);
> > + const struct arg *arg_addr = pop_args(ctx);
> > + char tmp[16]; /* Ought to be enough. */
> > + int ret;
> > + unsigned int hexlen = len;
> > + uint8_t hex_tmp[16];
> > +
> > + /* Arguments are expected. */
> > + if (!arg_data)
> > + return -1;
> > + if (!arg_len) {
> > + push_args(ctx, arg_data);
> > + return -1;
> > + }
> > + if (!arg_addr) {
> > + push_args(ctx, arg_len);
> > + push_args(ctx, arg_data);
> > + return -1;
> > + }
> > + size = arg_data->size;
> > + /* Bit-mask fill is not supported. */
> > + if (arg_data->mask || size < len)
> > + goto error;
> > + if (!ctx->object)
> > + return len;
> > +
> > + /* translate bytes string to array. */
> > + if (str[0] == '0' && ((str[1] == 'x') ||
> > + (str[1] == 'X'))) {
> > + str += 2;
> > + hexlen -= 2;
> > + }
> > + parse_hex_string(str, hex_tmp, &hexlen);
> > + /* Let parse_int() fill length information first. */
> > + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> > + if (ret < 0)
> > + goto error;
> > + push_args(ctx, arg_len);
> > + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> > + if (ret < 0) {
> > + pop_args(ctx);
> > + goto error;
> > + }
> > + buf = (uint8_t *)ctx->object + arg_data->offset;
> > + /* Output buffer is not necessarily NUL-terminated. */
> > + memcpy(buf, hex_tmp, hexlen);
> > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> > + if (ctx->objmask)
> > + memset((uint8_t *)ctx->objmask + arg_data->offset,
> > + 0xff, hexlen);
> > + /* Save address if requested. */
> > + if (arg_addr->size) {
> > + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > + (void *[]){
> > + (uint8_t *)ctx->object + arg_data->offset
> > + },
> > + arg_addr->size);
> > + if (ctx->objmask)
> > + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > + (void *[]){
> > + (uint8_t *)ctx->objmask + arg_data->offset
> > + },
> > + arg_addr->size);
> > + }
> > + return len;
> > +error:
> > + push_args(ctx, arg_addr);
> > + push_args(ctx, arg_len);
> > + push_args(ctx, arg_data);
> > + return -1;
> > +
> > +}
> > +
> > /**
> > * Parse a MAC address.
> > *
> > --
> > 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-18 8:49 ` Zhao1, Wei
@ 2019-03-21 14:03 ` Ananyev, Konstantin
2019-03-22 1:34 ` Zhao1, Wei
0 siblings, 1 reply; 24+ messages in thread
From: Ananyev, Konstantin @ 2019-03-21 14:03 UTC (permalink / raw)
To: Zhao1, Wei, dev; +Cc: stable, Zhang, Qi Z, adrien.mazarguil
Hi Wei,
> >
> > >
> > > There is need for users to set configuration of HEX number for RSS
> > > key. The key byte should be pass down as hex number not as char
> > > string. This patch enable cmdline flow parse HEX number, in order to
> > > not using string which pass ASIC number.
> > >
> > > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > > action")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > > ---
> > > app/test-pmd/cmdline_flow.c | 141
> > > +++++++++++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 140 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> > > index 36659a6..2445731 100644
> > > --- a/app/test-pmd/cmdline_flow.c
> > > +++ b/app/test-pmd/cmdline_flow.c
> > > @@ -35,6 +35,7 @@ enum index {
> > > PREFIX,
> > > BOOLEAN,
> > > STRING,
> > > + HEX,
> > > MAC_ADDR,
> > > IPV4_ADDR,
> > > IPV6_ADDR,
> > > @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const
> > > struct token *, static int parse_string(struct context *, const struct token *,
> > > const char *, unsigned int,
> > > void *, unsigned int);
> > > +static int parse_hex(struct context *ctx, const struct token *token,
> > > + const char *str, unsigned int len,
> > > + void *buf, unsigned int size);
> > > static int parse_mac_addr(struct context *, const struct token *,
> > > const char *, unsigned int,
> > > void *, unsigned int);
> > > @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> > > .call = parse_string,
> > > .comp = comp_none,
> > > },
> > > + [HEX] = {
> > > + .name = "{hex}",
> > > + .type = "HEX",
> > > + .help = "fixed string",
> > > + .call = parse_hex,
> > > + .comp = comp_none,
> > > + },
> > > [MAC_ADDR] = {
> > > .name = "{MAC address}",
> > > .type = "MAC-48",
> > > @@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
> > > .name = "pattern",
> > > .help = "byte string to look for",
> > > .next = NEXT(item_raw,
> > > - NEXT_ENTRY(STRING),
> > > + NEXT_ENTRY(HEX),
> > > NEXT_ENTRY(ITEM_PARAM_IS,
> > > ITEM_PARAM_SPEC,
> > > ITEM_PARAM_MASK)),
> > > @@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const struct
> > token *token,
> > > return -1;
> > > }
> > >
> > > +static uint32_t
> > > +get_hex_val(char c)
> > > +{
> > > + switch (c) {
> > > + case '0': case '1': case '2': case '3': case '4': case '5':
> > > + case '6': case '7': case '8': case '9':
> > > + return c - '0';
> > > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > > + return c - 'A' + 10;
> > > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > > + return c - 'a' + 10;
> > > + default:
> > > + return 0;
> > > + }
> > > +}
> > > +
> > > +static int
> > > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > > + const char *c;
> > > + uint32_t i;
> > > +
> > > + /* Check input parameters */
> > > + if ((src == NULL) ||
> > > + (dst == NULL) ||
> > > + (size == NULL) ||
> > > + (*size == 0))
> > > + return -1;
> > > + if ((*size & 1) != 0)
> > > + return -1;
> > > +
> > > + for (c = src, i = 0; i < *size; c++, i++) {
> > > + if ((((*c) >= '0') && ((*c) <= '9')) ||
> > > + (((*c) >= 'A') && ((*c) <= 'F')) ||
> > > + (((*c) >= 'a') && ((*c) <= 'f')))
> > > + continue;
> > > +
> > > + return -1;
> > > + }
> > > + *size = *size / 2;
> > > +
> > > + /* Convert chars to bytes */
> > > + for (i = 0; i < *size; i++)
> > > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > > + get_hex_val(src[2 * i + 1]);
> > > +
> > > + return 0;
> > > +}
> >
> > What's wrong with strtoul() and friends?
> > Why you need to write your own version?
> > Konstantin
>
>
> I have try to use strtoul(), but it do not work, because
> I want to pars char byte on bye one, not the long string by one time, and also
> RSS KEY can be as long as 100 byte long for i40e, but strtoul() return only 64 bit long.
> app\test-eventdev and examples\ip_pipeline , drivers\net\softnic also use get_hex_val(), so I will it also.
Ok, thanks for explanation.
Probably at least you can use isxdigit(), isdigit, toupper(), etc. as Stephen already suggested.
Konstantin
>
>
> >
> > > +
> > > +static int
> > > +parse_hex(struct context *ctx, const struct token *token,
> > > + const char *str, unsigned int len,
> > > + void *buf, unsigned int size)
> > > +{
> > > + const struct arg *arg_data = pop_args(ctx);
> > > + const struct arg *arg_len = pop_args(ctx);
> > > + const struct arg *arg_addr = pop_args(ctx);
> > > + char tmp[16]; /* Ought to be enough. */
> > > + int ret;
> > > + unsigned int hexlen = len;
> > > + uint8_t hex_tmp[16];
> > > +
> > > + /* Arguments are expected. */
> > > + if (!arg_data)
> > > + return -1;
> > > + if (!arg_len) {
> > > + push_args(ctx, arg_data);
> > > + return -1;
> > > + }
> > > + if (!arg_addr) {
> > > + push_args(ctx, arg_len);
> > > + push_args(ctx, arg_data);
> > > + return -1;
> > > + }
> > > + size = arg_data->size;
> > > + /* Bit-mask fill is not supported. */
> > > + if (arg_data->mask || size < len)
> > > + goto error;
> > > + if (!ctx->object)
> > > + return len;
> > > +
> > > + /* translate bytes string to array. */
> > > + if (str[0] == '0' && ((str[1] == 'x') ||
> > > + (str[1] == 'X'))) {
> > > + str += 2;
> > > + hexlen -= 2;
> > > + }
> > > + parse_hex_string(str, hex_tmp, &hexlen);
> > > + /* Let parse_int() fill length information first. */
> > > + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> > > + if (ret < 0)
> > > + goto error;
> > > + push_args(ctx, arg_len);
> > > + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> > > + if (ret < 0) {
> > > + pop_args(ctx);
> > > + goto error;
> > > + }
> > > + buf = (uint8_t *)ctx->object + arg_data->offset;
> > > + /* Output buffer is not necessarily NUL-terminated. */
> > > + memcpy(buf, hex_tmp, hexlen);
> > > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> > > + if (ctx->objmask)
> > > + memset((uint8_t *)ctx->objmask + arg_data->offset,
> > > + 0xff, hexlen);
> > > + /* Save address if requested. */
> > > + if (arg_addr->size) {
> > > + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > > + (void *[]){
> > > + (uint8_t *)ctx->object + arg_data->offset
> > > + },
> > > + arg_addr->size);
> > > + if (ctx->objmask)
> > > + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > > + (void *[]){
> > > + (uint8_t *)ctx->objmask + arg_data->offset
> > > + },
> > > + arg_addr->size);
> > > + }
> > > + return len;
> > > +error:
> > > + push_args(ctx, arg_addr);
> > > + push_args(ctx, arg_len);
> > > + push_args(ctx, arg_data);
> > > + return -1;
> > > +
> > > +}
> > > +
> > > /**
> > > * Parse a MAC address.
> > > *
> > > --
> > > 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-21 14:03 ` Ananyev, Konstantin
@ 2019-03-22 1:34 ` Zhao1, Wei
0 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-22 1:34 UTC (permalink / raw)
To: Ananyev, Konstantin, dev; +Cc: stable, Zhang, Qi Z, adrien.mazarguil
Hi, Konstantin
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, March 21, 2019 10:04 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> adrien.mazarguil@6wind.com
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix support of hex string
> parser for flow API
>
>
> Hi Wei,
>
> > >
> > > >
> > > > There is need for users to set configuration of HEX number for RSS
> > > > key. The key byte should be pass down as hex number not as char
> > > > string. This patch enable cmdline flow parse HEX number, in order
> > > > to not using string which pass ASIC number.
> > > >
> > > > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > > > action")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > > > ---
> > > > app/test-pmd/cmdline_flow.c | 141
> > > > +++++++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 140 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/app/test-pmd/cmdline_flow.c
> > > > b/app/test-pmd/cmdline_flow.c index 36659a6..2445731 100644
> > > > --- a/app/test-pmd/cmdline_flow.c
> > > > +++ b/app/test-pmd/cmdline_flow.c
> > > > @@ -35,6 +35,7 @@ enum index {
> > > > PREFIX,
> > > > BOOLEAN,
> > > > STRING,
> > > > + HEX,
> > > > MAC_ADDR,
> > > > IPV4_ADDR,
> > > > IPV6_ADDR,
> > > > @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *,
> > > > const struct token *, static int parse_string(struct context *, const
> struct token *,
> > > > const char *, unsigned int,
> > > > void *, unsigned int);
> > > > +static int parse_hex(struct context *ctx, const struct token *token,
> > > > + const char *str, unsigned int len,
> > > > + void *buf, unsigned int size);
> > > > static int parse_mac_addr(struct context *, const struct token *,
> > > > const char *, unsigned int,
> > > > void *, unsigned int);
> > > > @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> > > > .call = parse_string,
> > > > .comp = comp_none,
> > > > },
> > > > + [HEX] = {
> > > > + .name = "{hex}",
> > > > + .type = "HEX",
> > > > + .help = "fixed string",
> > > > + .call = parse_hex,
> > > > + .comp = comp_none,
> > > > + },
> > > > [MAC_ADDR] = {
> > > > .name = "{MAC address}",
> > > > .type = "MAC-48",
> > > > @@ -1544,7 +1555,7 @@ static const struct token token_list[] = {
> > > > .name = "pattern",
> > > > .help = "byte string to look for",
> > > > .next = NEXT(item_raw,
> > > > - NEXT_ENTRY(STRING),
> > > > + NEXT_ENTRY(HEX),
> > > > NEXT_ENTRY(ITEM_PARAM_IS,
> > > > ITEM_PARAM_SPEC,
> > > > ITEM_PARAM_MASK)),
> > > > @@ -4475,6 +4486,134 @@ parse_string(struct context *ctx, const
> > > > struct
> > > token *token,
> > > > return -1;
> > > > }
> > > >
> > > > +static uint32_t
> > > > +get_hex_val(char c)
> > > > +{
> > > > + switch (c) {
> > > > + case '0': case '1': case '2': case '3': case '4': case '5':
> > > > + case '6': case '7': case '8': case '9':
> > > > + return c - '0';
> > > > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > > > + return c - 'A' + 10;
> > > > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > > > + return c - 'a' + 10;
> > > > + default:
> > > > + return 0;
> > > > + }
> > > > +}
> > > > +
> > > > +static int
> > > > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > > > + const char *c;
> > > > + uint32_t i;
> > > > +
> > > > + /* Check input parameters */
> > > > + if ((src == NULL) ||
> > > > + (dst == NULL) ||
> > > > + (size == NULL) ||
> > > > + (*size == 0))
> > > > + return -1;
> > > > + if ((*size & 1) != 0)
> > > > + return -1;
> > > > +
> > > > + for (c = src, i = 0; i < *size; c++, i++) {
> > > > + if ((((*c) >= '0') && ((*c) <= '9')) ||
> > > > + (((*c) >= 'A') && ((*c) <= 'F')) ||
> > > > + (((*c) >= 'a') && ((*c) <= 'f')))
> > > > + continue;
> > > > +
> > > > + return -1;
> > > > + }
> > > > + *size = *size / 2;
> > > > +
> > > > + /* Convert chars to bytes */
> > > > + for (i = 0; i < *size; i++)
> > > > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > > > + get_hex_val(src[2 * i + 1]);
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > What's wrong with strtoul() and friends?
> > > Why you need to write your own version?
> > > Konstantin
> >
> >
> > I have try to use strtoul(), but it do not work, because I want to
> > pars char byte on bye one, not the long string by one time, and also
> > RSS KEY can be as long as 100 byte long for i40e, but strtoul() return only 64
> bit long.
> > app\test-eventdev and examples\ip_pipeline , drivers\net\softnic also use
> get_hex_val(), so I will it also.
>
> Ok, thanks for explanation.
> Probably at least you can use isxdigit(), isdigit, toupper(), etc. as Stephen
> already suggested.
> Konstantin
Ok, I will have a try of isxdigit() and update in v2.
>
> >
> >
> > >
> > > > +
> > > > +static int
> > > > +parse_hex(struct context *ctx, const struct token *token,
> > > > + const char *str, unsigned int len,
> > > > + void *buf, unsigned int size)
> > > > +{
> > > > + const struct arg *arg_data = pop_args(ctx);
> > > > + const struct arg *arg_len = pop_args(ctx);
> > > > + const struct arg *arg_addr = pop_args(ctx);
> > > > + char tmp[16]; /* Ought to be enough. */
> > > > + int ret;
> > > > + unsigned int hexlen = len;
> > > > + uint8_t hex_tmp[16];
> > > > +
> > > > + /* Arguments are expected. */
> > > > + if (!arg_data)
> > > > + return -1;
> > > > + if (!arg_len) {
> > > > + push_args(ctx, arg_data);
> > > > + return -1;
> > > > + }
> > > > + if (!arg_addr) {
> > > > + push_args(ctx, arg_len);
> > > > + push_args(ctx, arg_data);
> > > > + return -1;
> > > > + }
> > > > + size = arg_data->size;
> > > > + /* Bit-mask fill is not supported. */
> > > > + if (arg_data->mask || size < len)
> > > > + goto error;
> > > > + if (!ctx->object)
> > > > + return len;
> > > > +
> > > > + /* translate bytes string to array. */
> > > > + if (str[0] == '0' && ((str[1] == 'x') ||
> > > > + (str[1] == 'X'))) {
> > > > + str += 2;
> > > > + hexlen -= 2;
> > > > + }
> > > > + parse_hex_string(str, hex_tmp, &hexlen);
> > > > + /* Let parse_int() fill length information first. */
> > > > + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> > > > + if (ret < 0)
> > > > + goto error;
> > > > + push_args(ctx, arg_len);
> > > > + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> > > > + if (ret < 0) {
> > > > + pop_args(ctx);
> > > > + goto error;
> > > > + }
> > > > + buf = (uint8_t *)ctx->object + arg_data->offset;
> > > > + /* Output buffer is not necessarily NUL-terminated. */
> > > > + memcpy(buf, hex_tmp, hexlen);
> > > > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> > > > + if (ctx->objmask)
> > > > + memset((uint8_t *)ctx->objmask + arg_data->offset,
> > > > + 0xff, hexlen);
> > > > + /* Save address if requested. */
> > > > + if (arg_addr->size) {
> > > > + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> > > > + (void *[]){
> > > > + (uint8_t *)ctx->object + arg_data->offset
> > > > + },
> > > > + arg_addr->size);
> > > > + if (ctx->objmask)
> > > > + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> > > > + (void *[]){
> > > > + (uint8_t *)ctx->objmask + arg_data->offset
> > > > + },
> > > > + arg_addr->size);
> > > > + }
> > > > + return len;
> > > > +error:
> > > > + push_args(ctx, arg_addr);
> > > > + push_args(ctx, arg_len);
> > > > + push_args(ctx, arg_data);
> > > > + return -1;
> > > > +
> > > > +}
> > > > +
> > > > /**
> > > > * Parse a MAC address.
> > > > *
> > > > --
> > > > 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-stable] [PATCH v2] app/testpmd: fix support of hex string parser for flow API
2019-03-15 8:43 [dpdk-stable] [PATCH] app/testpmd: fix support of hex string parser for flow API Wei Zhao
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
@ 2019-03-18 8:16 ` Wei Zhao
2019-03-19 5:23 ` Zhao1, Wei
2019-03-22 3:15 ` [dpdk-stable] [PATCH v3] " Wei Zhao
2019-03-18 15:27 ` [dpdk-stable] [dpdk-dev] [PATCH] " Stephen Hemminger
2 siblings, 2 replies; 24+ messages in thread
From: Wei Zhao @ 2019-03-18 8:16 UTC (permalink / raw)
To: dev; +Cc: stable, qi.z.zhang, wenzhuo.lu, konstantin.ananyev, Wei Zhao
There is need for users to set configuration of HEX number for RSS
key. The key byte should be pass down as hex number not as char
string. This patch enable cmdline flow parse HEX number,
in order to not using string which pass ASIC number.
Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
Cc: stable@dpdk.org
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
---
v2:
fix bug of string length limite.
---
app/test-pmd/cmdline_flow.c | 146 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 145 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 36659a6..7d263c6 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -35,6 +35,7 @@ enum index {
PREFIX,
BOOLEAN,
STRING,
+ HEX,
MAC_ADDR,
IPV4_ADDR,
IPV6_ADDR,
@@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const struct token *,
static int parse_string(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size);
static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
.call = parse_string,
.comp = comp_none,
},
+ [HEX] = {
+ .name = "{hex}",
+ .type = "HEX",
+ .help = "fixed string",
+ .call = parse_hex,
+ .comp = comp_none,
+ },
[MAC_ADDR] = {
.name = "{MAC address}",
.type = "MAC-48",
@@ -2306,7 +2317,7 @@ static const struct token token_list[] = {
[ACTION_RSS_KEY] = {
.name = "key",
.help = "RSS hash key",
- .next = NEXT(action_rss, NEXT_ENTRY(STRING)),
+ .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
.args = ARGS(ARGS_ENTRY_ARB(0, 0),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
@@ -4475,6 +4486,139 @@ parse_string(struct context *ctx, const struct token *token,
return -1;
}
+static uint32_t
+get_hex_val(char c)
+{
+ switch (c) {
+ case '0': case '1': case '2': case '3': case '4': case '5':
+ case '6': case '7': case '8': case '9':
+ return c - '0';
+ case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
+ return c - 'A' + 10;
+ case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
+ return c - 'a' + 10;
+ default:
+ return 0;
+ }
+}
+
+static int
+parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
+{
+ const char *c;
+ uint32_t i;
+
+ /* Check input parameters */
+ if ((src == NULL) ||
+ (dst == NULL) ||
+ (size == NULL) ||
+ (*size == 0))
+ return -1;
+ if ((*size & 1) != 0)
+ return -1;
+
+ for (c = src, i = 0; i < *size; c++, i++) {
+ if ((((*c) >= '0') && ((*c) <= '9')) ||
+ (((*c) >= 'A') && ((*c) <= 'F')) ||
+ (((*c) >= 'a') && ((*c) <= 'f')))
+ continue;
+
+ return -1;
+ }
+ *size = *size / 2;
+
+ /* Convert chars to bytes */
+ for (i = 0; i < *size; i++)
+ dst[i] = get_hex_val(src[2 * i]) * 16 +
+ get_hex_val(src[2 * i + 1]);
+
+ return 0;
+}
+
+static int
+parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg_data = pop_args(ctx);
+ const struct arg *arg_len = pop_args(ctx);
+ const struct arg *arg_addr = pop_args(ctx);
+ char tmp[16]; /* Ought to be enough. */
+ int ret;
+ unsigned int hexlen = len;
+ unsigned int length = 256;
+ uint8_t hex_tmp[length];
+
+ /* Arguments are expected. */
+ if (!arg_data)
+ return -1;
+ if (!arg_len) {
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ if (!arg_addr) {
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ size = arg_data->size;
+ /* Bit-mask fill is not supported. */
+ if (arg_data->mask)
+ goto error;
+ if (!ctx->object)
+ return len;
+
+ /* translate bytes string to array. */
+ if (str[0] == '0' && ((str[1] == 'x') ||
+ (str[1] == 'X'))) {
+ str += 2;
+ hexlen -= 2;
+ }
+ if (hexlen > length)
+ return -1;
+ ret = parse_hex_string(str, hex_tmp, &hexlen);
+ if (ret < 0)
+ goto error;
+ /* Let parse_int() fill length information first. */
+ ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
+ if (ret < 0)
+ goto error;
+ push_args(ctx, arg_len);
+ ret = parse_int(ctx, token, tmp, ret, NULL, 0);
+ if (ret < 0) {
+ pop_args(ctx);
+ goto error;
+ }
+ buf = (uint8_t *)ctx->object + arg_data->offset;
+ /* Output buffer is not necessarily NUL-terminated. */
+ memcpy(buf, hex_tmp, hexlen);
+ memset((uint8_t *)buf + len, 0x00, size - hexlen);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg_data->offset,
+ 0xff, hexlen);
+ /* Save address if requested. */
+ if (arg_addr->size) {
+ memcpy((uint8_t *)ctx->object + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->object + arg_data->offset
+ },
+ arg_addr->size);
+ if (ctx->objmask)
+ memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->objmask + arg_data->offset
+ },
+ arg_addr->size);
+ }
+ return len;
+error:
+ push_args(ctx, arg_addr);
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+
+}
+
/**
* Parse a MAC address.
*
--
2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v2] app/testpmd: fix support of hex string parser for flow API
2019-03-18 8:16 ` [dpdk-stable] [PATCH v2] " Wei Zhao
@ 2019-03-19 5:23 ` Zhao1, Wei
2019-03-22 3:15 ` [dpdk-stable] [PATCH v3] " Wei Zhao
1 sibling, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-19 5:23 UTC (permalink / raw)
To: dev
Cc: stable, Zhang, Qi Z, Lu, Wenzhuo, Ananyev, Konstantin, Peng,
Yuan, Yigit, Ferruh
Send on behalf of pengyuan
Tested-by: Peng Yuan <yuan.peng@intel.com>
- Tested Branch: master
- Tested Commit: 239912fa798e6e671072ca7ff987afd74c1e506c
- OS: 4.13.9-300.fc27.x86_64
- GCC: gcc (GCC) 7.3.1 20180303 (Red Hat 7.3.1-5)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
- NIC: Intel Corporation Device Fortville [8086:1583]
- Default x86_64-native-linuxapp-gcc configuration
- Prerequisites:
- Total 1 case1, 1 passed, 0 failed
- Case steps:
1. Bind the pf port to dpdk driver:
./usertools/dpdk-devbind.py -b igb_uio 05:00.0 05:00.1
2. start testpmd:
./x86_64-native-linuxapp-gcc/app/testpmd -c 1ffff -n 4 - -i --nb-cores=8 --rxq=4 --txq=4 --port-topology=chained
3. testpmd> flow create 0 ingress pattern end actions rss types ipv4-udp end key 1234567890123456789012345678901234567890FFFFFFFFFFFF1234567890123456789012345678901234567890FFFFFFFFFFFF / end
testpmd> show port 0 rss-hash key
RSS functions:
ipv4-udp udp
RSS key:
1234567890123456789012345678901234567890FFFFFFFFFFFF1234567890123456789012345678901234567890FFFFFFFFFFFF
52bytes key can be set successfully.
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Monday, March 18, 2019 4:16 PM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [PATCH v2] app/testpmd: fix support of hex string parser for flow
> API
>
> There is need for users to set configuration of HEX number for RSS key. The
> key byte should be pass down as hex number not as char string. This patch
> enable cmdline flow parse HEX number, in order to not using string which
> pass ASIC number.
>
> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>
> ---
>
> v2:
> fix bug of string length limite.
> ---
> app/test-pmd/cmdline_flow.c | 146
> +++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 145 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 36659a6..7d263c6 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -35,6 +35,7 @@ enum index {
> PREFIX,
> BOOLEAN,
> STRING,
> + HEX,
> MAC_ADDR,
> IPV4_ADDR,
> IPV6_ADDR,
> @@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const
> struct token *, static int parse_string(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> +static int parse_hex(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size);
> static int parse_mac_addr(struct context *, const struct token *,
> const char *, unsigned int,
> void *, unsigned int);
> @@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
> .call = parse_string,
> .comp = comp_none,
> },
> + [HEX] = {
> + .name = "{hex}",
> + .type = "HEX",
> + .help = "fixed string",
> + .call = parse_hex,
> + .comp = comp_none,
> + },
> [MAC_ADDR] = {
> .name = "{MAC address}",
> .type = "MAC-48",
> @@ -2306,7 +2317,7 @@ static const struct token token_list[] = {
> [ACTION_RSS_KEY] = {
> .name = "key",
> .help = "RSS hash key",
> - .next = NEXT(action_rss, NEXT_ENTRY(STRING)),
> + .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
> .args = ARGS(ARGS_ENTRY_ARB(0, 0),
> ARGS_ENTRY_ARB
> (offsetof(struct action_rss_data, conf) + @@ -
> 4475,6 +4486,139 @@ parse_string(struct context *ctx, const struct token
> *token,
> return -1;
> }
>
> +static uint32_t
> +get_hex_val(char c)
> +{
> + switch (c) {
> + case '0': case '1': case '2': case '3': case '4': case '5':
> + case '6': case '7': case '8': case '9':
> + return c - '0';
> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> + return c - 'A' + 10;
> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> + return c - 'a' + 10;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> + const char *c;
> + uint32_t i;
> +
> + /* Check input parameters */
> + if ((src == NULL) ||
> + (dst == NULL) ||
> + (size == NULL) ||
> + (*size == 0))
> + return -1;
> + if ((*size & 1) != 0)
> + return -1;
> +
> + for (c = src, i = 0; i < *size; c++, i++) {
> + if ((((*c) >= '0') && ((*c) <= '9')) ||
> + (((*c) >= 'A') && ((*c) <= 'F')) ||
> + (((*c) >= 'a') && ((*c) <= 'f')))
> + continue;
> +
> + return -1;
> + }
> + *size = *size / 2;
> +
> + /* Convert chars to bytes */
> + for (i = 0; i < *size; i++)
> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> + get_hex_val(src[2 * i + 1]);
> +
> + return 0;
> +}
> +
> +static int
> +parse_hex(struct context *ctx, const struct token *token,
> + const char *str, unsigned int len,
> + void *buf, unsigned int size)
> +{
> + const struct arg *arg_data = pop_args(ctx);
> + const struct arg *arg_len = pop_args(ctx);
> + const struct arg *arg_addr = pop_args(ctx);
> + char tmp[16]; /* Ought to be enough. */
> + int ret;
> + unsigned int hexlen = len;
> + unsigned int length = 256;
> + uint8_t hex_tmp[length];
> +
> + /* Arguments are expected. */
> + if (!arg_data)
> + return -1;
> + if (!arg_len) {
> + push_args(ctx, arg_data);
> + return -1;
> + }
> + if (!arg_addr) {
> + push_args(ctx, arg_len);
> + push_args(ctx, arg_data);
> + return -1;
> + }
> + size = arg_data->size;
> + /* Bit-mask fill is not supported. */
> + if (arg_data->mask)
> + goto error;
> + if (!ctx->object)
> + return len;
> +
> + /* translate bytes string to array. */
> + if (str[0] == '0' && ((str[1] == 'x') ||
> + (str[1] == 'X'))) {
> + str += 2;
> + hexlen -= 2;
> + }
> + if (hexlen > length)
> + return -1;
> + ret = parse_hex_string(str, hex_tmp, &hexlen);
> + if (ret < 0)
> + goto error;
> + /* Let parse_int() fill length information first. */
> + ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
> + if (ret < 0)
> + goto error;
> + push_args(ctx, arg_len);
> + ret = parse_int(ctx, token, tmp, ret, NULL, 0);
> + if (ret < 0) {
> + pop_args(ctx);
> + goto error;
> + }
> + buf = (uint8_t *)ctx->object + arg_data->offset;
> + /* Output buffer is not necessarily NUL-terminated. */
> + memcpy(buf, hex_tmp, hexlen);
> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> + if (ctx->objmask)
> + memset((uint8_t *)ctx->objmask + arg_data->offset,
> + 0xff, hexlen);
> + /* Save address if requested. */
> + if (arg_addr->size) {
> + memcpy((uint8_t *)ctx->object + arg_addr->offset,
> + (void *[]){
> + (uint8_t *)ctx->object + arg_data->offset
> + },
> + arg_addr->size);
> + if (ctx->objmask)
> + memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
> + (void *[]){
> + (uint8_t *)ctx->objmask + arg_data->offset
> + },
> + arg_addr->size);
> + }
> + return len;
> +error:
> + push_args(ctx, arg_addr);
> + push_args(ctx, arg_len);
> + push_args(ctx, arg_data);
> + return -1;
> +
> +}
> +
> /**
> * Parse a MAC address.
> *
> --
> 2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-18 8:16 ` [dpdk-stable] [PATCH v2] " Wei Zhao
2019-03-19 5:23 ` Zhao1, Wei
@ 2019-03-22 3:15 ` Wei Zhao
2019-03-22 14:56 ` Ferruh Yigit
2019-04-09 8:41 ` [dpdk-stable] [PATCH v4] " Wei Zhao
1 sibling, 2 replies; 24+ messages in thread
From: Wei Zhao @ 2019-03-22 3:15 UTC (permalink / raw)
To: dev; +Cc: stable, stephen, konstantin.ananyev, ferruh.yigit, Wei Zhao
There is need for users to set configuration of HEX number for RSS
key. The key byte should be pass down as hex number not as char
string. This patch enable cmdline flow parse HEX number,
in order to not using string which pass ASIC number.
Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
Cc: stable@dpdk.org
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Tested-by: Peng Yuan <yuan.peng@intel.com>
---
v2:
fix bug of string length limite.
v3:
use isxdigit to check char string
---
app/test-pmd/cmdline_flow.c | 145 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 144 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 36659a6..4bc033c 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -35,6 +35,7 @@ enum index {
PREFIX,
BOOLEAN,
STRING,
+ HEX,
MAC_ADDR,
IPV4_ADDR,
IPV6_ADDR,
@@ -1122,6 +1123,9 @@ static int parse_boolean(struct context *, const struct token *,
static int parse_string(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size);
static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1198,6 +1202,13 @@ static const struct token token_list[] = {
.call = parse_string,
.comp = comp_none,
},
+ [HEX] = {
+ .name = "{hex}",
+ .type = "HEX",
+ .help = "fixed string",
+ .call = parse_hex,
+ .comp = comp_none,
+ },
[MAC_ADDR] = {
.name = "{MAC address}",
.type = "MAC-48",
@@ -2306,7 +2317,7 @@ static const struct token token_list[] = {
[ACTION_RSS_KEY] = {
.name = "key",
.help = "RSS hash key",
- .next = NEXT(action_rss, NEXT_ENTRY(STRING)),
+ .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
.args = ARGS(ARGS_ENTRY_ARB(0, 0),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
@@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct token *token,
return -1;
}
+static uint32_t
+get_hex_val(char c)
+{
+ switch (c) {
+ case '0': case '1': case '2': case '3': case '4': case '5':
+ case '6': case '7': case '8': case '9':
+ return c - '0';
+ case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
+ return c - 'A' + 10;
+ case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
+ return c - 'a' + 10;
+ default:
+ return 0;
+ }
+}
+
+static int
+parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
+{
+ const char *c;
+ uint32_t i;
+
+ /* Check input parameters */
+ if ((src == NULL) ||
+ (dst == NULL) ||
+ (size == NULL) ||
+ (*size == 0))
+ return -1;
+ if ((*size & 1) != 0)
+ return -1;
+
+ for (c = src, i = 0; i < *size; c++, i++) {
+ if (isxdigit(*c))
+ continue;
+ else
+ return -1;
+ }
+
+ *size = *size / 2;
+
+ /* Convert chars to bytes */
+ for (i = 0; i < *size; i++)
+ dst[i] = get_hex_val(src[2 * i]) * 16 +
+ get_hex_val(src[2 * i + 1]);
+
+ return 0;
+}
+
+static int
+parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg_data = pop_args(ctx);
+ const struct arg *arg_len = pop_args(ctx);
+ const struct arg *arg_addr = pop_args(ctx);
+ char tmp[16]; /* Ought to be enough. */
+ int ret;
+ unsigned int hexlen = len;
+ unsigned int length = 256;
+ uint8_t hex_tmp[length];
+
+ /* Arguments are expected. */
+ if (!arg_data)
+ return -1;
+ if (!arg_len) {
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ if (!arg_addr) {
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ size = arg_data->size;
+ /* Bit-mask fill is not supported. */
+ if (arg_data->mask)
+ goto error;
+ if (!ctx->object)
+ return len;
+
+ /* translate bytes string to array. */
+ if (str[0] == '0' && ((str[1] == 'x') ||
+ (str[1] == 'X'))) {
+ str += 2;
+ hexlen -= 2;
+ }
+ if (hexlen > length)
+ return -1;
+ ret = parse_hex_string(str, hex_tmp, &hexlen);
+ if (ret < 0)
+ goto error;
+ /* Let parse_int() fill length information first. */
+ ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
+ if (ret < 0)
+ goto error;
+ push_args(ctx, arg_len);
+ ret = parse_int(ctx, token, tmp, ret, NULL, 0);
+ if (ret < 0) {
+ pop_args(ctx);
+ goto error;
+ }
+ buf = (uint8_t *)ctx->object + arg_data->offset;
+ /* Output buffer is not necessarily NUL-terminated. */
+ memcpy(buf, hex_tmp, hexlen);
+ memset((uint8_t *)buf + len, 0x00, size - hexlen);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg_data->offset,
+ 0xff, hexlen);
+ /* Save address if requested. */
+ if (arg_addr->size) {
+ memcpy((uint8_t *)ctx->object + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->object + arg_data->offset
+ },
+ arg_addr->size);
+ if (ctx->objmask)
+ memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->objmask + arg_data->offset
+ },
+ arg_addr->size);
+ }
+ return len;
+error:
+ push_args(ctx, arg_addr);
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+
+}
+
/**
* Parse a MAC address.
*
--
2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-22 3:15 ` [dpdk-stable] [PATCH v3] " Wei Zhao
@ 2019-03-22 14:56 ` Ferruh Yigit
2019-03-25 3:39 ` Zhao1, Wei
` (3 more replies)
2019-04-09 8:41 ` [dpdk-stable] [PATCH v4] " Wei Zhao
1 sibling, 4 replies; 24+ messages in thread
From: Ferruh Yigit @ 2019-03-22 14:56 UTC (permalink / raw)
To: Wei Zhao, dev; +Cc: stable, stephen, konstantin.ananyev
On 3/22/2019 3:15 AM, Wei Zhao wrote:
> There is need for users to set configuration of HEX number for RSS
> key. The key byte should be pass down as hex number not as char
> string. This patch enable cmdline flow parse HEX number,
> in order to not using string which pass ASIC number.
>
> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> Tested-by: Peng Yuan <yuan.peng@intel.com>
<...>
> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct token *token,
> return -1;
> }
>
> +static uint32_t
> +get_hex_val(char c)
> +{
> + switch (c) {
> + case '0': case '1': case '2': case '3': case '4': case '5':
> + case '6': case '7': case '8': case '9':
> + return c - '0';
> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> + return c - 'A' + 10;
> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> + return c - 'a' + 10;
> + default:
> + return 0;
> + }
> +}
> +
> +static int
> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
> +{
> + const char *c;
> + uint32_t i;
> +
> + /* Check input parameters */
> + if ((src == NULL) ||
> + (dst == NULL) ||
> + (size == NULL) ||
> + (*size == 0))
> + return -1;
> + if ((*size & 1) != 0)
> + return -1;
> +
> + for (c = src, i = 0; i < *size; c++, i++) {
> + if (isxdigit(*c))
> + continue;
> + else
> + return -1;
> + }
> +
> + *size = *size / 2;
> +
> + /* Convert chars to bytes */
> + for (i = 0; i < *size; i++)
> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> + get_hex_val(src[2 * i + 1]);
> +
> + return 0;
> +}
I can see this has been discussed already but what would you think updating the
'parse_hex_string' something like following, it is less code to maintain:
static int
parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
{
int len;
int i
for (i = 0, len = 0; i < *size; i += 2) {
char tmp[3];
snprintf(tmp, 3, src + i);
dst[len++] = strtoul(tmp, NULL, 16);
}
dst[len] = 0;
*size = len;
return 0;
}
(indeed with better error checking on strtoul ;) )
<...>
> + /* Output buffer is not necessarily NUL-terminated. */
> + memcpy(buf, hex_tmp, hexlen);
> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
Can't this overflow the 'buf'? since "len = 2 * hexlen"
I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-22 14:56 ` Ferruh Yigit
@ 2019-03-25 3:39 ` Zhao1, Wei
2019-03-25 8:45 ` Ferruh Yigit
2019-04-09 7:38 ` Zhao1, Wei
` (2 subsequent siblings)
3 siblings, 1 reply; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-25 3:39 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, March 22, 2019 10:56 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> > There is need for users to set configuration of HEX number for RSS
> > key. The key byte should be pass down as hex number not as char
> > string. This patch enable cmdline flow parse HEX number, in order to
> > not using string which pass ASIC number.
> >
> > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > action")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > Tested-by: Peng Yuan <yuan.peng@intel.com>
>
> <...>
>
> > @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct
> token *token,
> > return -1;
> > }
> >
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > + switch (c) {
> > + case '0': case '1': case '2': case '3': case '4': case '5':
> > + case '6': case '7': case '8': case '9':
> > + return c - '0';
> > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > + return c - 'A' + 10;
> > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > + return c - 'a' + 10;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int
> > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > + const char *c;
> > + uint32_t i;
> > +
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if (isxdigit(*c))
> > + continue;
> > + else
> > + return -1;
> > + }
> > +
> > + *size = *size / 2;
> > +
> > + /* Convert chars to bytes */
> > + for (i = 0; i < *size; i++)
> > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > + get_hex_val(src[2 * i + 1]);
> > +
> > + return 0;
> > +}
>
> I can see this has been discussed already but what would you think updating
> the 'parse_hex_string' something like following, it is less code to maintain:
>
> static int
> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> int len;
> int i
> for (i = 0, len = 0; i < *size; i += 2) {
> char tmp[3];
> snprintf(tmp, 3, src + i);
> dst[len++] = strtoul(tmp, NULL, 16);
> }
> dst[len] = 0;
> *size = len;
> return 0;
> }
>
> (indeed with better error checking on strtoul ;) )
Why delete these check from parse_hex_string()?
/* Check input parameters */
if ((src == NULL) ||
(dst == NULL) ||
(size == NULL) ||
(*size == 0))
return -1;
if ((*size & 1) != 0)
return -1;
for (c = src, i = 0; i < *size; c++, i++) {
if (isxdigit(*c))
continue;
else
return -1;
}
>
> <...>
>
> > + /* Output buffer is not necessarily NUL-terminated. */
> > + memcpy(buf, hex_tmp, hexlen);
> > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>
> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-25 3:39 ` Zhao1, Wei
@ 2019-03-25 8:45 ` Ferruh Yigit
2019-03-25 9:25 ` Zhao1, Wei
0 siblings, 1 reply; 24+ messages in thread
From: Ferruh Yigit @ 2019-03-25 8:45 UTC (permalink / raw)
To: Zhao1, Wei, dev; +Cc: stable, stephen, Ananyev, Konstantin
On 3/25/2019 3:39 AM, Zhao1, Wei wrote:
> Hi,
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, March 22, 2019 10:56 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
>> parser for flow API
>>
>> On 3/22/2019 3:15 AM, Wei Zhao wrote:
>>> There is need for users to set configuration of HEX number for RSS
>>> key. The key byte should be pass down as hex number not as char
>>> string. This patch enable cmdline flow parse HEX number, in order to
>>> not using string which pass ASIC number.
>>>
>>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
>>> action")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>>> Tested-by: Peng Yuan <yuan.peng@intel.com>
>>
>> <...>
>>
>>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct
>> token *token,
>>> return -1;
>>> }
>>>
>>> +static uint32_t
>>> +get_hex_val(char c)
>>> +{
>>> + switch (c) {
>>> + case '0': case '1': case '2': case '3': case '4': case '5':
>>> + case '6': case '7': case '8': case '9':
>>> + return c - '0';
>>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>> + return c - 'A' + 10;
>>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>> + return c - 'a' + 10;
>>> + default:
>>> + return 0;
>>> + }
>>> +}
>>> +
>>> +static int
>>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>>> + const char *c;
>>> + uint32_t i;
>>> +
>>> + /* Check input parameters */
>>> + if ((src == NULL) ||
>>> + (dst == NULL) ||
>>> + (size == NULL) ||
>>> + (*size == 0))
>>> + return -1;
>>> + if ((*size & 1) != 0)
>>> + return -1;
>>> +
>>> + for (c = src, i = 0; i < *size; c++, i++) {
>>> + if (isxdigit(*c))
>>> + continue;
>>> + else
>>> + return -1;
>>> + }
>>> +
>>> + *size = *size / 2;
>>> +
>>> + /* Convert chars to bytes */
>>> + for (i = 0; i < *size; i++)
>>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
>>> + get_hex_val(src[2 * i + 1]);
>>> +
>>> + return 0;
>>> +}
>>
>> I can see this has been discussed already but what would you think updating
>> the 'parse_hex_string' something like following, it is less code to maintain:
>>
>> static int
>> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>> int len;
>> int i
>> for (i = 0, len = 0; i < *size; i += 2) {
>> char tmp[3];
>> snprintf(tmp, 3, src + i);
>> dst[len++] = strtoul(tmp, NULL, 16);
>> }
>> dst[len] = 0;
>> *size = len;
>> return 0;
>> }
>>
>> (indeed with better error checking on strtoul ;) )
>
>
> Why delete these check from parse_hex_string()?
The point is using 'strtoul' instead of your functions, so that you won't need
'get_hex_val()' at all, or won't need 'isxdigit()' because 'strtoul' will check
it, won't need size should be multiply of two restriction '(*size & 1)' because
of implementation change. Probably you will need NULL checks, but again point is
why not using 'strtoul' instead of writing your version of it?
>
> /* Check input parameters */
> if ((src == NULL) ||
> (dst == NULL) ||
> (size == NULL) ||
> (*size == 0))
> return -1;
> if ((*size & 1) != 0)
> return -1;
> for (c = src, i = 0; i < *size; c++, i++) {
> if (isxdigit(*c))
> continue;
> else
> return -1;
> }
>
>
>
>>
>> <...>
>>
>>> + /* Output buffer is not necessarily NUL-terminated. */
>>> + memcpy(buf, hex_tmp, hexlen);
>>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>>
>> Can't this overflow the 'buf'? since "len = 2 * hexlen"
>> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-25 8:45 ` Ferruh Yigit
@ 2019-03-25 9:25 ` Zhao1, Wei
2019-03-25 9:35 ` Ferruh Yigit
0 siblings, 1 reply; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-25 9:25 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi，Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 25, 2019 4:46 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 3/25/2019 3:39 AM, Zhao1, Wei wrote:
> > Hi,
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Friday, March 22, 2019 10:56 PM
> >> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev,
> Konstantin
> >> <konstantin.ananyev@intel.com>
> >> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex
> >> string parser for flow API
> >>
> >> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> >>> There is need for users to set configuration of HEX number for RSS
> >>> key. The key byte should be pass down as hex number not as char
> >>> string. This patch enable cmdline flow parse HEX number, in order to
> >>> not using string which pass ASIC number.
> >>>
> >>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> >>> action")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >>> Tested-by: Peng Yuan <yuan.peng@intel.com>
> >>
> >> <...>
> >>
> >>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const
> >>> struct
> >> token *token,
> >>> return -1;
> >>> }
> >>>
> >>> +static uint32_t
> >>> +get_hex_val(char c)
> >>> +{
> >>> + switch (c) {
> >>> + case '0': case '1': case '2': case '3': case '4': case '5':
> >>> + case '6': case '7': case '8': case '9':
> >>> + return c - '0';
> >>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> >>> + return c - 'A' + 10;
> >>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> >>> + return c - 'a' + 10;
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> +}
> >>> +
> >>> +static int
> >>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >>> + const char *c;
> >>> + uint32_t i;
> >>> +
> >>> + /* Check input parameters */
> >>> + if ((src == NULL) ||
> >>> + (dst == NULL) ||
> >>> + (size == NULL) ||
> >>> + (*size == 0))
> >>> + return -1;
> >>> + if ((*size & 1) != 0)
> >>> + return -1;
> >>> +
> >>> + for (c = src, i = 0; i < *size; c++, i++) {
> >>> + if (isxdigit(*c))
> >>> + continue;
> >>> + else
> >>> + return -1;
> >>> + }
> >>> +
> >>> + *size = *size / 2;
> >>> +
> >>> + /* Convert chars to bytes */
> >>> + for (i = 0; i < *size; i++)
> >>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> >>> + get_hex_val(src[2 * i + 1]);
> >>> +
> >>> + return 0;
> >>> +}
> >>
> >> I can see this has been discussed already but what would you think
> >> updating the 'parse_hex_string' something like following, it is less code to
> maintain:
> >>
> >> static int
> >> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >> int len;
> >> int i
> >> for (i = 0, len = 0; i < *size; i += 2) {
> >> char tmp[3];
> >> snprintf(tmp, 3, src + i);
> >> dst[len++] = strtoul(tmp, NULL, 16);
> >> }
> >> dst[len] = 0;
> >> *size = len;
> >> return 0;
> >> }
> >>
> >> (indeed with better error checking on strtoul ;) )
> >
> >
> > Why delete these check from parse_hex_string()?
>
> The point is using 'strtoul' instead of your functions, so that you won't need
> 'get_hex_val()' at all, or won't need 'isxdigit()' because 'strtoul' will check it,
> won't need size should be multiply of two restriction '(*size & 1)' because of
> implementation change. Probably you will need NULL checks, but again point
> is why not using 'strtoul' instead of writing your version of it?
Yes, we can use 'strtoul' , but my point is that I think we need these check code even if we use the code 'strtoul' .
isxdigit(*c)) is need because *c may be sring "0xrgh" which is not hex.
If we use strtoul will return 0 for that ,we can not distinguish between error or input is zero.
'(*size & 1) can be delete, I agree.
/* Check input parameters */
if ((src == NULL) ||
(dst == NULL) ||
(size == NULL) ||
(*size == 0))
return -1;
for (c = src, i = 0; i < *size; c++, i++) {
if (isxdigit(*c))
continue;
else
return -1;
}
> >
> > /* Check input parameters */
> > if ((src == NULL) ||
> > (dst == NULL) ||
> > (size == NULL) ||
> > (*size == 0))
> > return -1;
> > if ((*size & 1) != 0)
> > return -1;
> > for (c = src, i = 0; i < *size; c++, i++) {
> > if (isxdigit(*c))
> > continue;
> > else
> > return -1;
> > }
> >
> >
> >
> >>
> >> <...>
> >>
> >>> + /* Output buffer is not necessarily NUL-terminated. */
> >>> + memcpy(buf, hex_tmp, hexlen);
> >>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> >>
> >> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> >> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-25 9:25 ` Zhao1, Wei
@ 2019-03-25 9:35 ` Ferruh Yigit
2019-03-25 9:39 ` Zhao1, Wei
0 siblings, 1 reply; 24+ messages in thread
From: Ferruh Yigit @ 2019-03-25 9:35 UTC (permalink / raw)
To: Zhao1, Wei, dev; +Cc: stable, stephen, Ananyev, Konstantin
On 3/25/2019 9:25 AM, Zhao1, Wei wrote:
> Hi，Ferruh
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Monday, March 25, 2019 4:46 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
>> parser for flow API
>>
>> On 3/25/2019 3:39 AM, Zhao1, Wei wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh
>>>> Sent: Friday, March 22, 2019 10:56 PM
>>>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>>>> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev,
>> Konstantin
>>>> <konstantin.ananyev@intel.com>
>>>> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex
>>>> string parser for flow API
>>>>
>>>> On 3/22/2019 3:15 AM, Wei Zhao wrote:
>>>>> There is need for users to set configuration of HEX number for RSS
>>>>> key. The key byte should be pass down as hex number not as char
>>>>> string. This patch enable cmdline flow parse HEX number, in order to
>>>>> not using string which pass ASIC number.
>>>>>
>>>>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
>>>>> action")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>>>>> Tested-by: Peng Yuan <yuan.peng@intel.com>
>>>>
>>>> <...>
>>>>
>>>>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const
>>>>> struct
>>>> token *token,
>>>>> return -1;
>>>>> }
>>>>>
>>>>> +static uint32_t
>>>>> +get_hex_val(char c)
>>>>> +{
>>>>> + switch (c) {
>>>>> + case '0': case '1': case '2': case '3': case '4': case '5':
>>>>> + case '6': case '7': case '8': case '9':
>>>>> + return c - '0';
>>>>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>>>> + return c - 'A' + 10;
>>>>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>>>> + return c - 'a' + 10;
>>>>> + default:
>>>>> + return 0;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static int
>>>>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>>>>> + const char *c;
>>>>> + uint32_t i;
>>>>> +
>>>>> + /* Check input parameters */
>>>>> + if ((src == NULL) ||
>>>>> + (dst == NULL) ||
>>>>> + (size == NULL) ||
>>>>> + (*size == 0))
>>>>> + return -1;
>>>>> + if ((*size & 1) != 0)
>>>>> + return -1;
>>>>> +
>>>>> + for (c = src, i = 0; i < *size; c++, i++) {
>>>>> + if (isxdigit(*c))
>>>>> + continue;
>>>>> + else
>>>>> + return -1;
>>>>> + }
>>>>> +
>>>>> + *size = *size / 2;
>>>>> +
>>>>> + /* Convert chars to bytes */
>>>>> + for (i = 0; i < *size; i++)
>>>>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
>>>>> + get_hex_val(src[2 * i + 1]);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>
>>>> I can see this has been discussed already but what would you think
>>>> updating the 'parse_hex_string' something like following, it is less code to
>> maintain:
>>>>
>>>> static int
>>>> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>>>> int len;
>>>> int i
>>>> for (i = 0, len = 0; i < *size; i += 2) {
>>>> char tmp[3];
>>>> snprintf(tmp, 3, src + i);
>>>> dst[len++] = strtoul(tmp, NULL, 16);
>>>> }
>>>> dst[len] = 0;
>>>> *size = len;
>>>> return 0;
>>>> }
>>>>
>>>> (indeed with better error checking on strtoul ;) )
>>>
>>>
>>> Why delete these check from parse_hex_string()?
>>
>> The point is using 'strtoul' instead of your functions, so that you won't need
>> 'get_hex_val()' at all, or won't need 'isxdigit()' because 'strtoul' will check it,
>> won't need size should be multiply of two restriction '(*size & 1)' because of
>> implementation change. Probably you will need NULL checks, but again point
>> is why not using 'strtoul' instead of writing your version of it?
>
> Yes, we can use 'strtoul' , but my point is that I think we need these check code even if we use the code 'strtoul' .
> isxdigit(*c)) is need because *c may be sring "0xrgh" which is not hex.
> If we use strtoul will return 0 for that ,we can not distinguish between error or input is zero.
> '(*size & 1) can be delete, I agree.
>
> /* Check input parameters */
> if ((src == NULL) ||
> (dst == NULL) ||
> (size == NULL) ||
> (*size == 0))
> return -1;
> for (c = src, i = 0; i < *size; c++, i++) {
> if (isxdigit(*c))
> continue;
> else
> return -1;
> }
>
Please feel free to keep/add whatever check is required, I wasn't suggesting the
final implementation.
'strtol' can detect invalid chars, by providing non-NULL to second argument, so
'isxdigit()' is not required but more checks needed.
Overall you previously mentioned 'strtol' can't be used, but it can be used and
I believe it is better way to go, but I am asking what do you think about it?
Checks and implementation details can be handled.
>
>>>
>>> /* Check input parameters */
>>> if ((src == NULL) ||
>>> (dst == NULL) ||
>>> (size == NULL) ||
>>> (*size == 0))
>>> return -1;
>>> if ((*size & 1) != 0)
>>> return -1;
>>> for (c = src, i = 0; i < *size; c++, i++) {
>>> if (isxdigit(*c))
>>> continue;
>>> else
>>> return -1;
>>> }
>>>
>>>
>>>
>>>>
>>>> <...>
>>>>
>>>>> + /* Output buffer is not necessarily NUL-terminated. */
>>>>> + memcpy(buf, hex_tmp, hexlen);
>>>>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>>>>
>>>> Can't this overflow the 'buf'? since "len = 2 * hexlen"
>>>> I guess intention is "buf + hexlen"
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-25 9:35 ` Ferruh Yigit
@ 2019-03-25 9:39 ` Zhao1, Wei
0 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-25 9:39 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi，Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, March 25, 2019 5:36 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 3/25/2019 9:25 AM, Zhao1, Wei wrote:
> > Hi，Ferruh
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Monday, March 25, 2019 4:46 PM
> >> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev,
> Konstantin
> >> <konstantin.ananyev@intel.com>
> >> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex
> >> string parser for flow API
> >>
> >> On 3/25/2019 3:39 AM, Zhao1, Wei wrote:
> >>> Hi,
> >>>
> >>>> -----Original Message-----
> >>>> From: Yigit, Ferruh
> >>>> Sent: Friday, March 22, 2019 10:56 PM
> >>>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >>>> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev,
> >> Konstantin
> >>>> <konstantin.ananyev@intel.com>
> >>>> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of
> >>>> hex string parser for flow API
> >>>>
> >>>> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> >>>>> There is need for users to set configuration of HEX number for RSS
> >>>>> key. The key byte should be pass down as hex number not as char
> >>>>> string. This patch enable cmdline flow parse HEX number, in order
> >>>>> to not using string which pass ASIC number.
> >>>>>
> >>>>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> >>>>> action")
> >>>>> Cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >>>>> Tested-by: Peng Yuan <yuan.peng@intel.com>
> >>>>
> >>>> <...>
> >>>>
> >>>>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const
> >>>>> struct
> >>>> token *token,
> >>>>> return -1;
> >>>>> }
> >>>>>
> >>>>> +static uint32_t
> >>>>> +get_hex_val(char c)
> >>>>> +{
> >>>>> + switch (c) {
> >>>>> + case '0': case '1': case '2': case '3': case '4': case '5':
> >>>>> + case '6': case '7': case '8': case '9':
> >>>>> + return c - '0';
> >>>>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> >>>>> + return c - 'A' + 10;
> >>>>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> >>>>> + return c - 'a' + 10;
> >>>>> + default:
> >>>>> + return 0;
> >>>>> + }
> >>>>> +}
> >>>>> +
> >>>>> +static int
> >>>>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >>>>> + const char *c;
> >>>>> + uint32_t i;
> >>>>> +
> >>>>> + /* Check input parameters */
> >>>>> + if ((src == NULL) ||
> >>>>> + (dst == NULL) ||
> >>>>> + (size == NULL) ||
> >>>>> + (*size == 0))
> >>>>> + return -1;
> >>>>> + if ((*size & 1) != 0)
> >>>>> + return -1;
> >>>>> +
> >>>>> + for (c = src, i = 0; i < *size; c++, i++) {
> >>>>> + if (isxdigit(*c))
> >>>>> + continue;
> >>>>> + else
> >>>>> + return -1;
> >>>>> + }
> >>>>> +
> >>>>> + *size = *size / 2;
> >>>>> +
> >>>>> + /* Convert chars to bytes */
> >>>>> + for (i = 0; i < *size; i++)
> >>>>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> >>>>> + get_hex_val(src[2 * i + 1]);
> >>>>> +
> >>>>> + return 0;
> >>>>> +}
> >>>>
> >>>> I can see this has been discussed already but what would you think
> >>>> updating the 'parse_hex_string' something like following, it is
> >>>> less code to
> >> maintain:
> >>>>
> >>>> static int
> >>>> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >>>> int len;
> >>>> int i
> >>>> for (i = 0, len = 0; i < *size; i += 2) {
> >>>> char tmp[3];
> >>>> snprintf(tmp, 3, src + i);
> >>>> dst[len++] = strtoul(tmp, NULL, 16);
> >>>> }
> >>>> dst[len] = 0;
> >>>> *size = len;
> >>>> return 0;
> >>>> }
> >>>>
> >>>> (indeed with better error checking on strtoul ;) )
> >>>
> >>>
> >>> Why delete these check from parse_hex_string()?
> >>
> >> The point is using 'strtoul' instead of your functions, so that you
> >> won't need 'get_hex_val()' at all, or won't need 'isxdigit()' because
> >> 'strtoul' will check it, won't need size should be multiply of two
> >> restriction '(*size & 1)' because of implementation change. Probably
> >> you will need NULL checks, but again point is why not using 'strtoul'
> instead of writing your version of it?
> >
> > Yes, we can use 'strtoul' , but my point is that I think we need these check
> code even if we use the code 'strtoul' .
> > isxdigit(*c)) is need because *c may be sring "0xrgh" which is not hex.
> > If we use strtoul will return 0 for that ,we can not distinguish between
> error or input is zero.
> > '(*size & 1) can be delete, I agree.
> >
> > /* Check input parameters */
> > if ((src == NULL) ||
> > (dst == NULL) ||
> > (size == NULL) ||
> > (*size == 0))
> > return -1;
> > for (c = src, i = 0; i < *size; c++, i++) {
> > if (isxdigit(*c))
> > continue;
> > else
> > return -1;
> > }
> >
>
> Please feel free to keep/add whatever check is required, I wasn't suggesting
> the final implementation.
>
> 'strtol' can detect invalid chars, by providing non-NULL to second argument,
> so 'isxdigit()' is not required but more checks needed.
Get , thank you.
>
> Overall you previously mentioned 'strtol' can't be used, but it can be used
> and I believe it is better way to go, but I am asking what do you think about it?
> Checks and implementation details can be handled.
Ok, update in v4
>
> >
> >>>
> >>> /* Check input parameters */
> >>> if ((src == NULL) ||
> >>> (dst == NULL) ||
> >>> (size == NULL) ||
> >>> (*size == 0))
> >>> return -1;
> >>> if ((*size & 1) != 0)
> >>> return -1;
> >>> for (c = src, i = 0; i < *size; c++, i++) {
> >>> if (isxdigit(*c))
> >>> continue;
> >>> else
> >>> return -1;
> >>> }
> >>>
> >>>
> >>>
> >>>>
> >>>> <...>
> >>>>
> >>>>> + /* Output buffer is not necessarily NUL-terminated. */
> >>>>> + memcpy(buf, hex_tmp, hexlen);
> >>>>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> >>>>
> >>>> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> >>>> I guess intention is "buf + hexlen"
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-22 14:56 ` Ferruh Yigit
2019-03-25 3:39 ` Zhao1, Wei
@ 2019-04-09 7:38 ` Zhao1, Wei
2019-04-09 8:44 ` Ferruh Yigit
2019-04-09 7:40 ` Zhao1, Wei
2019-04-09 7:42 ` Zhao1, Wei
3 siblings, 1 reply; 24+ messages in thread
From: Zhao1, Wei @ 2019-04-09 7:38 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, March 22, 2019 10:56 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> > There is need for users to set configuration of HEX number for RSS
> > key. The key byte should be pass down as hex number not as char
> > string. This patch enable cmdline flow parse HEX number, in order to
> > not using string which pass ASIC number.
> >
> > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > action")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > Tested-by: Peng Yuan <yuan.peng@intel.com>
>
> <...>
>
> > @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct
> token *token,
> > return -1;
> > }
> >
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > + switch (c) {
> > + case '0': case '1': case '2': case '3': case '4': case '5':
> > + case '6': case '7': case '8': case '9':
> > + return c - '0';
> > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > + return c - 'A' + 10;
> > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > + return c - 'a' + 10;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int
> > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > + const char *c;
> > + uint32_t i;
> > +
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if (isxdigit(*c))
> > + continue;
> > + else
> > + return -1;
> > + }
> > +
> > + *size = *size / 2;
> > +
> > + /* Convert chars to bytes */
> > + for (i = 0; i < *size; i++)
> > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > + get_hex_val(src[2 * i + 1]);
> > +
> > + return 0;
> > +}
>
> I can see this has been discussed already but what would you think updating
> the 'parse_hex_string' something like following, it is less code to maintain:
>
> static int
> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> int len;
> int i
> for (i = 0, len = 0; i < *size; i += 2) {
> char tmp[3];
> snprintf(tmp, 3, src + i);
> dst[len++] = strtoul(tmp, NULL, 16);
> }
> dst[len] = 0;
> *size = len;
> return 0;
> }
>
> (indeed with better error checking on strtoul ;) )
>
> <...>
I have change code style as your guide as bellow,
but strtoul() seems do not work, it return c with a none-null every time.
you can have a try yourself.
static int
parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
{
char *c = NULL;
uint32_t i, len;
char tmp[3];
/* Check input parameters */
if ((src == NULL) ||
(dst == NULL) ||
(size == NULL) ||
(*size == 0))
return -1;
/* Convert chars to bytes */
for (i = 0, len = 0; i < *size; i += 2) {
snprintf(tmp, 3, "%s" ,src + i);
dst[len++] = strtoul(tmp, &c, 16);
if(c)
return -1;
}
dst[len] = 0;
*size = len;
return 0;
}
>
> > + /* Output buffer is not necessarily NUL-terminated. */
> > + memcpy(buf, hex_tmp, hexlen);
> > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>
> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-04-09 7:38 ` Zhao1, Wei
@ 2019-04-09 8:44 ` Ferruh Yigit
2019-04-09 9:12 ` Zhao1, Wei
0 siblings, 1 reply; 24+ messages in thread
From: Ferruh Yigit @ 2019-04-09 8:44 UTC (permalink / raw)
To: Zhao1, Wei, dev; +Cc: stable, stephen, Ananyev, Konstantin
On 4/9/2019 8:38 AM, Zhao1, Wei wrote:
> Hi, Ferruh
>
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Friday, March 22, 2019 10:56 PM
>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
>> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
>> parser for flow API
>>
>> On 3/22/2019 3:15 AM, Wei Zhao wrote:
>>> There is need for users to set configuration of HEX number for RSS
>>> key. The key byte should be pass down as hex number not as char
>>> string. This patch enable cmdline flow parse HEX number, in order to
>>> not using string which pass ASIC number.
>>>
>>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
>>> action")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
>>> Tested-by: Peng Yuan <yuan.peng@intel.com>
>>
>> <...>
>>
>>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct
>> token *token,
>>> return -1;
>>> }
>>>
>>> +static uint32_t
>>> +get_hex_val(char c)
>>> +{
>>> + switch (c) {
>>> + case '0': case '1': case '2': case '3': case '4': case '5':
>>> + case '6': case '7': case '8': case '9':
>>> + return c - '0';
>>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
>>> + return c - 'A' + 10;
>>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
>>> + return c - 'a' + 10;
>>> + default:
>>> + return 0;
>>> + }
>>> +}
>>> +
>>> +static int
>>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>>> + const char *c;
>>> + uint32_t i;
>>> +
>>> + /* Check input parameters */
>>> + if ((src == NULL) ||
>>> + (dst == NULL) ||
>>> + (size == NULL) ||
>>> + (*size == 0))
>>> + return -1;
>>> + if ((*size & 1) != 0)
>>> + return -1;
>>> +
>>> + for (c = src, i = 0; i < *size; c++, i++) {
>>> + if (isxdigit(*c))
>>> + continue;
>>> + else
>>> + return -1;
>>> + }
>>> +
>>> + *size = *size / 2;
>>> +
>>> + /* Convert chars to bytes */
>>> + for (i = 0; i < *size; i++)
>>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
>>> + get_hex_val(src[2 * i + 1]);
>>> +
>>> + return 0;
>>> +}
>>
>> I can see this has been discussed already but what would you think updating
>> the 'parse_hex_string' something like following, it is less code to maintain:
>>
>> static int
>> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
>> int len;
>> int i
>> for (i = 0, len = 0; i < *size; i += 2) {
>> char tmp[3];
>> snprintf(tmp, 3, src + i);
>> dst[len++] = strtoul(tmp, NULL, 16);
>> }
>> dst[len] = 0;
>> *size = len;
>> return 0;
>> }
>>
>> (indeed with better error checking on strtoul ;) )
>>
>> <...>
>
>
>
> I have change code style as your guide as bellow,
> but strtoul() seems do not work, it return c with a none-null every time.
> you can have a try yourself.
Hi Wei,
Yes it return 'c' none-null, which is OK/expected.
According man page:
"
If endptr is not NULL, strtol() stores the address of the first invalid
character in *endptr. If there were no digits at all, strtol() stores the
original value of nptr in *endptr (and returns 0). In particular, if *nptr is
not '\0' but **endptr is '\0' on return, the entire string is valid.
"
First invalid char in below is 'tmp[2]', which is '0', so 'strtol' returns
'&tmp[2]', this is aligned with what described above.
It says, "if *nptr is not '\0' but **endptr is '\0' on return, the entire string
is valid.", beware it says "**endptr is '\0'", so following check should work:
if (*c != 0) {
len--;
dst[len] = 0;
*size = len;
return -1;
}
>
>
> static int
> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
> {
> char *c = NULL;
> uint32_t i, len;
> char tmp[3];
>
> /* Check input parameters */
> if ((src == NULL) ||
> (dst == NULL) ||
> (size == NULL) ||
> (*size == 0))
> return -1;
>
> /* Convert chars to bytes */
> for (i = 0, len = 0; i < *size; i += 2) {
> snprintf(tmp, 3, "%s" ,src + i);
> dst[len++] = strtoul(tmp, &c, 16);
> if(c)
> return -1;
> }
> dst[len] = 0;
> *size = len;
>
> return 0;
> }
>
>>
>>> + /* Output buffer is not necessarily NUL-terminated. */
>>> + memcpy(buf, hex_tmp, hexlen);
>>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>>
>> Can't this overflow the 'buf'? since "len = 2 * hexlen"
>> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-04-09 8:44 ` Ferruh Yigit
@ 2019-04-09 9:12 ` Zhao1, Wei
0 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-04-09 9:12 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, April 9, 2019 4:45 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 4/9/2019 8:38 AM, Zhao1, Wei wrote:
> > Hi, Ferruh
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Friday, March 22, 2019 10:56 PM
> >> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev,
> Konstantin
> >> <konstantin.ananyev@intel.com>
> >> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex
> >> string parser for flow API
> >>
> >> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> >>> There is need for users to set configuration of HEX number for RSS
> >>> key. The key byte should be pass down as hex number not as char
> >>> string. This patch enable cmdline flow parse HEX number, in order to
> >>> not using string which pass ASIC number.
> >>>
> >>> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> >>> action")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >>> Tested-by: Peng Yuan <yuan.peng@intel.com>
> >>
> >> <...>
> >>
> >>> @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const
> >>> struct
> >> token *token,
> >>> return -1;
> >>> }
> >>>
> >>> +static uint32_t
> >>> +get_hex_val(char c)
> >>> +{
> >>> + switch (c) {
> >>> + case '0': case '1': case '2': case '3': case '4': case '5':
> >>> + case '6': case '7': case '8': case '9':
> >>> + return c - '0';
> >>> + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> >>> + return c - 'A' + 10;
> >>> + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> >>> + return c - 'a' + 10;
> >>> + default:
> >>> + return 0;
> >>> + }
> >>> +}
> >>> +
> >>> +static int
> >>> +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >>> + const char *c;
> >>> + uint32_t i;
> >>> +
> >>> + /* Check input parameters */
> >>> + if ((src == NULL) ||
> >>> + (dst == NULL) ||
> >>> + (size == NULL) ||
> >>> + (*size == 0))
> >>> + return -1;
> >>> + if ((*size & 1) != 0)
> >>> + return -1;
> >>> +
> >>> + for (c = src, i = 0; i < *size; c++, i++) {
> >>> + if (isxdigit(*c))
> >>> + continue;
> >>> + else
> >>> + return -1;
> >>> + }
> >>> +
> >>> + *size = *size / 2;
> >>> +
> >>> + /* Convert chars to bytes */
> >>> + for (i = 0; i < *size; i++)
> >>> + dst[i] = get_hex_val(src[2 * i]) * 16 +
> >>> + get_hex_val(src[2 * i + 1]);
> >>> +
> >>> + return 0;
> >>> +}
> >>
> >> I can see this has been discussed already but what would you think
> >> updating the 'parse_hex_string' something like following, it is less code to
> maintain:
> >>
> >> static int
> >> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> >> int len;
> >> int i
> >> for (i = 0, len = 0; i < *size; i += 2) {
> >> char tmp[3];
> >> snprintf(tmp, 3, src + i);
> >> dst[len++] = strtoul(tmp, NULL, 16);
> >> }
> >> dst[len] = 0;
> >> *size = len;
> >> return 0;
> >> }
> >>
> >> (indeed with better error checking on strtoul ;) )
> >>
> >> <...>
> >
> >
> >
> > I have change code style as your guide as bellow, but strtoul() seems
> > do not work, it return c with a none-null every time.
> > you can have a try yourself.
>
> Hi Wei,
>
> Yes it return 'c' none-null, which is OK/expected.
>
> According man page:
> "
> If endptr is not NULL, strtol() stores the address of the first invalid character
> in *endptr. If there were no digits at all, strtol() stores the original value of
> nptr in *endptr (and returns 0). In particular, if *nptr is not '\0' but **endptr
> is '\0' on return, the entire string is valid.
> "
>
> First invalid char in below is 'tmp[2]', which is '0', so 'strtol' returns '&tmp[2]',
> this is aligned with what described above.
>
> It says, "if *nptr is not '\0' but **endptr is '\0' on return, the entire string is
> valid.", beware it says "**endptr is '\0'", so following check should work:
>
> if (*c != 0) {
> len--;
> dst[len] = 0;
> *size = len;
> return -1;
> }
Yes, you are right, it keep address of 'tmp[2]' in c, update in v4 for check code .
>
> >
> >
> > static int
> > parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > char *c = NULL;
> > uint32_t i, len;
> > char tmp[3];
> >
> > /* Check input parameters */
> > if ((src == NULL) ||
> > (dst == NULL) ||
> > (size == NULL) ||
> > (*size == 0))
> > return -1;
> >
> > /* Convert chars to bytes */
> > for (i = 0, len = 0; i < *size; i += 2) {
> > snprintf(tmp, 3, "%s" ,src + i);
> > dst[len++] = strtoul(tmp, &c, 16);
> > if(c)
> > return -1;
> > }
> > dst[len] = 0;
> > *size = len;
> >
> > return 0;
> > }
> >
> >>
> >>> + /* Output buffer is not necessarily NUL-terminated. */
> >>> + memcpy(buf, hex_tmp, hexlen);
> >>> + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> >>
> >> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> >> I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-22 14:56 ` Ferruh Yigit
2019-03-25 3:39 ` Zhao1, Wei
2019-04-09 7:38 ` Zhao1, Wei
@ 2019-04-09 7:40 ` Zhao1, Wei
2019-04-09 7:42 ` Zhao1, Wei
3 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-04-09 7:40 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi, Ferruh
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Tuesday, April 9, 2019 3:38 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: RE: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> Hi, Ferruh
>
> > -----Original Message-----
> > From: Yigit, Ferruh
> > Sent: Friday, March 22, 2019 10:56 PM
> > To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> > Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>
> > Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex
> > string parser for flow API
> >
> > On 3/22/2019 3:15 AM, Wei Zhao wrote:
> > > There is need for users to set configuration of HEX number for RSS
> > > key. The key byte should be pass down as hex number not as char
> > > string. This patch enable cmdline flow parse HEX number, in order to
> > > not using string which pass ASIC number.
> > >
> > > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > > action")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > > Tested-by: Peng Yuan <yuan.peng@intel.com>
> >
> > <...>
> >
> > > @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const
> > > struct
> > token *token,
> > > return -1;
> > > }
> > >
> > > +static uint32_t
> > > +get_hex_val(char c)
> > > +{
> > > + switch (c) {
> > > + case '0': case '1': case '2': case '3': case '4': case '5':
> > > + case '6': case '7': case '8': case '9':
> > > + return c - '0';
> > > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > > + return c - 'A' + 10;
> > > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > > + return c - 'a' + 10;
> > > + default:
> > > + return 0;
> > > + }
> > > +}
> > > +
> > > +static int
> > > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > > + const char *c;
> > > + uint32_t i;
> > > +
> > > + /* Check input parameters */
> > > + if ((src == NULL) ||
> > > + (dst == NULL) ||
> > > + (size == NULL) ||
> > > + (*size == 0))
> > > + return -1;
> > > + if ((*size & 1) != 0)
> > > + return -1;
> > > +
> > > + for (c = src, i = 0; i < *size; c++, i++) {
> > > + if (isxdigit(*c))
> > > + continue;
> > > + else
> > > + return -1;
> > > + }
> > > +
> > > + *size = *size / 2;
> > > +
> > > + /* Convert chars to bytes */
> > > + for (i = 0; i < *size; i++)
> > > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > > + get_hex_val(src[2 * i + 1]);
> > > +
> > > + return 0;
> > > +}
> >
> > I can see this has been discussed already but what would you think
> > updating the 'parse_hex_string' something like following, it is less code to
> maintain:
> >
> > static int
> > parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > int len;
> > int i
> > for (i = 0, len = 0; i < *size; i += 2) {
> > char tmp[3];
> > snprintf(tmp, 3, src + i);
> > dst[len++] = strtoul(tmp, NULL, 16);
> > }
> > dst[len] = 0;
> > *size = len;
> > return 0;
> > }
> >
> > (indeed with better error checking on strtoul ;) )
> >
> > <...>
>
>
>
> I have change code style as your guide as bellow, but strtoul() seems do not
> work, it return c with a none-null every time.
> you can have a try yourself.
>
>
> static int
> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
> {
> char *c = NULL;
> uint32_t i, len;
> char tmp[3];
>
> /* Check input parameters */
> if ((src == NULL) ||
> (dst == NULL) ||
> (size == NULL) ||
> (*size == 0))
> return -1;
>
> /* Convert chars to bytes */
> for (i = 0, len = 0; i < *size; i += 2) {
> snprintf(tmp, 3, "%s" ,src + i);
> dst[len++] = strtoul(tmp, &c, 16);
> if(c)
> return -1;
> }
> dst[len] = 0;
> *size = len;
>
> return 0;
> }
>
As this code with strtoul do not work, I suggest we use v3 of get_hex_val(), do you think so?
> >
> > > + /* Output buffer is not necessarily NUL-terminated. */
> > > + memcpy(buf, hex_tmp, hexlen);
> > > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
> >
> > Can't this overflow the 'buf'? since "len = 2 * hexlen"
> > I guess intention is "buf + hexlen"
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string parser for flow API
2019-03-22 14:56 ` Ferruh Yigit
` (2 preceding siblings ...)
2019-04-09 7:40 ` Zhao1, Wei
@ 2019-04-09 7:42 ` Zhao1, Wei
3 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-04-09 7:42 UTC (permalink / raw)
To: Yigit, Ferruh, dev; +Cc: stable, stephen, Ananyev, Konstantin
Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Friday, March 22, 2019 10:56 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; stephen@networkplumber.org; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-stable] [PATCH v3] app/testpmd: fix support of hex string
> parser for flow API
>
> On 3/22/2019 3:15 AM, Wei Zhao wrote:
> > There is need for users to set configuration of HEX number for RSS
> > key. The key byte should be pass down as hex number not as char
> > string. This patch enable cmdline flow parse HEX number, in order to
> > not using string which pass ASIC number.
> >
> > Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow
> > action")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> > Tested-by: Peng Yuan <yuan.peng@intel.com>
>
> <...>
>
> > @@ -4475,6 +4486,138 @@ parse_string(struct context *ctx, const struct
> token *token,
> > return -1;
> > }
> >
> > +static uint32_t
> > +get_hex_val(char c)
> > +{
> > + switch (c) {
> > + case '0': case '1': case '2': case '3': case '4': case '5':
> > + case '6': case '7': case '8': case '9':
> > + return c - '0';
> > + case 'A': case 'B': case 'C': case 'D': case 'E': case 'F':
> > + return c - 'A' + 10;
> > + case 'a': case 'b': case 'c': case 'd': case 'e': case 'f':
> > + return c - 'a' + 10;
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > +static int
> > +parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> > + const char *c;
> > + uint32_t i;
> > +
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if (isxdigit(*c))
> > + continue;
> > + else
> > + return -1;
> > + }
> > +
> > + *size = *size / 2;
> > +
> > + /* Convert chars to bytes */
> > + for (i = 0; i < *size; i++)
> > + dst[i] = get_hex_val(src[2 * i]) * 16 +
> > + get_hex_val(src[2 * i + 1]);
> > +
> > + return 0;
> > +}
>
> I can see this has been discussed already but what would you think updating
> the 'parse_hex_string' something like following, it is less code to maintain:
>
> static int
> parse_hex_string(const char *src, uint8_t *dst, uint32_t *size) {
> int len;
> int i
> for (i = 0, len = 0; i < *size; i += 2) {
> char tmp[3];
> snprintf(tmp, 3, src + i);
> dst[len++] = strtoul(tmp, NULL, 16);
> }
> dst[len] = 0;
> *size = len;
> return 0;
> }
>
> (indeed with better error checking on strtoul ;) )
>
> <...>
>
> > + /* Output buffer is not necessarily NUL-terminated. */
> > + memcpy(buf, hex_tmp, hexlen);
> > + memset((uint8_t *)buf + len, 0x00, size - hexlen);
>
> Can't this overflow the 'buf'? since "len = 2 * hexlen"
> I guess intention is "buf + hexlen"
Yes, you are right, update in v4
^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-stable] [PATCH v4] app/testpmd: fix support of hex string parser for flow API
2019-03-22 3:15 ` [dpdk-stable] [PATCH v3] " Wei Zhao
2019-03-22 14:56 ` Ferruh Yigit
@ 2019-04-09 8:41 ` Wei Zhao
2019-04-11 18:40 ` Ferruh Yigit
1 sibling, 1 reply; 24+ messages in thread
From: Wei Zhao @ 2019-04-09 8:41 UTC (permalink / raw)
To: dev; +Cc: stable, ferruh.yigit, yuan.peng, Wei Zhao
There is need for users to set configuration of HEX number for RSS
key. The key byte should be pass down as hex number not as char
string. This patch enable cmdline flow parse HEX number,
in order to not using string which pass ASIC number.
Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
Cc: stable@dpdk.org
Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
Tested-by: Peng Yuan <yuan.peng@intel.com>
---
v2:
fix bug of string length limite.
v3:
use isxdigit to check char string
v4:
fix bug in set memory to zero and change code for translating
string to hex
---
app/test-pmd/cmdline_flow.c | 128 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 1 deletion(-)
diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
index 1c83bc9..54ff175 100644
--- a/app/test-pmd/cmdline_flow.c
+++ b/app/test-pmd/cmdline_flow.c
@@ -36,6 +36,7 @@ enum index {
PREFIX,
BOOLEAN,
STRING,
+ HEX,
MAC_ADDR,
IPV4_ADDR,
IPV6_ADDR,
@@ -1123,6 +1124,9 @@ static int parse_boolean(struct context *, const struct token *,
static int parse_string(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
+static int parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size);
static int parse_mac_addr(struct context *, const struct token *,
const char *, unsigned int,
void *, unsigned int);
@@ -1199,6 +1203,13 @@ static const struct token token_list[] = {
.call = parse_string,
.comp = comp_none,
},
+ [HEX] = {
+ .name = "{hex}",
+ .type = "HEX",
+ .help = "fixed string",
+ .call = parse_hex,
+ .comp = comp_none,
+ },
[MAC_ADDR] = {
.name = "{MAC address}",
.type = "MAC-48",
@@ -2307,7 +2318,7 @@ static const struct token token_list[] = {
[ACTION_RSS_KEY] = {
.name = "key",
.help = "RSS hash key",
- .next = NEXT(action_rss, NEXT_ENTRY(STRING)),
+ .next = NEXT(action_rss, NEXT_ENTRY(HEX)),
.args = ARGS(ARGS_ENTRY_ARB(0, 0),
ARGS_ENTRY_ARB
(offsetof(struct action_rss_data, conf) +
@@ -4476,6 +4487,121 @@ parse_string(struct context *ctx, const struct token *token,
return -1;
}
+static int
+parse_hex_string(const char *src, uint8_t *dst, uint32_t *size)
+{
+ char *c = NULL;
+ uint32_t i, len;
+ char tmp[3];
+
+ /* Check input parameters */
+ if ((src == NULL) ||
+ (dst == NULL) ||
+ (size == NULL) ||
+ (*size == 0))
+ return -1;
+
+ /* Convert chars to bytes */
+ for (i = 0, len = 0; i < *size; i += 2) {
+ snprintf(tmp, 3, "%s", src + i);
+ dst[len++] = strtoul(tmp, &c, 16);
+ if (*c != 0) {
+ len--;
+ dst[len] = 0;
+ *size = len;
+ return -1;
+ }
+ }
+ dst[len] = 0;
+ *size = len;
+
+ return 0;
+}
+
+static int
+parse_hex(struct context *ctx, const struct token *token,
+ const char *str, unsigned int len,
+ void *buf, unsigned int size)
+{
+ const struct arg *arg_data = pop_args(ctx);
+ const struct arg *arg_len = pop_args(ctx);
+ const struct arg *arg_addr = pop_args(ctx);
+ char tmp[16]; /* Ought to be enough. */
+ int ret;
+ unsigned int hexlen = len;
+ unsigned int length = 256;
+ uint8_t hex_tmp[length];
+
+ /* Arguments are expected. */
+ if (!arg_data)
+ return -1;
+ if (!arg_len) {
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ if (!arg_addr) {
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+ }
+ size = arg_data->size;
+ /* Bit-mask fill is not supported. */
+ if (arg_data->mask)
+ goto error;
+ if (!ctx->object)
+ return len;
+
+ /* translate bytes string to array. */
+ if (str[0] == '0' && ((str[1] == 'x') ||
+ (str[1] == 'X'))) {
+ str += 2;
+ hexlen -= 2;
+ }
+ if (hexlen > length)
+ return -1;
+ ret = parse_hex_string(str, hex_tmp, &hexlen);
+ if (ret < 0)
+ goto error;
+ /* Let parse_int() fill length information first. */
+ ret = snprintf(tmp, sizeof(tmp), "%u", hexlen);
+ if (ret < 0)
+ goto error;
+ push_args(ctx, arg_len);
+ ret = parse_int(ctx, token, tmp, ret, NULL, 0);
+ if (ret < 0) {
+ pop_args(ctx);
+ goto error;
+ }
+ buf = (uint8_t *)ctx->object + arg_data->offset;
+ /* Output buffer is not necessarily NUL-terminated. */
+ memcpy(buf, hex_tmp, hexlen);
+ memset((uint8_t *)buf + hexlen, 0x00, size - hexlen);
+ if (ctx->objmask)
+ memset((uint8_t *)ctx->objmask + arg_data->offset,
+ 0xff, hexlen);
+ /* Save address if requested. */
+ if (arg_addr->size) {
+ memcpy((uint8_t *)ctx->object + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->object + arg_data->offset
+ },
+ arg_addr->size);
+ if (ctx->objmask)
+ memcpy((uint8_t *)ctx->objmask + arg_addr->offset,
+ (void *[]){
+ (uint8_t *)ctx->objmask + arg_data->offset
+ },
+ arg_addr->size);
+ }
+ return len;
+error:
+ push_args(ctx, arg_addr);
+ push_args(ctx, arg_len);
+ push_args(ctx, arg_data);
+ return -1;
+
+}
+
/**
* Parse a MAC address.
*
--
2.7.5
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [PATCH v4] app/testpmd: fix support of hex string parser for flow API
2019-04-09 8:41 ` [dpdk-stable] [PATCH v4] " Wei Zhao
@ 2019-04-11 18:40 ` Ferruh Yigit
0 siblings, 0 replies; 24+ messages in thread
From: Ferruh Yigit @ 2019-04-11 18:40 UTC (permalink / raw)
To: Wei Zhao, dev; +Cc: stable, yuan.peng
On 4/9/2019 9:41 AM, Wei Zhao wrote:
> There is need for users to set configuration of HEX number for RSS
> key. The key byte should be pass down as hex number not as char
> string. This patch enable cmdline flow parse HEX number,
> in order to not using string which pass ASIC number.
>
> Fixes: f4d623f96119 ("app/testpmd: fix missing RSS fields in flow action")
> Cc: stable@dpdk.org
>
> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> Tested-by: Peng Yuan <yuan.peng@intel.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-15 8:43 [dpdk-stable] [PATCH] app/testpmd: fix support of hex string parser for flow API Wei Zhao
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
2019-03-18 8:16 ` [dpdk-stable] [PATCH v2] " Wei Zhao
@ 2019-03-18 15:27 ` Stephen Hemminger
2019-03-22 1:34 ` Zhao1, Wei
2 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2019-03-18 15:27 UTC (permalink / raw)
To: Wei Zhao; +Cc: dev, stable, qi.z.zhang, adrien.mazarguil
On Fri, 15 Mar 2019 16:43:55 +0800
Wei Zhao <wei.zhao1@intel.com> wrote:
> + /* Check input parameters */
> + if ((src == NULL) ||
> + (dst == NULL) ||
> + (size == NULL) ||
> + (*size == 0))
> + return -1;
> + if ((*size & 1) != 0)
> + return -1;
> +
> + for (c = src, i = 0; i < *size; c++, i++) {
> + if ((((*c) >= '0') && ((*c) <= '9')) ||
> + (((*c) >= 'A') && ((*c) <= 'F')) ||
> + (((*c) >= 'a') && ((*c) <= 'f')))
> + continue;
> +
Excessive number of parens here. You don't need them around (*c)
Why did you not use isxdigit() instead?
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix support of hex string parser for flow API
2019-03-18 15:27 ` [dpdk-stable] [dpdk-dev] [PATCH] " Stephen Hemminger
@ 2019-03-22 1:34 ` Zhao1, Wei
0 siblings, 0 replies; 24+ messages in thread
From: Zhao1, Wei @ 2019-03-22 1:34 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, stable, Zhang, Qi Z, adrien.mazarguil
Hi, Stephen Hemminger
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Monday, March 18, 2019 11:27 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>;
> adrien.mazarguil@6wind.com
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix support of hex string
> parser for flow API
>
> On Fri, 15 Mar 2019 16:43:55 +0800
> Wei Zhao <wei.zhao1@intel.com> wrote:
>
> > + /* Check input parameters */
> > + if ((src == NULL) ||
> > + (dst == NULL) ||
> > + (size == NULL) ||
> > + (*size == 0))
> > + return -1;
> > + if ((*size & 1) != 0)
> > + return -1;
> > +
> > + for (c = src, i = 0; i < *size; c++, i++) {
> > + if ((((*c) >= '0') && ((*c) <= '9')) ||
> > + (((*c) >= 'A') && ((*c) <= 'F')) ||
> > + (((*c) >= 'a') && ((*c) <= 'f')))
> > + continue;
> > +
>
> Excessive number of parens here. You don't need them around (*c)
>
> Why did you not use isxdigit() instead?
Ok, I will have a try of isxdigit() and update in v2.
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2019-04-11 18:40 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-15 8:43 [dpdk-stable] [PATCH] app/testpmd: fix support of hex string parser for flow API Wei Zhao
2019-03-15 9:44 ` [dpdk-stable] [dpdk-dev] " Ananyev, Konstantin
2019-03-18 1:36 ` Zhao1, Wei
2019-03-18 8:49 ` Zhao1, Wei
2019-03-21 14:03 ` Ananyev, Konstantin
2019-03-22 1:34 ` Zhao1, Wei
2019-03-18 8:16 ` [dpdk-stable] [PATCH v2] " Wei Zhao
2019-03-19 5:23 ` Zhao1, Wei
2019-03-22 3:15 ` [dpdk-stable] [PATCH v3] " Wei Zhao
2019-03-22 14:56 ` Ferruh Yigit
2019-03-25 3:39 ` Zhao1, Wei
2019-03-25 8:45 ` Ferruh Yigit
2019-03-25 9:25 ` Zhao1, Wei
2019-03-25 9:35 ` Ferruh Yigit
2019-03-25 9:39 ` Zhao1, Wei
2019-04-09 7:38 ` Zhao1, Wei
2019-04-09 8:44 ` Ferruh Yigit
2019-04-09 9:12 ` Zhao1, Wei
2019-04-09 7:40 ` Zhao1, Wei
2019-04-09 7:42 ` Zhao1, Wei
2019-04-09 8:41 ` [dpdk-stable] [PATCH v4] " Wei Zhao
2019-04-11 18:40 ` Ferruh Yigit
2019-03-18 15:27 ` [dpdk-stable] [dpdk-dev] [PATCH] " Stephen Hemminger
2019-03-22 1:34 ` Zhao1, Wei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).