From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 2B9317D01 for ; Wed, 18 Apr 2018 14:26:15 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id E6AA1480088; Wed, 18 Apr 2018 12:26:12 +0000 (UTC) Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Wed, 18 Apr 2018 13:26:04 +0100 To: Adrien Mazarguil , Thomas Monjalon , Ferruh Yigit , CC: Ajit Khaparde , Wenzhuo Lu , John Daley , Gaetan Rivet , Beilei Xing , "Konstantin Ananyev" , Nelio Laranjeiro , Andrew Rybchenko , Pascal Mazon References: <20180410162022.9101-1-adrien.mazarguil@6wind.com> <20180416150525.2817-1-adrien.mazarguil@6wind.com> <20180416150525.2817-6-adrien.mazarguil@6wind.com> From: Andrew Rybchenko Message-ID: <1caed91d-c762-7e5c-af33-a6dd5f1ef319@solarflare.com> Date: Wed, 18 Apr 2018 15:26:00 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180416150525.2817-6-adrien.mazarguil@6wind.com> Content-Language: en-GB X-Originating-IP: [84.52.114.114] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23790.003 X-TM-AS-Result: No--13.228800-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1524054374-uKuuUkaKqFxJ Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v4 05/16] ethdev: alter behavior of flow API actions 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: Wed, 18 Apr 2018 12:26:15 -0000 On 04/16/2018 07:22 PM, Adrien Mazarguil wrote: > This patch makes the following changes to flow rule actions: > > - List order now matters, they are redefined as performed first to last > instead of "all simultaneously". > > - Repeated actions are now supported (e.g. specifying QUEUE multiple times > now duplicates traffic among them). Previously only the last action of > any given kind was taken into account. > > - No more distinction between terminating/non-terminating/meta actions. > Flow rules themselves are now defined as always terminating unless a > PASSTHRU action is specified. > > These changes alter the behavior of flow rules in corner cases in order to > prepare the flow API for actions that modify traffic contents or properties > (e.g. encapsulation, compression) and for which order matter when combined. > > Previously one would have to do so through multiple flow rules by combining > PASSTRHU with priority levels, however this proved overly complex to > implement at the PMD level, hence this simpler approach. > > This breaks ABI compatibility for the following public functions: > > - rte_flow_create() > - rte_flow_validate() > > PMDs with rte_flow support are modified accordingly: > > - bnxt: no change, implementation already forbids multiple actions and does > not support PASSTHRU. > > - e1000: no change, same as bnxt. > > - enic: modified to forbid redundant actions, no support for default drop. > > - failsafe: no change needed. > > - i40e: no change, implementation already forbids multiple actions. > > - ixgbe: same as i40e. > > - mlx4: modified to forbid multiple fate-deciding actions and drop when > unspecified. > > - mlx5: same as mlx4, with other redundant actions also forbidden. > > - sfc: same as mlx4. > > - tap: implementation already complies with the new behavior except for > the default pass-through modified as a default drop. > > Signed-off-by: Adrien Mazarguil > Reviewed-by: Andrew Rybchenko > Cc: Ajit Khaparde > Cc: Wenzhuo Lu > Cc: John Daley > Cc: Gaetan Rivet > Cc: Beilei Xing > Cc: Konstantin Ananyev > Cc: Nelio Laranjeiro > Cc: Andrew Rybchenko > Cc: Pascal Mazon > --- > doc/guides/prog_guide/rte_flow.rst | 67 +++++++++++++------------------- > drivers/net/enic/enic_flow.c | 25 ++++++++++++ > drivers/net/mlx4/mlx4_flow.c | 21 +++++++--- > drivers/net/mlx5/mlx5_flow.c | 69 ++++++++++++++------------------- > drivers/net/sfc/sfc_flow.c | 22 +++++++---- > drivers/net/tap/tap_flow.c | 11 ++++++ > lib/librte_ether/rte_flow.h | 54 +++++++------------------- > 7 files changed, 138 insertions(+), 131 deletions(-) [...] > diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c > index b9f36587c..a5c6a1670 100644 > --- a/drivers/net/enic/enic_flow.c > +++ b/drivers/net/enic/enic_flow.c > @@ -975,6 +979,10 @@ enic_copy_action_v1(const struct rte_flow_action actions[], > const struct rte_flow_action_queue *queue = > (const struct rte_flow_action_queue *) > actions->conf; > + > + if (overlap & FATE) > + return ENOTSUP; > + overlap |= FATE; > enic_action->rq_idx = > enic_rte_rq_idx_to_sop_idx(queue->index); > break; > @@ -984,6 +992,8 @@ enic_copy_action_v1(const struct rte_flow_action actions[], > break; > } > } > + if (!overlap & FATE) Build using clang on Ubuntu 17.10 fails: /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: fatal error: logical not is only applied to       the left hand side of this bitwise operator [-Wlogical-not-parentheses]         if (!overlap & FATE)             ^        ~ /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add parentheses after the '!' to       evaluate the bitwise operator first         if (!overlap & FATE)             ^              (             ) /var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add parentheses around left hand side       expression to silence this warning         if (!overlap & FATE)             ^             (       ) 1 error generated. /var/tmp/dpdk-next-net/mk/internal/rte.compile-pre.mk:114: recipe for target 'enic_flow.o' failed make[4]: *** [enic_flow.o] Error 1 /var/tmp/dpdk-next-net/mk/rte.subdir.mk:35: recipe for target 'enic' failed make[3]: *** [enic] Error 2   CC nfp_cpp_pcie_ops.o make[3]: *** Waiting for unfinished jobs.... $ clang --version clang version 4.0.1-6 (tags/RELEASE_401/final) Target: x86_64-pc-linux-gnu Thread model: posix InstalledDir: /usr/bin > + return ENOTSUP; > enic_action->type = FILTER_ACTION_RQ_STEERING; > return 0; > } > @@ -1001,6 +1011,9 @@ static int > enic_copy_action_v2(const struct rte_flow_action actions[], > struct filter_action_v2 *enic_action) > { > + enum { FATE = 1, MARK = 2, }; > + uint32_t overlap = 0; > + > FLOW_TRACE(); > > for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) { > @@ -1009,6 +1022,10 @@ enic_copy_action_v2(const struct rte_flow_action actions[], > const struct rte_flow_action_queue *queue = > (const struct rte_flow_action_queue *) > actions->conf; > + > + if (overlap & FATE) > + return ENOTSUP; > + overlap |= FATE; > enic_action->rq_idx = > enic_rte_rq_idx_to_sop_idx(queue->index); > enic_action->flags |= FILTER_ACTION_RQ_STEERING_FLAG; > @@ -1019,6 +1036,9 @@ enic_copy_action_v2(const struct rte_flow_action actions[], > (const struct rte_flow_action_mark *) > actions->conf; > > + if (overlap & MARK) Same > + return ENOTSUP; > + overlap |= MARK; > /* ENIC_MAGIC_FILTER_ID is reserved and is the highest > * in the range of allows mark ids. > */ > @@ -1029,6 +1049,9 @@ enic_copy_action_v2(const struct rte_flow_action actions[], > break; > } > case RTE_FLOW_ACTION_TYPE_FLAG: { > + if (overlap & MARK) > + return ENOTSUP; > + overlap |= MARK; > enic_action->filter_id = ENIC_MAGIC_FILTER_ID; > enic_action->flags |= FILTER_ACTION_FILTER_ID_FLAG; > break; > @@ -1044,6 +1067,8 @@ enic_copy_action_v2(const struct rte_flow_action actions[], > break; > } > } > + if (!overlap & FATE) Same > + return ENOTSUP; > enic_action->type = FILTER_ACTION_V2; > return 0; > } [...]