[dpdk-dev] [PATCH v4 3/3] examples/ipsec-secgw: add Egress flow actions

[Nelio Laranjeiro]

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 <nelio.laranjeiro at 6wind.com>
> > ---
> >   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+<RSS/QUEUE/PASSTHRU>+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