[dpdk-dev] [PATCH] ethdev: add isolated mode to flow API

Adrien Mazarguil adrien.mazarguil at 6wind.com
Fri May 19 10:14:09 CEST 2017


Re-sending this due to the lack of feedback. Anyone?

On Thu, Mar 30, 2017 at 11:08:16AM +0200, Adrien Mazarguil wrote:
> Isolated mode can be requested by applications on individual ports to avoid
> ingress traffic outside of the flow rules they define.
> 
> Besides making ingress more deterministic, it allows PMDs to safely reuse
> resources otherwise assigned to handle the remaining traffic, such as
> global RSS configuration settings, VLAN filters, MAC address entries,
> legacy filter API rules and so on in order to expand the set of possible
> flow rule types.
> 
> To minimize code complexity, PMDs implementing this mode may provide
> partial (or even no) support for flow rules when not enabled (e.g. no
> priorities, no RSS action). Applications written to use the flow API are
> therefore encouraged to enable it.
> 
> Once effective, leaving isolated mode may not be possible depending on PMD
> implementation.
> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil at 6wind.com>
> Cc: Ferruh Yigit <ferruh.yigit at intel.com>
> Cc: John McNamara <john.mcnamara at intel.com>
> Cc: "O'Driscoll, Tim" <tim.odriscoll at intel.com>
> Cc: "Lu, Wenzhuo" <wenzhuo.lu at intel.com>
> Cc: John Daley <johndale at cisco.com>
> Cc: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Cc: Helin Zhang <helin.zhang at intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> Cc: Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: Pascal Mazon <pascal.mazon at 6wind.com>
> ---
>  app/test-pmd/cmdline.c                      |  4 ++
>  app/test-pmd/cmdline_flow.c                 | 49 ++++++++++++++++++++-
>  app/test-pmd/config.c                       | 16 +++++++
>  app/test-pmd/testpmd.h                      |  1 +
>  doc/guides/prog_guide/rte_flow.rst          | 56 ++++++++++++++++++++++++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 47 +++++++++++++++++++-
>  drivers/net/ixgbe/ixgbe_flow.c              |  9 ++--
>  lib/librte_ether/rte_ether_version.map      |  7 +++
>  lib/librte_ether/rte_flow.c                 | 18 ++++++++
>  lib/librte_ether/rte_flow.h                 | 33 ++++++++++++++
>  lib/librte_ether/rte_flow_driver.h          |  5 +++
>  11 files changed, 238 insertions(+), 7 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index e0d54d4..dbab647 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -892,6 +892,10 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"flow list {port_id} [group {group_id}] [...]\n"
>  			"    List existing flow rules sorted by priority,"
>  			" filtered by group identifiers.\n\n"
> +
> +			"flow isolate {port_id} {boolean}\n"
> +			"    Restrict ingress traffic to the defined"
> +			" flow rules\n\n"
>  		);
>  	}
>  }
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 4e99f0f..b783b81 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -80,6 +80,7 @@ enum index {
>  	FLUSH,
>  	QUERY,
>  	LIST,
> +	ISOLATE,
>  
>  	/* Destroy arguments. */
>  	DESTROY_RULE,
> @@ -361,6 +362,9 @@ struct buffer {
>  			uint32_t *group;
>  			uint32_t group_n;
>  		} list; /**< List arguments. */
> +		struct {
> +			int set;
> +		} isolate; /**< Isolated mode arguments. */
>  	} args; /**< Command arguments. */
>  };
>  
> @@ -631,6 +635,9 @@ static int parse_action(struct context *, const struct token *,
>  static int parse_list(struct context *, const struct token *,
>  		      const char *, unsigned int,
>  		      void *, unsigned int);
> +static int parse_isolate(struct context *, const struct token *,
> +			 const char *, unsigned int,
> +			 void *, unsigned int);
>  static int parse_int(struct context *, const struct token *,
>  		     const char *, unsigned int,
>  		     void *, unsigned int);
> @@ -777,7 +784,8 @@ static const struct token token_list[] = {
>  			      DESTROY,
>  			      FLUSH,
>  			      LIST,
> -			      QUERY)),
> +			      QUERY,
> +			      ISOLATE)),
>  		.call = parse_init,
>  	},
>  	/* Sub-level commands. */
> @@ -827,6 +835,15 @@ static const struct token token_list[] = {
>  		.args = ARGS(ARGS_ENTRY(struct buffer, port)),
>  		.call = parse_list,
>  	},
> +	[ISOLATE] = {
> +		.name = "isolate",
> +		.help = "restrict ingress traffic to the defined flow rules",
> +		.next = NEXT(NEXT_ENTRY(BOOLEAN),
> +			     NEXT_ENTRY(PORT_ID)),
> +		.args = ARGS(ARGS_ENTRY(struct buffer, args.isolate.set),
> +			     ARGS_ENTRY(struct buffer, port)),
> +		.call = parse_isolate,
> +	},
>  	/* Destroy arguments. */
>  	[DESTROY_RULE] = {
>  		.name = "rule",
> @@ -2039,6 +2056,33 @@ parse_list(struct context *ctx, const struct token *token,
>  	return len;
>  }
>  
> +/** Parse tokens for isolate command. */
> +static int
> +parse_isolate(struct context *ctx, const struct token *token,
> +	      const char *str, unsigned int len,
> +	      void *buf, unsigned int size)
> +{
> +	struct buffer *out = buf;
> +
> +	/* Token name must match. */
> +	if (parse_default(ctx, token, str, len, NULL, 0) < 0)
> +		return -1;
> +	/* Nothing else to do if there is no buffer. */
> +	if (!out)
> +		return len;
> +	if (!out->command) {
> +		if (ctx->curr != ISOLATE)
> +			return -1;
> +		if (sizeof(*out) > size)
> +			return -1;
> +		out->command = ctx->curr;
> +		ctx->objdata = 0;
> +		ctx->object = out;
> +		ctx->objmask = NULL;
> +	}
> +	return len;
> +}
> +
>  /**
>   * Parse signed/unsigned integers 8 to 64-bit long.
>   *
> @@ -2735,6 +2779,9 @@ cmd_flow_parsed(const struct buffer *in)
>  		port_flow_list(in->port, in->args.list.group_n,
>  			       in->args.list.group);
>  		break;
> +	case ISOLATE:
> +		port_flow_isolate(in->port, in->args.isolate.set);
> +		break;
>  	default:
>  		break;
>  	}
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 4d873cd..f07c238 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1435,6 +1435,22 @@ port_flow_list(portid_t port_id, uint32_t n, const uint32_t group[n])
>  	}
>  }
>  
> +/** Restrict ingress traffic to the defined flow rules. */
> +int
> +port_flow_isolate(portid_t port_id, int set)
> +{
> +	struct rte_flow_error error;
> +
> +	/* Poisoning to make sure PMDs update it in case of error. */
> +	memset(&error, 0x66, sizeof(error));
> +	if (rte_flow_isolate(port_id, set, &error))
> +		return port_flow_complain(&error);
> +	printf("Ingress traffic on port %u is %s to the defined flow rules\n",
> +	       port_id,
> +	       set ? "now restricted" : "not restricted anymore");
> +	return 0;
> +}
> +
>  /*
>   * RX/TX ring descriptors display functions.
>   */
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 642796e..369c4ed 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -531,6 +531,7 @@ int port_flow_flush(portid_t port_id);
>  int port_flow_query(portid_t port_id, uint32_t rule,
>  		    enum rte_flow_action_type action);
>  void port_flow_list(portid_t port_id, uint32_t n, const uint32_t *group);
> +int port_flow_isolate(portid_t port_id, int set);
>  
>  void rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id);
>  void tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id);
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 5ee045e..6441282 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1485,6 +1485,62 @@ Return values:
>  
>  - 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
>  
> +Isolated mode
> +-------------
> +
> +The general expectation for ingress traffic is that flow rules process it
> +first; the remaining unmatched or pass-through traffic usually ends up in a
> +queue (with or without RSS, locally or in some sub-device instance)
> +depending on the global configuration settings of a port.
> +
> +While fine from a compatibility standpoint, this approach makes drivers more
> +complex as they have to check for possible side effects outside of this API
> +when creating or destroying flow rules. It results in a more limited set of
> +available rule types due to the way device resources are assigned (e.g. no
> +support for the RSS action even on capable hardware).
> +
> +Given that nonspecific traffic can be handled by flow rules as well,
> +isolated mode is a means for applications to tell a driver that ingress on
> +the underlying port must be injected from the defined flow rules only; that
> +no default traffic is expected outside those rules.
> +
> +This has the following benefits:
> +
> +- Applications get finer-grained control over the kind of traffic they want
> +  to receive (no traffic by default).
> +
> +- More importantly they control at what point nonspecific traffic is handled
> +  relative to other flow rules, by adjusting priority levels.
> +
> +- Drivers can assign more hardware resources to flow rules and expand the
> +  set of supported rule types.
> +
> +Because toggling isolated mode may cause profound changes to the ingress
> +processing path of a driver, it may not be possible to leave it once
> +entered. Likewise, existing flow rules or global configuration settings may
> +prevent a driver from entering isolated mode.
> +
> +Applications relying on this mode are therefore encouraged to toggle it as
> +soon as possible after device initialization, ideally before the first call
> +to ``rte_eth_dev_configure()`` to avoid possible failures due to conflicting
> +settings.
> +
> +.. code-block:: c
> +
> +   int
> +   rte_flow_isolate(uint8_t port_id, int set, struct rte_flow_error *error);
> +
> +Arguments:
> +
> +- ``port_id``: port identifier of Ethernet device.
> +- ``set``: nonzero to enter isolated mode, attempt to leave it otherwise.
> +- ``error``: perform verbose error reporting if not NULL. PMDs initialize
> +  this structure in case of error only.
> +
> +Return values:
> +
> +- 0 on success, a negative errno value otherwise and ``rte_errno`` is set.
> +
>  Verbose error reporting
>  -----------------------
>  
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 6b8fc17..412dfa6 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2173,7 +2173,8 @@ Flow rules management
>  ---------------------
>  
>  Control of the generic flow API (*rte_flow*) is fully exposed through the
> -``flow`` command (validation, creation, destruction and queries).
> +``flow`` command (validation, creation, destruction, queries and operation
> +modes).
>  
>  Considering *rte_flow* overlaps with all `Filter Functions`_, using both
>  features simultaneously may cause undefined side-effects and is therefore
> @@ -2227,6 +2228,10 @@ following sections.
>  
>     flow list {port_id} [group {group_id}] [...]
>  
> +- Restrict ingress traffic to the defined flow rules::
> +
> +   flow isolate {port_id} {boolean}
> +
>  Validating flow rules
>  ~~~~~~~~~~~~~~~~~~~~~
>  
> @@ -2794,3 +2799,43 @@ Output can be limited to specific groups::
>     5       0       1000    i-      ETH IPV6 ICMP => QUEUE
>     7       63      0       i-      ETH IPV6 UDP VXLAN => MARK QUEUE
>     testpmd>
> +
> +Toggling isolated mode
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +``flow isolate`` can be used to tell the underlying PMD that ingress traffic
> +must only be injected from the defined flow rules; that no default traffic
> +is expected outside those rules and the driver is free to assign more
> +resources to handle them. It is bound to ``rte_flow_isolate()``::
> +
> + flow isolate {port_id} {boolean}
> +
> +If successful, enabling or disabling isolated mode shows either::
> +
> + Ingress traffic on port [...]
> +    is now restricted to the defined flow rules
> +
> +Or::
> +
> + Ingress traffic on port [...]
> +    is not restricted anymore to the defined flow rules
> +
> +Otherwise, in case of error::
> +
> +   Caught error type [...] ([...]): [...]
> +
> +Mainly due to its side effects, PMDs supporting this mode may not have the
> +ability to toggle it more than once without reinitializing affected ports
> +first (e.g. by exiting testpmd).
> +
> +Enabling isolated mode::
> +
> + testpmd> flow isolate 0 true
> + Ingress traffic on port 0 is now restricted to the defined flow rules
> + testpmd>
> +
> +Disabling isolated mode::
> +
> + testpmd> flow isolate 0 false
> + Ingress traffic on port 0 is not restricted anymore to the defined flow rules
> + testpmd>
> diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
> index e2ba9c2..faba219 100644
> --- a/drivers/net/ixgbe/ixgbe_flow.c
> +++ b/drivers/net/ixgbe/ixgbe_flow.c
> @@ -2787,9 +2787,8 @@ ixgbe_flow_flush(struct rte_eth_dev *dev,
>  }
>  
>  const struct rte_flow_ops ixgbe_flow_ops = {
> -	ixgbe_flow_validate,
> -	ixgbe_flow_create,
> -	ixgbe_flow_destroy,
> -	ixgbe_flow_flush,
> -	NULL,
> +	.validate = ixgbe_flow_validate,
> +	.create = ixgbe_flow_create,
> +	.destroy = ixgbe_flow_destroy,
> +	.flush = ixgbe_flow_flush,
>  };
> diff --git a/lib/librte_ether/rte_ether_version.map b/lib/librte_ether/rte_ether_version.map
> index 238c2a1..660e466 100644
> --- a/lib/librte_ether/rte_ether_version.map
> +++ b/lib/librte_ether/rte_ether_version.map
> @@ -153,3 +153,10 @@ DPDK_17.05 {
>  	rte_eth_find_next;
>  
>  } DPDK_17.02;
> +
> +DPDK_17.08 {
> +	global:
> +
> +	rte_flow_isolate;
> +
> +} DPDK_17.05;
> diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c
> index aaa70d6..c1de31b 100644
> --- a/lib/librte_ether/rte_flow.c
> +++ b/lib/librte_ether/rte_flow.c
> @@ -157,3 +157,21 @@ rte_flow_query(uint8_t port_id,
>  				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>  				   NULL, rte_strerror(ENOSYS));
>  }
> +
> +/* Restrict ingress traffic to the defined flow rules. */
> +int
> +rte_flow_isolate(uint8_t port_id,
> +		 int set,
> +		 struct rte_flow_error *error)
> +{
> +	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
> +	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
> +
> +	if (!ops)
> +		return -rte_errno;
> +	if (likely(!!ops->isolate))
> +		return ops->isolate(dev, set, error);
> +	return -rte_flow_error_set(error, ENOSYS,
> +				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				   NULL, rte_strerror(ENOSYS));
> +}
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index 8013eca..f90b997 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -1134,6 +1134,39 @@ rte_flow_query(uint8_t port_id,
>  	       void *data,
>  	       struct rte_flow_error *error);
>  
> +/**
> + * Restrict ingress traffic to the defined flow rules.
> + *
> + * Isolated mode guarantees that all ingress traffic comes from defined flow
> + * rules only (current and future).
> + *
> + * Besides making ingress more deterministic, it allows PMDs to safely reuse
> + * resources otherwise assigned to handle the remaining traffic, such as
> + * global RSS configuration settings, VLAN filters, MAC address entries,
> + * legacy filter API rules and so on in order to expand the set of possible
> + * flow rule types.
> + *
> + * Calling this function as soon as possible after device initialization,
> + * ideally before the first call to rte_eth_dev_configure(), is recommended
> + * to avoid possible failures due to conflicting settings.
> + *
> + * Once effective, leaving isolated mode may not be possible depending on
> + * PMD implementation.
> + *
> + * @param port_id
> + *   Port identifier of Ethernet device.
> + * @param set
> + *   Nonzero to enter isolated mode, attempt to leave it otherwise.
> + * @param[out] error
> + *   Perform verbose error reporting if not NULL. PMDs initialize this
> + *   structure in case of error only.
> + *
> + * @return
> + *   0 on success, a negative errno value otherwise and rte_errno is set.
> + */
> +int
> +rte_flow_isolate(uint8_t port_id, int set, struct rte_flow_error *error);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/librte_ether/rte_flow_driver.h b/lib/librte_ether/rte_flow_driver.h
> index da5749d..4d95391 100644
> --- a/lib/librte_ether/rte_flow_driver.h
> +++ b/lib/librte_ether/rte_flow_driver.h
> @@ -120,6 +120,11 @@ struct rte_flow_ops {
>  		 enum rte_flow_action_type,
>  		 void *,
>  		 struct rte_flow_error *);
> +	/** See rte_flow_isolate(). */
> +	int (*isolate)
> +		(struct rte_eth_dev *,
> +		 int,
> +		 struct rte_flow_error *);
>  };
>  
>  /**
> -- 
> 2.1.4

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list