* [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 14:55 [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len Lukasz Bartosik
@ 2019-04-30 14:55 ` Lukasz Bartosik
2019-04-30 15:05 ` Ananyev, Konstantin
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: Lukasz Bartosik @ 2019-04-30 14:55 UTC (permalink / raw)
To: konstantin.ananyev; +Cc: dev, anoobj, Lukasz Bartosik
When esn is used then high-order 32 bits are included in ICV
calculation however are not transmitted. Update packet length
to be consistent with auth data offset and length before crypto
operation. High-order 32 bits of esn will be removed from packet
length in crypto post processing.
Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
---
lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
lib/librte_ipsec/sa.c | 4 ++--
lib/librte_ipsec/sa.h | 8 +++++++
4 files changed, 87 insertions(+), 12 deletions(-)
diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
index 4e0e12a..eb899e3 100644
--- a/lib/librte_ipsec/esp_inb.c
+++ b/lib/librte_ipsec/esp_inb.c
@@ -16,7 +16,8 @@
#include "pad.h"
typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
- struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
+ struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
+ uint8_t is_inline);
/*
* helper function to fill crypto_sym op for cipher+auth algorithms.
@@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
+ /*
+ * if esn is used then high-order 32 bits are also used in ICV
+ * calculation but are not transmitted, update packet length
+ * to be consistent with auth data length and offset, this will
+ * be subtracted from packet length in post crypto processing
+ */
+ mb->pkt_len += sa->sqh_len;
+ ml->data_len += sa->sqh_len;
+
inb_pkt_xprepare(sa, sqn, icv);
return plen;
}
@@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
*/
static inline uint16_t
tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
{
uint32_t adj, i, k, tl;
uint32_t hl[num];
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove high-order 32 bits of esn from packet length
+ * which was added before crypto processing, this doesn't
+ * apply to inline case
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
+ (is_inline ? 0 : sa->sqh_len);
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
*/
static inline uint16_t
trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
{
char *np;
uint32_t i, k, l2, tl;
@@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove high-order 32 bits of esn from packet length
+ * which was added before crypto processing, this doesn't
+ * apply to inline case
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
+ (is_inline ? 0 : sa->sqh_len);
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
* process group of ESP inbound packets.
*/
static inline uint16_t
-esp_inb_pkt_process(const struct rte_ipsec_session *ss,
- struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
+esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
+ uint16_t num, uint8_t is_inline, esp_inb_process_t process)
{
uint32_t k, n;
struct rte_ipsec_sa *sa;
@@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
sa = ss->sa;
/* process packets, extract seq numbers */
- k = process(sa, mb, sqn, dr, num);
+ k = process(sa, mb, sqn, dr, num, is_inline);
/* handle unprocessed mbufs */
if (k != num && k != 0)
@@ -533,7 +555,14 @@ uint16_t
esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, tun_process);
+ return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
+}
+
+uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
}
/*
@@ -543,5 +572,12 @@ uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, trs_process);
+ return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
+}
+
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
}
diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
index c798bc4..71a595e 100644
--- a/lib/librte_ipsec/esp_outb.c
+++ b/lib/librte_ipsec/esp_outb.c
@@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
uint32_t i, k, n;
uint64_t sqn;
rte_be64_t sqc;
+ struct rte_mbuf *ml;
struct rte_ipsec_sa *sa;
struct rte_cryptodev_sym_session *cs;
union sym_op_data icv;
@@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
/* success, setup crypto op */
if (rc >= 0) {
+ /*
+ * if esn is used then high-order 32 bits are also
+ * used in ICV calculation but are not transmitted,
+ * update packet length to be consistent with auth
+ * data length and offset, this will be subtracted
+ * from packet length in post crypto processing
+ */
+ if (sa->sqh_len) {
+ mb[i]->pkt_len += sa->sqh_len;
+ ml = rte_pktmbuf_lastseg(mb[i]);
+ ml->data_len += sa->sqh_len;
+ }
+
outb_pkt_xprepare(sa, sqc, &icv);
lksd_none_cop_prepare(cop[k], cs, mb[i]);
outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
@@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
uint32_t i, k, n, l2, l3;
uint64_t sqn;
rte_be64_t sqc;
+ struct rte_mbuf *ml;
struct rte_ipsec_sa *sa;
struct rte_cryptodev_sym_session *cs;
union sym_op_data icv;
@@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
/* success, setup crypto op */
if (rc >= 0) {
+ /*
+ * if esn is used then high-order 32 bits are also
+ * used in ICV calculation but are not transmitted,
+ * update packet length to be consistent with auth
+ * data length and offset, this will be subtracted
+ * from packet length in post crypto processing
+ */
+ if (sa->sqh_len) {
+ mb[i]->pkt_len += sa->sqh_len;
+ ml = rte_pktmbuf_lastseg(mb[i]);
+ ml->data_len += sa->sqh_len;
+ }
+
outb_pkt_xprepare(sa, sqc, &icv);
lksd_none_cop_prepare(cop[k], cs, mb[i]);
outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
@@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
for (i = 0; i != num; i++) {
if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
ml = rte_pktmbuf_lastseg(mb[i]);
+ /* remove high-order 32 bits of esn from packet len */
+ mb[i]->pkt_len -= sa->sqh_len;
+ ml->data_len -= sa->sqh_len;
icv = rte_pktmbuf_mtod_offset(ml, void *,
ml->data_len - icv_len);
remove_sqh(icv, icv_len);
diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
index 846e317..ff01358 100644
--- a/lib/librte_ipsec/sa.c
+++ b/lib/librte_ipsec/sa.c
@@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
switch (sa->type & msk) {
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
- pf->process = esp_inb_tun_pkt_process;
+ pf->process = inline_inb_tun_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
- pf->process = esp_inb_trs_pkt_process;
+ pf->process = inline_inb_trs_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
index ffb5fb4..20c0a65 100644
--- a/lib/librte_ipsec/sa.h
+++ b/lib/librte_ipsec/sa.h
@@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
+uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
/* outbound processing */
uint16_t
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 14:55 [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len Lukasz Bartosik
2019-04-30 14:55 ` Lukasz Bartosik
@ 2019-04-30 15:05 ` Ananyev, Konstantin
2019-04-30 15:05 ` Ananyev, Konstantin
2019-04-30 15:38 ` Lukas Bartosik
2019-05-19 14:47 ` [dpdk-dev] " Ananyev, Konstantin
2019-05-23 12:11 ` [dpdk-dev] [PATCH v2] " Lukasz Bartosik
3 siblings, 2 replies; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-04-30 15:05 UTC (permalink / raw)
To: Lukasz Bartosik; +Cc: dev, anoobj
> -----Original Message-----
> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
> Sent: Tuesday, April 30, 2019 3:56 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
Hi Lukasz,
Why you want to do it?
I deliberately didn't include SQH bits into the pkt_len/data_len,
because it is a temporary data and we are going to drop it anyway.
Konstantin
>
> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
> lib/librte_ipsec/sa.c | 4 ++--
> lib/librte_ipsec/sa.h | 8 +++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
> index 4e0e12a..eb899e3 100644
> --- a/lib/librte_ipsec/esp_inb.c
> +++ b/lib/librte_ipsec/esp_inb.c
> @@ -16,7 +16,8 @@
> #include "pad.h"
>
> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
> + uint8_t is_inline);
>
> /*
> * helper function to fill crypto_sym op for cipher+auth algorithms.
> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>
> + /*
> + * if esn is used then high-order 32 bits are also used in ICV
> + * calculation but are not transmitted, update packet length
> + * to be consistent with auth data length and offset, this will
> + * be subtracted from packet length in post crypto processing
> + */
> + mb->pkt_len += sa->sqh_len;
> + ml->data_len += sa->sqh_len;
> +
> inb_pkt_xprepare(sa, sqn, icv);
> return plen;
> }
> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
> */
> static inline uint16_t
> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> uint32_t adj, i, k, tl;
> uint32_t hl[num];
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> */
> static inline uint16_t
> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> char *np;
> uint32_t i, k, l2, tl;
> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
> * process group of ESP inbound packets.
> */
> static inline uint16_t
> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
> {
> uint32_t k, n;
> struct rte_ipsec_sa *sa;
> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> sa = ss->sa;
>
> /* process packets, extract seq numbers */
> - k = process(sa, mb, sqn, dr, num);
> + k = process(sa, mb, sqn, dr, num, is_inline);
>
> /* handle unprocessed mbufs */
> if (k != num && k != 0)
> @@ -533,7 +555,14 @@ uint16_t
> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, tun_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
> +}
> +
> +uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
> }
>
> /*
> @@ -543,5 +572,12 @@ uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, trs_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
> +}
> +
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
> }
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..71a595e 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n, l2, l3;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index 846e317..ff01358 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
> switch (sa->type & msk) {
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
> - pf->process = esp_inb_tun_pkt_process;
> + pf->process = inline_inb_tun_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
> - pf->process = esp_inb_trs_pkt_process;
> + pf->process = inline_inb_trs_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index ffb5fb4..20c0a65 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> +uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> /* outbound processing */
>
> uint16_t
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 15:05 ` Ananyev, Konstantin
@ 2019-04-30 15:05 ` Ananyev, Konstantin
2019-04-30 15:38 ` Lukas Bartosik
1 sibling, 0 replies; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-04-30 15:05 UTC (permalink / raw)
To: Lukasz Bartosik; +Cc: dev, anoobj
> -----Original Message-----
> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
> Sent: Tuesday, April 30, 2019 3:56 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
Hi Lukasz,
Why you want to do it?
I deliberately didn't include SQH bits into the pkt_len/data_len,
because it is a temporary data and we are going to drop it anyway.
Konstantin
>
> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
> lib/librte_ipsec/sa.c | 4 ++--
> lib/librte_ipsec/sa.h | 8 +++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
> index 4e0e12a..eb899e3 100644
> --- a/lib/librte_ipsec/esp_inb.c
> +++ b/lib/librte_ipsec/esp_inb.c
> @@ -16,7 +16,8 @@
> #include "pad.h"
>
> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
> + uint8_t is_inline);
>
> /*
> * helper function to fill crypto_sym op for cipher+auth algorithms.
> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>
> + /*
> + * if esn is used then high-order 32 bits are also used in ICV
> + * calculation but are not transmitted, update packet length
> + * to be consistent with auth data length and offset, this will
> + * be subtracted from packet length in post crypto processing
> + */
> + mb->pkt_len += sa->sqh_len;
> + ml->data_len += sa->sqh_len;
> +
> inb_pkt_xprepare(sa, sqn, icv);
> return plen;
> }
> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
> */
> static inline uint16_t
> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> uint32_t adj, i, k, tl;
> uint32_t hl[num];
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> */
> static inline uint16_t
> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> char *np;
> uint32_t i, k, l2, tl;
> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
> * process group of ESP inbound packets.
> */
> static inline uint16_t
> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
> {
> uint32_t k, n;
> struct rte_ipsec_sa *sa;
> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> sa = ss->sa;
>
> /* process packets, extract seq numbers */
> - k = process(sa, mb, sqn, dr, num);
> + k = process(sa, mb, sqn, dr, num, is_inline);
>
> /* handle unprocessed mbufs */
> if (k != num && k != 0)
> @@ -533,7 +555,14 @@ uint16_t
> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, tun_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
> +}
> +
> +uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
> }
>
> /*
> @@ -543,5 +572,12 @@ uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, trs_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
> +}
> +
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
> }
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..71a595e 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n, l2, l3;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index 846e317..ff01358 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
> switch (sa->type & msk) {
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
> - pf->process = esp_inb_tun_pkt_process;
> + pf->process = inline_inb_tun_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
> - pf->process = esp_inb_trs_pkt_process;
> + pf->process = inline_inb_trs_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index ffb5fb4..20c0a65 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> +uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> /* outbound processing */
>
> uint16_t
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 15:05 ` Ananyev, Konstantin
2019-04-30 15:05 ` Ananyev, Konstantin
@ 2019-04-30 15:38 ` Lukas Bartosik
2019-04-30 15:38 ` Lukas Bartosik
2019-05-07 14:48 ` [dpdk-dev] [EXT] " Lukas Bartosik
1 sibling, 2 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-04-30 15:38 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
________________________________
From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
Sent: Tuesday, April 30, 2019 5:05 PM
To: Lukas Bartosik
Cc: dev@dpdk.org; Anoob Joseph
Subject: RE: [PATCH] ipsec: include high order bytes of esn in pkt len
> -----Original Message-----
> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
> Sent: Tuesday, April 30, 2019 3:56 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
Hi Lukasz,
Why you want to do it?
I deliberately didn't include SQH bits into the pkt_len/data_len,
because it is a temporary data and we are going to drop it anyway.
Konstantin
Hi Konstantin,
Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
because it is told to authenticate more data that a packet holds (according to pkt_len).
I came across this when running IPSec tests which use esn.
I understand that sqh 32 bits are temporary and included only for ICV calculation however
not including them in pkt_len for crypto processing is inconsistent in my opinion.
Thanks,
Lukasz
>
> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
> lib/librte_ipsec/sa.c | 4 ++--
> lib/librte_ipsec/sa.h | 8 +++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
> index 4e0e12a..eb899e3 100644
> --- a/lib/librte_ipsec/esp_inb.c
> +++ b/lib/librte_ipsec/esp_inb.c
> @@ -16,7 +16,8 @@
> #include "pad.h"
>
> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
> + uint8_t is_inline);
>
> /*
> * helper function to fill crypto_sym op for cipher+auth algorithms.
> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>
> + /*
> + * if esn is used then high-order 32 bits are also used in ICV
> + * calculation but are not transmitted, update packet length
> + * to be consistent with auth data length and offset, this will
> + * be subtracted from packet length in post crypto processing
> + */
> + mb->pkt_len += sa->sqh_len;
> + ml->data_len += sa->sqh_len;
> +
> inb_pkt_xprepare(sa, sqn, icv);
> return plen;
> }
> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
> */
> static inline uint16_t
> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> uint32_t adj, i, k, tl;
> uint32_t hl[num];
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> */
> static inline uint16_t
> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> char *np;
> uint32_t i, k, l2, tl;
> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
> * process group of ESP inbound packets.
> */
> static inline uint16_t
> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
> {
> uint32_t k, n;
> struct rte_ipsec_sa *sa;
> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> sa = ss->sa;
>
> /* process packets, extract seq numbers */
> - k = process(sa, mb, sqn, dr, num);
> + k = process(sa, mb, sqn, dr, num, is_inline);
>
> /* handle unprocessed mbufs */
> if (k != num && k != 0)
> @@ -533,7 +555,14 @@ uint16_t
> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, tun_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
> +}
> +
> +uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
> }
>
> /*
> @@ -543,5 +572,12 @@ uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, trs_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
> +}
> +
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
> }
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..71a595e 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n, l2, l3;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index 846e317..ff01358 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
> switch (sa->type & msk) {
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
> - pf->process = esp_inb_tun_pkt_process;
> + pf->process = inline_inb_tun_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
> - pf->process = esp_inb_trs_pkt_process;
> + pf->process = inline_inb_trs_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index ffb5fb4..20c0a65 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> +uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> /* outbound processing */
>
> uint16_t
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 15:38 ` Lukas Bartosik
@ 2019-04-30 15:38 ` Lukas Bartosik
2019-05-07 14:48 ` [dpdk-dev] [EXT] " Lukas Bartosik
1 sibling, 0 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-04-30 15:38 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
________________________________
From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
Sent: Tuesday, April 30, 2019 5:05 PM
To: Lukas Bartosik
Cc: dev@dpdk.org; Anoob Joseph
Subject: RE: [PATCH] ipsec: include high order bytes of esn in pkt len
> -----Original Message-----
> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
> Sent: Tuesday, April 30, 2019 3:56 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
Hi Lukasz,
Why you want to do it?
I deliberately didn't include SQH bits into the pkt_len/data_len,
because it is a temporary data and we are going to drop it anyway.
Konstantin
Hi Konstantin,
Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
because it is told to authenticate more data that a packet holds (according to pkt_len).
I came across this when running IPSec tests which use esn.
I understand that sqh 32 bits are temporary and included only for ICV calculation however
not including them in pkt_len for crypto processing is inconsistent in my opinion.
Thanks,
Lukasz
>
> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
> lib/librte_ipsec/sa.c | 4 ++--
> lib/librte_ipsec/sa.h | 8 +++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
> index 4e0e12a..eb899e3 100644
> --- a/lib/librte_ipsec/esp_inb.c
> +++ b/lib/librte_ipsec/esp_inb.c
> @@ -16,7 +16,8 @@
> #include "pad.h"
>
> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
> + uint8_t is_inline);
>
> /*
> * helper function to fill crypto_sym op for cipher+auth algorithms.
> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>
> + /*
> + * if esn is used then high-order 32 bits are also used in ICV
> + * calculation but are not transmitted, update packet length
> + * to be consistent with auth data length and offset, this will
> + * be subtracted from packet length in post crypto processing
> + */
> + mb->pkt_len += sa->sqh_len;
> + ml->data_len += sa->sqh_len;
> +
> inb_pkt_xprepare(sa, sqn, icv);
> return plen;
> }
> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
> */
> static inline uint16_t
> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> uint32_t adj, i, k, tl;
> uint32_t hl[num];
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> */
> static inline uint16_t
> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> char *np;
> uint32_t i, k, l2, tl;
> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
> * process group of ESP inbound packets.
> */
> static inline uint16_t
> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
> {
> uint32_t k, n;
> struct rte_ipsec_sa *sa;
> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> sa = ss->sa;
>
> /* process packets, extract seq numbers */
> - k = process(sa, mb, sqn, dr, num);
> + k = process(sa, mb, sqn, dr, num, is_inline);
>
> /* handle unprocessed mbufs */
> if (k != num && k != 0)
> @@ -533,7 +555,14 @@ uint16_t
> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, tun_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
> +}
> +
> +uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
> }
>
> /*
> @@ -543,5 +572,12 @@ uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, trs_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
> +}
> +
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
> }
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..71a595e 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n, l2, l3;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index 846e317..ff01358 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
> switch (sa->type & msk) {
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
> - pf->process = esp_inb_tun_pkt_process;
> + pf->process = inline_inb_tun_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
> - pf->process = esp_inb_trs_pkt_process;
> + pf->process = inline_inb_trs_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index ffb5fb4..20c0a65 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> +uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> /* outbound processing */
>
> uint16_t
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 15:38 ` Lukas Bartosik
2019-04-30 15:38 ` Lukas Bartosik
@ 2019-05-07 14:48 ` Lukas Bartosik
2019-05-07 14:48 ` Lukas Bartosik
` (2 more replies)
1 sibling, 3 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-07 14:48 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
On 30.04.2019 17:38, Lukas Bartosik wrote:
> External Email
>
> ----------------------------------------------------------------------
>
>
>
> ________________________________
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, April 30, 2019 5:05 PM
> To: Lukas Bartosik
> Cc: dev@dpdk.org; Anoob Joseph
> Subject: RE: [PATCH] ipsec: include high order bytes of esn in pkt len
>
>
>
>> -----Original Message-----
>> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
>> Sent: Tuesday, April 30, 2019 3:56 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
>> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>>
>> When esn is used then high-order 32 bits are included in ICV
>> calculation however are not transmitted. Update packet length
>> to be consistent with auth data offset and length before crypto
>> operation. High-order 32 bits of esn will be removed from packet
>> length in crypto post processing.
>
> Hi Lukasz,
> Why you want to do it?
> I deliberately didn't include SQH bits into the pkt_len/data_len,
> because it is a temporary data and we are going to drop it anyway.
> Konstantin
>
> Hi Konstantin,
> Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> because it is told to authenticate more data that a packet holds (according to pkt_len).
> I came across this when running IPSec tests which use esn.
> I understand that sqh 32 bits are temporary and included only for ICV calculation however
> not including them in pkt_len for crypto processing is inconsistent in my opinion.
> Thanks,
> Lukasz
>
Hi Konstantin,
I should have elaborated more. When 32 high bits of esn are not included in
packet length then auth offset and data point to data which is outside packet
(according to packet length).
This makes crypto request (auth data length and offset) incoherent with a packet
which the crypto request points to.
This is my argument for including 32 high bits of esn into packet length even
though the inclusion is only temporary.
Thanks,
Lukasz
>>
>> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
>> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
>> ---
>> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
>> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
>> lib/librte_ipsec/sa.c | 4 ++--
>> lib/librte_ipsec/sa.h | 8 +++++++
>> 4 files changed, 87 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
>> index 4e0e12a..eb899e3 100644
>> --- a/lib/librte_ipsec/esp_inb.c
>> +++ b/lib/librte_ipsec/esp_inb.c
>> @@ -16,7 +16,8 @@
>> #include "pad.h"
>>
>> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
>> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
>> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
>> + uint8_t is_inline);
>>
>> /*
>> * helper function to fill crypto_sym op for cipher+auth algorithms.
>> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
>> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
>> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>>
>> + /*
>> + * if esn is used then high-order 32 bits are also used in ICV
>> + * calculation but are not transmitted, update packet length
>> + * to be consistent with auth data length and offset, this will
>> + * be subtracted from packet length in post crypto processing
>> + */
>> + mb->pkt_len += sa->sqh_len;
>> + ml->data_len += sa->sqh_len;
>> +
>> inb_pkt_xprepare(sa, sqn, icv);
>> return plen;
>> }
>> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
>> */
>> static inline uint16_t
>> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> uint32_t adj, i, k, tl;
>> uint32_t hl[num];
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> */
>> static inline uint16_t
>> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> char *np;
>> uint32_t i, k, l2, tl;
>> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
>> * process group of ESP inbound packets.
>> */
>> static inline uint16_t
>> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
>> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
>> {
>> uint32_t k, n;
>> struct rte_ipsec_sa *sa;
>> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> sa = ss->sa;
>>
>> /* process packets, extract seq numbers */
>> - k = process(sa, mb, sqn, dr, num);
>> + k = process(sa, mb, sqn, dr, num, is_inline);
>>
>> /* handle unprocessed mbufs */
>> if (k != num && k != 0)
>> @@ -533,7 +555,14 @@ uint16_t
>> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, tun_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
>> +}
>> +
>> +uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
>> }
>>
>> /*
>> @@ -543,5 +572,12 @@ uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, trs_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
>> +}
>> +
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
>> }
>> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
>> index c798bc4..71a595e 100644
>> --- a/lib/librte_ipsec/esp_outb.c
>> +++ b/lib/librte_ipsec/esp_outb.c
>> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
>> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n, l2, l3;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
>> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> for (i = 0; i != num; i++) {
>> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
>> ml = rte_pktmbuf_lastseg(mb[i]);
>> + /* remove high-order 32 bits of esn from packet len */
>> + mb[i]->pkt_len -= sa->sqh_len;
>> + ml->data_len -= sa->sqh_len;
>> icv = rte_pktmbuf_mtod_offset(ml, void *,
>> ml->data_len - icv_len);
>> remove_sqh(icv, icv_len);
>> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
>> index 846e317..ff01358 100644
>> --- a/lib/librte_ipsec/sa.c
>> +++ b/lib/librte_ipsec/sa.c
>> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
>> switch (sa->type & msk) {
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> - pf->process = esp_inb_tun_pkt_process;
>> + pf->process = inline_inb_tun_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
>> - pf->process = esp_inb_trs_pkt_process;
>> + pf->process = inline_inb_trs_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
>> index ffb5fb4..20c0a65 100644
>> --- a/lib/librte_ipsec/sa.h
>> +++ b/lib/librte_ipsec/sa.h
>> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> +uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> /* outbound processing */
>>
>> uint16_t
>> --
>> 2.7.4
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-07 14:48 ` [dpdk-dev] [EXT] " Lukas Bartosik
@ 2019-05-07 14:48 ` Lukas Bartosik
2019-05-09 11:59 ` Ananyev, Konstantin
2019-05-14 13:52 ` Ananyev, Konstantin
2 siblings, 0 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-07 14:48 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
On 30.04.2019 17:38, Lukas Bartosik wrote:
> External Email
>
> ----------------------------------------------------------------------
>
>
>
> ________________________________
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, April 30, 2019 5:05 PM
> To: Lukas Bartosik
> Cc: dev@dpdk.org; Anoob Joseph
> Subject: RE: [PATCH] ipsec: include high order bytes of esn in pkt len
>
>
>
>> -----Original Message-----
>> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
>> Sent: Tuesday, April 30, 2019 3:56 PM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
>> Subject: [PATCH] ipsec: include high order bytes of esn in pkt len
>>
>> When esn is used then high-order 32 bits are included in ICV
>> calculation however are not transmitted. Update packet length
>> to be consistent with auth data offset and length before crypto
>> operation. High-order 32 bits of esn will be removed from packet
>> length in crypto post processing.
>
> Hi Lukasz,
> Why you want to do it?
> I deliberately didn't include SQH bits into the pkt_len/data_len,
> because it is a temporary data and we are going to drop it anyway.
> Konstantin
>
> Hi Konstantin,
> Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> because it is told to authenticate more data that a packet holds (according to pkt_len).
> I came across this when running IPSec tests which use esn.
> I understand that sqh 32 bits are temporary and included only for ICV calculation however
> not including them in pkt_len for crypto processing is inconsistent in my opinion.
> Thanks,
> Lukasz
>
Hi Konstantin,
I should have elaborated more. When 32 high bits of esn are not included in
packet length then auth offset and data point to data which is outside packet
(according to packet length).
This makes crypto request (auth data length and offset) incoherent with a packet
which the crypto request points to.
This is my argument for including 32 high bits of esn into packet length even
though the inclusion is only temporary.
Thanks,
Lukasz
>>
>> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
>> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
>> ---
>> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
>> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
>> lib/librte_ipsec/sa.c | 4 ++--
>> lib/librte_ipsec/sa.h | 8 +++++++
>> 4 files changed, 87 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
>> index 4e0e12a..eb899e3 100644
>> --- a/lib/librte_ipsec/esp_inb.c
>> +++ b/lib/librte_ipsec/esp_inb.c
>> @@ -16,7 +16,8 @@
>> #include "pad.h"
>>
>> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
>> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
>> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
>> + uint8_t is_inline);
>>
>> /*
>> * helper function to fill crypto_sym op for cipher+auth algorithms.
>> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
>> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
>> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>>
>> + /*
>> + * if esn is used then high-order 32 bits are also used in ICV
>> + * calculation but are not transmitted, update packet length
>> + * to be consistent with auth data length and offset, this will
>> + * be subtracted from packet length in post crypto processing
>> + */
>> + mb->pkt_len += sa->sqh_len;
>> + ml->data_len += sa->sqh_len;
>> +
>> inb_pkt_xprepare(sa, sqn, icv);
>> return plen;
>> }
>> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
>> */
>> static inline uint16_t
>> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> uint32_t adj, i, k, tl;
>> uint32_t hl[num];
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> */
>> static inline uint16_t
>> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> char *np;
>> uint32_t i, k, l2, tl;
>> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
>> * process group of ESP inbound packets.
>> */
>> static inline uint16_t
>> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
>> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
>> {
>> uint32_t k, n;
>> struct rte_ipsec_sa *sa;
>> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> sa = ss->sa;
>>
>> /* process packets, extract seq numbers */
>> - k = process(sa, mb, sqn, dr, num);
>> + k = process(sa, mb, sqn, dr, num, is_inline);
>>
>> /* handle unprocessed mbufs */
>> if (k != num && k != 0)
>> @@ -533,7 +555,14 @@ uint16_t
>> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, tun_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
>> +}
>> +
>> +uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
>> }
>>
>> /*
>> @@ -543,5 +572,12 @@ uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, trs_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
>> +}
>> +
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
>> }
>> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
>> index c798bc4..71a595e 100644
>> --- a/lib/librte_ipsec/esp_outb.c
>> +++ b/lib/librte_ipsec/esp_outb.c
>> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
>> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n, l2, l3;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
>> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> for (i = 0; i != num; i++) {
>> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
>> ml = rte_pktmbuf_lastseg(mb[i]);
>> + /* remove high-order 32 bits of esn from packet len */
>> + mb[i]->pkt_len -= sa->sqh_len;
>> + ml->data_len -= sa->sqh_len;
>> icv = rte_pktmbuf_mtod_offset(ml, void *,
>> ml->data_len - icv_len);
>> remove_sqh(icv, icv_len);
>> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
>> index 846e317..ff01358 100644
>> --- a/lib/librte_ipsec/sa.c
>> +++ b/lib/librte_ipsec/sa.c
>> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
>> switch (sa->type & msk) {
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> - pf->process = esp_inb_tun_pkt_process;
>> + pf->process = inline_inb_tun_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
>> - pf->process = esp_inb_trs_pkt_process;
>> + pf->process = inline_inb_trs_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
>> index ffb5fb4..20c0a65 100644
>> --- a/lib/librte_ipsec/sa.h
>> +++ b/lib/librte_ipsec/sa.h
>> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> +uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> /* outbound processing */
>>
>> uint16_t
>> --
>> 2.7.4
>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-07 14:48 ` [dpdk-dev] [EXT] " Lukas Bartosik
2019-05-07 14:48 ` Lukas Bartosik
@ 2019-05-09 11:59 ` Ananyev, Konstantin
2019-05-09 11:59 ` Ananyev, Konstantin
2019-05-14 13:52 ` Ananyev, Konstantin
2 siblings, 1 reply; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-09 11:59 UTC (permalink / raw)
To: Lukas Bartosik; +Cc: dev, Anoob Joseph
> >>
> >> When esn is used then high-order 32 bits are included in ICV
> >> calculation however are not transmitted. Update packet length
> >> to be consistent with auth data offset and length before crypto
> >> operation. High-order 32 bits of esn will be removed from packet
> >> length in crypto post processing.
> >
> > Hi Lukasz,
> > Why you want to do it?
> > I deliberately didn't include SQH bits into the pkt_len/data_len,
> > because it is a temporary data and we are going to drop it anyway.
> > Konstantin
> >
> > Hi Konstantin,
> > Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> > because it is told to authenticate more data that a packet holds (according to pkt_len).
> > I came across this when running IPSec tests which use esn.
> > I understand that sqh 32 bits are temporary and included only for ICV calculation however
> > not including them in pkt_len for crypto processing is inconsistent in my opinion.
> > Thanks,
> > Lukasz
> >
>
> Hi Konstantin,
>
> I should have elaborated more. When 32 high bits of esn are not included in
> packet length then auth offset and data point to data which is outside packet
> (according to packet length).
> This makes crypto request (auth data length and offset) incoherent with a packet
> which the crypto request points to.
>
> This is my argument for including 32 high bits of esn into packet length even
> though the inclusion is only temporary.
I understood the reasoning.
Will try to have a closer look in next few days.
Thanks
Konstantin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-09 11:59 ` Ananyev, Konstantin
@ 2019-05-09 11:59 ` Ananyev, Konstantin
0 siblings, 0 replies; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-09 11:59 UTC (permalink / raw)
To: Lukas Bartosik; +Cc: dev, Anoob Joseph
> >>
> >> When esn is used then high-order 32 bits are included in ICV
> >> calculation however are not transmitted. Update packet length
> >> to be consistent with auth data offset and length before crypto
> >> operation. High-order 32 bits of esn will be removed from packet
> >> length in crypto post processing.
> >
> > Hi Lukasz,
> > Why you want to do it?
> > I deliberately didn't include SQH bits into the pkt_len/data_len,
> > because it is a temporary data and we are going to drop it anyway.
> > Konstantin
> >
> > Hi Konstantin,
> > Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> > because it is told to authenticate more data that a packet holds (according to pkt_len).
> > I came across this when running IPSec tests which use esn.
> > I understand that sqh 32 bits are temporary and included only for ICV calculation however
> > not including them in pkt_len for crypto processing is inconsistent in my opinion.
> > Thanks,
> > Lukasz
> >
>
> Hi Konstantin,
>
> I should have elaborated more. When 32 high bits of esn are not included in
> packet length then auth offset and data point to data which is outside packet
> (according to packet length).
> This makes crypto request (auth data length and offset) incoherent with a packet
> which the crypto request points to.
>
> This is my argument for including 32 high bits of esn into packet length even
> though the inclusion is only temporary.
I understood the reasoning.
Will try to have a closer look in next few days.
Thanks
Konstantin
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-07 14:48 ` [dpdk-dev] [EXT] " Lukas Bartosik
2019-05-07 14:48 ` Lukas Bartosik
2019-05-09 11:59 ` Ananyev, Konstantin
@ 2019-05-14 13:52 ` Ananyev, Konstantin
2019-05-14 13:52 ` Ananyev, Konstantin
2019-05-14 14:31 ` Lukas Bartosik
2 siblings, 2 replies; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-14 13:52 UTC (permalink / raw)
To: Lukas Bartosik; +Cc: dev, Anoob Joseph
Hi Lukasz,
> >>
> >> When esn is used then high-order 32 bits are included in ICV
> >> calculation however are not transmitted. Update packet length
> >> to be consistent with auth data offset and length before crypto
> >> operation. High-order 32 bits of esn will be removed from packet
> >> length in crypto post processing.
> >
> > Hi Lukasz,
> > Why you want to do it?
> > I deliberately didn't include SQH bits into the pkt_len/data_len,
> > because it is a temporary data and we are going to drop it anyway.
> > Konstantin
> >
> > Hi Konstantin,
> > Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> > because it is told to authenticate more data that a packet holds (according to pkt_len).
Thanks for explanation, just to confirm about the check in your PMD:
You are talking about struct rte_crypto_sym_op auth.data.offset and auth.data.length,
i.e: auth.data.offset + auth.data.length > pkt_len
Or something else?
find drivers/*/octeon* -type f | xargs grep -l 'auth\.data\.'
returns no results.
Konstantin
> > I came across this when running IPSec tests which use esn.
> > I understand that sqh 32 bits are temporary and included only for ICV calculation however
> > not including them in pkt_len for crypto processing is inconsistent in my opinion.
> > Thanks,
> > Lukasz
> >
>
> Hi Konstantin,
>
> I should have elaborated more. When 32 high bits of esn are not included in
> packet length then auth offset and data point to data which is outside packet
> (according to packet length).
> This makes crypto request (auth data length and offset) incoherent with a packet
> which the crypto request points to.
>
> This is my argument for including 32 high bits of esn into packet length even
> though the inclusion is only temporary.
>
> Thanks,
> Lukasz
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-14 13:52 ` Ananyev, Konstantin
@ 2019-05-14 13:52 ` Ananyev, Konstantin
2019-05-14 14:31 ` Lukas Bartosik
1 sibling, 0 replies; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-14 13:52 UTC (permalink / raw)
To: Lukas Bartosik; +Cc: dev, Anoob Joseph
Hi Lukasz,
> >>
> >> When esn is used then high-order 32 bits are included in ICV
> >> calculation however are not transmitted. Update packet length
> >> to be consistent with auth data offset and length before crypto
> >> operation. High-order 32 bits of esn will be removed from packet
> >> length in crypto post processing.
> >
> > Hi Lukasz,
> > Why you want to do it?
> > I deliberately didn't include SQH bits into the pkt_len/data_len,
> > because it is a temporary data and we are going to drop it anyway.
> > Konstantin
> >
> > Hi Konstantin,
> > Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
> > because it is told to authenticate more data that a packet holds (according to pkt_len).
Thanks for explanation, just to confirm about the check in your PMD:
You are talking about struct rte_crypto_sym_op auth.data.offset and auth.data.length,
i.e: auth.data.offset + auth.data.length > pkt_len
Or something else?
find drivers/*/octeon* -type f | xargs grep -l 'auth\.data\.'
returns no results.
Konstantin
> > I came across this when running IPSec tests which use esn.
> > I understand that sqh 32 bits are temporary and included only for ICV calculation however
> > not including them in pkt_len for crypto processing is inconsistent in my opinion.
> > Thanks,
> > Lukasz
> >
>
> Hi Konstantin,
>
> I should have elaborated more. When 32 high bits of esn are not included in
> packet length then auth offset and data point to data which is outside packet
> (according to packet length).
> This makes crypto request (auth data length and offset) incoherent with a packet
> which the crypto request points to.
>
> This is my argument for including 32 high bits of esn into packet length even
> though the inclusion is only temporary.
>
> Thanks,
> Lukasz
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-14 13:52 ` Ananyev, Konstantin
2019-05-14 13:52 ` Ananyev, Konstantin
@ 2019-05-14 14:31 ` Lukas Bartosik
2019-05-14 14:31 ` Lukas Bartosik
1 sibling, 1 reply; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-14 14:31 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
On 14.05.2019 15:52, Ananyev, Konstantin wrote:
> Hi Lukasz,
>
>>>>
>>>> When esn is used then high-order 32 bits are included in ICV
>>>> calculation however are not transmitted. Update packet length
>>>> to be consistent with auth data offset and length before crypto
>>>> operation. High-order 32 bits of esn will be removed from packet
>>>> length in crypto post processing.
>>>
>>> Hi Lukasz,
>>> Why you want to do it?
>>> I deliberately didn't include SQH bits into the pkt_len/data_len,
>>> because it is a temporary data and we are going to drop it anyway.
>>> Konstantin
>>>
>>> Hi Konstantin,
>>> Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
>>> because it is told to authenticate more data that a packet holds (according to pkt_len).
>
> Thanks for explanation, just to confirm about the check in your PMD:
> You are talking about struct rte_crypto_sym_op auth.data.offset and auth.data.length,
> i.e: auth.data.offset + auth.data.length > pkt_len
> Or something else?
>
> find drivers/*/octeon* -type f | xargs grep -l 'auth\.data\.'
> returns no results.
>
> Konstantin
>
Hi Konstantin
This is exactly auth.data.length and auth.data.offset from rte_crypto_sym_op.
The check takes place in drivers/common/cpt/cpt_ucode.h in cpt_dec_hmac_prep function
although there is no direct check for auth.data.offset + auth.data.length > pkt_len
as at this point auth.data.offset, auth.data.length and pkt_len are stored in
internal structures related to how we process crypto requests.
Thanks,
Lukasz
>>> I came across this when running IPSec tests which use esn.
>>> I understand that sqh 32 bits are temporary and included only for ICV calculation however
>>> not including them in pkt_len for crypto processing is inconsistent in my opinion.
>>> Thanks,
>>> Lukasz
>>>
>>
>> Hi Konstantin,
>>
>> I should have elaborated more. When 32 high bits of esn are not included in
>> packet length then auth offset and data point to data which is outside packet
>> (according to packet length).
>> This makes crypto request (auth data length and offset) incoherent with a packet
>> which the crypto request points to.
>>
>> This is my argument for including 32 high bits of esn into packet length even
>> though the inclusion is only temporary.
>>
>> Thanks,
>> Lukasz
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-14 14:31 ` Lukas Bartosik
@ 2019-05-14 14:31 ` Lukas Bartosik
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-14 14:31 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
On 14.05.2019 15:52, Ananyev, Konstantin wrote:
> Hi Lukasz,
>
>>>>
>>>> When esn is used then high-order 32 bits are included in ICV
>>>> calculation however are not transmitted. Update packet length
>>>> to be consistent with auth data offset and length before crypto
>>>> operation. High-order 32 bits of esn will be removed from packet
>>>> length in crypto post processing.
>>>
>>> Hi Lukasz,
>>> Why you want to do it?
>>> I deliberately didn't include SQH bits into the pkt_len/data_len,
>>> because it is a temporary data and we are going to drop it anyway.
>>> Konstantin
>>>
>>> Hi Konstantin,
>>> Our OcteonTx crypto driver validates pkt_len with auth data length/offset and it complains
>>> because it is told to authenticate more data that a packet holds (according to pkt_len).
>
> Thanks for explanation, just to confirm about the check in your PMD:
> You are talking about struct rte_crypto_sym_op auth.data.offset and auth.data.length,
> i.e: auth.data.offset + auth.data.length > pkt_len
> Or something else?
>
> find drivers/*/octeon* -type f | xargs grep -l 'auth\.data\.'
> returns no results.
>
> Konstantin
>
Hi Konstantin
This is exactly auth.data.length and auth.data.offset from rte_crypto_sym_op.
The check takes place in drivers/common/cpt/cpt_ucode.h in cpt_dec_hmac_prep function
although there is no direct check for auth.data.offset + auth.data.length > pkt_len
as at this point auth.data.offset, auth.data.length and pkt_len are stored in
internal structures related to how we process crypto requests.
Thanks,
Lukasz
>>> I came across this when running IPSec tests which use esn.
>>> I understand that sqh 32 bits are temporary and included only for ICV calculation however
>>> not including them in pkt_len for crypto processing is inconsistent in my opinion.
>>> Thanks,
>>> Lukasz
>>>
>>
>> Hi Konstantin,
>>
>> I should have elaborated more. When 32 high bits of esn are not included in
>> packet length then auth offset and data point to data which is outside packet
>> (according to packet length).
>> This makes crypto request (auth data length and offset) incoherent with a packet
>> which the crypto request points to.
>>
>> This is my argument for including 32 high bits of esn into packet length even
>> though the inclusion is only temporary.
>>
>> Thanks,
>> Lukasz
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-04-30 14:55 [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len Lukasz Bartosik
2019-04-30 14:55 ` Lukasz Bartosik
2019-04-30 15:05 ` Ananyev, Konstantin
@ 2019-05-19 14:47 ` Ananyev, Konstantin
2019-05-20 11:13 ` Lukas Bartosik
2019-05-23 12:11 ` [dpdk-dev] [PATCH v2] " Lukasz Bartosik
3 siblings, 1 reply; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-19 14:47 UTC (permalink / raw)
To: Lukasz Bartosik; +Cc: dev, anoobj
Hi Lukasz,
Thanks for clarifications.
Looks good in general.
Few small comments below.
Konstantin
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
>
> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
> lib/librte_ipsec/sa.c | 4 ++--
> lib/librte_ipsec/sa.h | 8 +++++++
> 4 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
> index 4e0e12a..eb899e3 100644
> --- a/lib/librte_ipsec/esp_inb.c
> +++ b/lib/librte_ipsec/esp_inb.c
> @@ -16,7 +16,8 @@
> #include "pad.h"
>
> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
> + uint8_t is_inline);
>
> /*
> * helper function to fill crypto_sym op for cipher+auth algorithms.
> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>
> + /*
> + * if esn is used then high-order 32 bits are also used in ICV
> + * calculation but are not transmitted, update packet length
> + * to be consistent with auth data length and offset, this will
> + * be subtracted from packet length in post crypto processing
Here and in several other comments below - you repeat basically the same thing.
Seems a bit excessive. I suppose just to put in in one place, or probably even
in patch description will be enough.
> + */
> + mb->pkt_len += sa->sqh_len;
> + ml->data_len += sa->sqh_len;
> +
> inb_pkt_xprepare(sa, sqn, icv);
> return plen;
> }
> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
> */
> static inline uint16_t
> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> uint32_t adj, i, k, tl;
> uint32_t hl[num];
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
This comment seems a bit misleading, as we have remove not only sqh,
but also icv, espt, padding.
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> */
> static inline uint16_t
> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> - uint32_t sqn[], uint32_t dr[], uint16_t num)
> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
> {
> char *np;
> uint32_t i, k, l2, tl;
> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
> struct esp_tail espt[num];
> struct rte_mbuf *ml[num];
>
> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
> + /*
> + * remove high-order 32 bits of esn from packet length
> + * which was added before crypto processing, this doesn't
> + * apply to inline case
> + */
> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
> + (is_inline ? 0 : sa->sqh_len);
> const uint32_t cofs = sa->ctp.cipher.offset;
>
> /*
> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
> * process group of ESP inbound packets.
> */
> static inline uint16_t
> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
> {
> uint32_t k, n;
> struct rte_ipsec_sa *sa;
> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
> sa = ss->sa;
>
> /* process packets, extract seq numbers */
> - k = process(sa, mb, sqn, dr, num);
> + k = process(sa, mb, sqn, dr, num, is_inline);
>
> /* handle unprocessed mbufs */
> if (k != num && k != 0)
> @@ -533,7 +555,14 @@ uint16_t
> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, tun_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
To make things a bit cleaner, can I suggest the following:
1. make esp_inb_pkt_process() take as a parameters:
const struct rte_ipsec_sa *sa (instead of const struct rte_ipsec_session *ss)
uint32_t sqh_len instead of is_inlne
So here it would become:
sa = ss->sa;
return esp_inb_pkt_process(ss, mb, num, sa->sqh_len, tun_process);
For inline it would be:
sa = ss->sa;
return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
> +}
> +
> +uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
> }
>
> /*
> @@ -543,5 +572,12 @@ uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num)
> {
> - return esp_inb_pkt_process(ss, mb, num, trs_process);
> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
> +}
> +
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num)
> +{
> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
> }
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..71a595e 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
That means we have to go through our 'next' list once again.
Seems suboptimal.
I think would be better to make outb_tun_pkt_prepare() to take sqh_len as extra parameter.
Then inside it we can just:
tlen = pdlen + sa->icv_len + sqh_len;
...
if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
return -ENOSPC;
...
ml->data_len += tlen;
mb->pkt_len += tlen;
...
pdofs += pdlen + sqh_len;
Same for transport mode.
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> uint32_t i, k, n, l2, l3;
> uint64_t sqn;
> rte_be64_t sqc;
> + struct rte_mbuf *ml;
> struct rte_ipsec_sa *sa;
> struct rte_cryptodev_sym_session *cs;
> union sym_op_data icv;
> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>
> /* success, setup crypto op */
> if (rc >= 0) {
> + /*
> + * if esn is used then high-order 32 bits are also
> + * used in ICV calculation but are not transmitted,
> + * update packet length to be consistent with auth
> + * data length and offset, this will be subtracted
> + * from packet length in post crypto processing
> + */
> + if (sa->sqh_len) {
> + mb[i]->pkt_len += sa->sqh_len;
> + ml = rte_pktmbuf_lastseg(mb[i]);
> + ml->data_len += sa->sqh_len;
> + }
> +
> outb_pkt_xprepare(sa, sqc, &icv);
> lksd_none_cop_prepare(cop[k], cs, mb[i]);
> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
> index 846e317..ff01358 100644
> --- a/lib/librte_ipsec/sa.c
> +++ b/lib/librte_ipsec/sa.c
> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
> switch (sa->type & msk) {
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
> - pf->process = esp_inb_tun_pkt_process;
> + pf->process = inline_inb_tun_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
> - pf->process = esp_inb_trs_pkt_process;
> + pf->process = inline_inb_trs_pkt_process;
> break;
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
> index ffb5fb4..20c0a65 100644
> --- a/lib/librte_ipsec/sa.h
> +++ b/lib/librte_ipsec/sa.h
> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> uint16_t
> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> +uint16_t
> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> struct rte_mbuf *mb[], uint16_t num);
>
> +uint16_t
> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
> + struct rte_mbuf *mb[], uint16_t num);
> +
> /* outbound processing */
>
> uint16_t
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len
2019-05-19 14:47 ` [dpdk-dev] " Ananyev, Konstantin
@ 2019-05-20 11:13 ` Lukas Bartosik
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-20 11:13 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
Hi Konstantin,
Thank you for the review.
I will send a revised patch which addresses your comments.
Thanks,
Lukasz
On 19.05.2019 16:47, Ananyev, Konstantin wrote:
>
> Hi Lukasz,
> Thanks for clarifications.
> Looks good in general.
> Few small comments below.
> Konstantin
>
>> When esn is used then high-order 32 bits are included in ICV
>> calculation however are not transmitted. Update packet length
>> to be consistent with auth data offset and length before crypto
>> operation. High-order 32 bits of esn will be removed from packet
>> length in crypto post processing.
>>
>> Change-Id: I5ba50e341059a8d6a5e4ce7c626dfe7b9173740b
>> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
>> ---
>> lib/librte_ipsec/esp_inb.c | 56 +++++++++++++++++++++++++++++++++++++--------
>> lib/librte_ipsec/esp_outb.c | 31 +++++++++++++++++++++++++
>> lib/librte_ipsec/sa.c | 4 ++--
>> lib/librte_ipsec/sa.h | 8 +++++++
>> 4 files changed, 87 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
>> index 4e0e12a..eb899e3 100644
>> --- a/lib/librte_ipsec/esp_inb.c
>> +++ b/lib/librte_ipsec/esp_inb.c
>> @@ -16,7 +16,8 @@
>> #include "pad.h"
>>
>> typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
>> - struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
>> + struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
>> + uint8_t is_inline);
>>
>> /*
>> * helper function to fill crypto_sym op for cipher+auth algorithms.
>> @@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
>> icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
>> icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
>>
>> + /*
>> + * if esn is used then high-order 32 bits are also used in ICV
>> + * calculation but are not transmitted, update packet length
>> + * to be consistent with auth data length and offset, this will
>> + * be subtracted from packet length in post crypto processing
>
> Here and in several other comments below - you repeat basically the same thing.
> Seems a bit excessive. I suppose just to put in in one place, or probably even
> in patch description will be enough.
>
>> + */
>> + mb->pkt_len += sa->sqh_len;
>> + ml->data_len += sa->sqh_len;
>> +
>> inb_pkt_xprepare(sa, sqn, icv);
>> return plen;
>> }
>> @@ -373,14 +383,20 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
>> */
>> static inline uint16_t
>> tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> uint32_t adj, i, k, tl;
>> uint32_t hl[num];
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>
> This comment seems a bit misleading, as we have remove not only sqh,
> but also icv, espt, padding.
>
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -420,7 +436,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> */
>> static inline uint16_t
>> trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> - uint32_t sqn[], uint32_t dr[], uint16_t num)
>> + uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t is_inline)
>> {
>> char *np;
>> uint32_t i, k, l2, tl;
>> @@ -428,7 +444,13 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
>> struct esp_tail espt[num];
>> struct rte_mbuf *ml[num];
>>
>> - const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
>> + /*
>> + * remove high-order 32 bits of esn from packet length
>> + * which was added before crypto processing, this doesn't
>> + * apply to inline case
>> + */
>> + const uint32_t tlen = sa->icv_len + sizeof(espt[0]) +
>> + (is_inline ? 0 : sa->sqh_len);
>> const uint32_t cofs = sa->ctp.cipher.offset;
>>
>> /*
>> @@ -496,8 +518,8 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
>> * process group of ESP inbound packets.
>> */
>> static inline uint16_t
>> -esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> - struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
>> +esp_inb_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> + uint16_t num, uint8_t is_inline, esp_inb_process_t process)
>> {
>> uint32_t k, n;
>> struct rte_ipsec_sa *sa;
>> @@ -507,7 +529,7 @@ esp_inb_pkt_process(const struct rte_ipsec_session *ss,
>> sa = ss->sa;
>>
>> /* process packets, extract seq numbers */
>> - k = process(sa, mb, sqn, dr, num);
>> + k = process(sa, mb, sqn, dr, num, is_inline);
>>
>> /* handle unprocessed mbufs */
>> if (k != num && k != 0)
>> @@ -533,7 +555,14 @@ uint16_t
>> esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, tun_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
>
> To make things a bit cleaner, can I suggest the following:
> 1. make esp_inb_pkt_process() take as a parameters:
> const struct rte_ipsec_sa *sa (instead of const struct rte_ipsec_session *ss)
> uint32_t sqh_len instead of is_inlne
>
> So here it would become:
>
> sa = ss->sa;
> return esp_inb_pkt_process(ss, mb, num, sa->sqh_len, tun_process);
>
> For inline it would be:
>
> sa = ss->sa;
> return esp_inb_pkt_process(ss, mb, num, 0, tun_process);
>
>
>> +}
>> +
>> +uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, tun_process);
>> }
>>
>> /*
>> @@ -543,5 +572,12 @@ uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num)
>> {
>> - return esp_inb_pkt_process(ss, mb, num, trs_process);
>> + return esp_inb_pkt_process(ss, mb, num, 0, trs_process);
>> +}
>> +
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num)
>> +{
>> + return esp_inb_pkt_process(ss, mb, num, 1, trs_process);
>> }
>> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
>> index c798bc4..71a595e 100644
>> --- a/lib/librte_ipsec/esp_outb.c
>> +++ b/lib/librte_ipsec/esp_outb.c
>> @@ -221,6 +221,7 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -246,6 +247,19 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>
> That means we have to go through our 'next' list once again.
> Seems suboptimal.
> I think would be better to make outb_tun_pkt_prepare() to take sqh_len as extra parameter.
> Then inside it we can just:
> tlen = pdlen + sa->icv_len + sqh_len;
> ...
> if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
> return -ENOSPC;
> ...
> ml->data_len += tlen;
> mb->pkt_len += tlen;
> ...
> pdofs += pdlen + sqh_len;
>
> Same for transport mode.
>
>
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, 0, rc);
>> @@ -356,6 +370,7 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> uint32_t i, k, n, l2, l3;
>> uint64_t sqn;
>> rte_be64_t sqc;
>> + struct rte_mbuf *ml;
>> struct rte_ipsec_sa *sa;
>> struct rte_cryptodev_sym_session *cs;
>> union sym_op_data icv;
>> @@ -384,6 +399,19 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>>
>> /* success, setup crypto op */
>> if (rc >= 0) {
>> + /*
>> + * if esn is used then high-order 32 bits are also
>> + * used in ICV calculation but are not transmitted,
>> + * update packet length to be consistent with auth
>> + * data length and offset, this will be subtracted
>> + * from packet length in post crypto processing
>> + */
>> + if (sa->sqh_len) {
>> + mb[i]->pkt_len += sa->sqh_len;
>> + ml = rte_pktmbuf_lastseg(mb[i]);
>> + ml->data_len += sa->sqh_len;
>> + }
>> +
>> outb_pkt_xprepare(sa, sqc, &icv);
>> lksd_none_cop_prepare(cop[k], cs, mb[i]);
>> outb_cop_prepare(cop[k], sa, iv, &icv, l2 + l3, rc);
>> @@ -425,6 +453,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> for (i = 0; i != num; i++) {
>> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
>> ml = rte_pktmbuf_lastseg(mb[i]);
>> + /* remove high-order 32 bits of esn from packet len */
>> + mb[i]->pkt_len -= sa->sqh_len;
>> + ml->data_len -= sa->sqh_len;
>> icv = rte_pktmbuf_mtod_offset(ml, void *,
>> ml->data_len - icv_len);
>> remove_sqh(icv, icv_len);
>> diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
>> index 846e317..ff01358 100644
>> --- a/lib/librte_ipsec/sa.c
>> +++ b/lib/librte_ipsec/sa.c
>> @@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
>> switch (sa->type & msk) {
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> - pf->process = esp_inb_tun_pkt_process;
>> + pf->process = inline_inb_tun_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
>> - pf->process = esp_inb_trs_pkt_process;
>> + pf->process = inline_inb_trs_pkt_process;
>> break;
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
>> case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
>> diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
>> index ffb5fb4..20c0a65 100644
>> --- a/lib/librte_ipsec/sa.h
>> +++ b/lib/librte_ipsec/sa.h
>> @@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> uint16_t
>> +inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> +uint16_t
>> esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> struct rte_mbuf *mb[], uint16_t num);
>>
>> +uint16_t
>> +inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
>> + struct rte_mbuf *mb[], uint16_t num);
>> +
>> /* outbound processing */
>>
>> uint16_t
>> --
>> 2.7.4
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v2] ipsec: include high order bytes of esn in pkt len
2019-04-30 14:55 [dpdk-dev] [PATCH] ipsec: include high order bytes of esn in pkt len Lukasz Bartosik
` (2 preceding siblings ...)
2019-05-19 14:47 ` [dpdk-dev] " Ananyev, Konstantin
@ 2019-05-23 12:11 ` Lukasz Bartosik
2019-05-30 16:51 ` Ananyev, Konstantin
2019-06-05 15:31 ` [dpdk-dev] [PATCH v3] " Lukasz Bartosik
3 siblings, 2 replies; 23+ messages in thread
From: Lukasz Bartosik @ 2019-05-23 12:11 UTC (permalink / raw)
To: konstantin.ananyev; +Cc: dev, anoobj, Lukasz Bartosik
When esn is used then high-order 32 bits are included in ICV
calculation however are not transmitted. Update packet length
to be consistent with auth data offset and length before crypto
operation. High-order 32 bits of esn will be removed from packet
length in crypto post processing.
Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
---
lib/librte_ipsec/esp_inb.c | 59 +++++++++++++++++++++++++++++++++++----------
lib/librte_ipsec/esp_outb.c | 19 +++++++++------
lib/librte_ipsec/sa.c | 4 +--
lib/librte_ipsec/sa.h | 8 ++++++
4 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
index 4e0e12a..23e4945 100644
--- a/lib/librte_ipsec/esp_inb.c
+++ b/lib/librte_ipsec/esp_inb.c
@@ -16,7 +16,8 @@
#include "pad.h"
typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
- struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
+ struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
+ uint8_t sqh_len);
/*
* helper function to fill crypto_sym op for cipher+auth algorithms.
@@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
+ /*
+ * if esn is used then high-order 32 bits are also used in ICV
+ * calculation but are not transmitted, update packet length
+ * to be consistent with auth data length and offset, this will
+ * be subtracted from packet length in post crypto processing
+ */
+ mb->pkt_len += sa->sqh_len;
+ ml->data_len += sa->sqh_len;
+
inb_pkt_xprepare(sa, sqn, icv);
return plen;
}
@@ -373,14 +383,18 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
*/
static inline uint16_t
tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t sqh_len)
{
uint32_t adj, i, k, tl;
uint32_t hl[num];
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove icv, esp trailer and high-order
+ * 32 bits of esn from packet length
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) + sqh_len;
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -420,7 +434,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
*/
static inline uint16_t
trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t sqh_len)
{
char *np;
uint32_t i, k, l2, tl;
@@ -428,7 +442,11 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove icv, esp trailer and high-order
+ * 32 bits of esn from packet length
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) + sqh_len;
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -496,18 +514,15 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
* process group of ESP inbound packets.
*/
static inline uint16_t
-esp_inb_pkt_process(const struct rte_ipsec_session *ss,
- struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
+esp_inb_pkt_process(struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
+ uint16_t num, uint8_t sqh_len, esp_inb_process_t process)
{
uint32_t k, n;
- struct rte_ipsec_sa *sa;
uint32_t sqn[num];
uint32_t dr[num];
- sa = ss->sa;
-
/* process packets, extract seq numbers */
- k = process(sa, mb, sqn, dr, num);
+ k = process(sa, mb, sqn, dr, num, sqh_len);
/* handle unprocessed mbufs */
if (k != num && k != 0)
@@ -533,7 +548,16 @@ uint16_t
esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, tun_process);
+ struct rte_ipsec_sa *sa = ss->sa;
+
+ return esp_inb_pkt_process(sa, mb, num, sa->sqh_len, tun_process);
+}
+
+uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss->sa, mb, num, 0, tun_process);
}
/*
@@ -543,5 +567,14 @@ uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, trs_process);
+ struct rte_ipsec_sa *sa = ss->sa;
+
+ return esp_inb_pkt_process(sa, mb, num, sa->sqh_len, trs_process);
+}
+
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss->sa, mb, num, 0, trs_process);
}
diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
index c798bc4..ed5974b 100644
--- a/lib/librte_ipsec/esp_outb.c
+++ b/lib/librte_ipsec/esp_outb.c
@@ -126,11 +126,11 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
/* pad length + esp tail */
pdlen = clen - plen;
- tlen = pdlen + sa->icv_len;
+ tlen = pdlen + sa->icv_len + sa->sqh_len;
/* do append and prepend */
ml = rte_pktmbuf_lastseg(mb);
- if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
+ if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
return -ENOSPC;
/* prepend header */
@@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
rte_memcpy(ph, sa->hdr, sa->hdr_len);
/* update original and new ip header fields */
- update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len, sa->hdr_l3_off,
- sqn_low16(sqc));
+ update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len - sa->sqh_len,
+ sa->hdr_l3_off, sqn_low16(sqc));
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + sa->hdr_len);
@@ -292,11 +292,11 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
/* pad length + esp tail */
pdlen = clen - plen;
- tlen = pdlen + sa->icv_len;
+ tlen = pdlen + sa->icv_len + sa->sqh_len;
/* do append and insert */
ml = rte_pktmbuf_lastseg(mb);
- if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
+ if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
return -ENOSPC;
/* prepend space for ESP header */
@@ -314,8 +314,8 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
insert_esph(ph, ph + hlen, uhlen);
/* update ip header fields */
- np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len, l2len, l3len,
- IPPROTO_ESP);
+ np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len - sa->sqh_len, l2len,
+ l3len, IPPROTO_ESP);
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + uhlen);
@@ -425,6 +425,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
for (i = 0; i != num; i++) {
if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
ml = rte_pktmbuf_lastseg(mb[i]);
+ /* remove high-order 32 bits of esn from packet len */
+ mb[i]->pkt_len -= sa->sqh_len;
+ ml->data_len -= sa->sqh_len;
icv = rte_pktmbuf_mtod_offset(ml, void *,
ml->data_len - icv_len);
remove_sqh(icv, icv_len);
diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
index 846e317..ff01358 100644
--- a/lib/librte_ipsec/sa.c
+++ b/lib/librte_ipsec/sa.c
@@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
switch (sa->type & msk) {
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
- pf->process = esp_inb_tun_pkt_process;
+ pf->process = inline_inb_tun_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
- pf->process = esp_inb_trs_pkt_process;
+ pf->process = inline_inb_trs_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
index ffb5fb4..20c0a65 100644
--- a/lib/librte_ipsec/sa.h
+++ b/lib/librte_ipsec/sa.h
@@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
+uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
/* outbound processing */
uint16_t
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH v2] ipsec: include high order bytes of esn in pkt len
2019-05-23 12:11 ` [dpdk-dev] [PATCH v2] " Lukasz Bartosik
@ 2019-05-30 16:51 ` Ananyev, Konstantin
2019-05-31 16:09 ` Lukas Bartosik
2019-06-05 15:31 ` [dpdk-dev] [PATCH v3] " Lukasz Bartosik
1 sibling, 1 reply; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-05-30 16:51 UTC (permalink / raw)
To: Lukasz Bartosik; +Cc: dev, anoobj
Hi Lukasz,
> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
> index c798bc4..ed5974b 100644
> --- a/lib/librte_ipsec/esp_outb.c
> +++ b/lib/librte_ipsec/esp_outb.c
> @@ -126,11 +126,11 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>
> /* pad length + esp tail */
> pdlen = clen - plen;
> - tlen = pdlen + sa->icv_len;
> + tlen = pdlen + sa->icv_len + sa->sqh_len;
We probably don't want to increase pkt_len by sa->sqh_len for inline case.
That's why I suggested to pass sqh_len as parameter to that function.
Then for inline we can just pass 0.
Do you see any obstacles with that approach?
Same thought for transport mode.
Konstantin
>
> /* do append and prepend */
> ml = rte_pktmbuf_lastseg(mb);
> - if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
> + if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
> return -ENOSPC;
>
> /* prepend header */
> @@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
> rte_memcpy(ph, sa->hdr, sa->hdr_len);
>
> /* update original and new ip header fields */
> - update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len, sa->hdr_l3_off,
> - sqn_low16(sqc));
> + update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len - sa->sqh_len,
> + sa->hdr_l3_off, sqn_low16(sqc));
>
> /* update spi, seqn and iv */
> esph = (struct esp_hdr *)(ph + sa->hdr_len);
> @@ -292,11 +292,11 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>
> /* pad length + esp tail */
> pdlen = clen - plen;
> - tlen = pdlen + sa->icv_len;
> + tlen = pdlen + sa->icv_len + sa->sqh_len;
>
> /* do append and insert */
> ml = rte_pktmbuf_lastseg(mb);
> - if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
> + if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
> return -ENOSPC;
>
> /* prepend space for ESP header */
> @@ -314,8 +314,8 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
> insert_esph(ph, ph + hlen, uhlen);
>
> /* update ip header fields */
> - np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len, l2len, l3len,
> - IPPROTO_ESP);
> + np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len - sa->sqh_len, l2len,
> + l3len, IPPROTO_ESP);
>
> /* update spi, seqn and iv */
> esph = (struct esp_hdr *)(ph + uhlen);
> @@ -425,6 +425,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
> for (i = 0; i != num; i++) {
> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
> ml = rte_pktmbuf_lastseg(mb[i]);
> + /* remove high-order 32 bits of esn from packet len */
> + mb[i]->pkt_len -= sa->sqh_len;
> + ml->data_len -= sa->sqh_len;
> icv = rte_pktmbuf_mtod_offset(ml, void *,
> ml->data_len - icv_len);
> remove_sqh(icv, icv_len);
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH v2] ipsec: include high order bytes of esn in pkt len
2019-05-30 16:51 ` Ananyev, Konstantin
@ 2019-05-31 16:09 ` Lukas Bartosik
0 siblings, 0 replies; 23+ messages in thread
From: Lukas Bartosik @ 2019-05-31 16:09 UTC (permalink / raw)
To: Ananyev, Konstantin; +Cc: dev, Anoob Joseph
Hi Konstantin
On 30.05.2019 18:51, Ananyev, Konstantin wrote:
> Hi Lukasz,
>
>> diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
>> index c798bc4..ed5974b 100644
>> --- a/lib/librte_ipsec/esp_outb.c
>> +++ b/lib/librte_ipsec/esp_outb.c
>> @@ -126,11 +126,11 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>
>> /* pad length + esp tail */
>> pdlen = clen - plen;
>> - tlen = pdlen + sa->icv_len;
>> + tlen = pdlen + sa->icv_len + sa->sqh_len;
>
> We probably don't want to increase pkt_len by sa->sqh_len for inline case.
> That's why I suggested to pass sqh_len as parameter to that function.
> Then for inline we can just pass 0.
> Do you see any obstacles with that approach?
> Same thought for transport mode.
> Konstantin
>
I agree this is incorrect. I have missed inline case.
I will send revised patch.
Thanks,
Lukasz
>>
>> /* do append and prepend */
>> ml = rte_pktmbuf_lastseg(mb);
>> - if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> + if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> return -ENOSPC;
>>
>> /* prepend header */
>> @@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>> rte_memcpy(ph, sa->hdr, sa->hdr_len);
>>
>> /* update original and new ip header fields */
>> - update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len, sa->hdr_l3_off,
>> - sqn_low16(sqc));
>> + update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len - sa->sqh_len,
>> + sa->hdr_l3_off, sqn_low16(sqc));
>>
>> /* update spi, seqn and iv */
>> esph = (struct esp_hdr *)(ph + sa->hdr_len);
>> @@ -292,11 +292,11 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>>
>> /* pad length + esp tail */
>> pdlen = clen - plen;
>> - tlen = pdlen + sa->icv_len;
>> + tlen = pdlen + sa->icv_len + sa->sqh_len;
>>
>> /* do append and insert */
>> ml = rte_pktmbuf_lastseg(mb);
>> - if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> + if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
>> return -ENOSPC;
>>
>> /* prepend space for ESP header */
>> @@ -314,8 +314,8 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
>> insert_esph(ph, ph + hlen, uhlen);
>>
>> /* update ip header fields */
>> - np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len, l2len, l3len,
>> - IPPROTO_ESP);
>> + np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len - sa->sqh_len, l2len,
>> + l3len, IPPROTO_ESP);
>>
>> /* update spi, seqn and iv */
>> esph = (struct esp_hdr *)(ph + uhlen);
>> @@ -425,6 +425,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
>> for (i = 0; i != num; i++) {
>> if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
>> ml = rte_pktmbuf_lastseg(mb[i]);
>> + /* remove high-order 32 bits of esn from packet len */
>> + mb[i]->pkt_len -= sa->sqh_len;
>> + ml->data_len -= sa->sqh_len;
>> icv = rte_pktmbuf_mtod_offset(ml, void *,
>> ml->data_len - icv_len);
>> remove_sqh(icv, icv_len);
^ permalink raw reply [flat|nested] 23+ messages in thread
* [dpdk-dev] [PATCH v3] ipsec: include high order bytes of esn in pkt len
2019-05-23 12:11 ` [dpdk-dev] [PATCH v2] " Lukasz Bartosik
2019-05-30 16:51 ` Ananyev, Konstantin
@ 2019-06-05 15:31 ` Lukasz Bartosik
2019-06-06 14:45 ` Ananyev, Konstantin
1 sibling, 1 reply; 23+ messages in thread
From: Lukasz Bartosik @ 2019-06-05 15:31 UTC (permalink / raw)
To: konstantin.ananyev; +Cc: dev, anoobj, Lukasz Bartosik
When esn is used then high-order 32 bits are included in ICV
calculation however are not transmitted. Update packet length
to be consistent with auth data offset and length before crypto
operation. High-order 32 bits of esn will be removed from packet
length in crypto post processing.
Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
---
lib/librte_ipsec/esp_inb.c | 59 +++++++++++++++++++++++++++++++++++----------
lib/librte_ipsec/esp_outb.c | 36 +++++++++++++++------------
lib/librte_ipsec/sa.c | 4 +--
lib/librte_ipsec/sa.h | 8 ++++++
4 files changed, 76 insertions(+), 31 deletions(-)
diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c
index 4e0e12a..23e4945 100644
--- a/lib/librte_ipsec/esp_inb.c
+++ b/lib/librte_ipsec/esp_inb.c
@@ -16,7 +16,8 @@
#include "pad.h"
typedef uint16_t (*esp_inb_process_t)(const struct rte_ipsec_sa *sa,
- struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num);
+ struct rte_mbuf *mb[], uint32_t sqn[], uint32_t dr[], uint16_t num,
+ uint8_t sqh_len);
/*
* helper function to fill crypto_sym op for cipher+auth algorithms.
@@ -181,6 +182,15 @@ inb_pkt_prepare(const struct rte_ipsec_sa *sa, const struct replay_sqn *rsn,
icv->va = rte_pktmbuf_mtod_offset(ml, void *, icv_ofs);
icv->pa = rte_pktmbuf_iova_offset(ml, icv_ofs);
+ /*
+ * if esn is used then high-order 32 bits are also used in ICV
+ * calculation but are not transmitted, update packet length
+ * to be consistent with auth data length and offset, this will
+ * be subtracted from packet length in post crypto processing
+ */
+ mb->pkt_len += sa->sqh_len;
+ ml->data_len += sa->sqh_len;
+
inb_pkt_xprepare(sa, sqn, icv);
return plen;
}
@@ -373,14 +383,18 @@ tun_process_step3(struct rte_mbuf *mb, uint64_t txof_msk, uint64_t txof_val)
*/
static inline uint16_t
tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t sqh_len)
{
uint32_t adj, i, k, tl;
uint32_t hl[num];
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove icv, esp trailer and high-order
+ * 32 bits of esn from packet length
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) + sqh_len;
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -420,7 +434,7 @@ tun_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
*/
static inline uint16_t
trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
- uint32_t sqn[], uint32_t dr[], uint16_t num)
+ uint32_t sqn[], uint32_t dr[], uint16_t num, uint8_t sqh_len)
{
char *np;
uint32_t i, k, l2, tl;
@@ -428,7 +442,11 @@ trs_process(const struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
struct esp_tail espt[num];
struct rte_mbuf *ml[num];
- const uint32_t tlen = sa->icv_len + sizeof(espt[0]);
+ /*
+ * remove icv, esp trailer and high-order
+ * 32 bits of esn from packet length
+ */
+ const uint32_t tlen = sa->icv_len + sizeof(espt[0]) + sqh_len;
const uint32_t cofs = sa->ctp.cipher.offset;
/*
@@ -496,18 +514,15 @@ esp_inb_rsn_update(struct rte_ipsec_sa *sa, const uint32_t sqn[],
* process group of ESP inbound packets.
*/
static inline uint16_t
-esp_inb_pkt_process(const struct rte_ipsec_session *ss,
- struct rte_mbuf *mb[], uint16_t num, esp_inb_process_t process)
+esp_inb_pkt_process(struct rte_ipsec_sa *sa, struct rte_mbuf *mb[],
+ uint16_t num, uint8_t sqh_len, esp_inb_process_t process)
{
uint32_t k, n;
- struct rte_ipsec_sa *sa;
uint32_t sqn[num];
uint32_t dr[num];
- sa = ss->sa;
-
/* process packets, extract seq numbers */
- k = process(sa, mb, sqn, dr, num);
+ k = process(sa, mb, sqn, dr, num, sqh_len);
/* handle unprocessed mbufs */
if (k != num && k != 0)
@@ -533,7 +548,16 @@ uint16_t
esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, tun_process);
+ struct rte_ipsec_sa *sa = ss->sa;
+
+ return esp_inb_pkt_process(sa, mb, num, sa->sqh_len, tun_process);
+}
+
+uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss->sa, mb, num, 0, tun_process);
}
/*
@@ -543,5 +567,14 @@ uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num)
{
- return esp_inb_pkt_process(ss, mb, num, trs_process);
+ struct rte_ipsec_sa *sa = ss->sa;
+
+ return esp_inb_pkt_process(sa, mb, num, sa->sqh_len, trs_process);
+}
+
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num)
+{
+ return esp_inb_pkt_process(ss->sa, mb, num, 0, trs_process);
}
diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c
index c798bc4..71bc3a6 100644
--- a/lib/librte_ipsec/esp_outb.c
+++ b/lib/librte_ipsec/esp_outb.c
@@ -104,7 +104,7 @@ outb_cop_prepare(struct rte_crypto_op *cop,
static inline int32_t
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)
+ union sym_op_data *icv, uint8_t sqh_len)
{
uint32_t clen, hlen, l2len, pdlen, pdofs, plen, tlen;
struct rte_mbuf *ml;
@@ -126,11 +126,11 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
/* pad length + esp tail */
pdlen = clen - plen;
- tlen = pdlen + sa->icv_len;
+ tlen = pdlen + sa->icv_len + sqh_len;
/* do append and prepend */
ml = rte_pktmbuf_lastseg(mb);
- if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
+ if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
return -ENOSPC;
/* prepend header */
@@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
rte_memcpy(ph, sa->hdr, sa->hdr_len);
/* update original and new ip header fields */
- update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len, sa->hdr_l3_off,
- sqn_low16(sqc));
+ update_tun_l3hdr(sa, ph + sa->hdr_l3_off, mb->pkt_len - sqh_len,
+ sa->hdr_l3_off, sqn_low16(sqc));
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + sa->hdr_len);
@@ -242,8 +242,8 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
gen_iv(iv, sqc);
/* try to update the packet itself */
- rc = outb_tun_pkt_prepare(sa, sqc, iv, mb[i], &icv);
-
+ rc = outb_tun_pkt_prepare(sa, sqc, iv, mb[i], &icv,
+ sa->sqh_len);
/* success, setup crypto op */
if (rc >= 0) {
outb_pkt_xprepare(sa, sqc, &icv);
@@ -270,7 +270,8 @@ esp_outb_tun_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
static inline int32_t
outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
const uint64_t ivp[IPSEC_MAX_IV_QWORD], struct rte_mbuf *mb,
- uint32_t l2len, uint32_t l3len, union sym_op_data *icv)
+ uint32_t l2len, uint32_t l3len, union sym_op_data *icv,
+ uint8_t sqh_len)
{
uint8_t np;
uint32_t clen, hlen, pdlen, pdofs, plen, tlen, uhlen;
@@ -292,11 +293,11 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
/* pad length + esp tail */
pdlen = clen - plen;
- tlen = pdlen + sa->icv_len;
+ tlen = pdlen + sa->icv_len + sqh_len;
/* do append and insert */
ml = rte_pktmbuf_lastseg(mb);
- if (tlen + sa->sqh_len + sa->aad_len > rte_pktmbuf_tailroom(ml))
+ if (tlen + sa->aad_len > rte_pktmbuf_tailroom(ml))
return -ENOSPC;
/* prepend space for ESP header */
@@ -314,8 +315,8 @@ outb_trs_pkt_prepare(struct rte_ipsec_sa *sa, rte_be64_t sqc,
insert_esph(ph, ph + hlen, uhlen);
/* update ip header fields */
- np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len, l2len, l3len,
- IPPROTO_ESP);
+ np = update_trs_l3hdr(sa, ph + l2len, mb->pkt_len - sqh_len, l2len,
+ l3len, IPPROTO_ESP);
/* update spi, seqn and iv */
esph = (struct esp_hdr *)(ph + uhlen);
@@ -380,8 +381,8 @@ esp_outb_trs_prepare(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
gen_iv(iv, sqc);
/* try to update the packet itself */
- rc = outb_trs_pkt_prepare(sa, sqc, iv, mb[i], l2, l3, &icv);
-
+ rc = outb_trs_pkt_prepare(sa, sqc, iv, mb[i], l2, l3, &icv,
+ sa->sqh_len);
/* success, setup crypto op */
if (rc >= 0) {
outb_pkt_xprepare(sa, sqc, &icv);
@@ -425,6 +426,9 @@ esp_outb_sqh_process(const struct rte_ipsec_session *ss, struct rte_mbuf *mb[],
for (i = 0; i != num; i++) {
if ((mb[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) == 0) {
ml = rte_pktmbuf_lastseg(mb[i]);
+ /* remove high-order 32 bits of esn from packet len */
+ mb[i]->pkt_len -= sa->sqh_len;
+ ml->data_len -= sa->sqh_len;
icv = rte_pktmbuf_mtod_offset(ml, void *,
ml->data_len - icv_len);
remove_sqh(icv, icv_len);
@@ -494,7 +498,7 @@ inline_outb_tun_pkt_process(const struct rte_ipsec_session *ss,
gen_iv(iv, sqc);
/* try to update the packet itself */
- rc = outb_tun_pkt_prepare(sa, sqc, iv, mb[i], &icv);
+ rc = outb_tun_pkt_prepare(sa, sqc, iv, mb[i], &icv, 0);
k += (rc >= 0);
@@ -548,7 +552,7 @@ inline_outb_trs_pkt_process(const struct rte_ipsec_session *ss,
/* try to update the packet itself */
rc = outb_trs_pkt_prepare(sa, sqc, iv, mb[i],
- l2, l3, &icv);
+ l2, l3, &icv, 0);
k += (rc >= 0);
diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c
index 846e317..ff01358 100644
--- a/lib/librte_ipsec/sa.c
+++ b/lib/librte_ipsec/sa.c
@@ -610,10 +610,10 @@ inline_crypto_pkt_func_select(const struct rte_ipsec_sa *sa,
switch (sa->type & msk) {
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TUNLV6):
- pf->process = esp_inb_tun_pkt_process;
+ pf->process = inline_inb_tun_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_IB | RTE_IPSEC_SATP_MODE_TRANS):
- pf->process = esp_inb_trs_pkt_process;
+ pf->process = inline_inb_trs_pkt_process;
break;
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV4):
case (RTE_IPSEC_SATP_DIR_OB | RTE_IPSEC_SATP_MODE_TUNLV6):
diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h
index ffb5fb4..20c0a65 100644
--- a/lib/librte_ipsec/sa.h
+++ b/lib/librte_ipsec/sa.h
@@ -143,9 +143,17 @@ esp_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
uint16_t
+inline_inb_tun_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
+uint16_t
esp_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
struct rte_mbuf *mb[], uint16_t num);
+uint16_t
+inline_inb_trs_pkt_process(const struct rte_ipsec_session *ss,
+ struct rte_mbuf *mb[], uint16_t num);
+
/* outbound processing */
uint16_t
--
2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [dpdk-dev] [PATCH v3] ipsec: include high order bytes of esn in pkt len
2019-06-05 15:31 ` [dpdk-dev] [PATCH v3] " Lukasz Bartosik
@ 2019-06-06 14:45 ` Ananyev, Konstantin
2019-06-20 13:25 ` Akhil Goyal
0 siblings, 1 reply; 23+ messages in thread
From: Ananyev, Konstantin @ 2019-06-06 14:45 UTC (permalink / raw)
To: Lukasz Bartosik; +Cc: dev, anoobj
> -----Original Message-----
> From: Lukasz Bartosik [mailto:lbartosik@marvell.com]
> Sent: Wednesday, June 5, 2019 4:31 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; anoobj@marvell.com; Lukasz Bartosik <lbartosik@marvell.com>
> Subject: [PATCH v3] ipsec: include high order bytes of esn in pkt len
>
> When esn is used then high-order 32 bits are included in ICV
> calculation however are not transmitted. Update packet length
> to be consistent with auth data offset and length before crypto
> operation. High-order 32 bits of esn will be removed from packet
> length in crypto post processing.
>
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ipsec: include high order bytes of esn in pkt len
2019-06-06 14:45 ` Ananyev, Konstantin
@ 2019-06-20 13:25 ` Akhil Goyal
2019-06-25 12:49 ` Akhil Goyal
0 siblings, 1 reply; 23+ messages in thread
From: Akhil Goyal @ 2019-06-20 13:25 UTC (permalink / raw)
To: Ananyev, Konstantin, Lukasz Bartosik; +Cc: dev, anoobj
> >
> > When esn is used then high-order 32 bits are included in ICV
> > calculation however are not transmitted. Update packet length
> > to be consistent with auth data offset and length before crypto
> > operation. High-order 32 bits of esn will be removed from packet
> > length in crypto post processing.
> >
> > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > ---
>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [dpdk-dev] [PATCH v3] ipsec: include high order bytes of esn in pkt len
2019-06-20 13:25 ` Akhil Goyal
@ 2019-06-25 12:49 ` Akhil Goyal
0 siblings, 0 replies; 23+ messages in thread
From: Akhil Goyal @ 2019-06-25 12:49 UTC (permalink / raw)
To: Akhil Goyal, Ananyev, Konstantin, Lukasz Bartosik; +Cc: dev, anoobj
>
> > >
> > > When esn is used then high-order 32 bits are included in ICV
> > > calculation however are not transmitted. Update packet length
> > > to be consistent with auth data offset and length before crypto
> > > operation. High-order 32 bits of esn will be removed from packet
> > > length in crypto post processing.
> > >
> > > Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> > > ---
> >
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
Applied to dpdk-next-crypto
Thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread