From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f49.google.com (mail-wm0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id C296F1B00B for ; Fri, 15 Dec 2017 14:34:48 +0100 (CET) Received: by mail-wm0-f49.google.com with SMTP id 64so17596535wme.3 for ; Fri, 15 Dec 2017 05:34:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=in+AIFxYKkz01TZ6NDsCi0X3fxTWTMHi/YD3O9zOvyc=; b=RjhZ7EJMz6/++E/Ha1k/5Iu0/B9ryVCGkSV+pgBs40K5OHFmseISR8VQjSYts3LkO7 rgA7HRPDbq5WyAud4ZXmBQbMy7+XlGYR/WSdkb8lfdTyR/Gpe7CJrRO+AwJ0R3mJeT5N ZOLBKudui4KuShJtRe9cvL8xlAZbLtScYPOPo0f8symPTVHKEF/gEK+5LjjAr9MRmUoR Bz2s5GLbytGoMMFrVlOG6G5X/mi7/+WvUMcOuqUuy2oXb8GowRMnkwKsUfnyC8baNQiN Xa4vaXcdyiP9OOeQauwHrlvyrQCcCuhp1ap7IeR2AQ87+lVSdR7owMy97Zs9GtEP/XiB oz5g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=in+AIFxYKkz01TZ6NDsCi0X3fxTWTMHi/YD3O9zOvyc=; b=R+5tBO9Nhf46t6V/GQb3g5FpRorYBGHu3ahK6E/NE0efmpHz/B/68cn8yWW7C0UgCT 5aMZpeqcIn+EjCMaxkTS9O0acu8A1iYArpeMcm9B01DLQrwI2H4gb+4iC0SZkdN7Kax7 1LPEGeXsticR0Js718D6dJrGyo1pB6f0lFWKjCwR3zeAyTLEzpu4spFGnZRbXjq+r6Ag ve+EM+oq/Duxo1v7qJEMLoiKK3h+R/A2hdapbLk5ormwBsutdfq3pvCfDuQaM26KlGbQ F2UD9c15/I0KdCBsrvMvTWPHlw13o3dzT0ypKU84iBucLFlR5AnAK/yeHTWWWaPvXjE0 4+IQ== X-Gm-Message-State: AKGB3mIZ79v/12agQzu95zF+V+TYYbzTAv4j6MWuNN41jQy0K2vFwR45 KVTfP80zmv8TaELkkTx7fMsW X-Google-Smtp-Source: ACJfBos7d8N5nAs/e+KhMKDkth9qAtLwgeL8PywGcpmDIG70QINX2pGwjhatug3/XqjaMIfBSnXsWw== X-Received: by 10.80.148.10 with SMTP id p10mr17350119eda.250.1513344888265; Fri, 15 Dec 2017 05:34:48 -0800 (PST) Received: from laranjeiro-vm.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p37sm5331343eda.96.2017.12.15.05.34.46 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 15 Dec 2017 05:34:47 -0800 (PST) Date: Fri, 15 Dec 2017 14:35:23 +0100 From: Nelio Laranjeiro To: Anoob Joseph Cc: Akhil Goyal , Declan Doherty , Radu Nicolau , Sergio Gonzalez Monroy , Jerin Jacob , Narayana Prasad , dev@dpdk.org Message-ID: <20171215133523.jvoccqcnlnnhpktu@laranjeiro-vm.dev.6wind.com> References: <1513326606-21970-1-git-send-email-anoob.joseph@caviumnetworks.com> <1513327396-22178-1-git-send-email-anoob.joseph@caviumnetworks.com> <1513327396-22178-3-git-send-email-anoob.joseph@caviumnetworks.com> <20171215093931.rfpqwzincbcob6q4@laranjeiro-vm.dev.6wind.com> <923257f6-4ddf-6885-f7ad-caf5926a0545@caviumnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <923257f6-4ddf-6885-f7ad-caf5926a0545@caviumnetworks.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v5 2/2] examples/ipsec-secgw: add support for inline protocol 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: , X-List-Received-Date: Fri, 15 Dec 2017 13:34:48 -0000 On Fri, Dec 15, 2017 at 04:33:25PM +0530, Anoob Joseph wrote: > Hi Nelio, > > > On 12/15/2017 03:09 PM, Nelio Laranjeiro wrote: > > Hi Anoob, > > > > On Fri, Dec 15, 2017 at 08:43:16AM +0000, Anoob Joseph wrote: > > > Adding support for inline protocol processing > > > > > > In ingress side, application will receive regular IP packets, without > > > any IPsec related info. Application will do a selector check (SP-SA > > > check) by making use of the metadata from the packet. The > > > device-specific metadata in mbuf would aid in determing the security > > > session which processed the packet. > > This means that your devices removes the tunnel header? What happens > > for packets which could not be decrypted correctly, is also the header > > removed? > > Anyway this description is wrong as it is not true for all inline > > devices. > This is particularly for inline protocol processed packets. For inline > crypto, tunnel headers will be present, but for inline protocol tunnel > headers need not be present. Ok, my bad, I did not see the RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL additional flag. Sorry, > > In addition, in ingress, the same result can be archived by using a mark > > id to identify the flow and thus its security association. > > > > > In egress side, the plain packet would be submitted to the driver. The > > > packet will have optional metadata, which could be used to identify the > > > security session associated with the packet. > > Not true for all inline devices, some only need the next-proto for the > > ESP part. > This is more or less existing behavior for inline crypto. Inline protocol > shares most of it's behavior with inline crypto. > > > > > Signed-off-by: Anoob Joseph > > > --- > > > v5: > > > * Fixed checkpatch reported warnings > > > > > > v4: > > > * Directly using rte_mbuf.udata64 as the metadata from the packet > > > * Removed usage of rte_security_get_pkt_metadata API > > > > > > v3: > > > * Using (void *)userdata instead of 64 bit metadata in conf > > > * Changes parallel to the change in API > > > > > > v2: > > > * Using get_pkt_metadata API instead of get_session & get_cookie APIs > > > > > > examples/ipsec-secgw/esp.c | 6 +- > > > examples/ipsec-secgw/ipsec-secgw.c | 42 ++++++++++++- > > > examples/ipsec-secgw/ipsec.c | 121 +++++++++++++++++++++++++++++++------ > > > 3 files changed, 147 insertions(+), 22 deletions(-) > > > > > > diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c > > > index c3efe52..561f873 100644 > > > --- a/examples/ipsec-secgw/esp.c > > > +++ b/examples/ipsec-secgw/esp.c > > > @@ -178,7 +178,8 @@ esp_inbound_post(struct rte_mbuf *m, struct ipsec_sa *sa, > > > RTE_ASSERT(sa != NULL); > > > RTE_ASSERT(cop != NULL); > > > - if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) { > > > + if ((sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) || > > > + (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)) { > > > if (m->ol_flags & PKT_RX_SEC_OFFLOAD) { > > > if (m->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) > > > cop->status = RTE_CRYPTO_OP_STATUS_ERROR; > > > @@ -474,7 +475,8 @@ esp_outbound_post(struct rte_mbuf *m, > > > RTE_ASSERT(m != NULL); > > > RTE_ASSERT(sa != NULL); > > > - if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) { > > > + if ((sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) || > > > + (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)) { > > > m->ol_flags |= PKT_TX_SEC_OFFLOAD; > > > } else { > > > RTE_ASSERT(cop != NULL); > > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c > > > index c98454a..8254056 100644 > > > --- a/examples/ipsec-secgw/ipsec-secgw.c > > > +++ b/examples/ipsec-secgw/ipsec-secgw.c > > > @@ -265,6 +265,40 @@ prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t) > > > RTE_LOG(ERR, IPSEC, "Unsupported packet type\n"); > > > rte_pktmbuf_free(pkt); > > > } > > > + > > > + /* Check if the packet has been processed inline. For inline protocol > > > + * processed packets, the metadata in the mbuf can be used to identify > > > + * the security processing done on the packet. The metadata will be > > > + * used to retrieve the application registered userdata associated > > > + * with the security session. > > > + */ > > > + > > > + if (pkt->ol_flags & PKT_RX_SEC_OFFLOAD) { > > > + struct ipsec_sa *sa; > > > + struct ipsec_mbuf_metadata *priv; > > > + struct rte_security_ctx *ctx = (struct rte_security_ctx *) > > > + rte_eth_dev_get_sec_ctx( > > > + pkt->port); > > > + > > > + /* Retrieve the userdata registered. Here, the userdata > > > + * registered is the SA pointer. > > > + */ > > > + > > > + sa = (struct ipsec_sa *) > > > + rte_security_get_userdata(ctx, pkt->udata64); > > > + > > > + if (sa == NULL) { > > > + /* userdata could not be retrieved */ > > > + return; > > > + } > > > + > > > + /* Save SA as priv member in mbuf. This will be used in the > > > + * IPsec selector(SP-SA) check. > > > + */ > > > + > > > + priv = get_priv(pkt); > > > + priv->sa = sa; > > > + } > > You must verify the function exists on all drivers, those who don't > > support such userdata to be inserted won't implement it. > That check is there in library. It will return NULL and then the function > would exit > > > > What happens on security session where the application don't set such > > information? How the application knows the pointer is set correctly? > Application is the one who sets and gets the pointer. If application doesn't > set it correctly, application will not able to use it. If application > doesn't set this, for inline protocol, the application won't have any > information which it can use to identify the security processing done. > > > > > } > > > static inline void > > > @@ -401,11 +435,17 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip, > > > ip->pkts[j++] = m; > > > continue; > > > } > > > - if (res & DISCARD || i < lim) { > > > + if (res & DISCARD) { > > > rte_pktmbuf_free(m); > > > continue; > > > } > > > + > > > /* Only check SPI match for processed IPSec packets */ > > > + if (i < lim && ((m->ol_flags & PKT_RX_SEC_OFFLOAD) == 0)) { > > > + rte_pktmbuf_free(m); > > > + continue; > > > + } > > > + > > > sa_idx = ip->res[i] & PROTECT_MASK; > > > if (sa_idx == 0 || !inbound_sa_check(sa, m, sa_idx)) { > > > rte_pktmbuf_free(m); > > > diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c > > > index 70ed227..bd68ec6 100644 > > > --- a/examples/ipsec-secgw/ipsec.c > > > +++ b/examples/ipsec-secgw/ipsec.c > > > @@ -46,6 +46,27 @@ > > > #include "ipsec.h" > > > #include "esp.h" > > > +static inline void > > > +set_ipsec_conf(struct ipsec_sa *sa, struct rte_security_ipsec_xform *ipsec) > > > +{ > > > + if (ipsec->mode == RTE_SECURITY_IPSEC_SA_MODE_TUNNEL) { > > > + struct rte_security_ipsec_tunnel_param *tunnel = > > > + &ipsec->tunnel; > > > + if (sa->flags == IP4_TUNNEL) { > > > + tunnel->type = > > > + RTE_SECURITY_IPSEC_TUNNEL_IPV4; > > > + tunnel->ipv4.ttl = IPDEFTTL; > > > + > > > + memcpy((uint8_t *)&tunnel->ipv4.src_ip, > > > + (uint8_t *)&sa->src.ip.ip4, 4); > > > + > > > + memcpy((uint8_t *)&tunnel->ipv4.dst_ip, > > > + (uint8_t *)&sa->dst.ip.ip4, 4); > > > + } > > > + /* TODO support for Transport and IPV6 tunnel */ > > > + } > > > +} > > > + > > > static inline int > > > create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > > { > > > @@ -95,7 +116,8 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > > RTE_SECURITY_IPSEC_SA_MODE_TUNNEL : > > > RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT, > > > } }, > > > - .crypto_xform = sa->xforms > > > + .crypto_xform = sa->xforms, > > > + .userdata = NULL, > > > }; > > > @@ -104,23 +126,8 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > > rte_cryptodev_get_sec_ctx( > > > ipsec_ctx->tbl[cdev_id_qp].id); > > > - if (sess_conf.ipsec.mode == > > > - RTE_SECURITY_IPSEC_SA_MODE_TUNNEL) { > > > - struct rte_security_ipsec_tunnel_param *tunnel = > > > - &sess_conf.ipsec.tunnel; > > > - if (sa->flags == IP4_TUNNEL) { > > > - tunnel->type = > > > - RTE_SECURITY_IPSEC_TUNNEL_IPV4; > > > - tunnel->ipv4.ttl = IPDEFTTL; > > > - > > > - memcpy((uint8_t *)&tunnel->ipv4.src_ip, > > > - (uint8_t *)&sa->src.ip.ip4, 4); > > > - > > > - memcpy((uint8_t *)&tunnel->ipv4.dst_ip, > > > - (uint8_t *)&sa->dst.ip.ip4, 4); > > > - } > > > - /* TODO support for Transport and IPV6 tunnel */ > > > - } > > > + /* Set IPsec parameters in conf */ > > > + set_ipsec_conf(sa, &(sess_conf.ipsec)); > > > sa->sec_session = rte_security_session_create(ctx, > > > &sess_conf, ipsec_ctx->session_pool); > > > @@ -206,6 +213,70 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) > > > err.message); > > > return -1; > > > } > > > + } else if (sa->type == > > > + RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) { > > > + struct rte_security_ctx *ctx = > > > + (struct rte_security_ctx *) > > > + rte_eth_dev_get_sec_ctx(sa->portid); > > > + const struct rte_security_capability *sec_cap; > > > + > > > + if (ctx == NULL) { > > > + RTE_LOG(ERR, IPSEC, > > > + "Ethernet device doesn't have security features registered\n"); > > > + return -1; > > > + } > > > + > > > + /* Set IPsec parameters in conf */ > > > + set_ipsec_conf(sa, &(sess_conf.ipsec)); > > > + > > > + /* Save SA as userdata for the security session. When > > > + * the packet is received, this userdata will be > > > + * retrieved using the metadata from the packet. > > > + * > > > + * This is required only for inbound SAs. > > > + */ > > Again not true for all inline devices. > Should be valid for all inline protocol devices. May be addition of > capability would look better? > > > > > + > > > + if (sa->direction == RTE_SECURITY_IPSEC_SA_DIR_INGRESS) > > > + sess_conf.userdata = (void *) sa; > > > + > > > + sa->sec_session = rte_security_session_create(ctx, > > > + &sess_conf, ipsec_ctx->session_pool); > > > + if (sa->sec_session == NULL) { > > > + RTE_LOG(ERR, IPSEC, > > > + "SEC Session init failed: err: %d\n", ret); > > > + return -1; > > > + } > > > + > > > + sec_cap = rte_security_capabilities_get(ctx); > > > + > > > + if (sec_cap == NULL) { > > > + RTE_LOG(ERR, IPSEC, > > > + "No capabilities registered\n"); > > > + return -1; > > > + } > > > + > > > + /* iterate until ESP tunnel*/ > > > + while (sec_cap->action != > > > + RTE_SECURITY_ACTION_TYPE_NONE) { > > > + > > > + if (sec_cap->action == sa->type && > > > + sec_cap->protocol == > > > + RTE_SECURITY_PROTOCOL_IPSEC && > > > + sec_cap->ipsec.mode == > > > + RTE_SECURITY_IPSEC_SA_MODE_TUNNEL && > > > + sec_cap->ipsec.direction == sa->direction) > > > + break; > > > + sec_cap++; > > > + } > > > + > > > + if (sec_cap->action == RTE_SECURITY_ACTION_TYPE_NONE) { > > > + RTE_LOG(ERR, IPSEC, > > > + "No suitable security capability found\n"); > > > + return -1; > > > + } > > > + > > > + sa->ol_flags = sec_cap->ol_flags; > > > + sa->security_ctx = ctx; > > > } > > > } else { > > > sa->crypto_session = rte_cryptodev_sym_session_create( > > > @@ -323,7 +394,19 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx, > > > } > > > break; > > > case RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL: > > > - break; > > > + if ((unlikely(sa->sec_session == NULL)) && > > > + create_session(ipsec_ctx, sa)) { > > > + rte_pktmbuf_free(pkts[i]); > > > + continue; > > > + } > > > + > > > + cqp = &ipsec_ctx->tbl[sa->cdev_id_qp]; > > > + cqp->ol_pkts[cqp->ol_pkts_cnt++] = pkts[i]; > > > + if (sa->ol_flags & RTE_SECURITY_TX_OLOAD_NEED_MDATA) > > > + rte_security_set_pkt_metadata( > > > + sa->security_ctx, > > > + sa->sec_session, pkts[i], NULL); > > > + continue; > > > case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO: > > > priv->cop.type = RTE_CRYPTO_OP_TYPE_SYMMETRIC; > > > priv->cop.status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED; > > > -- > > > 2.7.4 > > > > > Thanks, > > > Thanks, > Anoob -- Nélio Laranjeiro 6WIND