From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 09CF6A05D3 for ; Sun, 19 May 2019 18:26:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D14444F91; Sun, 19 May 2019 18:26:33 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 052C0152A for ; Sun, 19 May 2019 18:26:31 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 May 2019 09:26:30 -0700 X-ExtLoop1: 1 Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by orsmga003.jf.intel.com with ESMTP; 19 May 2019 09:26:29 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.155]) by IRSMSX102.ger.corp.intel.com ([169.254.2.108]) with mapi id 14.03.0415.000; Sun, 19 May 2019 17:26:28 +0100 From: "Ananyev, Konstantin" To: "Kovacevic, Marko" , "dev@dpdk.org" CC: "akhil.goyal@nxp.com" , "Zhang, Roy Fan" Thread-Topic: [PATCH v1] lib/ipsec: add support for header construction Thread-Index: AQHVDMoVocEtyiYEQkaKrIs9UmYIB6ZyoI3A Date: Sun, 19 May 2019 16:26:28 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772580161635D40@irsmsx105.ger.corp.intel.com> References: <20190517160319.2468-1-marko.kovacevic@intel.com> In-Reply-To: <20190517160319.2468-1-marko.kovacevic@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjBmNmMwYWQtMzk3MC00ZWRiLWExODYtNWZlZWY2Y2JkMTI0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRXpmeHJXR0poS2UyN3N1WVRkUlFEZDlvYnVtZHRwZ3QxS3lQTmpvZkZTMytOcE1jNVIxYnJEbTd4V0crZHZaaCJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v1] lib/ipsec: add support for header construction X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, >=20 > Add support for RFC 4301(5.1.2) to update of > Type of service field and Traffic class field > bits inside ipv4/ipv6 packets for outbound cases > and inbound cases which deals with the update of > the DSCP/ENC bits inside each of the fields. >=20 > Signed-off-by: Marko Kovacevic > --- > examples/ipsec-secgw/sa.c | 2 + > lib/librte_ipsec/esp_inb.c | 14 ++++- > lib/librte_ipsec/esp_outb.c | 4 +- > lib/librte_ipsec/iph.h | 119 +++++++++++++++++++++++++++++++= ++++-- > lib/librte_ipsec/rte_ipsec_sa.h | 25 ++++++++ > lib/librte_ipsec/sa.c | 17 ++++++ > lib/librte_ipsec/sa.h | 2 + > lib/librte_net/rte_ip.h | 8 +++ > lib/librte_security/rte_security.h | 9 +++ > 9 files changed, 191 insertions(+), 9 deletions(-) Looks good in general, some generic comments: - I think it is better to split the patch into few sub-pathces: One for rte_security, second for rte_net, third - rte_ipsec, forth - exam= ples/ipsec-secgw - Would be good to add support for other options too (ttl, etc.) - Would be good to add new test-case for it into examples/ipsec-secgw/test/ Plus few nits in the code below. Konstantin >=20 > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c > index b850e9839..4d85d09df 100644 > --- a/examples/ipsec-secgw/sa.c > +++ b/examples/ipsec-secgw/sa.c > @@ -991,6 +991,8 @@ fill_ipsec_sa_prm(struct rte_ipsec_sa_prm *prm, const= struct ipsec_sa *ss, > prm->ipsec_xform.mode =3D (ss->flags =3D=3D TRANSPORT) ? > RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT : > RTE_SECURITY_IPSEC_SA_MODE_TUNNEL; > + prm->ipsec_xform.options.ecn =3D 1; > + prm->ipsec_xform.options.copy_dscp =3D 1; >=20 > if (ss->flags =3D=3D IP4_TUNNEL) { > prm->ipsec_xform.tunnel.type =3D RTE_SECURITY_IPSEC_TUNNEL_IPV4; > diff --git a/lib/librte_ipsec/esp_inb.c b/lib/librte_ipsec/esp_inb.c > index 4e0e12a85..8a3cb8a15 100644 > --- a/lib/librte_ipsec/esp_inb.c > +++ b/lib/librte_ipsec/esp_inb.c > @@ -377,9 +377,10 @@ tun_process(const struct rte_ipsec_sa *sa, struct rt= e_mbuf *mb[], > { > uint32_t adj, i, k, tl; > uint32_t hl[num]; > + void *inner_h; > + const void *outter_h; > struct esp_tail espt[num]; > struct rte_mbuf *ml[num]; > - > const uint32_t tlen =3D sa->icv_len + sizeof(espt[0]); > const uint32_t cofs =3D sa->ctp.cipher.offset; >=20 > @@ -400,9 +401,16 @@ tun_process(const struct rte_ipsec_sa *sa, struct rt= e_mbuf *mb[], > if (tun_process_check(mb[i], ml[i], espt[i], adj, tl, > sa->proto) =3D=3D 0) { >=20 > + outter_h =3D rte_pktmbuf_mtod_offset(mb[i], uint8_t *, > + mb[i]->l2_len); > + > /* modify packet's layout */ > - tun_process_step2(mb[i], ml[i], hl[i], adj, > - tl, sqn + k); > + inner_h =3D tun_process_step2(mb[i], ml[i], hl[i], adj, > + tl, sqn + k); > + > + if ((sa->type & INB_TUN_HDR_MSK) !=3D 0) > + update_inb_tun_l3_hdr(sa, inner_h, outter_h); > + > /* update mbuf's metadata */ > tun_process_step3(mb[i], sa->tx_offload.msk, > sa->tx_offload.val); > diff --git a/lib/librte_ipsec/esp_outb.c b/lib/librte_ipsec/esp_outb.c > index c798bc4c4..a71164e0c 100644 > --- a/lib/librte_ipsec/esp_outb.c > +++ b/lib/librte_ipsec/esp_outb.c > @@ -152,8 +152,8 @@ outb_tun_pkt_prepare(struct rte_ipsec_sa *sa, rte_be6= 4_t sqc, > rte_memcpy(ph, sa->hdr, sa->hdr_len); >=20 > /* 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_outb_tun_l3hdr(sa, ph + sa->hdr_l3_off, ph + hlen, mb->pkt_len, > + sa->hdr_l3_off, sqn_low16(sqc)); >=20 > /* update spi, seqn and iv */ > esph =3D (struct esp_hdr *)(ph + sa->hdr_len); > diff --git a/lib/librte_ipsec/iph.h b/lib/librte_ipsec/iph.h > index 58930cf18..f45db5d4a 100644 > --- a/lib/librte_ipsec/iph.h > +++ b/lib/librte_ipsec/iph.h > @@ -11,6 +11,11 @@ > * used internally by ipsec library. > */ >=20 > +#define IPV6_DSCP_MASK (DSCP_MASK << IPV6_HDR_TC_SHIFT) > +#define IPV6_ECN_MASK (ECN_MASK << IPV6_HDR_TC_SHIFT) > +#define IPV6_TOS_MASK (IPV6_ECN_MASK | IPV6_DSCP_MASK) > +#define IPV6_ECN_CE IPV6_ECN_MASK > + > /* > * Move preceding (L3) headers down to remove ESP header and IV. > */ > @@ -35,6 +40,26 @@ insert_esph(char *np, char *op, uint32_t hlen) > np[i] =3D op[i]; > } >=20 > +static inline uint8_t > +get_ipv6_tos(rte_be32_t vtc_flow) > +{ > + uint32_t v; > + > + v =3D rte_be_to_cpu_32(vtc_flow); > + return v >> IPV6_HDR_TC_SHIFT; > +} > + > +static inline rte_be32_t > +set_ipv6_tos(rte_be32_t vtc_flow, uint32_t tos) > +{ > + uint32_t v; > + > + v =3D rte_cpu_to_be_32(tos << IPV6_HDR_TC_SHIFT); > + vtc_flow &=3D ~rte_cpu_to_be_32(IPV6_TOS_MASK); > + > + return (v | vtc_flow); > +} > + > /* update original ip header fields for transport case */ > static inline int > update_trs_l3hdr(const struct rte_ipsec_sa *sa, void *p, uint32_t plen, > @@ -64,20 +89,106 @@ update_trs_l3hdr(const struct rte_ipsec_sa *sa, void= *p, uint32_t plen, >=20 > /* update original and new ip header fields for tunnel case */ > static inline void > -update_tun_l3hdr(const struct rte_ipsec_sa *sa, void *p, uint32_t plen, > - uint32_t l2len, rte_be16_t pid) > +update_outb_tun_l3hdr(const struct rte_ipsec_sa *sa, void *outh, > + const void *inh, uint32_t plen, uint32_t l2len, rte_be16_t pid) > { > struct ipv4_hdr *v4h; > struct ipv6_hdr *v6h; > + uint32_t itp, otp; > + const struct ipv4_hdr *v4in_h; > + const struct ipv6_hdr *v6in_h; >=20 > if (sa->type & RTE_IPSEC_SATP_MODE_TUNLV4) { > - v4h =3D p; > + v4h =3D outh; > v4h->packet_id =3D pid; > v4h->total_length =3D rte_cpu_to_be_16(plen - l2len); I think it makes sense to invoke the code below, only when: ((sa->type & INB_TUN_HDR_MSK) !=3D 0) Same as we doing for onbound. Also probably worth to put it into a separate inline function. > + > + if (sa->proto =3D=3D IPPROTO_IPIP) { For consistency with the check above, seems a bit better: if ((sa->type & RTE_IPSEC_SATP_IPV_MASK) =3D=3D RTE_IPSEC_SATP_IPV4) > + /* ipv4 inner header */ > + v4in_h =3D inh; > + > + otp =3D v4h->type_of_service & ~sa->tos_mask; > + itp =3D v4in_h->type_of_service & sa->tos_mask; > + v4h->type_of_service =3D (otp | itp); > + } else { > + /* ipv6 inner header */ > + v6in_h =3D inh; > + > + otp =3D v4h->type_of_service & ~sa->tos_mask; > + itp =3D get_ipv6_tos(v6in_h->vtc_flow) & sa->tos_mask; > + v4h->type_of_service =3D (otp | itp); > + } > } else { > - v6h =3D p; > + v6h =3D outh; > v6h->payload_len =3D rte_cpu_to_be_16(plen - l2len - > sizeof(*v6h)); > + > + if (sa->proto =3D=3D IPPROTO_IPIP) { Same comment as above here. > + /* ipv4 inner header */ > + v4in_h =3D inh; > + > + otp =3D get_ipv6_tos(v6h->vtc_flow) & ~sa->tos_mask; > + itp =3D v4in_h->type_of_service & sa->tos_mask; > + v6h->vtc_flow =3D set_ipv6_tos(v6h->vtc_flow, otp | itp); > + } else { > + /* ipv6 inner header */ > + v6in_h =3D inh; > + > + otp =3D get_ipv6_tos(v6h->vtc_flow) & ~sa->tos_mask; > + itp =3D get_ipv6_tos(v6in_h->vtc_flow) & sa->tos_mask; > + v6h->vtc_flow =3D set_ipv6_tos(v6h->vtc_flow, otp | itp); > + } > + } > +} > + > +static inline void > +update_inb_tun_l3_hdr(const struct rte_ipsec_sa *sa, void *ip_inner, > + const void *ip_outter) > +{ > + struct ipv4_hdr *inner_v4h; > + const struct ipv4_hdr *outter_v4h; > + struct ipv6_hdr *inner_v6h; > + const struct ipv6_hdr *outter_v6h; > + uint8_t ecn_v4out, ecn_v4in; > + uint32_t ecn_v6out, ecn_v6in; > + > + inner_v4h =3D ip_inner; > + outter_v4h =3D ip_outter; > + > + inner_v6h =3D ip_inner; > + outter_v6h =3D ip_outter; > + > + /* */ > + if (sa->type & RTE_IPSEC_SATP_MODE_TUNLV4) { > + > + ecn_v4out =3D outter_v4h->type_of_service & ECN_MASK; > + > + if ((sa->type & RTE_IPSEC_SATP_IPV_MASK) =3D=3D RTE_IPSEC_SATP_IPV4) { > + ecn_v4in =3D inner_v4h->type_of_service & ECN_MASK; > + if (ecn_v4out =3D=3D ECN_CE && ecn_v4in !=3D 0) > + inner_v4h->type_of_service |=3D ECN_CE; > + } else { > + ecn_v6in =3D inner_v6h->vtc_flow & > + rte_cpu_to_be_32(IPV6_ECN_MASK); > + if (ecn_v4out =3D=3D ECN_CE && ecn_v6in !=3D 0) > + inner_v6h->vtc_flow |=3D > + rte_cpu_to_be_32(IPV6_ECN_CE); > + } > + } else { > + ecn_v6out =3D outter_v6h->vtc_flow & > + rte_cpu_to_be_32(IPV6_ECN_MASK); > + > + if ((sa->type & RTE_IPSEC_SATP_IPV_MASK) =3D=3D RTE_IPSEC_SATP_IPV6) { > + ecn_v6in =3D inner_v6h->vtc_flow & > + rte_cpu_to_be_32(IPV6_ECN_MASK); > + if (ecn_v6out =3D=3D IPV6_ECN_CE && ecn_v6in !=3D 0) > + inner_v6h->vtc_flow |=3D > + rte_cpu_to_be_32(IPV6_ECN_CE); > + } else { > + ecn_v4in =3D inner_v4h->type_of_service & ECN_MASK; > + if (ecn_v6out =3D=3D ECN_CE && ecn_v4in !=3D 0) > + inner_v4h->type_of_service |=3D ECN_CE; > + } > } > } >=20 > diff --git a/lib/librte_ipsec/rte_ipsec_sa.h b/lib/librte_ipsec/rte_ipsec= _sa.h > index fd9b3ed60..8f179ee9d 100644 > --- a/lib/librte_ipsec/rte_ipsec_sa.h > +++ b/lib/librte_ipsec/rte_ipsec_sa.h > @@ -95,6 +95,11 @@ enum { > RTE_SATP_LOG2_MODE, > RTE_SATP_LOG2_SQN =3D RTE_SATP_LOG2_MODE + 2, > RTE_SATP_LOG2_ESN, > + RTE_SATP_LOG2_ECN, > + RTE_SATP_LOG2_DSCP, > + RTE_SATP_LOG2_TTL, > + RTE_SATP_LOG2_DF, > + RTE_SATP_LOG2_FLABEL, > RTE_SATP_LOG2_NUM > }; >=20 > @@ -123,6 +128,26 @@ enum { > #define RTE_IPSEC_SATP_ESN_DISABLE (0ULL << RTE_SATP_LOG2_ESN) > #define RTE_IPSEC_SATP_ESN_ENABLE (1ULL << RTE_SATP_LOG2_ESN) >=20 > +#define RTE_IPSEC_SATP_ECN_MASK (1ULL << RTE_SATP_LOG2_ECN) > +#define RTE_IPSEC_SATP_ECN_DISABLE (0ULL << RTE_SATP_LOG2_ECN) > +#define RTE_IPSEC_SATP_ECN_ENABLE (1ULL << RTE_SATP_LOG2_ECN) > + > +#define RTE_IPSEC_SATP_DSCP_MASK (1ULL << RTE_SATP_LOG2_DSCP) > +#define RTE_IPSEC_SATP_DSCP_DISABLE (0ULL << RTE_SATP_LOG2_DSCP) > +#define RTE_IPSEC_SATP_DSCP_ENABLE (1ULL << RTE_SATP_LOG2_DSCP) > + > +#define RTE_IPSEC_SATP_TTL_MASK (1ULL << RTE_SATP_LOG2_TTL) > +#define RTE_IPSEC_SATP_TTL_DISABLE (0ULL << RTE_SATP_LOG2_TTL) > +#define RTE_IPSEC_SATP_TTL_ENABLE (1ULL << RTE_SATP_LOG2_TTL) > + > +#define RTE_IPSEC_SATP_DF_MASK (1ULL << RTE_SATP_LOG2_DF) > +#define RTE_IPSEC_SATP_DF_DISABLE (0ULL << RTE_SATP_LOG2_DF) > +#define RTE_IPSEC_SATP_DF_ENABLE (1ULL << RTE_SATP_LOG2_DF) > + > +#define RTE_IPSEC_SATP_FLABEL_MASK (1ULL << RTE_SATP_LOG2_FLABEL) > +#define RTE_IPSEC_SATP_FLABEL_DISABLE (0ULL << RTE_SATP_LOG2_FLABEL) > +#define RTE_IPSEC_SATP_FLABEL_ENABLE (1ULL << RTE_SATP_LOG2_FLABEL) > + > /** > * get type of given SA > * @return > diff --git a/lib/librte_ipsec/sa.c b/lib/librte_ipsec/sa.c > index 846e317fe..d48acd117 100644 > --- a/lib/librte_ipsec/sa.c > +++ b/lib/librte_ipsec/sa.c > @@ -220,6 +220,17 @@ fill_sa_type(const struct rte_ipsec_sa_prm *prm, uin= t64_t *type) > else > tp |=3D RTE_IPSEC_SATP_SQN_RAW; >=20 > + /* check for ECN flag */ > + if (prm->ipsec_xform.options.ecn =3D=3D 0) > + tp |=3D RTE_IPSEC_SATP_ECN_DISABLE; > + else > + tp |=3D RTE_IPSEC_SATP_ECN_ENABLE; > + /* check for DSCP flag */ > + if (prm->ipsec_xform.options.copy_dscp =3D=3D 0) > + tp |=3D RTE_IPSEC_SATP_DSCP_DISABLE; > + else > + tp |=3D RTE_IPSEC_SATP_DSCP_ENABLE; > + > *type =3D tp; > return 0; > } > @@ -308,6 +319,12 @@ esp_sa_init(struct rte_ipsec_sa *sa, const struct rt= e_ipsec_sa_prm *prm, > static const uint64_t msk =3D RTE_IPSEC_SATP_DIR_MASK | > RTE_IPSEC_SATP_MODE_MASK; >=20 > + if (prm->ipsec_xform.options.ecn) > + sa->tos_mask |=3D ECN_MASK; > + > + if (prm->ipsec_xform.options.copy_dscp) > + sa->tos_mask |=3D DSCP_MASK; > + > if (cxf->aead !=3D NULL) { > switch (cxf->aead->algo) { > case RTE_CRYPTO_AEAD_AES_GCM: > diff --git a/lib/librte_ipsec/sa.h b/lib/librte_ipsec/sa.h > index ffb5fb4f8..41e0b78c9 100644 > --- a/lib/librte_ipsec/sa.h > +++ b/lib/librte_ipsec/sa.h > @@ -10,6 +10,7 @@ > #define IPSEC_MAX_HDR_SIZE 64 > #define IPSEC_MAX_IV_SIZE 16 > #define IPSEC_MAX_IV_QWORD (IPSEC_MAX_IV_SIZE / sizeof(uint64_t)) > +#define INB_TUN_HDR_MSK (RTE_IPSEC_SATP_ECN_MASK | RTE_IPSEC_SATP_DSCP_M= ASK) >=20 > /* padding alignment for different algorithms */ > enum { > @@ -103,6 +104,7 @@ struct rte_ipsec_sa { > uint8_t iv_ofs; /* offset for algo-specific IV inside crypto op */ > uint8_t iv_len; > uint8_t pad_align; > + uint8_t tos_mask; >=20 > /* template for tunnel header */ > uint8_t hdr[IPSEC_MAX_HDR_SIZE]; > diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h > index f9b909090..6592637f7 100644 > --- a/lib/librte_net/rte_ip.h > +++ b/lib/librte_net/rte_ip.h > @@ -47,6 +47,14 @@ struct ipv4_hdr { > (((c) & 0xff) << 8) | \ > ((d) & 0xff)) >=20 > + > +/** RFC 3168 */ > +#define ECN_MASK (0x03) > +#define ECN_CE ECN_MASK > + > +/** Packet Option Masks */ > +#define DSCP_MASK (0xFC) Might be worth to add some prefix: IP_ECN_... Or even RTE_IP_ECN_... > + > /** Maximal IPv4 packet length (including a header) */ > #define IPV4_MAX_PKT_LEN 65535 >=20 > diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte= _security.h > index 76f54e0e0..577eff766 100644 > --- a/lib/librte_security/rte_security.h > +++ b/lib/librte_security/rte_security.h > @@ -163,6 +163,15 @@ struct rte_security_ipsec_sa_options { > * * 0: Inner packet is not modified. > */ > uint32_t dec_ttl : 1; > + > + /**< Explicit Congestion Notification (ECN) > + * > + * * ECT(1) (ECN-Capable Transport(1)) > + * * ECT(0) (ECN-Capable Transport(0)) > + * * ECT(CE)(CE (Congestion Experienced)) I think, that comment (possible ECN values) better move into rte_ip.h. And here explain briefly what would be behavior for ipsec implementation for 0/1 values. > + */ > + > + uint32_t ecn : 1; > }; >=20 > /** IPSec security association direction */ > -- > 2.13.6