From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f181.google.com (mail-wr0-f181.google.com [209.85.128.181]) by dpdk.org (Postfix) with ESMTP id CFEA21F5 for ; Fri, 19 May 2017 10:14:16 +0200 (CEST) Received: by mail-wr0-f181.google.com with SMTP id z52so11773889wrc.2 for ; Fri, 19 May 2017 01:14:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=g7ij/JSLtqHlCJQ+kfYc7/bJYftPCCDH9rnv0RblIsQ=; b=mYzQu0104rjtk6VeHevf9Om+cI+u/e1RAeoc4rvT/saRb8bUWy5ffjs+ziJqvhbreH FscuMgtRedKcweFABAXEThSgFDKEg8mrch51svpSRnWuMAezlOn4usolTtVEcQih9Gqn bbS1pf8b83q+4RdvLELSjRg+gHTuXDZbSZcDEdHkM9dIeb4UQnTh5z1dQCZNTZvCs8a7 h+7NZYVLl4nxgs5Rla2SAjwrdmPJj2/7REtpVK/n8LcVDIgFduYErvBq6h/qry06yMFl FlHPO1tlktTRP/ms5hpiz5Oew2uyWEXEE01Fhp2VdOx4mvyTjK7HDX6MIhQ9EOQvkc26 +HXQ== 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:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=g7ij/JSLtqHlCJQ+kfYc7/bJYftPCCDH9rnv0RblIsQ=; b=TkH6ceHTbQdcMCGfkPkd2nmXImLTw6XS1qKu2Y3BhmQDC0p6oAgkvI5QWw8BVUTVul vakZIsqQ5tzeJOXcZlylFRfspkY0C+4UrCfRWALC+Vf1jVwzeVp4cvxbmShSEoWa76jx RlCdS+pJ322GJ2o0jFLjNhZEnSQFdTJ2DyFCwDTdDJmn2jWgLOKBax+YdgFP8PGRSCdI fZ/K4FxFDaw1GdBXwtXG6p063meK7br1EANajuBUZNB8HzaGpx6XAlzim0p2pxawMDgh oOf6NEx6/q5K3zuVmO4M+J38UERHBkkj4TciafAAbQJhSTOL8rsHRMpRH/MdzdiEFxo/ eKmA== X-Gm-Message-State: AODbwcBGZca0Jxv3jVBGwAy391wxV1JVMW57vA9MlEoRCn5AU7XJYSuR E65MyunvtECVNl0k3BQ= X-Received: by 10.223.149.33 with SMTP id 30mr2011985wrs.61.1495181656220; Fri, 19 May 2017 01:14:16 -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 v22sm903516wrd.38.2017.05.19.01.14.15 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 May 2017 01:14:15 -0700 (PDT) Date: Fri, 19 May 2017 10:14:09 +0200 From: Adrien Mazarguil To: dev@dpdk.org Message-ID: <20170519081409.GM1758@6wind.com> References: <4a269110b5623a0a72e4fe9d2636414de5921473.1492609343.git.adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4a269110b5623a0a72e4fe9d2636414de5921473.1492609343.git.adrien.mazarguil@6wind.com> Subject: Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API 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: Fri, 19 May 2017 08:14:17 -0000 Re-sending this due to the lack of feedback. Anyone? On Thu, Mar 30, 2017 at 11:08:16AM +0200, Adrien Mazarguil wrote: > Isolated mode can be requested by applications on individual ports to avoid > ingress traffic outside of the flow rules they define. > > Besides making ingress more deterministic, it allows PMDs to safely reuse > resources otherwise assigned to handle the remaining traffic, such as > global RSS configuration settings, VLAN filters, MAC address entries, > legacy filter API rules and so on in order to expand the set of possible > flow rule types. > > To minimize code complexity, PMDs implementing this mode may provide > partial (or even no) support for flow rules when not enabled (e.g. no > priorities, no RSS action). Applications written to use the flow API are > therefore encouraged to enable it. > > Once effective, leaving isolated mode may not be possible depending on PMD > implementation. > > Signed-off-by: Adrien Mazarguil > Cc: Ferruh Yigit > Cc: John McNamara > Cc: "O'Driscoll, Tim" > Cc: "Lu, Wenzhuo" > Cc: John Daley > Cc: Konstantin Ananyev > Cc: Helin Zhang > Cc: Nelio Laranjeiro > Cc: Andrew Rybchenko > Cc: Pascal Mazon > --- > app/test-pmd/cmdline.c | 4 ++ > app/test-pmd/cmdline_flow.c | 49 ++++++++++++++++++++- > app/test-pmd/config.c | 16 +++++++ > app/test-pmd/testpmd.h | 1 + > doc/guides/prog_guide/rte_flow.rst | 56 ++++++++++++++++++++++++ > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 47 +++++++++++++++++++- > drivers/net/ixgbe/ixgbe_flow.c | 9 ++-- > lib/librte_ether/rte_ether_version.map | 7 +++ > lib/librte_ether/rte_flow.c | 18 ++++++++ > lib/librte_ether/rte_flow.h | 33 ++++++++++++++ > lib/librte_ether/rte_flow_driver.h | 5 +++ > 11 files changed, 238 insertions(+), 7 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index e0d54d4..dbab647 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -892,6 +892,10 @@ static void cmd_help_long_parsed(void *parsed_result, > "flow list {port_id} [group {group_id}] [...]\n" > " List existing flow rules sorted by priority," > " filtered by group identifiers.\n\n" > + > + "flow isolate {port_id} {boolean}\n" > + " Restrict ingress traffic to the defined" > + " flow rules\n\n" > ); > } > } > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > index 4e99f0f..b783b81 100644 > --- a/app/test-pmd/cmdline_flow.c > +++ b/app/test-pmd/cmdline_flow.c > @@ -80,6 +80,7 @@ enum index { > FLUSH, > QUERY, > LIST, > + ISOLATE, > > /* Destroy arguments. */ > DESTROY_RULE, > @@ -361,6 +362,9 @@ struct buffer { > uint32_t *group; > uint32_t group_n; > } list; /**< List arguments. */ > + struct { > + int set; > + } isolate; /**< Isolated mode arguments. */ > } args; /**< Command arguments. */ > }; > > @@ -631,6 +635,9 @@ static int parse_action(struct context *, const struct token *, > static int parse_list(struct context *, const struct token *, > const char *, unsigned int, > void *, unsigned int); > +static int parse_isolate(struct context *, const struct token *, > + const char *, unsigned int, > + void *, unsigned int); > static int parse_int(struct context *, const struct token *, > const char *, unsigned int, > void *, unsigned int); > @@ -777,7 +784,8 @@ static const struct token token_list[] = { > DESTROY, > FLUSH, > LIST, > - QUERY)), > + QUERY, > + ISOLATE)), > .call = parse_init, > }, > /* Sub-level commands. */ > @@ -827,6 +835,15 @@ static const struct token token_list[] = { > .args = ARGS(ARGS_ENTRY(struct buffer, port)), > .call = parse_list, > }, > + [ISOLATE] = { > + .name = "isolate", > + .help = "restrict ingress traffic to the defined flow rules", > + .next = NEXT(NEXT_ENTRY(BOOLEAN), > + NEXT_ENTRY(PORT_ID)), > + .args = ARGS(ARGS_ENTRY(struct buffer, args.isolate.set), > + ARGS_ENTRY(struct buffer, port)), > + .call = parse_isolate, > + }, > /* Destroy arguments. */ > [DESTROY_RULE] = { > .name = "rule", > @@ -2039,6 +2056,33 @@ parse_list(struct context *ctx, const struct token *token, > return len; > } > > +/** Parse tokens for isolate command. */ > +static int > +parse_isolate(struct context *ctx, const struct token *token, > + const char *str, unsigned int len, > + void *buf, unsigned int size) > +{ > + struct buffer *out = buf; > + > + /* Token name must match. */ > + if (parse_default(ctx, token, str, len, NULL, 0) < 0) > + return -1; > + /* Nothing else to do if there is no buffer. */ > + if (!out) > + return len; > + if (!out->command) { > + if (ctx->curr != ISOLATE) > + return -1; > + if (sizeof(*out) > size) > + return -1; > + out->command = ctx->curr; > + ctx->objdata = 0; > + ctx->object = out; > + ctx->objmask = NULL; > + } > + return len; > +} > + > /** > * Parse signed/unsigned integers 8 to 64-bit long. > * > @@ -2735,6 +2779,9 @@ cmd_flow_parsed(const struct buffer *in) > port_flow_list(in->port, in->args.list.group_n, > in->args.list.group); > break; > + case ISOLATE: > + port_flow_isolate(in->port, in->args.isolate.set); > + break; > default: > break; > } > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > index 4d873cd..f07c238 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -1435,6 +1435,22 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n]) > } > } > > +/** Restrict ingress traffic to the defined flow rules. */ > +int > +port_flow_isolate(portid_t port_id, int set) > +{ > + struct rte_flow_error error; > + > + /* Poisoning to make sure PMDs update it in case of error. */ > + memset(&error, 0x66, sizeof(error)); > + if (rte_flow_isolate(port_id, set, &error)) > + return port_flow_complain(&error); > + printf("Ingress traffic on port %u is %s to the defined flow rules\n", > + port_id, > + set ? "now restricted" : "not restricted anymore"); > + return 0; > +} > + > /* > * RX/TX ring descriptors display functions. > */ > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index 642796e..369c4ed 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -531,6 +531,7 @@ int port_flow_flush(portid_t port_id); > int port_flow_query(portid_t port_id, uint32_t rule, > enum rte_flow_action_type action); > void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group); > +int port_flow_isolate(portid_t port_id, int set); > > void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id); > void tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id); > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > index 5ee045e..6441282 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -1485,6 +1485,62 @@ Return values: > > - 0 on success, a negative errno value otherwise and ``rte_errno`` is set. > > +Isolated mode > +------------- > + > +The general expectation for ingress traffic is that flow rules process it > +first; the remaining unmatched or pass-through traffic usually ends up in a > +queue (with or without RSS, locally or in some sub-device instance) > +depending on the global configuration settings of a port. > + > +While fine from a compatibility standpoint, this approach makes drivers more > +complex as they have to check for possible side effects outside of this API > +when creating or destroying flow rules. It results in a more limited set of > +available rule types due to the way device resources are assigned (e.g. no > +support for the RSS action even on capable hardware). > + > +Given that nonspecific traffic can be handled by flow rules as well, > +isolated mode is a means for applications to tell a driver that ingress on > +the underlying port must be injected from the defined flow rules only; that > +no default traffic is expected outside those rules. > + > +This has the following benefits: > + > +- Applications get finer-grained control over the kind of traffic they want > + to receive (no traffic by default). > + > +- More importantly they control at what point nonspecific traffic is handled > + relative to other flow rules, by adjusting priority levels. > + > +- Drivers can assign more hardware resources to flow rules and expand the > + set of supported rule types. > + > +Because toggling isolated mode may cause profound changes to the ingress > +processing path of a driver, it may not be possible to leave it once > +entered. Likewise, existing flow rules or global configuration settings may > +prevent a driver from entering isolated mode. > + > +Applications relying on this mode are therefore encouraged to toggle it as > +soon as possible after device initialization, ideally before the first call > +to ``rte_eth_dev_configure()`` to avoid possible failures due to conflicting > +settings. > + > +.. code-block:: c > + > + int > + rte_flow_isolate(uint8_t port_id, int set, struct rte_flow_error *error); > + > +Arguments: > + > +- ``port_id``: port identifier of Ethernet device. > +- ``set``: nonzero to enter isolated mode, attempt to leave it otherwise. > +- ``error``: perform verbose error reporting if not NULL. PMDs initialize > + this structure in case of error only. > + > +Return values: > + > +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set. > + > Verbose error reporting > ----------------------- > > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > index 6b8fc17..412dfa6 100644 > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst > @@ -2173,7 +2173,8 @@ Flow rules management > --------------------- > > Control of the generic flow API (*rte_flow*) is fully exposed through the > -``flow`` command (validation, creation, destruction and queries). > +``flow`` command (validation, creation, destruction, queries and operation > +modes). > > Considering *rte_flow* overlaps with all `Filter Functions`_, using both > features simultaneously may cause undefined side-effects and is therefore > @@ -2227,6 +2228,10 @@ following sections. > > flow list {port_id} [group {group_id}] [...] > > +- Restrict ingress traffic to the defined flow rules:: > + > + flow isolate {port_id} {boolean} > + > Validating flow rules > ~~~~~~~~~~~~~~~~~~~~~ > > @@ -2794,3 +2799,43 @@ Output can be limited to specific groups:: > 5 0 1000 i- ETH IPV6 ICMP => QUEUE > 7 63 0 i- ETH IPV6 UDP VXLAN => MARK QUEUE > testpmd> > + > +Toggling isolated mode > +~~~~~~~~~~~~~~~~~~~~~~ > + > +``flow isolate`` can be used to tell the underlying PMD that ingress traffic > +must only be injected from the defined flow rules; that no default traffic > +is expected outside those rules and the driver is free to assign more > +resources to handle them. It is bound to ``rte_flow_isolate()``:: > + > + flow isolate {port_id} {boolean} > + > +If successful, enabling or disabling isolated mode shows either:: > + > + Ingress traffic on port [...] > + is now restricted to the defined flow rules > + > +Or:: > + > + Ingress traffic on port [...] > + is not restricted anymore to the defined flow rules > + > +Otherwise, in case of error:: > + > + Caught error type [...] ([...]): [...] > + > +Mainly due to its side effects, PMDs supporting this mode may not have the > +ability to toggle it more than once without reinitializing affected ports > +first (e.g. by exiting testpmd). > + > +Enabling isolated mode:: > + > + testpmd> flow isolate 0 true > + Ingress traffic on port 0 is now restricted to the defined flow rules > + testpmd> > + > +Disabling isolated mode:: > + > + testpmd> flow isolate 0 false > + Ingress traffic on port 0 is not restricted anymore to the defined flow rules > + testpmd> > diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c > index e2ba9c2..faba219 100644 > --- a/drivers/net/ixgbe/ixgbe_flow.c > +++ b/drivers/net/ixgbe/ixgbe_flow.c > @@ -2787,9 +2787,8 @@ ixgbe_flow_flush(struct rte_eth_dev *dev, > } > > const struct rte_flow_ops ixgbe_flow_ops = { > - ixgbe_flow_validate, > - ixgbe_flow_create, > - ixgbe_flow_destroy, > - ixgbe_flow_flush, > - NULL, > + .validate = ixgbe_flow_validate, > + .create = ixgbe_flow_create, > + .destroy = ixgbe_flow_destroy, > + .flush = ixgbe_flow_flush, > }; > diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map > index 238c2a1..660e466 100644 > --- a/lib/librte_ether/rte_ether_version.map > +++ b/lib/librte_ether/rte_ether_version.map > @@ -153,3 +153,10 @@ DPDK_17.05 { > rte_eth_find_next; > > } DPDK_17.02; > + > +DPDK_17.08 { > + global: > + > + rte_flow_isolate; > + > +} DPDK_17.05; > diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c > index aaa70d6..c1de31b 100644 > --- a/lib/librte_ether/rte_flow.c > +++ b/lib/librte_ether/rte_flow.c > @@ -157,3 +157,21 @@ rte_flow_query(uint8_t port_id, > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > NULL, rte_strerror(ENOSYS)); > } > + > +/* Restrict ingress traffic to the defined flow rules. */ > +int > +rte_flow_isolate(uint8_t port_id, > + int set, > + struct rte_flow_error *error) > +{ > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); > + > + if (!ops) > + return -rte_errno; > + if (likely(!!ops->isolate)) > + return ops->isolate(dev, set, error); > + return -rte_flow_error_set(error, ENOSYS, > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > + NULL, rte_strerror(ENOSYS)); > +} > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index 8013eca..f90b997 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -1134,6 +1134,39 @@ rte_flow_query(uint8_t port_id, > void *data, > struct rte_flow_error *error); > > +/** > + * Restrict ingress traffic to the defined flow rules. > + * > + * Isolated mode guarantees that all ingress traffic comes from defined flow > + * rules only (current and future). > + * > + * Besides making ingress more deterministic, it allows PMDs to safely reuse > + * resources otherwise assigned to handle the remaining traffic, such as > + * global RSS configuration settings, VLAN filters, MAC address entries, > + * legacy filter API rules and so on in order to expand the set of possible > + * flow rule types. > + * > + * Calling this function as soon as possible after device initialization, > + * ideally before the first call to rte_eth_dev_configure(), is recommended > + * to avoid possible failures due to conflicting settings. > + * > + * Once effective, leaving isolated mode may not be possible depending on > + * PMD implementation. > + * > + * @param port_id > + * Port identifier of Ethernet device. > + * @param set > + * Nonzero to enter isolated mode, attempt to leave it otherwise. > + * @param[out] error > + * Perform verbose error reporting if not NULL. PMDs initialize this > + * structure in case of error only. > + * > + * @return > + * 0 on success, a negative errno value otherwise and rte_errno is set. > + */ > +int > +rte_flow_isolate(uint8_t port_id, int set, struct rte_flow_error *error); > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_ether/rte_flow_driver.h b/lib/librte_ether/rte_flow_driver.h > index da5749d..4d95391 100644 > --- a/lib/librte_ether/rte_flow_driver.h > +++ b/lib/librte_ether/rte_flow_driver.h > @@ -120,6 +120,11 @@ struct rte_flow_ops { > enum rte_flow_action_type, > void *, > struct rte_flow_error *); > + /** See rte_flow_isolate(). */ > + int (*isolate) > + (struct rte_eth_dev *, > + int, > + struct rte_flow_error *); > }; > > /** > -- > 2.1.4 -- Adrien Mazarguil 6WIND