From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 90CC0A034F; Sun, 3 Oct 2021 20:10:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 282CE41254; Sun, 3 Oct 2021 20:10:47 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id CA8C841251 for ; Sun, 3 Oct 2021 20:10:45 +0200 (CEST) Received: from [100.65.5.102] (unknown [5.144.121.112]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 4CD057F66F; Sun, 3 Oct 2021 21:10:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 4CD057F66F DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1633284645; bh=EOrpiHJl8VBnb1tow/1YNQqezKUfYAIeLikLrAq0EOs=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=nQQs3uaGx4DPM/OJ/DkC6V6Khs6xa3yMX4j4RXSaG2gQabMr6hRAZjjEfyadtr6Nc PveFa9sOHxcKTc/2FTn//KEbY28lmcS3VUIqniKcIYTE151VOeI9k9VkUiCqnJrZJa ykirZb+M3XbgfcBEIDEQnqivBLF1RU1Ws7unpANo= To: Ori Kam , Andrew Rybchenko , Xiaoyun Li , NBU-Contact-Thomas Monjalon , Ferruh Yigit Cc: "dev@dpdk.org" References: <20210907125157.3843-1-ivan.malov@oktetlabs.ru> <20211001134716.1608857-1-andrew.rybchenko@oktetlabs.ru> <20211001134716.1608857-3-andrew.rybchenko@oktetlabs.ru> From: Ivan Malov Message-ID: Date: Sun, 3 Oct 2021 21:10:33 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 02/12] ethdev: add eswitch port item to flow API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 03/10/2021 15:40, Ori Kam wrote: > Hi Andrew and Ivan, > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Friday, October 1, 2021 4:47 PM >> Subject: [PATCH v1 02/12] ethdev: add eswitch port item to flow API >> >> From: Ivan Malov >> >> For use with "transfer" flows. Supposed to match traffic entering the e-switch >> from the external world (network, guests) via the port which is logically >> connected with the given ethdev. >> >> Must not be combined with attributes "ingress" / "egress". >> >> This item is meant to use the same structure as ethdev item. >> > > In case the app is not working with representors, meaning > each switch port is mapped to ethdev. > both items (ethdev and eswitch port ) have the same meaning? No. Ethdev means ethdev, and e-switch port is the point where this ethdev is plugged to. For example, "transfer + ESWITCH_PORT" for a regular PF ethdev typically means the network port (maybe you can recall the idea that a PF ethdev "represents" the network port it's associated with). I believe, that diagrams which these patches add to "doc/guides/prog_guide/rte_flow.rst" may come in handy to understand the meaning. Also, you can take a look at our larger diagram from the Sep 14 gathering. > >> Signed-off-by: Ivan Malov >> Signed-off-by: Andrew Rybchenko >> --- >> app/test-pmd/cmdline_flow.c | 27 +++++++++++++++++++++ >> doc/guides/prog_guide/rte_flow.rst | 22 +++++++++++++++++ >> doc/guides/rel_notes/release_21_11.rst | 2 +- >> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 4 +++ >> lib/ethdev/rte_flow.c | 1 + >> lib/ethdev/rte_flow.h | 12 ++++++++- >> 6 files changed, 66 insertions(+), 2 deletions(-) >> >> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c >> index e05b0d83d2..188d0ee39d 100644 >> --- a/app/test-pmd/cmdline_flow.c >> +++ b/app/test-pmd/cmdline_flow.c >> @@ -308,6 +308,8 @@ enum index { >> ITEM_POL_POLICY, >> ITEM_ETHDEV, >> ITEM_ETHDEV_ID, >> + ITEM_ESWITCH_PORT, >> + ITEM_ESWITCH_PORT_ETHDEV_ID, > > Like my comment from previous patch, I'm not sure the correct > term for ETHDEV is ID is should be port. Please see my reply in the previous thread. "ethdev" here is an "anchor", a "beacon" of sorts which allows either to refer namely to this ethdev or to the e-switch port associated with it. > >> >> /* Validate/create actions. */ >> ACTIONS, >> @@ -1003,6 +1005,7 @@ static const enum index next_item[] = { >> ITEM_INTEGRITY, >> ITEM_CONNTRACK, >> ITEM_ETHDEV, >> + ITEM_ESWITCH_PORT, >> END_SET, >> ZERO, >> }; >> @@ -1377,6 +1380,12 @@ static const enum index item_ethdev[] = { >> ZERO, >> }; >> >> +static const enum index item_eswitch_port[] = { >> + ITEM_ESWITCH_PORT_ETHDEV_ID, >> + ITEM_NEXT, >> + ZERO, >> +}; >> + >> static const enum index next_action[] = { >> ACTION_END, >> ACTION_VOID, >> @@ -3632,6 +3641,21 @@ static const struct token token_list[] = { >> item_param), >> .args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev, id)), >> }, >> + [ITEM_ESWITCH_PORT] = { >> + .name = "eswitch_port", >> + .help = "match traffic at e-switch going from the external port >> associated with the given ethdev", > > Missing the word logically since if we are talking about representor the connected port > is the PF while we want to match traffic on one of the FVs. Doesn't the word "external" say it all? Representor Ethdev <--> Admin ethdev's PF <--> E-Switch <--> VF (external / the most remote endpoint). > >> + .priv = PRIV_ITEM(ESWITCH_PORT, >> + sizeof(struct rte_flow_item_ethdev)), >> + .next = NEXT(item_eswitch_port), >> + .call = parse_vc, >> + }, >> + [ITEM_ESWITCH_PORT_ETHDEV_ID] = { >> + .name = "ethdev_id", >> + .help = "ethdev ID", >> + .next = NEXT(item_eswitch_port, >> NEXT_ENTRY(COMMON_UNSIGNED), >> + item_param), >> + .args = ARGS(ARGS_ENTRY(struct rte_flow_item_ethdev, id)), >> + }, >> /* Validate/create actions. */ >> [ACTIONS] = { >> .name = "actions", >> @@ -8370,6 +8394,9 @@ flow_item_default_mask(const struct >> rte_flow_item *item) >> case RTE_FLOW_ITEM_TYPE_ETHDEV: >> mask = &rte_flow_item_ethdev_mask; >> break; >> + case RTE_FLOW_ITEM_TYPE_ESWITCH_PORT: >> + mask = &rte_flow_item_ethdev_mask; >> + break; > > Not sure maybe merged the two cases? A noble idea. > >> default: >> break; >> } >> diff --git a/doc/guides/prog_guide/rte_flow.rst >> b/doc/guides/prog_guide/rte_flow.rst >> index ab628d9139..292bb42410 100644 >> --- a/doc/guides/prog_guide/rte_flow.rst >> +++ b/doc/guides/prog_guide/rte_flow.rst >> @@ -1460,6 +1460,28 @@ Use this with attribute **transfer**. Attributes >> **ingress** and >> | ``mask`` | ``id`` | zeroed for wildcard match | >> +----------+----------+---------------------------+ >> >> +Item: ``ESWITCH_PORT`` >> +^^^^^^^^^^^^^^^^^^^^^^ >> + >> +Matches traffic at e-switch going from the external port associated >> +with the given ethdev, for example, traffic from net. port or guest. > > Maybe replace external with e-switch? The word "e-switch" is already here. Basically, all "external" ports are "e-switch external ports" = ports which connect the e-switch with non-DPDK world: network ports, VFs passed to guests, etc. > >> + >> +:: >> + >> + * (Ethdev) ~~~~~~~~~~~~ (Internal Port) ~~~~ [] <<<< (External Port) >> + * : SW : Logical Net / Guest : >> + * : : : >> + * | ---- PMD Layer ---- | ------------ E-Switch Layer ------------ | >> + * >> + * [] shows the effective ("transfer") standpoint, the match engine; >> + * << shows the traffic flow in question hitting the match engine; >> + * ~~ shows logical interconnects between the endpoints. >> + > > I'm not sure I understand this diagram. Thanks for the feedback. I have no way with drawing fancy diagrams, so this one may not be ideal, yes. But could you please elaborate on your concerns. Maybe you understand it the right way but just unsure. If you describe what you see when looking at the diagram, I'll be able to either confirm your vision or debunk any misunderstanding and possibly improve the drawing. > >> +Use this with attribute **transfer**. Attributes **ingress** and >> +**egress** (`Attribute: Traffic direction`_) must not be used. >> + >> +This item is meant to use the same structure as `Item: ETHDEV`_. >> + >> Actions >> ~~~~~~~ >> >> diff --git a/doc/guides/rel_notes/release_21_11.rst >> b/doc/guides/rel_notes/release_21_11.rst >> index 91631adb4e..b2b27de3f0 100644 >> --- a/doc/guides/rel_notes/release_21_11.rst >> +++ b/doc/guides/rel_notes/release_21_11.rst >> @@ -167,7 +167,7 @@ API Changes >> Also, make sure to start the actual text at the margin. >> ======================================================= >> >> -* ethdev: Added item ``ETHDEV`` to flow API. >> +* ethdev: Added items ``ETHDEV``, ``ESWITCH_PORT`` to flow API. >> >> * cryptodev: The API rte_cryptodev_pmd_is_valid_dev is modified to >> rte_cryptodev_is_valid_dev as it can be used by the application as diff --git >> a/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> index 6d5de5457c..9a5c2a2d82 100644 >> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst >> @@ -3824,6 +3824,10 @@ This section lists supported pattern items and >> their attributes, if any. >> >> - ``id {unsigned}``: ethdev ID >> >> +- ``eswitch_port``: match traffic at e-switch going from the external >> +port associated with the given ethdev >> + >> + - ``ethdev_id {unsigned}``: ethdev ID >> + >> Actions list >> ^^^^^^^^^^^^ >> >> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index >> 84eb61cb6c..c4aea5625f 100644 >> --- a/lib/ethdev/rte_flow.c >> +++ b/lib/ethdev/rte_flow.c >> @@ -101,6 +101,7 @@ static const struct rte_flow_desc_data >> rte_flow_desc_item[] = { >> MK_FLOW_ITEM(INTEGRITY, sizeof(struct rte_flow_item_integrity)), >> MK_FLOW_ITEM(CONNTRACK, sizeof(uint32_t)), >> MK_FLOW_ITEM(ETHDEV, sizeof(struct rte_flow_item_ethdev)), >> + MK_FLOW_ITEM(ESWITCH_PORT, sizeof(struct >> rte_flow_item_ethdev)), >> }; >> >> /** Generate flow_action[] entry. */ >> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index >> 880502098e..1a7e4c2e3d 100644 >> --- a/lib/ethdev/rte_flow.h >> +++ b/lib/ethdev/rte_flow.h >> @@ -583,6 +583,16 @@ enum rte_flow_item_type { >> * @see struct rte_flow_item_ethdev >> */ >> RTE_FLOW_ITEM_TYPE_ETHDEV, >> + >> + /** >> + * [META] >> + * >> + * Matches traffic at e-switch going from the external port associated >> + * with the given ethdev, for example, traffic from net. port or guest. >> + * >> + * @see struct rte_flow_item_ethdev >> + */ >> + RTE_FLOW_ITEM_TYPE_ESWITCH_PORT, >> }; >> >> /** >> @@ -1813,7 +1823,7 @@ static const struct rte_flow_item_conntrack >> rte_flow_item_conntrack_mask = { >> * @b EXPERIMENTAL: this structure may change without prior notice >> * >> * Provides an ethdev ID for use with items which are as follows: >> - * RTE_FLOW_ITEM_TYPE_ETHDEV. >> + * RTE_FLOW_ITEM_TYPE_ETHDEV, >> RTE_FLOW_ITEM_TYPE_ESWITCH_PORT. >> */ >> struct rte_flow_item_ethdev { >> uint16_t id; /**< Ethdev ID */ >> -- >> 2.30.2 > > Best, > Ori > -- Ivan M