From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id 81C4B1BABD for ; Tue, 10 Apr 2018 18:34:29 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id l49so13460645wrl.4 for ; Tue, 10 Apr 2018 09:34:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=X0BYeewM6jVwwE9PLVlIuizB5p5KWkQCOOQ89bs/0qM=; b=xNHeR+XaaYPq+/TiLCG5UsJTJculKQJkZo+YpJZGl0Yz2lEojZH5p2gLhgOOqba7nW t6D0jRgl0cpLb8gn5KafVZVBKSuKT5x3xoD9VlcEoQUw7mKOo2q4WyiMk8okp7aN2nax WXufj5hca0Qd4ZGDGvo65ItlZVBQj77n/TsY9fRXS6PB5+IJSzSkHdcOJ2GDcWznmMoj 3MUvsUusSdHisYYIrryZMixLWWRXP87kGfICp7sTKn9dp5JVwAU1YlLAQkfo5c5GZ/0+ jPSFroc/hpQEXcMtXNEDTn+aUoWoM4di6p9AyFHJHi9Hx96KcVQoZODkMuWNbJ3Ayeob dr8Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=X0BYeewM6jVwwE9PLVlIuizB5p5KWkQCOOQ89bs/0qM=; b=rhrNZ2Cg229z2Q7MJESoAMHYtk16c2Blk31VAfVxuSMdgfg44patHoOQvlZKu8iamO BEDZU5uBeS/GTDq64wQ6B6mvblbUHe1npM8yPRLIUcUSYy47lIZ54BbTCkNZop6JaI+W l+/Iw/I9i5H+gSYTHRECygh0DnOrT0cEZaVN5PSRwbwTxB+TO6HqIRRPwm3+HU0/UpNj q8XjwajUQBViSMsMphzOEuIrWXdqiYeBTVlUcyQItaeDwKhdytaognWEPs9GYtwxGJME 3tBHcQXFa0gXut4oqlZWkdn/X6JV3vZfYMx9aP46zZvb8mmxEUwNWBQD5905999Yp0dE 7Rtg== X-Gm-Message-State: ALQs6tAWoUaZUnf3vmqw/kkBc6dm9GY4Bk1Ua/2PsQVtkLzGRBZm+bUg kNUxZ47nmg/pXCsnAy6xehab/8do X-Google-Smtp-Source: AIpwx4+1W+vMIkgxZs9WkNIkw4R6cO/p3s0YbIGsN61nM60yZY/L9E4lu091lNbANBGj0Cy2Bp1BPg== X-Received: by 10.223.134.103 with SMTP id 36mr909116wrw.23.1523378069165; Tue, 10 Apr 2018 09:34:29 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g38sm4840917wra.77.2018.04.10.09.34.28 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 10 Apr 2018 09:34:28 -0700 (PDT) Date: Tue, 10 Apr 2018 18:34:15 +0200 From: Adrien Mazarguil To: dev@dpdk.org Cc: stable@dpdk.org, Wenzhuo Lu , Jingjing Wu , Xueming Li Message-ID: <20180410161132.8776-8-adrien.mazarguil@6wind.com> References: <20180406131243.19037-1-adrien.mazarguil@6wind.com> <20180410161132.8776-1-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180410161132.8776-1-adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH v4 07/11] 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: Tue, 10 Apr 2018 16:34:30 -0000 Except for a list of queues, RSS configuration (hash key and fields) cannot be specified from the flow command line and testpmd does not provide safe defaults either. 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. This patch makes testpmd always provide default values. The list of RSS types to use is exclusively taken from the global "rss_hf" variable, itself configured through the "port config all rss" command or --rss-ip/--rss-udp command-line options. Fixes: 05d34c6e9d2c ("app/testpmd: add queue actions to flow command") Cc: stable@dpdk.org Signed-off-by: Adrien Mazarguil Acked-by: Nelio Laranjeiro Cc: Wenzhuo Lu Cc: Jingjing Wu Cc: Xueming Li --- v4 changes: Removed reliance on rte_eth_dev_rss_hash_conf_get(), which as reported by Xueming, is not necessarily supported and triggers a misleading "Function not implemented" warning. Updated commit log to reflect this. --- app/test-pmd/cmdline.c | 2 + app/test-pmd/cmdline_flow.c | 101 ++++++++++++++++++++++++---- app/test-pmd/config.c | 140 +++++++++++++++++++++++++++------------ 3 files changed, 190 insertions(+), 53 deletions(-) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 40b31ad7e..da784187d 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -1914,6 +1914,8 @@ cmd_config_rss_parsed(void *parsed_result, return; } rss_conf.rss_key = NULL; + /* Update global configuration for RSS types. */ + rss_hf = rss_conf.rss_hf; for (i = 0; i < rte_eth_dev_count(); i++) { diag = rte_eth_dev_rss_hash_update(i, &rss_conf); if (diag < 0) diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c index c2cf415ef..0e250d595 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) -/** 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; +}; /** Maximum number of subsequent tokens and arguments on the stack. */ #define CTX_STACK_SIZE 16 @@ -316,6 +322,13 @@ struct token { .size = (sz), \ }) +/** Static initializer for ARGS() with arbitrary offset and size. */ +#define ARGS_ENTRY_ARB(o, s) \ + (&(const struct arg){ \ + .offset = (o), \ + .size = (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 struct 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 token *, const char *, unsigned int, void *, unsigned int); @@ -1573,9 +1589,9 @@ static const struct token token_list[] = { [ACTION_RSS] = { .name = "rss", .help = "spread packets among several queues", - .priv = PRIV_ACTION(RSS, ACTION_RSS_SIZE), + .priv = PRIV_ACTION(RSS, sizeof(union action_rss_data)), .next = NEXT(action_rss), - .call = parse_vc, + .call = parse_vc_action_rss, }, [ACTION_RSS_QUEUES] = { .name = "queues", @@ -2004,6 +2020,61 @@ parse_vc_conf(struct context *ctx, const struct token *token, return len; } +/** 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 = buf; + struct rte_flow_action *action; + union action_rss_data *action_rss_data; + unsigned int i; + int ret; + + ret = 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 = &out->args.vc.actions[out->args.vc.actions_n - 1]; + /* Point to selected object. */ + ctx->object = out->args.vc.data; + ctx->objmask = NULL; + /* Set up default configuration. */ + action_rss_data = ctx->object; + *action_rss_data = (union action_rss_data){ + .conf = (struct rte_flow_action_rss){ + .rss_conf = &action_rss_data->s.rss_conf, + .num = RTE_MIN(nb_rxq, ACTION_RSS_QUEUE_NUM), + }, + }; + action_rss_data->s.rss_conf = (struct rte_eth_rss_conf){ + .rss_key = action_rss_data->s.rss_key, + .rss_key_len = sizeof(action_rss_data->s.rss_key), + .rss_hf = 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 = 0; i < action_rss_data->conf.num; ++i) + action_rss_data->conf.queue[i] = i; + if (!port_id_is_invalid(ctx->port, DISABLED_WARN) && + ctx->port != (portid_t)RTE_PORT_ALL) { + struct rte_eth_dev_info info; + + rte_eth_dev_info_get(ctx->port, &info); + action_rss_data->s.rss_conf.rss_key_len = + RTE_MIN(sizeof(action_rss_data->s.rss_key), + info.hash_key_size); + } + action->conf = &action_rss_data->conf; + return ret; +} + /** * Parse queue field for RSS action. * @@ -2015,6 +2086,7 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token, void *buf, unsigned int size) { static const enum index next[] = NEXT_ENTRY(ACTION_RSS_QUEUE); + union action_rss_data *action_rss_data; int ret; int i; @@ -2028,9 +2100,13 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token, ctx->objdata &= 0xffff; return len; } - if (i >= ACTION_RSS_NUM) + if (i >= 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 = parse_int(ctx, token, str, len, NULL, 0); if (ret < 0) { @@ -2045,7 +2121,8 @@ parse_vc_action_rss_queue(struct context *ctx, const struct token *token, ctx->next[ctx->next_num++] = next; if (!ctx->object) return len; - ((struct rte_flow_action_rss *)ctx->object)->num = i; + action_rss_data = ctx->object; + action_rss_data->conf.num = i; return len; } diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index 4bb255c62..aab5db355 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -982,31 +982,51 @@ static const struct { MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)), }; -/** 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 = 0; + size_t size = 0; + const void *item_spec = + type == ITEM_SPEC ? item->spec : + type == ITEM_LAST ? item->last : + type == 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; case RTE_FLOW_ITEM_TYPE_RAW: - spec.raw = item->spec; - *size = offsetof(struct rte_flow_item_raw, pattern) + - spec.raw->length * sizeof(*spec.raw->pattern); + src.raw = item_spec; + dst.raw = buf; + size = 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 = flow_item[item->type].size; + size = flow_item[item->type].size; + if (buf) + memcpy(buf, item_spec, size); break; } empty: - *pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size; + return RTE_ALIGN_CEIL(size, sizeof(double)); } /** Generate flow_action[] entry. */ @@ -1036,31 +1056,72 @@ static const struct { MK_FLOW_ACTION(METER, sizeof(struct rte_flow_action_meter)), }; -/** Compute storage space needed by action configuration. */ -static void -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 = 0; + size_t size = 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; case RTE_FLOW_ACTION_TYPE_RSS: - conf.rss = action->conf; - *size = offsetof(struct rte_flow_action_rss, queue) + - conf.rss->num * sizeof(*conf.rss->queue); + src.rss = action->conf; + dst.rss = buf; + off = 0; + if (dst.rss) + *dst.rss = (struct rte_flow_action_rss){ + .num = src.rss->num, + }; + off += offsetof(struct rte_flow_action_rss, queue); + if (src.rss->num) { + size = sizeof(*src.rss->queue) * src.rss->num; + if (dst.rss) + memcpy(dst.rss->queue, src.rss->queue, size); + off += size; + } + off = RTE_ALIGN_CEIL(off, sizeof(double)); + if (dst.rss) { + dst.rss->rss_conf = (void *)((uintptr_t)dst.rss + off); + *(struct rte_eth_rss_conf *)(uintptr_t) + dst.rss->rss_conf = (struct rte_eth_rss_conf){ + .rss_key_len = src.rss->rss_conf->rss_key_len, + .rss_hf = src.rss->rss_conf->rss_hf, + }; + } + off += sizeof(*src.rss->rss_conf); + if (src.rss->rss_conf->rss_key_len) { + off = RTE_ALIGN_CEIL(off, sizeof(double)); + size = 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 = + (void *)((uintptr_t)dst.rss + off); + memcpy(dst.rss->rss_conf->rss_key, + src.rss->rss_conf->rss_key, + size); + } + off += size; + } + size = off; break; default: - *size = flow_action[action->type].size; + size = flow_action[action->type].size; + if (buf) + memcpy(buf, action->conf, size); break; } empty: - *pad = RTE_ALIGN_CEIL(*size, sizeof(double)) - *size; + return RTE_ALIGN_CEIL(size, sizeof(double)); } /** Generate a port_flow entry from attributes/pattern/actions. */ @@ -1073,7 +1134,6 @@ port_flow_new(const struct rte_flow_attr *attr, const struct rte_flow_action *action; struct port_flow *pf = NULL; size_t tmp; - size_t pad; size_t off1 = 0; size_t off2 = 0; int err = ENOTSUP; @@ -1091,24 +1151,23 @@ port_flow_new(const struct rte_flow_attr *attr, if (pf) dst = memcpy(pf->data + off1, item, sizeof(*item)); off1 += sizeof(*item); - flow_item_spec_size(item, &tmp, &pad); if (item->spec) { if (pf) - dst->spec = memcpy(pf->data + off2, - item->spec, tmp); - off2 += tmp + pad; + dst->spec = pf->data + off2; + off2 += flow_item_spec_copy + (pf ? pf->data + off2 : NULL, item, ITEM_SPEC); } if (item->last) { if (pf) - dst->last = memcpy(pf->data + off2, - item->last, tmp); - off2 += tmp + pad; + dst->last = pf->data + off2; + off2 += flow_item_spec_copy + (pf ? pf->data + off2 : NULL, item, ITEM_LAST); } if (item->mask) { if (pf) - dst->mask = memcpy(pf->data + off2, - item->mask, tmp); - off2 += tmp + pad; + dst->mask = pf->data + off2; + off2 += flow_item_spec_copy + (pf ? pf->data + off2 : NULL, item, ITEM_MASK); } off2 = RTE_ALIGN_CEIL(off2, sizeof(double)); } while ((item++)->type != RTE_FLOW_ITEM_TYPE_END); @@ -1125,12 +1184,11 @@ port_flow_new(const struct rte_flow_attr *attr, if (pf) dst = memcpy(pf->data + off1, action, sizeof(*action)); off1 += sizeof(*action); - flow_action_conf_size(action, &tmp, &pad); if (action->conf) { if (pf) - dst->conf = memcpy(pf->data + off2, - action->conf, tmp); - off2 += tmp + pad; + dst->conf = pf->data + off2; + off2 += flow_action_conf_copy + (pf ? pf->data + off2 : NULL, action); } off2 = RTE_ALIGN_CEIL(off2, sizeof(double)); } while ((action++)->type != RTE_FLOW_ACTION_TYPE_END); -- 2.11.0