* [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
@ 2017-04-19 13:45 Adrien Mazarguil
2017-05-19 8:14 ` Adrien Mazarguil
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-04-19 13:45 UTC (permalink / raw)
To: Thomas Monjalon, dev
Cc: Ferruh Yigit, John McNamara, O'Driscoll, Tim, Lu, Wenzhuo,
John Daley, Konstantin Ananyev, Helin Zhang, Nelio Laranjeiro,
Andrew Rybchenko, Pascal Mazon
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@6wind.com>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: John McNamara <john.mcnamara@intel.com>
Cc: "O'Driscoll, Tim" <tim.odriscoll@intel.com>
Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Cc: John Daley <johndale@cisco.com>
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: Helin Zhang <helin.zhang@intel.com>
Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Cc: Pascal Mazon <pascal.mazon@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
2017-04-19 13:45 [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API Adrien Mazarguil
@ 2017-05-19 8:14 ` Adrien Mazarguil
2017-05-19 21:00 ` John Daley (johndale)
2017-05-24 13:53 ` Nélio Laranjeiro
2017-05-24 14:57 ` [dpdk-dev] [PATCH v2] " Adrien Mazarguil
2 siblings, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-05-19 8:14 UTC (permalink / raw)
To: dev
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@6wind.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: John McNamara <john.mcnamara@intel.com>
> Cc: "O'Driscoll, Tim" <tim.odriscoll@intel.com>
> Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
> Cc: John Daley <johndale@cisco.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Helin Zhang <helin.zhang@intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Pascal Mazon <pascal.mazon@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
2017-05-19 8:14 ` Adrien Mazarguil
@ 2017-05-19 21:00 ` John Daley (johndale)
2017-05-22 10:17 ` Adrien Mazarguil
0 siblings, 1 reply; 13+ messages in thread
From: John Daley (johndale) @ 2017-05-19 21:00 UTC (permalink / raw)
To: Adrien Mazarguil, dev
Hi Adrien,
I understand why applications may want to operate in this sort of isolated mode. Just to better understand the value of adding this feature can you contrast it to simply having the application add a very low priority "catch all" flow rule with DROP action?
Thanks,
Johnd
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> Sent: Friday, May 19, 2017 1:14 AM
> To: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
>
> 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@6wind.com>
> > Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> > Cc: John McNamara <john.mcnamara@intel.com>
> > Cc: "O'Driscoll, Tim" <tim.odriscoll@intel.com>
> > Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
> > Cc: John Daley <johndale@cisco.com>
> > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Cc: Helin Zhang <helin.zhang@intel.com>
> > Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> > Cc: Pascal Mazon <pascal.mazon@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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
2017-05-19 21:00 ` John Daley (johndale)
@ 2017-05-22 10:17 ` Adrien Mazarguil
0 siblings, 0 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-05-22 10:17 UTC (permalink / raw)
To: John Daley (johndale); +Cc: dev
Hi John,
On Fri, May 19, 2017 at 09:00:34PM +0000, John Daley (johndale) wrote:
> Hi Adrien,
>
> I understand why applications may want to operate in this sort of isolated mode. Just to better understand the value of adding this feature can you contrast it to simply having the application add a very low priority "catch all" flow rule with DROP action?
Actually the same can be expressed through a very low priority "catch all"
rule. The problem has more to do with code complexity on the PMD side to
achieve this behavior, and is one of the reasons isolated mode should be
requested before calling rte_eth_dev_configure() and may not be disabled
afterward depending on PMD implementation:
- Determining that a rule is really "catch all" and with the lowest priority
is not so easy. What happens if a rule with an even lower priority is
added afterward for instance.
- It might not be possible for PMDs to recycle MAC, VLAN filter, RSS and all
other allocated resources at run-time without prior notice. PMDs should be
guaranteed applications will not expect anything from these anymore.
- As a result, removing the "catch all" rule and restoring everything could
be tricky, possibly not even doable.
- Isolated mode can be implemented cleanly without maintaining compatibility
with other APIs (hence "isolated"), this should lead to simpler code.
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrien Mazarguil
> > Sent: Friday, May 19, 2017 1:14 AM
> > To: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
> >
> > 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@6wind.com>
> > > Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> > > Cc: John McNamara <john.mcnamara@intel.com>
> > > Cc: "O'Driscoll, Tim" <tim.odriscoll@intel.com>
> > > Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
> > > Cc: John Daley <johndale@cisco.com>
> > > Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > > Cc: Helin Zhang <helin.zhang@intel.com>
> > > Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> > > Cc: Pascal Mazon <pascal.mazon@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
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API
2017-04-19 13:45 [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API Adrien Mazarguil
2017-05-19 8:14 ` Adrien Mazarguil
@ 2017-05-24 13:53 ` Nélio Laranjeiro
2017-05-24 14:57 ` [dpdk-dev] [PATCH v2] " Adrien Mazarguil
2 siblings, 0 replies; 13+ messages in thread
From: Nélio Laranjeiro @ 2017-05-24 13:53 UTC (permalink / raw)
To: Adrien Mazarguil
Cc: Thomas Monjalon, dev, Ferruh Yigit, John McNamara,
O'Driscoll, Tim, Lu, Wenzhuo, John Daley, Konstantin Ananyev,
Helin Zhang, Andrew Rybchenko, Pascal Mazon
On Wed, Apr 19, 2017 at 03:45:31PM +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@6wind.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: John McNamara <john.mcnamara@intel.com>
> Cc: "O'Driscoll, Tim" <tim.odriscoll@intel.com>
> Cc: "Lu, Wenzhuo" <wenzhuo.lu@intel.com>
> Cc: John Daley <johndale@cisco.com>
> Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Cc: Helin Zhang <helin.zhang@intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
> Cc: Pascal Mazon <pascal.mazon@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
By the way I have implemented it for mlx5 driver [1], your patch needs
to be updated on master-net, is does not apply like this.
Thanks,
[1] http://dpdk.org/dev/patchwork/patch/24487/
--
Nélio Laranjeiro
6WIND
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v2] ethdev: add isolated mode to flow API
2017-04-19 13:45 [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API Adrien Mazarguil
2017-05-19 8:14 ` Adrien Mazarguil
2017-05-24 13:53 ` Nélio Laranjeiro
@ 2017-05-24 14:57 ` Adrien Mazarguil
2017-06-14 12:45 ` [dpdk-dev] [PATCH v3] " Adrien Mazarguil
2 siblings, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-05-24 14:57 UTC (permalink / raw)
To: dev
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@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@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 | 48 +++++++++++++++++++-
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(+), 8 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0afac68..b10eacb 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -904,6 +904,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 0fd69f9..0bc6d35 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,
@@ -365,6 +366,9 @@ struct buffer {
uint32_t *group;
uint32_t group_n;
} list; /**< List arguments. */
+ struct {
+ int set;
+ } isolate; /**< Isolated mode arguments. */
} args; /**< Command arguments. */
};
@@ -649,6 +653,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);
@@ -795,7 +802,8 @@ static const struct token token_list[] = {
DESTROY,
FLUSH,
LIST,
- QUERY)),
+ QUERY,
+ ISOLATE)),
.call = parse_init,
},
/* Sub-level commands. */
@@ -845,6 +853,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",
@@ -2087,6 +2104,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.
*
@@ -2786,6 +2830,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 e6c43ba..6919cbf 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -543,6 +543,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 b587ba9..699d2b2 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1517,6 +1517,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 0e50c10..2b9a1ea 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2264,7 +2264,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
@@ -2318,6 +2319,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
~~~~~~~~~~~~~~~~~~~~~
@@ -2894,6 +2899,46 @@ Output can be limited to specific groups::
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>
+
Sample QinQ flow rules
~~~~~~~~~~~~~~~~~~~~~~
@@ -2942,4 +2987,3 @@ Validate and create a QinQ rule on port 0 to steer traffic to a queue on the hos
ID Group Prio Attr Rule
0 0 0 i- ETH VLAN VLAN=>VF QUEUE
1 0 0 i- ETH VLAN VLAN=>PF QUEUE
-
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index 29e5ddc..fe7487e 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -2770,9 +2770,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 d6726bb..b279b1f 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -156,3 +156,10 @@ DPDK_17.05 {
rte_eth_xstats_get_names_by_id;
} 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 c47edbc..1302a25 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -1191,6 +1191,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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v3] ethdev: add isolated mode to flow API
2017-05-24 14:57 ` [dpdk-dev] [PATCH v2] " Adrien Mazarguil
@ 2017-06-14 12:45 ` Adrien Mazarguil
2017-06-14 13:01 ` Andrew Rybchenko
2017-06-14 14:48 ` [dpdk-dev] [PATCH v4] " Adrien Mazarguil
0 siblings, 2 replies; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 12:45 UTC (permalink / raw)
To: dev
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@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
v3:
- Rebased on next-net/master. Note this patch depends on
commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
necessary fix in igb's rte_flow_ops definition to avoid a compilation
issue.
v2:
- Rebased on master.
---
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 | 48 +++++++++++++++++++-
drivers/net/e1000/igb_flow.c | 9 ++--
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 +++
12 files changed, 242 insertions(+), 13 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9354270..b84c1ab 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -906,6 +906,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 3e5803a..a2a4a7e 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,
@@ -366,6 +367,9 @@ struct buffer {
uint32_t *group;
uint32_t group_n;
} list; /**< List arguments. */
+ struct {
+ int set;
+ } isolate; /**< Isolated mode arguments. */
} args; /**< Command arguments. */
};
@@ -651,6 +655,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);
@@ -797,7 +804,8 @@ static const struct token token_list[] = {
DESTROY,
FLUSH,
LIST,
- QUERY)),
+ QUERY,
+ ISOLATE)),
.call = parse_init,
},
/* Sub-level commands. */
@@ -847,6 +855,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",
@@ -2096,6 +2113,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.
*
@@ -2795,6 +2839,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 10b98b1..3cd4f31 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1411,6 +1411,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 2efa3cc..364502d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -543,6 +543,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 b587ba9..699d2b2 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1517,6 +1517,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 0e50c10..2b9a1ea 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2264,7 +2264,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
@@ -2318,6 +2319,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
~~~~~~~~~~~~~~~~~~~~~
@@ -2894,6 +2899,46 @@ Output can be limited to specific groups::
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>
+
Sample QinQ flow rules
~~~~~~~~~~~~~~~~~~~~~~
@@ -2942,4 +2987,3 @@ Validate and create a QinQ rule on port 0 to steer traffic to a queue on the hos
ID Group Prio Attr Rule
0 0 0 i- ETH VLAN VLAN=>VF QUEUE
1 0 0 i- ETH VLAN VLAN=>PF QUEUE
-
diff --git a/drivers/net/e1000/igb_flow.c b/drivers/net/e1000/igb_flow.c
index ce48c0d..8d59cd0 100644
--- a/drivers/net/e1000/igb_flow.c
+++ b/drivers/net/e1000/igb_flow.c
@@ -1673,9 +1673,8 @@ igb_flow_flush(struct rte_eth_dev *dev,
}
const struct rte_flow_ops igb_flow_ops = {
- igb_flow_validate,
- igb_flow_create,
- igb_flow_destroy,
- igb_flow_flush,
- NULL,
+ .validate = igb_flow_validate,
+ .create = igb_flow_create,
+ .destroy = igb_flow_destroy,
+ .flush = igb_flow_flush,
};
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index 1cb04f1..ccc73fa 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -2897,9 +2897,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 1d22434..805e6de 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -147,3 +147,10 @@ DPDK_17.05 {
rte_eth_xstats_get_names_by_id;
} 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 c47edbc..1302a25 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -1191,6 +1191,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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: add isolated mode to flow API
2017-06-14 12:45 ` [dpdk-dev] [PATCH v3] " Adrien Mazarguil
@ 2017-06-14 13:01 ` Andrew Rybchenko
2017-06-14 13:35 ` Adrien Mazarguil
2017-06-14 14:48 ` [dpdk-dev] [PATCH v4] " Adrien Mazarguil
1 sibling, 1 reply; 13+ messages in thread
From: Andrew Rybchenko @ 2017-06-14 13:01 UTC (permalink / raw)
To: Adrien Mazarguil, dev
On 06/14/2017 03:45 PM, 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@6wind.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>
> ---
>
> v3:
> - Rebased on next-net/master. Note this patch depends on
> commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
> necessary fix in igb's rte_flow_ops definition to avoid a compilation
> issue.
>
> v2:
> - Rebased on master.
> ---
> 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 | 48 +++++++++++++++++++-
> drivers/net/e1000/igb_flow.c | 9 ++--
> 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 +++
> 12 files changed, 242 insertions(+), 13 deletions(-)
<snip>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index b587ba9..699d2b2 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1517,6 +1517,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.
> +
I think it would be useful to highlight how isolated mode coexists with
promiscuous
and all-multicast. What is the expected behaviour of the functions which
toggle
promiscuous and all-multicast mode if isolated mode is enabled? These
functions
return void right now, so it is impossible to return error. What should
rte_eth_promiscuous_get() and rte_eth_allmulticast_get() return?
<snip>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: add isolated mode to flow API
2017-06-14 13:01 ` Andrew Rybchenko
@ 2017-06-14 13:35 ` Adrien Mazarguil
2017-06-14 14:04 ` Andrew Rybchenko
0 siblings, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 13:35 UTC (permalink / raw)
To: Andrew Rybchenko; +Cc: dev
On Wed, Jun 14, 2017 at 04:01:46PM +0300, Andrew Rybchenko wrote:
> On 06/14/2017 03:45 PM, 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@6wind.com>
> >Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> >
> >---
> >
> >v3:
> >- Rebased on next-net/master. Note this patch depends on
> > commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
> > necessary fix in igb's rte_flow_ops definition to avoid a compilation
> > issue.
> >
> >v2:
> >- Rebased on master.
> >---
> > 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 | 48 +++++++++++++++++++-
> > drivers/net/e1000/igb_flow.c | 9 ++--
> > 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 +++
> > 12 files changed, 242 insertions(+), 13 deletions(-)
>
> <snip>
>
> >diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> >index b587ba9..699d2b2 100644
> >--- a/doc/guides/prog_guide/rte_flow.rst
> >+++ b/doc/guides/prog_guide/rte_flow.rst
> >@@ -1517,6 +1517,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.
> >+
>
> I think it would be useful to highlight how isolated mode coexists with
> promiscuous
> and all-multicast. What is the expected behaviour of the functions which
> toggle
> promiscuous and all-multicast mode if isolated mode is enabled? These
> functions
> return void right now, so it is impossible to return error. What should
> rte_eth_promiscuous_get() and rte_eth_allmulticast_get() return?
They can technically return nothing/anything as long as they have no effect
on received traffic, as described.
Modifying existing wrappers that currently return void instead of an error
is outside the scope of this patch and requires ABI breakage. This can be
done later when the need arises.
For mlx4/mlx5, we plan to expose a different set of rte_eth_dev_ops
depending on whether isolated mode is toggled. When enabled, the
allmulti/promisc/MAC/VLAN/etc callbacks would be NULL for instance, and the
associated ethdev wrappers would automatically return an error where
applicable.
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ethdev: add isolated mode to flow API
2017-06-14 13:35 ` Adrien Mazarguil
@ 2017-06-14 14:04 ` Andrew Rybchenko
0 siblings, 0 replies; 13+ messages in thread
From: Andrew Rybchenko @ 2017-06-14 14:04 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev
On 06/14/2017 04:35 PM, Adrien Mazarguil wrote:
> On Wed, Jun 14, 2017 at 04:01:46PM +0300, Andrew Rybchenko wrote:
>> On 06/14/2017 03:45 PM, 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@6wind.com>
>>> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>
>>> ---
>>>
>>> v3:
>>> - Rebased on next-net/master. Note this patch depends on
>>> commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
>>> necessary fix in igb's rte_flow_ops definition to avoid a compilation
>>> issue.
>>>
>>> v2:
>>> - Rebased on master.
>>> ---
>>> 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 | 48 +++++++++++++++++++-
>>> drivers/net/e1000/igb_flow.c | 9 ++--
>>> 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 +++
>>> 12 files changed, 242 insertions(+), 13 deletions(-)
>> <snip>
>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
>>> index b587ba9..699d2b2 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -1517,6 +1517,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.
>>> +
>> I think it would be useful to highlight how isolated mode coexists with
>> promiscuous
>> and all-multicast. What is the expected behaviour of the functions which
>> toggle
>> promiscuous and all-multicast mode if isolated mode is enabled? These
>> functions
>> return void right now, so it is impossible to return error. What should
>> rte_eth_promiscuous_get() and rte_eth_allmulticast_get() return?
> They can technically return nothing/anything as long as they have no effect
> on received traffic, as described.
I was just asking to highlight it in the documentation. Yes, idea of the
isolated
mode is clear and may be it is enough.
> Modifying existing wrappers that currently return void instead of an error
> is outside the scope of this patch and requires ABI breakage. This can be
> done later when the need arises.
It is perfectly clear.
> For mlx4/mlx5, we plan to expose a different set of rte_eth_dev_ops
> depending on whether isolated mode is toggled. When enabled, the
> allmulti/promisc/MAC/VLAN/etc callbacks would be NULL for instance, and the
> associated ethdev wrappers would automatically return an error where
> applicable.
Thanks for the idea. We'll consider it as well.
Andrew.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [dpdk-dev] [PATCH v4] ethdev: add isolated mode to flow API
2017-06-14 12:45 ` [dpdk-dev] [PATCH v3] " Adrien Mazarguil
2017-06-14 13:01 ` Andrew Rybchenko
@ 2017-06-14 14:48 ` Adrien Mazarguil
2017-06-14 15:41 ` Andrew Rybchenko
1 sibling, 1 reply; 13+ messages in thread
From: Adrien Mazarguil @ 2017-06-14 14:48 UTC (permalink / raw)
To: dev; +Cc: Andrew Rybchenko
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@6wind.com>
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: Andrew Rybchenko <arybchenko@solarflare.com>
---
v4:
- Added detailed info about Rx functionality overridden by isolated mode,
such as promiscuous and allmulticast modes.
v3:
- Rebased on next-net/master. Note this patch depends on
commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
necessary fix in igb's rte_flow_ops definition to avoid a compilation
issue.
v2:
- Rebased on master.
---
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 | 67 ++++++++++++++++++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 48 ++++++++++++++++-
drivers/net/e1000/igb_flow.c | 9 ++--
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 | 44 ++++++++++++++++
lib/librte_ether/rte_flow_driver.h | 5 ++
12 files changed, 264 insertions(+), 13 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9354270..b84c1ab 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -906,6 +906,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 3e5803a..a2a4a7e 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,
@@ -366,6 +367,9 @@ struct buffer {
uint32_t *group;
uint32_t group_n;
} list; /**< List arguments. */
+ struct {
+ int set;
+ } isolate; /**< Isolated mode arguments. */
} args; /**< Command arguments. */
};
@@ -651,6 +655,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);
@@ -797,7 +804,8 @@ static const struct token token_list[] = {
DESTROY,
FLUSH,
LIST,
- QUERY)),
+ QUERY,
+ ISOLATE)),
.call = parse_init,
},
/* Sub-level commands. */
@@ -847,6 +855,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",
@@ -2096,6 +2113,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.
*
@@ -2795,6 +2839,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 10b98b1..3cd4f31 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1411,6 +1411,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 2efa3cc..364502d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -543,6 +543,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 b587ba9..6c26c37 100644
--- a/doc/guides/prog_guide/rte_flow.rst
+++ b/doc/guides/prog_guide/rte_flow.rst
@@ -1517,6 +1517,73 @@ 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.
+
+Once effective, the following functionality has no effect on the underlying
+port and may return errors such as ``ENOTSUP`` ("not supported"):
+
+- Toggling promiscuous mode.
+- Toggling allmulticast mode.
+- Configuring MAC addresses.
+- Configuring multicast addresses.
+- Configuring VLAN filters.
+- Configuring Rx filters through the legacy API (e.g. FDIR).
+- Configuring global RSS 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 0e50c10..2b9a1ea 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2264,7 +2264,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
@@ -2318,6 +2319,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
~~~~~~~~~~~~~~~~~~~~~
@@ -2894,6 +2899,46 @@ Output can be limited to specific groups::
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>
+
Sample QinQ flow rules
~~~~~~~~~~~~~~~~~~~~~~
@@ -2942,4 +2987,3 @@ Validate and create a QinQ rule on port 0 to steer traffic to a queue on the hos
ID Group Prio Attr Rule
0 0 0 i- ETH VLAN VLAN=>VF QUEUE
1 0 0 i- ETH VLAN VLAN=>PF QUEUE
-
diff --git a/drivers/net/e1000/igb_flow.c b/drivers/net/e1000/igb_flow.c
index ce48c0d..8d59cd0 100644
--- a/drivers/net/e1000/igb_flow.c
+++ b/drivers/net/e1000/igb_flow.c
@@ -1673,9 +1673,8 @@ igb_flow_flush(struct rte_eth_dev *dev,
}
const struct rte_flow_ops igb_flow_ops = {
- igb_flow_validate,
- igb_flow_create,
- igb_flow_destroy,
- igb_flow_flush,
- NULL,
+ .validate = igb_flow_validate,
+ .create = igb_flow_create,
+ .destroy = igb_flow_destroy,
+ .flush = igb_flow_flush,
};
diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
index 1cb04f1..ccc73fa 100644
--- a/drivers/net/ixgbe/ixgbe_flow.c
+++ b/drivers/net/ixgbe/ixgbe_flow.c
@@ -2897,9 +2897,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 1d22434..805e6de 100644
--- a/lib/librte_ether/rte_ether_version.map
+++ b/lib/librte_ether/rte_ether_version.map
@@ -147,3 +147,10 @@ DPDK_17.05 {
rte_eth_xstats_get_names_by_id;
} 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 c47edbc..34a5876 100644
--- a/lib/librte_ether/rte_flow.h
+++ b/lib/librte_ether/rte_flow.h
@@ -1191,6 +1191,50 @@ 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.
+ *
+ * Additionally, the following functionality has no effect on the underlying
+ * port and may return errors such as ENOTSUP ("not supported"):
+ *
+ * - Toggling promiscuous mode.
+ * - Toggling allmulticast mode.
+ * - Configuring MAC addresses.
+ * - Configuring multicast addresses.
+ * - Configuring VLAN filters.
+ * - Configuring Rx filters through the legacy API (e.g. FDIR).
+ * - Configuring global RSS settings.
+ *
+ * @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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: add isolated mode to flow API
2017-06-14 14:48 ` [dpdk-dev] [PATCH v4] " Adrien Mazarguil
@ 2017-06-14 15:41 ` Andrew Rybchenko
2017-06-14 21:33 ` Thomas Monjalon
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Rybchenko @ 2017-06-14 15:41 UTC (permalink / raw)
To: Adrien Mazarguil, dev
On 06/14/2017 05:48 PM, 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@6wind.com>
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>
> v4:
> - Added detailed info about Rx functionality overridden by isolated mode,
> such as promiscuous and allmulticast modes.
>
> v3:
> - Rebased on next-net/master. Note this patch depends on
> commit c0688ef1eded ("net/igb: parse flow API n-tuple filter") due to a
> necessary fix in igb's rte_flow_ops definition to avoid a compilation
> issue.
>
> v2:
> - Rebased on master.
> ---
> 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 | 67 ++++++++++++++++++++++++
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 48 ++++++++++++++++-
> drivers/net/e1000/igb_flow.c | 9 ++--
> 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 | 44 ++++++++++++++++
> lib/librte_ether/rte_flow_driver.h | 5 ++
> 12 files changed, 264 insertions(+), 13 deletions(-)
<snip>
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index b587ba9..6c26c37 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1517,6 +1517,73 @@ 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.
> +
> +Once effective, the following functionality has no effect on the underlying
> +port and may return errors such as ``ENOTSUP`` ("not supported"):
> +
> +- Toggling promiscuous mode.
> +- Toggling allmulticast mode.
> +- Configuring MAC addresses.
> +- Configuring multicast addresses.
> +- Configuring VLAN filters.
> +- Configuring Rx filters through the legacy API (e.g. FDIR).
> +- Configuring global RSS settings.
Many thanks. I think it is very useful since it is a check-list for PMD
maintainers.
<snip>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [dpdk-dev] [PATCH v4] ethdev: add isolated mode to flow API
2017-06-14 15:41 ` Andrew Rybchenko
@ 2017-06-14 21:33 ` Thomas Monjalon
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Monjalon @ 2017-06-14 21:33 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: dev, Andrew Rybchenko
14/06/2017 17:41, Andrew Rybchenko:
> On 06/14/2017 05:48 PM, 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@6wind.com>
> > Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>
>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
Applied, thanks
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-06-14 21:33 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-19 13:45 [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API Adrien Mazarguil
2017-05-19 8:14 ` Adrien Mazarguil
2017-05-19 21:00 ` John Daley (johndale)
2017-05-22 10:17 ` Adrien Mazarguil
2017-05-24 13:53 ` Nélio Laranjeiro
2017-05-24 14:57 ` [dpdk-dev] [PATCH v2] " Adrien Mazarguil
2017-06-14 12:45 ` [dpdk-dev] [PATCH v3] " Adrien Mazarguil
2017-06-14 13:01 ` Andrew Rybchenko
2017-06-14 13:35 ` Adrien Mazarguil
2017-06-14 14:04 ` Andrew Rybchenko
2017-06-14 14:48 ` [dpdk-dev] [PATCH v4] " Adrien Mazarguil
2017-06-14 15:41 ` Andrew Rybchenko
2017-06-14 21:33 ` Thomas Monjalon
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).