[dpdk-dev] [PATCH 6/9] ipsec: implement SA data-path API

[Ananyev, Konstantin]

Hi Qi,

> 
> Hi Konstantin and Awal:
> 
> 	I have couple questions for this patch.
> 	please forgive me if they are obvious, since I don't have much insight on IPsec, but I may work on related stuff in future :)
> 
> > +static inline int32_t
> > +esp_outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
> > +	const uint64_t ivp[IPSEC_MAX_IV_QWORD], struct rte_mbuf *mb,
> > +	union sym_op_data *icv)
> > +{
> > +	uint32_t clen, hlen, pdlen, pdofs, tlen;
> > +	struct rte_mbuf *ml;
> > +	struct esp_hdr *esph;
> > +	struct esp_tail *espt;
> > +	char *ph, *pt;
> > +	uint64_t *iv;
> > +
> > +	/* calculate extra header space required */
> > +	hlen = sa->hdr_len + sa->iv_len + sizeof(*esph);
> > +
> > +	/* number of bytes to encrypt */
> > +	clen = mb->pkt_len + sizeof(*espt);
> > +	clen = RTE_ALIGN_CEIL(clen, sa->pad_align);
> > +
> > +	/* pad length + esp tail */
> > +	pdlen = clen - mb->pkt_len;
> > +	tlen = pdlen + sa->icv_len;
> > +
> > +	/* do append and prepend */
> > +	ml = rte_pktmbuf_lastseg(mb);
> > +	if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
> > +		return -ENOSPC;
> > +
> > +	/* prepend header */
> > +	ph = rte_pktmbuf_prepend(mb, hlen);
> > +	if (ph == NULL)
> > +		return -ENOSPC;
> > +
> > +	/* append tail */
> > +	pdofs = ml->data_len;
> > +	ml->data_len += tlen;
> > +	mb->pkt_len += tlen;
> > +	pt = rte_pktmbuf_mtod_offset(ml, typeof(pt), pdofs);
> > +
> > +	/* update pkt l2/l3 len */
> > +	mb->l2_len = sa->hdr_l3_off;
> > +	mb->l3_len = sa->hdr_len - sa->hdr_l3_off;
> > +
> > +	/* copy tunnel pkt header */
> > +	rte_memcpy(ph, sa->hdr, sa->hdr_len);
> 
> I didn't get this, my understand is:
> 
> for tunnel mode if an original packet is
> 
> 	Eth + IP + UDP/TCP + data,

It  is assumed that input mbuf doesn't contain L2 header already (only L3/L4/...)
That's why we don't shift L2 header.
Probably have to put that into public API comments.

> 
> after encap, it should become
> 
> 	Eth + encap header (IP or IP + UDP) + ESP Header + IP + UDP/TCP + Data + ESP Tailer...
> 
> So after rte_pktmbuf_prepend shouldn't we do below
> 
> 1) shift L2 HEAD (Eth) ahead
> 2) copy encap header and ESP header to the hole.
> ?
> 
> But now we just copy the sa->hdr on the pre-pend space directly? What is the sa->hdr supposed to be?

Optional L2 header and new L3 header.

> but no matter what is it, we encap
> everything before the packet?
> BTW, is UDP encapsulation also be considered here?

Right now - no.
Might be later, if there would be a request for it.

>, I didn't figure out how a IP + UDP header should be configured with sa->hdr , sa-
> >hdr_l3_off, sa->hdr_len for this case
> 
> > +static inline int
> > +esp_inb_tun_single_pkt_process(struct rte_ipsec_sa *sa, struct rte_mbuf
> > *mb,
> > +	uint32_t *sqn)
> > +{
> > +	uint32_t hlen, icv_len, tlen;
> > +	struct esp_hdr *esph;
> > +	struct esp_tail *espt;
> > +	struct rte_mbuf *ml;
> > +	char *pd;
> > +
> > +	if (mb->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED)
> > +		return -EBADMSG;
> > +
> > +	icv_len = sa->icv_len;
> > +
> > +	ml = rte_pktmbuf_lastseg(mb);
> > +	espt = rte_pktmbuf_mtod_offset(ml, struct esp_tail *,
> > +		ml->data_len - icv_len - sizeof(*espt));
> 
> What kind of mechanism is to guarantee that last segment will always cover the esp tail?( data_len >= icv_len + sizeof (*espt))
> Is that possible the esp tail be split into multi-segment for jumbo frames caes?

It is possible, though right now we don't support such cases.
Right now it is up to the caller to make sure last segment contains espt+icv (plus enough free space for AAD, ESN.hi, etc.).
Plan to add proper multi-seg support later (most likely 19.05).
Konstantin