From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Doherty, Declan" <declan.doherty@intel.com>
Subject: Re: [PATCH v3 6/9] ipsec: implement SA data-path API
Date: Wed, 12 Dec 2018 17:47:20 +0000
Message-ID: <024a941e-04b2-795e-0884-095d40ae7f6d@intel.com>
References: <1543596366-22617-2-git-send-email-konstantin.ananyev@intel.com>
 <1544110714-4514-7-git-send-email-konstantin.ananyev@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>
To: Konstantin Ananyev <konstantin.ananyev@intel.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id 91FAA1B42E
 for <dev@dpdk.org>; Wed, 12 Dec 2018 18:47:26 +0100 (CET)
In-Reply-To: <1544110714-4514-7-git-send-email-konstantin.ananyev@intel.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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@intel.com>