Re: [PATCH v4 10/12] net/ixgbe: enable inline ipsec

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Thursday, October 19, 2017 1:17 PM
> To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
> borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net; sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com;
> Mcnamara, John <john.mcnamara@intel.com>; shahafs@mellanox.com; olivier.matz@6wind.com
> Subject: Re: [dpdk-dev] [PATCH v4 10/12] net/ixgbe: enable inline ipsec
> 
> 
> 
> > -----Original Message-----
> > From: Nicolau, Radu
> > Sent: Thursday, October 19, 2017 12:57 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org
> > Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
> > borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net; sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com;
> > Mcnamara, John <john.mcnamara@intel.com>; shahafs@mellanox.com; olivier.matz@6wind.com
> > Subject: RE: [PATCH v4 10/12] net/ixgbe: enable inline ipsec
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Thursday, October 19, 2017 12:04 PM
> > > To: Nicolau, Radu <radu.nicolau@intel.com>; Akhil Goyal
> > > <akhil.goyal@nxp.com>; dev@dpdk.org
> > > Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
> > > borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net;
> > > sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com; Mcnamara,
> > > John <john.mcnamara@intel.com>; shahafs@mellanox.com;
> > > olivier.matz@6wind.com
> > > Subject: RE: [PATCH v4 10/12] net/ixgbe: enable inline ipsec
> > >
> > >
> > >
> > > > >
> > > > >> <snip>
> > > > >> +
> > > > >> +static int
> > > > >> +ixgbe_crypto_update_mb(void *device __rte_unused,
> > > > >> +		struct rte_security_session *session,
> > > > >> +		       struct rte_mbuf *m, void *params __rte_unused) {



Another sort of generic question - why not make security_set_pkt_metadata function
to accept  bulk of packets?
In that case o can minimize the cost of function calls, accessing session data, etc.
Though I suppose that could wait till next patch series.
Konstantin


> > > > >> +	struct ixgbe_crypto_session *ic_session =
> > > > >> +			get_sec_session_private_data(session);
> > > > >> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
> > > > >> +		struct ixgbe_crypto_tx_desc_md *mdata =
> > > > >> +			(struct ixgbe_crypto_tx_desc_md *)&m->udata64;
> > > > >> +		mdata->enc = 1;
> > > > >> +		mdata->sa_idx = ic_session->sa_index;
> > > > >> +		mdata->pad_len = *rte_pktmbuf_mtod_offset(m,
> > > > >> +			uint8_t *, rte_pktmbuf_pkt_len(m) - 18) + 18;
> > > > > Could you explain what pad_len supposed to contain?
> > > > > Also what is a magical constant '18'?
> > > > > Could you create some macro if needed?
> > > > I added an explanation in the code, we read the payload padding size
> > > > that is stored at the len-18 bytes and add 18 bytes, 2 for ESP trailer
> > > > and 16 for ICV.
> > >
> > > Ok, can we at least have a macros for all these constants?
> > > Another question: you do use pkt_len() here - does it mean that multi-
> > > segment packets are not supported by ixgbe-ipsec?
> > > Konstantin
> > It does support multisegment, but the pad_len has to be set only for single send, it will be ignored otherwise. I have updated the code to
> set
> > it for single segment packets only.
> 
> Sorry, I didn't understand that.
> If that function does support multiseg packets, then it has to go to the last segment via m->next,
> If it doesn't, then it should return an error I case of m->nb_seg != 1.
> Right?
> 
> > Also, our test app does not support multisegment packets.
> 
> Ok, I suppose that means, multi-seg case wasn't tested :)
> 
> 
>