From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-1.cisco.com (rcdn-iport-1.cisco.com [173.37.86.72]) by dpdk.org (Postfix) with ESMTP id 87A5D2901 for ; Fri, 19 May 2017 23:00:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=18620; q=dns/txt; s=iport; t=1495227637; x=1496437237; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=mpGDihoN8PBL4EnFr7dDDUSWo6zZbkL+ENPLylh/Etc=; b=O2o3fxbX3fWgdyPgQUczLPshTYI3UibreRoOUnXTO1oAQT8ggqhbYyrB C/AYPjVHRPQxnDEZdUWxS85T4QuXwE6XBgGg491g1hXb+lw3lmUNPDCtI cKIEysvqgMptuICSXtMx4k/a5ApZTv7zNX3MvrzlPZJjzVP5eU1XE/nz+ o=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0CYAAAiXB9Z/40NJK1cGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBg1WBbgeNf5FxlXaCD4YkAoV+PxgBAgEBAQEBAQFrKIUYAQEBAQM?= =?us-ascii?q?nE0sEAgEIEQQBAQEeCQcyFAkIAgQBEgiKHLJCOosZAQEBAQEBAQEBAQEBAQEBA?= =?us-ascii?q?QEBAQEBHYZfgV6DG4Q/hhYFiUaGYo1tAYpRiEGCDY9qiQGLRAEfOIEKcBVGhHc?= =?us-ascii?q?cgWN2hl8CJAeBA4ENAQEB?= X-IronPort-AV: E=Sophos;i="5.38,365,1491264000"; d="scan'208";a="251211875" Received: from alln-core-8.cisco.com ([173.36.13.141]) by rcdn-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 19 May 2017 21:00:35 +0000 Received: from XCH-ALN-008.cisco.com (xch-aln-008.cisco.com [173.36.7.18]) by alln-core-8.cisco.com (8.14.5/8.14.5) with ESMTP id v4JL0ZqY021772 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 19 May 2017 21:00:35 GMT Received: from xch-rcd-007.cisco.com (173.37.102.17) by XCH-ALN-008.cisco.com (173.36.7.18) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Fri, 19 May 2017 16:00:34 -0500 Received: from xch-rcd-007.cisco.com ([173.37.102.17]) by XCH-RCD-007.cisco.com ([173.37.102.17]) with mapi id 15.00.1210.000; Fri, 19 May 2017 16:00:34 -0500 From: "John Daley (johndale)" To: Adrien Mazarguil , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API Thread-Index: AQHSuRNNDmpDRDI/YUmx7l89qw7dI6H70XKAgACAR7A= Date: Fri, 19 May 2017 21:00:34 +0000 Message-ID: <6c69620a704443978dfe97f6ef41d5fa@XCH-RCD-007.cisco.com> References: <4a269110b5623a0a72e4fe9d2636414de5921473.1492609343.git.adrien.mazarguil@6wind.com> <20170519081409.GM1758@6wind.com> In-Reply-To: <20170519081409.GM1758@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.19.145.150] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] ethdev: add isolated mode to flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 May 2017 21:00:38 -0000 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 co= ntrast it to simply having the application add a very low priority "catch a= ll" 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 >=20 > Re-sending this due to the lack of feedback. Anyone? >=20 > On Thu, Mar 30, 2017 at 11:08:16AM +0200, Adrien Mazarguil wrote: > > Isolated mode can be requested by applications on individual ports to > > avoid ingress traffic outside of the flow rules they define. > > > > Besides making ingress more deterministic, it allows PMDs to safely > > reuse resources otherwise assigned to handle the remaining traffic, > > such as global RSS configuration settings, VLAN filters, MAC address > > entries, legacy filter API rules and so on in order to expand the set > > of possible flow rule types. > > > > To minimize code complexity, PMDs implementing this mode may provide > > partial (or even no) support for flow rules when not enabled (e.g. no > > priorities, no RSS action). Applications written to use the flow API > > are therefore encouraged to enable it. > > > > Once effective, leaving isolated mode may not be possible depending on > > PMD implementation. > > > > Signed-off-by: Adrien Mazarguil > > Cc: Ferruh Yigit > > Cc: John McNamara > > Cc: "O'Driscoll, Tim" > > Cc: "Lu, Wenzhuo" > > Cc: John Daley > > Cc: Konstantin Ananyev > > Cc: Helin Zhang > > Cc: Nelio Laranjeiro > > Cc: Andrew Rybchenko > > Cc: Pascal Mazon > > --- > > app/test-pmd/cmdline.c | 4 ++ > > app/test-pmd/cmdline_flow.c | 49 ++++++++++++++++++++- > > app/test-pmd/config.c | 16 +++++++ > > app/test-pmd/testpmd.h | 1 + > > doc/guides/prog_guide/rte_flow.rst | 56 > ++++++++++++++++++++++++ > > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 47 > +++++++++++++++++++- > > drivers/net/ixgbe/ixgbe_flow.c | 9 ++-- > > lib/librte_ether/rte_ether_version.map | 7 +++ > > lib/librte_ether/rte_flow.c | 18 ++++++++ > > lib/librte_ether/rte_flow.h | 33 ++++++++++++++ > > lib/librte_ether/rte_flow_driver.h | 5 +++ > > 11 files changed, 238 insertions(+), 7 deletions(-) > > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > > e0d54d4..dbab647 100644 > > --- a/app/test-pmd/cmdline.c > > +++ b/app/test-pmd/cmdline.c > > @@ -892,6 +892,10 @@ static void cmd_help_long_parsed(void > *parsed_result, > > "flow list {port_id} [group {group_id}] [...]\n" > > " List existing flow rules sorted by priority," > > " filtered by group identifiers.\n\n" > > + > > + "flow isolate {port_id} {boolean}\n" > > + " Restrict ingress traffic to the defined" > > + " flow rules\n\n" > > ); > > } > > } > > diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c > > index 4e99f0f..b783b81 100644 > > --- a/app/test-pmd/cmdline_flow.c > > +++ b/app/test-pmd/cmdline_flow.c > > @@ -80,6 +80,7 @@ enum index { > > FLUSH, > > QUERY, > > LIST, > > + ISOLATE, > > > > /* Destroy arguments. */ > > DESTROY_RULE, > > @@ -361,6 +362,9 @@ struct buffer { > > uint32_t *group; > > uint32_t group_n; > > } list; /**< List arguments. */ > > + struct { > > + int set; > > + } isolate; /**< Isolated mode arguments. */ > > } args; /**< Command arguments. */ > > }; > > > > @@ -631,6 +635,9 @@ static int parse_action(struct context *, const > > struct token *, static int parse_list(struct context *, const struct t= oken *, > > 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[] =3D { > > DESTROY, > > FLUSH, > > LIST, > > - QUERY)), > > + QUERY, > > + ISOLATE)), > > .call =3D parse_init, > > }, > > /* Sub-level commands. */ > > @@ -827,6 +835,15 @@ static const struct token token_list[] =3D { > > .args =3D ARGS(ARGS_ENTRY(struct buffer, port)), > > .call =3D parse_list, > > }, > > + [ISOLATE] =3D { > > + .name =3D "isolate", > > + .help =3D "restrict ingress traffic to the defined flow rules", > > + .next =3D NEXT(NEXT_ENTRY(BOOLEAN), > > + NEXT_ENTRY(PORT_ID)), > > + .args =3D ARGS(ARGS_ENTRY(struct buffer, args.isolate.set), > > + ARGS_ENTRY(struct buffer, port)), > > + .call =3D parse_isolate, > > + }, > > /* Destroy arguments. */ > > [DESTROY_RULE] =3D { > > .name =3D "rule", > > @@ -2039,6 +2056,33 @@ parse_list(struct context *ctx, const struct tok= en > *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 =3D 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 !=3D ISOLATE) > > + return -1; > > + if (sizeof(*out) > size) > > + return -1; > > + out->command =3D ctx->curr; > > + ctx->objdata =3D 0; > > + ctx->object =3D out; > > + ctx->objmask =3D 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 mod= e. > > + > > +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 otherwi= se. > > +- ``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 =3D> QUEUE > > 7 63 0 i- ETH IPV6 UDP VXLAN =3D> 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 =3D { > > - ixgbe_flow_validate, > > - ixgbe_flow_create, > > - ixgbe_flow_destroy, > > - ixgbe_flow_flush, > > - NULL, > > + .validate =3D ixgbe_flow_validate, > > + .create =3D ixgbe_flow_create, > > + .destroy =3D ixgbe_flow_destroy, > > + .flush =3D 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 =3D &rte_eth_devices[port_id]; > > + const struct rte_flow_ops *ops =3D 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 s= et. > > + */ > > +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 >=20 > -- > Adrien Mazarguil > 6WIND