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

[Doherty, Declan]

Only some minor some cosmetic questions

On 06/12/2018 3:38 PM, Konstantin Ananyev wrote:
> Provide implementation for rte_ipsec_pkt_crypto_prepare() and
...

> +/*
> + * Move preceding (L3) headers up to free space for ESP header and IV.
> + */
> +static inline void
> +insert_esph(char *np, char *op, uint32_t hlen)
> +{
> +	uint32_t i;
> +
> +	for (i = 0; i != hlen; i++)
> +		np[i] = op[i];
> +}
> +
> +/* update original ip header fields for trasnport case */

                                            ^^typo
> +static inline int
> +update_trs_l3hdr(const struct rte_ipsec_sa *sa, void *p, uint32_t plen,
> +		uint32_t l2len, uint32_t l3len, uint8_t proto)
> +{
> +	struct ipv4_hdr *v4h;
> +	struct ipv6_hdr *v6h;
> +	int32_t rc;
> +
> +	if ((sa->type & RTE_IPSEC_SATP_IPV_MASK) == RTE_IPSEC_SATP_IPV4) {
> +		v4h = p;
> +		rc = v4h->next_proto_id;
> +		v4h->next_proto_id = proto;
> +		v4h->total_length = rte_cpu_to_be_16(plen - l2len);
> +	} else if (l3len == sizeof(*v6h)) {

why are you using a different method of identifying ipv6 vs ipv4, would 
checking (sa->type & RTE_IPSEC_SATP_IPV_MASK)== RTE_IPSEC_SATP_IPV6 be 
not be valid here also?

> 
...
> diff --git a/lib/librte_ipsec/ipsec_sqn.h b/lib/librte_ipsec/ipsec_sqn.h
> index 4471814f9..a33ff9cca 100644
> --- a/lib/librte_ipsec/ipsec_sqn.h
> +++ b/lib/librte_ipsec/ipsec_sqn.h
> @@ -15,6 +15,45 @@
>   
>   #define IS_ESN(sa)	((sa)->sqn_mask == UINT64_MAX)

Would it make more sense to have ESN as RTE_IPSEC_SATP_ property so it 
can be retrieve through the sa type API?

>   
...>

Acked-by: Declan Doherty <declan.doherty at intel.com>