[dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
Adrien Mazarguil
adrien.mazarguil at 6wind.com
Fri May 19 10:14:09 CEST 2017
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 <adrien.mazarguil at 6wind.com>
> Cc: Ferruh Yigit <ferruh.yigit at intel.com>
> Cc: John McNamara <john.mcnamara at intel.com>
> Cc: "O'Driscoll, Tim" <tim.odriscoll at intel.com>
> Cc: "Lu, Wenzhuo" <wenzhuo.lu at intel.com>
> Cc: John Daley <johndale at cisco.com>
> Cc: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Cc: Helin Zhang <helin.zhang at intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> Cc: Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: Pascal Mazon <pascal.mazon at 6wind.com>
> ---
> 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
More information about the dev
mailing list