From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nelio Laranjeiro Subject: Re: [PATCH v4 3/3] examples/ipsec-secgw: add Egress flow actions Date: Fri, 15 Dec 2017 14:53:00 +0100 Message-ID: <20171215135300.zm6ubao24qqxstpl@laranjeiro-vm.dev.6wind.com> References: <5d3fdd0c05d5f8afd3f8e38ca03eaf25187d5c98.1513000931.git.nelio.laranjeiro@6wind.com> <89add3272024fefe644a9e636a476c85d39e398b.1513264386.git.nelio.laranjeiro@6wind.com> <78f97959-bf6c-33cc-e758-d232013ea159@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: Sergio Gonzalez Monroy , Radu Nicolau , dev@dpdk.org To: Anoob Joseph Return-path: Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by dpdk.org (Postfix) with ESMTP id E4060397D for ; Fri, 15 Dec 2017 14:52:24 +0100 (CET) Received: by mail-wm0-f41.google.com with SMTP id b199so17747921wme.1 for ; Fri, 15 Dec 2017 05:52:24 -0800 (PST) Content-Disposition: inline In-Reply-To: <78f97959-bf6c-33cc-e758-d232013ea159@caviumnetworks.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Anoob, On Fri, Dec 15, 2017 at 02:35:12PM +0530, Anoob Joseph wrote: > Hi Nelio, > > On 12/14/2017 08:44 PM, Nelio Laranjeiro wrote: > > Add Egress flow create for devices supporting > > RTE_SECURITY_TX_HW_TRAILER_OFFLOAD. > > > > Signed-off-by: Nelio Laranjeiro > > --- > > examples/ipsec-secgw/ipsec.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c > > index 8e8dc6df7..d49970ad8 100644 > > --- a/examples/ipsec-secgw/ipsec.c > > +++ b/examples/ipsec-secgw/ipsec.c > > @@ -201,6 +201,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > sa->action[0].type = RTE_FLOW_ACTION_TYPE_SECURITY; > > sa->action[0].conf = sa->sec_session; > > + sa->action[1].type = RTE_FLOW_ACTION_TYPE_END; > > sa->attr.egress = (sa->direction == > > RTE_SECURITY_IPSEC_SA_DIR_EGRESS); > > @@ -253,6 +254,13 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > &err); > > if (ret) > > goto flow_create_failure; > > + } else if (sa->attr.egress && > > + (sa->ol_flags & > > + RTE_SECURITY_TX_HW_TRAILER_OFFLOAD)) { > If this flag is not set, the following code won't be executed, but it would > still try to create the flow. Right, with actions Security + END as the original code. > And if the flow create fails in that case then create_session would fail. Do you mean the original code is also wrong? > I would suggest moving the flow_create also into the block (for > ingress and egress). Or may be initialize the flow with > actions END+END+END, and add SECURITY++END as it hits > various conditions. I'm not sure what the flow_create would do for such an > action. This would look ugly in any case. See if you get any better ideas! I think this comment is related to second patch where the "sa->action[1].type = RTE_FLOW_ACTION_TYPE_END;" is wrongly removed. Can you confirm before I send a new revision? > > + sa->action[1].type = > > + RTE_FLOW_ACTION_TYPE_PASSTHRU; > > + sa->action[2].type = > > + RTE_FLOW_ACTION_TYPE_END; > > } > > flow_create: > > sa->flow = rte_flow_create(sa->portid, > Thanks, -- Nélio Laranjeiro 6WIND