From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 4F0662C6E for ; Mon, 7 May 2018 08:54:02 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 May 2018 23:54:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,373,1520924400"; d="scan'208";a="52085054" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by fmsmga004.fm.intel.com with ESMTP; 06 May 2018 23:54:00 -0700 Received: from pgsmsx101.gar.corp.intel.com ([169.254.1.211]) by KMSMSX151.gar.corp.intel.com ([169.254.10.77]) with mapi id 14.03.0319.002; Mon, 7 May 2018 14:53:58 +0800 From: "Zhao1, Wei" To: Adrien Mazarguil , "dev@dpdk.org" CC: "Peng, Yuan" Thread-Topic: [dpdk-dev] [PATCH v1 5/9] app/testpmd: fix RSS flow action configuration Thread-Index: AQHTwqbUULxJysDZ8kKqxKEC25j0y6QkGOPw Date: Mon, 7 May 2018 06:53:57 +0000 Message-ID: References: <20180323124725.15806-1-adrien.mazarguil@6wind.com> <20180323124725.15806-6-adrien.mazarguil@6wind.com> In-Reply-To: <20180323124725.15806-6-adrien.mazarguil@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [172.30.20.206] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1 5/9] app/testpmd: fix RSS flow action configuration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 May 2018 06:54:03 -0000 Hi, Adrien Mazarguil Once we apply this patch ,we will meet an abnormal issue, rte_flow command= will cause core dump. If we do not apply it, this will disappear. I have check this issue, it see= ms there is something wrong in function of port_flow_new() in Config.c file, core dump happen here every time. This is find out by Peng yuan first, she will tell you how reappear this i= ssue. > -----Original Message----- > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil > Sent: Friday, March 23, 2018 8:58 PM > To: dev@dpdk.org > Cc: stable@dpdk.org; Lu, Wenzhuo ; Wu, Jingjing > > Subject: [dpdk-dev] [PATCH v1 5/9] app/testpmd: fix RSS flow action > configuration >=20 > Except for a list of queues, RSS configuration (hash key and fields) cann= ot be > specified from the flow command line and testpmd does not provide safe > defaults either. >=20 > In order to validate their implementation with testpmd, PMDs had to > interpret its NULL RSS configuration parameters somehow, however this has > never been valid to begin with. >=20 > This patch makes testpmd always provide default values. >=20 > Fixes: 05d34c6e9d2c ("app/testpmd: add queue actions to flow command") > Cc: stable@dpdk.org >=20 > Signed-off-by: Adrien Mazarguil > Cc: Wenzhuo Lu > Cc: Jingjing Wu > --- > app/test-pmd/cmdline_flow.c | 104 +++++++++++++++++++++++++---- > app/test-pmd/config.c | 141 +++++++++++++++++++++++++++----------- > - > 2 files changed, 192 insertions(+), 53 deletions(-) >=20 > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > index c2cf415ef..890c36d8e 100644 > --- a/app/test-pmd/cmdline_flow.c > +++ b/app/test-pmd/cmdline_flow.c > @@ -184,13 +184,19 @@ enum index { > #define ITEM_RAW_SIZE \ > (offsetof(struct rte_flow_item_raw, pattern) + > ITEM_RAW_PATTERN_SIZE) >=20 > -/** Number of queue[] entries in struct rte_flow_action_rss. */ -#define > ACTION_RSS_NUM 32 > - > -/** Storage size for struct rte_flow_action_rss including queues. */ - > #define ACTION_RSS_SIZE \ > - (offsetof(struct rte_flow_action_rss, queue) + \ > - sizeof(*((struct rte_flow_action_rss *)0)->queue) * > ACTION_RSS_NUM) > +/** Maximum number of queue indices in struct rte_flow_action_rss. */ > +#define ACTION_RSS_QUEUE_NUM 32 > + > +/** Storage for struct rte_flow_action_rss including external data. */ > +union action_rss_data { > + struct rte_flow_action_rss conf; > + struct { > + uint8_t conf_data[offsetof(struct rte_flow_action_rss, > queue)]; > + uint16_t queue[ACTION_RSS_QUEUE_NUM]; > + struct rte_eth_rss_conf rss_conf; > + uint8_t rss_key[RSS_HASH_KEY_LENGTH]; > + } s; > +}; >=20 > /** Maximum number of subsequent tokens and arguments on the stack. > */ #define CTX_STACK_SIZE 16 @@ -316,6 +322,13 @@ struct token { > .size =3D (sz), \ > }) >=20 > +/** Static initializer for ARGS() with arbitrary offset and size. */ > +#define ARGS_ENTRY_ARB(o, s) \ > + (&(const struct arg){ \ > + .offset =3D (o), \ > + .size =3D (s), \ > + }) > + > /** Same as ARGS_ENTRY() using network byte ordering. */ #define > ARGS_ENTRY_HTON(s, f) \ > (&(const struct arg){ \ > @@ -650,6 +663,9 @@ static int parse_vc_spec(struct context *, const stru= ct > token *, > const char *, unsigned int, void *, unsigned int); > static int parse_vc_conf(struct context *, const struct token *, > const char *, unsigned int, void *, unsigned int); > +static int parse_vc_action_rss(struct context *, const struct token *, > + const char *, unsigned int, void *, > + unsigned int); > static int parse_vc_action_rss_queue(struct context *, const struct toke= n *, > const char *, unsigned int, void *, > unsigned int); > @@ -1573,9 +1589,9 @@ static const struct token token_list[] =3D { > [ACTION_RSS] =3D { > .name =3D "rss", > .help =3D "spread packets among several queues", > - .priv =3D PRIV_ACTION(RSS, ACTION_RSS_SIZE), > + .priv =3D PRIV_ACTION(RSS, sizeof(union action_rss_data)), > .next =3D NEXT(action_rss), > - .call =3D parse_vc, > + .call =3D parse_vc_action_rss, > }, > [ACTION_RSS_QUEUES] =3D { > .name =3D "queues", > @@ -2004,6 +2020,64 @@ parse_vc_conf(struct context *ctx, const struct > token *token, > return len; > } >=20 > +/** Parse RSS action. */ > +static int > +parse_vc_action_rss(struct context *ctx, const struct token *token, > + const char *str, unsigned int len, > + void *buf, unsigned int size) > +{ > + struct buffer *out =3D buf; > + struct rte_flow_action *action; > + union action_rss_data *action_rss_data; > + unsigned int i; > + int ret; > + > + ret =3D parse_vc(ctx, token, str, len, buf, size); > + if (ret < 0) > + return ret; > + /* Nothing else to do if there is no buffer. */ > + if (!out) > + return ret; > + if (!out->args.vc.actions_n) > + return -1; > + action =3D &out->args.vc.actions[out->args.vc.actions_n - 1]; > + /* Point to selected object. */ > + ctx->object =3D out->args.vc.data; > + ctx->objmask =3D NULL; > + /* Set up default configuration. */ > + action_rss_data =3D ctx->object; > + *action_rss_data =3D (union action_rss_data){ > + .conf =3D (struct rte_flow_action_rss){ > + .rss_conf =3D &action_rss_data->s.rss_conf, > + .num =3D RTE_MIN(nb_rxq, > ACTION_RSS_QUEUE_NUM), > + }, > + }; > + action_rss_data->s.rss_conf =3D (struct rte_eth_rss_conf){ > + .rss_key =3D action_rss_data->s.rss_key, > + .rss_key_len =3D sizeof(action_rss_data->s.rss_key), > + .rss_hf =3D rss_hf, > + }; > + strncpy((void *)action_rss_data->s.rss_key, > + "testpmd's default RSS hash key", > + sizeof(action_rss_data->s.rss_key)); > + for (i =3D 0; i < action_rss_data->conf.num; ++i) > + action_rss_data->conf.queue[i] =3D i; > + if (!port_id_is_invalid(ctx->port, DISABLED_WARN) && > + ctx->port !=3D (portid_t)RTE_PORT_ALL) { > + if (rte_eth_dev_rss_hash_conf_get > + (ctx->port, &action_rss_data->s.rss_conf) < 0) { > + struct rte_eth_dev_info info; > + > + rte_eth_dev_info_get(ctx->port, &info); > + action_rss_data->s.rss_conf.rss_key_len =3D > + RTE_MIN(sizeof(action_rss_data->s.rss_key), > + info.hash_key_size); > + } > + } > + action->conf =3D &action_rss_data->conf; > + return ret; > +} > + > /** > * Parse queue field for RSS action. > * > @@ -2015,6 +2089,7 @@ parse_vc_action_rss_queue(struct context *ctx, > const struct token *token, > void *buf, unsigned int size) > { > static const enum index next[] =3D NEXT_ENTRY(ACTION_RSS_QUEUE); > + union action_rss_data *action_rss_data; > int ret; > int i; >=20 > @@ -2028,9 +2103,13 @@ parse_vc_action_rss_queue(struct context *ctx, > const struct token *token, > ctx->objdata &=3D 0xffff; > return len; > } > - if (i >=3D ACTION_RSS_NUM) > + if (i >=3D ACTION_RSS_QUEUE_NUM) > return -1; > - if (push_args(ctx, ARGS_ENTRY(struct rte_flow_action_rss, > queue[i]))) > + if (push_args(ctx, > + ARGS_ENTRY_ARB(offsetof(struct rte_flow_action_rss, > + queue) + > + i * sizeof(action_rss_data->s.queue[i]), > + sizeof(action_rss_data->s.queue[i])))) > return -1; > ret =3D parse_int(ctx, token, str, len, NULL, 0); > if (ret < 0) { > @@ -2045,7 +2124,8 @@ parse_vc_action_rss_queue(struct context *ctx, > const struct token *token, > ctx->next[ctx->next_num++] =3D next; > if (!ctx->object) > return len; > - ((struct rte_flow_action_rss *)ctx->object)->num =3D i; > + action_rss_data =3D ctx->object; > + action_rss_data->conf.num =3D i; > return len; > } >=20 > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index > 4bb255c62..a4b54f86a 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -982,31 +982,52 @@ static const struct { > MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)), }; >=20 > -/** Compute storage space needed by item specification. */ -static void = - > flow_item_spec_size(const struct rte_flow_item *item, > - size_t *size, size_t *pad) > +/** Pattern item specification types. */ enum item_spec_type { > + ITEM_SPEC, > + ITEM_LAST, > + ITEM_MASK, > +}; > + > +/** Compute storage space needed by item specification and copy it. */ > +static size_t flow_item_spec_copy(void *buf, const struct rte_flow_item > +*item, > + enum item_spec_type type) > { > - if (!item->spec) { > - *size =3D 0; > + size_t size =3D 0; > + > + const void *item_spec =3D > + type =3D=3D ITEM_SPEC ? item->spec : > + type =3D=3D ITEM_LAST ? item->last : > + type =3D=3D ITEM_MASK ? item->mask : > + NULL; > + > + if (!item_spec) > goto empty; > - } > switch (item->type) { > union { > const struct rte_flow_item_raw *raw; > - } spec; > + } src; > + union { > + struct rte_flow_item_raw *raw; > + } dst; >=20 > case RTE_FLOW_ITEM_TYPE_RAW: > - spec.raw =3D item->spec; > - *size =3D offsetof(struct rte_flow_item_raw, pattern) + > - spec.raw->length * sizeof(*spec.raw->pattern); > + src.raw =3D item_spec; > + dst.raw =3D buf; > + size =3D offsetof(struct rte_flow_item_raw, pattern) + > + src.raw->length * sizeof(*src.raw->pattern); > + if (dst.raw) > + memcpy(dst.raw, src.raw, size); > break; > default: > - *size =3D flow_item[item->type].size; > + size =3D flow_item[item->type].size; > + if (buf) > + memcpy(buf, item_spec, size); > break; > } > empty: > - *pad =3D RTE_ALIGN_CEIL(*size, sizeof(double)) - *size; > + return RTE_ALIGN_CEIL(size, sizeof(double)); > } >=20 > /** Generate flow_action[] entry. */ > @@ -1036,31 +1057,72 @@ static const struct { > MK_FLOW_ACTION(METER, sizeof(struct > rte_flow_action_meter)), }; >=20 > -/** Compute storage space needed by action configuration. */ -static voi= d - > flow_action_conf_size(const struct rte_flow_action *action, > - size_t *size, size_t *pad) > +/** Compute storage space needed by action configuration and copy it. > +*/ static size_t flow_action_conf_copy(void *buf, const struct > +rte_flow_action *action) > { > - if (!action->conf) { > - *size =3D 0; > + size_t size =3D 0; > + > + if (!action->conf) > goto empty; > - } > switch (action->type) { > union { > const struct rte_flow_action_rss *rss; > - } conf; > + } src; > + union { > + struct rte_flow_action_rss *rss; > + } dst; > + size_t off; >=20 > case RTE_FLOW_ACTION_TYPE_RSS: > - conf.rss =3D action->conf; > - *size =3D offsetof(struct rte_flow_action_rss, queue) + > - conf.rss->num * sizeof(*conf.rss->queue); > + src.rss =3D action->conf; > + dst.rss =3D buf; > + off =3D 0; > + if (dst.rss) > + *dst.rss =3D (struct rte_flow_action_rss){ > + .num =3D src.rss->num, > + }; > + off +=3D offsetof(struct rte_flow_action_rss, queue); > + if (src.rss->num) { > + size =3D sizeof(*src.rss->queue) * src.rss->num; > + if (dst.rss) > + memcpy(dst.rss->queue, src.rss->queue, > size); > + off +=3D size; > + } > + off =3D RTE_ALIGN_CEIL(off, sizeof(double)); > + if (dst.rss) { > + dst.rss->rss_conf =3D (void *)((uintptr_t)dst.rss + off); > + *(struct rte_eth_rss_conf *)(uintptr_t) > + dst.rss->rss_conf =3D (struct > rte_eth_rss_conf){ > + .rss_key_len =3D src.rss->rss_conf- > >rss_key_len, > + .rss_hf =3D src.rss->rss_conf->rss_hf, > + }; > + } > + off +=3D sizeof(*src.rss->rss_conf); > + if (src.rss->rss_conf->rss_key_len) { > + off =3D RTE_ALIGN_CEIL(off, sizeof(double)); > + size =3D sizeof(*src.rss->rss_conf->rss_key) * > + src.rss->rss_conf->rss_key_len; > + if (dst.rss) { > + ((struct rte_eth_rss_conf *)(uintptr_t) > + dst.rss->rss_conf)->rss_key =3D > + (void *)((uintptr_t)dst.rss + off); > + memcpy(dst.rss->rss_conf->rss_key, > + src.rss->rss_conf->rss_key, > + size); > + } > + off +=3D size; > + } > + size =3D off; > break; > default: > - *size =3D flow_action[action->type].size; > + size =3D flow_action[action->type].size; > + if (buf) > + memcpy(buf, action->conf, size); > break; > } > empty: > - *pad =3D RTE_ALIGN_CEIL(*size, sizeof(double)) - *size; > + return RTE_ALIGN_CEIL(size, sizeof(double)); > } >=20 > /** Generate a port_flow entry from attributes/pattern/actions. */ @@ - > 1073,7 +1135,6 @@ port_flow_new(const struct rte_flow_attr *attr, > const struct rte_flow_action *action; > struct port_flow *pf =3D NULL; > size_t tmp; > - size_t pad; > size_t off1 =3D 0; > size_t off2 =3D 0; > int err =3D ENOTSUP; > @@ -1091,24 +1152,23 @@ port_flow_new(const struct rte_flow_attr *attr, > if (pf) > dst =3D memcpy(pf->data + off1, item, sizeof(*item)); > off1 +=3D sizeof(*item); > - flow_item_spec_size(item, &tmp, &pad); > if (item->spec) { > if (pf) > - dst->spec =3D memcpy(pf->data + off2, > - item->spec, tmp); > - off2 +=3D tmp + pad; > + dst->spec =3D pf->data + off2; > + off2 +=3D flow_item_spec_copy > + (pf ? pf->data + off2 : NULL, item, > ITEM_SPEC); > } > if (item->last) { > if (pf) > - dst->last =3D memcpy(pf->data + off2, > - item->last, tmp); > - off2 +=3D tmp + pad; > + dst->last =3D pf->data + off2; > + off2 +=3D flow_item_spec_copy > + (pf ? pf->data + off2 : NULL, item, > ITEM_LAST); > } > if (item->mask) { > if (pf) > - dst->mask =3D memcpy(pf->data + off2, > - item->mask, tmp); > - off2 +=3D tmp + pad; > + dst->mask =3D pf->data + off2; > + off2 +=3D flow_item_spec_copy > + (pf ? pf->data + off2 : NULL, item, > ITEM_MASK); > } > off2 =3D RTE_ALIGN_CEIL(off2, sizeof(double)); > } while ((item++)->type !=3D RTE_FLOW_ITEM_TYPE_END); @@ - > 1125,12 +1185,11 @@ port_flow_new(const struct rte_flow_attr *attr, > if (pf) > dst =3D memcpy(pf->data + off1, action, > sizeof(*action)); > off1 +=3D sizeof(*action); > - flow_action_conf_size(action, &tmp, &pad); > if (action->conf) { > if (pf) > - dst->conf =3D memcpy(pf->data + off2, > - action->conf, tmp); > - off2 +=3D tmp + pad; > + dst->conf =3D pf->data + off2; > + off2 +=3D flow_action_conf_copy > + (pf ? pf->data + off2 : NULL, action); > } > off2 =3D RTE_ALIGN_CEIL(off2, sizeof(double)); > } while ((action++)->type !=3D RTE_FLOW_ACTION_TYPE_END); > -- > 2.11.0