* [dpdk-stable] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
[not found] <1544805623-18150-2-git-send-email-konstantin.ananyev@intel.com>
@ 2018-12-28 15:33 ` Konstantin Ananyev
2019-01-02 11:44 ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 04/10] examples/ipsec-secgw: fix outbound codepath for single SA Konstantin Ananyev
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Konstantin Ananyev @ 2018-12-28 15:33 UTC (permalink / raw)
To: dev, dev; +Cc: akhil.goyal, Konstantin Ananyev, stable
In some cases crypto-ops could never be dequeued from the crypto-device.
The easiest way to reproduce:
start ipsec-secgw with crypto-dev and send to it less then 32 packets.
none packets will be forwarded.
Reason for that is that the application does dequeue() from crypto-queues
only when new packets arrive.
This patch makes sure it calls dequeue() on a regular basis.
Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop")
Cc: stable@dpdk.org
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++-----
examples/ipsec-secgw/ipsec.c | 60 ++++++++-----
examples/ipsec-secgw/ipsec.h | 11 +++
3 files changed, 165 insertions(+), 42 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 274a49cbb..797bd6435 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -469,38 +469,55 @@ inbound_sp_sa(struct sp_ctx *sp, struct sa_ctx *sa, struct traffic_type *ip,
ip->num = j;
}
-static inline void
-process_pkts_inbound(struct ipsec_ctx *ipsec_ctx,
- struct ipsec_traffic *traffic)
+static void
+split46_traffic(struct ipsec_traffic *trf, struct rte_mbuf *mb[], uint32_t num)
{
+ uint32_t i, n4, n6;
+ struct ip *ip;
struct rte_mbuf *m;
- uint16_t idx, nb_pkts_in, i, n_ip4, n_ip6;
- nb_pkts_in = ipsec_inbound(ipsec_ctx, traffic->ipsec.pkts,
- traffic->ipsec.num, MAX_PKT_BURST);
+ n4 = trf->ip4.num;
+ n6 = trf->ip6.num;
- n_ip4 = traffic->ip4.num;
- n_ip6 = traffic->ip6.num;
+ for (i = 0; i < num; i++) {
+
+ m = mb[i];
+ ip = rte_pktmbuf_mtod(m, struct ip *);
- /* SP/ACL Inbound check ipsec and ip4 */
- for (i = 0; i < nb_pkts_in; i++) {
- m = traffic->ipsec.pkts[i];
- struct ip *ip = rte_pktmbuf_mtod(m, struct ip *);
if (ip->ip_v == IPVERSION) {
- idx = traffic->ip4.num++;
- traffic->ip4.pkts[idx] = m;
- traffic->ip4.data[idx] = rte_pktmbuf_mtod_offset(m,
+ trf->ip4.pkts[n4] = m;
+ trf->ip4.data[n4] = rte_pktmbuf_mtod_offset(m,
uint8_t *, offsetof(struct ip, ip_p));
+ n4++;
} else if (ip->ip_v == IP6_VERSION) {
- idx = traffic->ip6.num++;
- traffic->ip6.pkts[idx] = m;
- traffic->ip6.data[idx] = rte_pktmbuf_mtod_offset(m,
+ trf->ip6.pkts[n6] = m;
+ trf->ip6.data[n6] = rte_pktmbuf_mtod_offset(m,
uint8_t *,
offsetof(struct ip6_hdr, ip6_nxt));
+ n6++;
} else
rte_pktmbuf_free(m);
}
+ trf->ip4.num = n4;
+ trf->ip6.num = n6;
+}
+
+
+static inline void
+process_pkts_inbound(struct ipsec_ctx *ipsec_ctx,
+ struct ipsec_traffic *traffic)
+{
+ uint16_t nb_pkts_in, n_ip4, n_ip6;
+
+ n_ip4 = traffic->ip4.num;
+ n_ip6 = traffic->ip6.num;
+
+ nb_pkts_in = ipsec_inbound(ipsec_ctx, traffic->ipsec.pkts,
+ traffic->ipsec.num, MAX_PKT_BURST);
+
+ split46_traffic(traffic, traffic->ipsec.pkts, nb_pkts_in);
+
inbound_sp_sa(ipsec_ctx->sp4_ctx, ipsec_ctx->sa_ctx, &traffic->ip4,
n_ip4);
@@ -795,7 +812,7 @@ process_pkts(struct lcore_conf *qconf, struct rte_mbuf **pkts,
}
static inline void
-drain_buffers(struct lcore_conf *qconf)
+drain_tx_buffers(struct lcore_conf *qconf)
{
struct buffer *buf;
uint32_t portid;
@@ -809,6 +826,81 @@ drain_buffers(struct lcore_conf *qconf)
}
}
+static inline void
+drain_crypto_buffers(struct lcore_conf *qconf)
+{
+ uint32_t i;
+ struct ipsec_ctx *ctx;
+
+ /* drain inbound buffers*/
+ ctx = &qconf->inbound;
+ for (i = 0; i != ctx->nb_qps; i++) {
+ if (ctx->tbl[i].len != 0)
+ enqueue_cop_burst(ctx->tbl + i);
+ }
+
+ /* drain outbound buffers*/
+ ctx = &qconf->outbound;
+ for (i = 0; i != ctx->nb_qps; i++) {
+ if (ctx->tbl[i].len != 0)
+ enqueue_cop_burst(ctx->tbl + i);
+ }
+}
+
+static void
+drain_inbound_crypto_queues(const struct lcore_conf *qconf,
+ struct ipsec_ctx *ctx)
+{
+ uint32_t n;
+ struct ipsec_traffic trf;
+
+ /* dequeue packets from crypto-queue */
+ n = ipsec_inbound_cqp_dequeue(ctx, trf.ipsec.pkts,
+ RTE_DIM(trf.ipsec.pkts));
+ if (n == 0)
+ return;
+
+ trf.ip4.num = 0;
+ trf.ip6.num = 0;
+
+ /* split traffic by ipv4-ipv6 */
+ split46_traffic(&trf, trf.ipsec.pkts, n);
+
+ /* process ipv4 packets */
+ inbound_sp_sa(ctx->sp4_ctx, ctx->sa_ctx, &trf.ip4, 0);
+ route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
+
+ /* process ipv6 packets */
+ inbound_sp_sa(ctx->sp6_ctx, ctx->sa_ctx, &trf.ip6, 0);
+ route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
+}
+
+static void
+drain_outbound_crypto_queues(const struct lcore_conf *qconf,
+ struct ipsec_ctx *ctx)
+{
+ uint32_t n;
+ struct ipsec_traffic trf;
+
+ /* dequeue packets from crypto-queue */
+ n = ipsec_outbound_cqp_dequeue(ctx, trf.ipsec.pkts,
+ RTE_DIM(trf.ipsec.pkts));
+ if (n == 0)
+ return;
+
+ trf.ip4.num = 0;
+ trf.ip6.num = 0;
+
+ /* split traffic by ipv4-ipv6 */
+ split46_traffic(&trf, trf.ipsec.pkts, n);
+
+ /* process ipv4 packets */
+ route4_pkts(qconf->rt4_ctx, trf.ip4.pkts, trf.ip4.num);
+
+ /* process ipv6 packets */
+ route6_pkts(qconf->rt6_ctx, trf.ip6.pkts, trf.ip6.num);
+}
+
/* main processing loop */
static int32_t
main_loop(__attribute__((unused)) void *dummy)
@@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy)
diff_tsc = cur_tsc - prev_tsc;
if (unlikely(diff_tsc > drain_tsc)) {
- drain_buffers(qconf);
+ drain_tx_buffers(qconf);
+ drain_crypto_buffers(qconf);
prev_tsc = cur_tsc;
}
@@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy)
if (nb_rx > 0)
process_pkts(qconf, pkts, nb_rx, portid);
}
+
+ drain_inbound_crypto_queues(qconf, &qconf->inbound);
+ drain_outbound_crypto_queues(qconf, &qconf->outbound);
}
}
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 3d415f1af..8bf928a23 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -333,33 +333,35 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
return 0;
}
+/*
+ * queue crypto-ops into PMD queue.
+ */
+void
+enqueue_cop_burst(struct cdev_qp *cqp)
+{
+ uint32_t i, len, ret;
+
+ len = cqp->len;
+ ret = rte_cryptodev_enqueue_burst(cqp->id, cqp->qp, cqp->buf, len);
+ if (ret < len) {
+ RTE_LOG_DP(DEBUG, IPSEC, "Cryptodev %u queue %u:"
+ " enqueued %u crypto ops out of %u\n",
+ cqp->id, cqp->qp, ret, len);
+ /* drop packets that we fail to enqueue */
+ for (i = ret; i < len; i++)
+ rte_pktmbuf_free(cqp->buf[i]->sym->m_src);
+ }
+ cqp->in_flight += ret;
+ cqp->len = 0;
+}
+
static inline void
enqueue_cop(struct cdev_qp *cqp, struct rte_crypto_op *cop)
{
- int32_t ret = 0, i;
-
cqp->buf[cqp->len++] = cop;
- if (cqp->len == MAX_PKT_BURST) {
- int enq_size = cqp->len;
- if ((cqp->in_flight + enq_size) > MAX_INFLIGHT)
- enq_size -=
- (int)((cqp->in_flight + enq_size) - MAX_INFLIGHT);
-
- if (enq_size > 0)
- ret = rte_cryptodev_enqueue_burst(cqp->id, cqp->qp,
- cqp->buf, enq_size);
- if (ret < cqp->len) {
- RTE_LOG_DP(DEBUG, IPSEC, "Cryptodev %u queue %u:"
- " enqueued %u crypto ops out of %u\n",
- cqp->id, cqp->qp,
- ret, cqp->len);
- for (i = ret; i < cqp->len; i++)
- rte_pktmbuf_free(cqp->buf[i]->sym->m_src);
- }
- cqp->in_flight += ret;
- cqp->len = 0;
- }
+ if (cqp->len == MAX_PKT_BURST)
+ enqueue_cop_burst(cqp);
}
static inline void
@@ -548,6 +550,13 @@ ipsec_inbound(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
return ipsec_dequeue(esp_inbound_post, ctx, pkts, len);
}
+uint16_t
+ipsec_inbound_cqp_dequeue(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
+ uint16_t len)
+{
+ return ipsec_dequeue(esp_inbound_post, ctx, pkts, len);
+}
+
uint16_t
ipsec_outbound(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
uint32_t sa_idx[], uint16_t nb_pkts, uint16_t len)
@@ -560,3 +569,10 @@ ipsec_outbound(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
return ipsec_dequeue(esp_outbound_post, ctx, pkts, len);
}
+
+uint16_t
+ipsec_outbound_cqp_dequeue(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
+ uint16_t len)
+{
+ return ipsec_dequeue(esp_outbound_post, ctx, pkts, len);
+}
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index 580f7876b..2f04b7d68 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -184,6 +184,14 @@ uint16_t
ipsec_outbound(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
uint32_t sa_idx[], uint16_t nb_pkts, uint16_t len);
+uint16_t
+ipsec_inbound_cqp_dequeue(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
+ uint16_t len);
+
+uint16_t
+ipsec_outbound_cqp_dequeue(struct ipsec_ctx *ctx, struct rte_mbuf *pkts[],
+ uint16_t len);
+
static inline uint16_t
ipsec_metadata_size(void)
{
@@ -248,4 +256,7 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
int
add_dst_ethaddr(uint16_t port, const struct ether_addr *addr);
+void
+enqueue_cop_burst(struct cdev_qp *cqp);
+
#endif /* __IPSEC_H__ */
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5 04/10] examples/ipsec-secgw: fix outbound codepath for single SA
[not found] <1544805623-18150-2-git-send-email-konstantin.ananyev@intel.com>
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued Konstantin Ananyev
@ 2018-12-28 15:33 ` Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 05/10] examples/ipsec-secgw: make local variables static Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 06/10] examples/ipsec-secgw: fix inbound SA checking Konstantin Ananyev
3 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2018-12-28 15:33 UTC (permalink / raw)
To: dev, dev; +Cc: akhil.goyal, Konstantin Ananyev, stable
Looking at process_pkts_outbound_nosp() there seems few issues:
- accessing mbuf after it was freed
- invoking ipsec_outbound() for ipv4 packets only
- copying number of packets, but not the mbuf pointers itself
that patch provides fixes for that issues.
Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
Cc: stable@dpdk.org
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
examples/ipsec-secgw/ipsec-secgw.c | 33 +++++++++++++++++++++---------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 797bd6435..cc812d502 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -629,32 +629,45 @@ process_pkts_outbound_nosp(struct ipsec_ctx *ipsec_ctx,
struct ipsec_traffic *traffic)
{
struct rte_mbuf *m;
- uint32_t nb_pkts_out, i;
+ uint32_t nb_pkts_out, i, n;
struct ip *ip;
/* Drop any IPsec traffic from protected ports */
for (i = 0; i < traffic->ipsec.num; i++)
rte_pktmbuf_free(traffic->ipsec.pkts[i]);
- traffic->ipsec.num = 0;
+ n = 0;
- for (i = 0; i < traffic->ip4.num; i++)
- traffic->ip4.res[i] = single_sa_idx;
+ for (i = 0; i < traffic->ip4.num; i++) {
+ traffic->ipsec.pkts[n] = traffic->ip4.pkts[i];
+ traffic->ipsec.res[n++] = single_sa_idx;
+ }
- for (i = 0; i < traffic->ip6.num; i++)
- traffic->ip6.res[i] = single_sa_idx;
+ for (i = 0; i < traffic->ip6.num; i++) {
+ traffic->ipsec.pkts[n] = traffic->ip6.pkts[i];
+ traffic->ipsec.res[n++] = single_sa_idx;
+ }
+
+ traffic->ip4.num = 0;
+ traffic->ip6.num = 0;
+ traffic->ipsec.num = n;
- nb_pkts_out = ipsec_outbound(ipsec_ctx, traffic->ip4.pkts,
- traffic->ip4.res, traffic->ip4.num,
+ nb_pkts_out = ipsec_outbound(ipsec_ctx, traffic->ipsec.pkts,
+ traffic->ipsec.res, traffic->ipsec.num,
MAX_PKT_BURST);
/* They all sue the same SA (ip4 or ip6 tunnel) */
m = traffic->ipsec.pkts[i];
ip = rte_pktmbuf_mtod(m, struct ip *);
- if (ip->ip_v == IPVERSION)
+ if (ip->ip_v == IPVERSION) {
traffic->ip4.num = nb_pkts_out;
- else
+ for (i = 0; i < nb_pkts_out; i++)
+ traffic->ip4.pkts[i] = traffic->ipsec.pkts[i];
+ } else {
traffic->ip6.num = nb_pkts_out;
+ for (i = 0; i < nb_pkts_out; i++)
+ traffic->ip6.pkts[i] = traffic->ipsec.pkts[i];
+ }
}
static inline int32_t
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5 05/10] examples/ipsec-secgw: make local variables static
[not found] <1544805623-18150-2-git-send-email-konstantin.ananyev@intel.com>
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 04/10] examples/ipsec-secgw: fix outbound codepath for single SA Konstantin Ananyev
@ 2018-12-28 15:33 ` Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 06/10] examples/ipsec-secgw: fix inbound SA checking Konstantin Ananyev
3 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2018-12-28 15:33 UTC (permalink / raw)
To: dev, dev; +Cc: akhil.goyal, Konstantin Ananyev, stable
in sp4.c and sp6.c there are few globals that used only locally.
Define them as static ones.
Cc: stable@dpdk.org
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
---
examples/ipsec-secgw/sp4.c | 10 +++++-----
examples/ipsec-secgw/sp6.c | 10 +++++-----
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/examples/ipsec-secgw/sp4.c b/examples/ipsec-secgw/sp4.c
index 8d3d3d8e0..6b05daaa9 100644
--- a/examples/ipsec-secgw/sp4.c
+++ b/examples/ipsec-secgw/sp4.c
@@ -44,7 +44,7 @@ enum {
RTE_ACL_IPV4_NUM
};
-struct rte_acl_field_def ip4_defs[NUM_FIELDS_IPV4] = {
+static struct rte_acl_field_def ip4_defs[NUM_FIELDS_IPV4] = {
{
.type = RTE_ACL_FIELD_TYPE_BITMASK,
.size = sizeof(uint8_t),
@@ -85,11 +85,11 @@ struct rte_acl_field_def ip4_defs[NUM_FIELDS_IPV4] = {
RTE_ACL_RULE_DEF(acl4_rules, RTE_DIM(ip4_defs));
-struct acl4_rules acl4_rules_out[MAX_ACL_RULE_NUM];
-uint32_t nb_acl4_rules_out;
+static struct acl4_rules acl4_rules_out[MAX_ACL_RULE_NUM];
+static uint32_t nb_acl4_rules_out;
-struct acl4_rules acl4_rules_in[MAX_ACL_RULE_NUM];
-uint32_t nb_acl4_rules_in;
+static struct acl4_rules acl4_rules_in[MAX_ACL_RULE_NUM];
+static uint32_t nb_acl4_rules_in;
void
parse_sp4_tokens(char **tokens, uint32_t n_tokens,
diff --git a/examples/ipsec-secgw/sp6.c b/examples/ipsec-secgw/sp6.c
index 6002afef3..dc5b94c6a 100644
--- a/examples/ipsec-secgw/sp6.c
+++ b/examples/ipsec-secgw/sp6.c
@@ -34,7 +34,7 @@ enum {
#define IP6_ADDR_SIZE 16
-struct rte_acl_field_def ip6_defs[IP6_NUM] = {
+static struct rte_acl_field_def ip6_defs[IP6_NUM] = {
{
.type = RTE_ACL_FIELD_TYPE_BITMASK,
.size = sizeof(uint8_t),
@@ -116,11 +116,11 @@ struct rte_acl_field_def ip6_defs[IP6_NUM] = {
RTE_ACL_RULE_DEF(acl6_rules, RTE_DIM(ip6_defs));
-struct acl6_rules acl6_rules_out[MAX_ACL_RULE_NUM];
-uint32_t nb_acl6_rules_out;
+static struct acl6_rules acl6_rules_out[MAX_ACL_RULE_NUM];
+static uint32_t nb_acl6_rules_out;
-struct acl6_rules acl6_rules_in[MAX_ACL_RULE_NUM];
-uint32_t nb_acl6_rules_in;
+static struct acl6_rules acl6_rules_in[MAX_ACL_RULE_NUM];
+static uint32_t nb_acl6_rules_in;
void
parse_sp6_tokens(char **tokens, uint32_t n_tokens,
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-stable] [PATCH v5 06/10] examples/ipsec-secgw: fix inbound SA checking
[not found] <1544805623-18150-2-git-send-email-konstantin.ananyev@intel.com>
` (2 preceding siblings ...)
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 05/10] examples/ipsec-secgw: make local variables static Konstantin Ananyev
@ 2018-12-28 15:33 ` Konstantin Ananyev
3 siblings, 0 replies; 8+ messages in thread
From: Konstantin Ananyev @ 2018-12-28 15:33 UTC (permalink / raw)
To: dev, dev; +Cc: akhil.goyal, Konstantin Ananyev, stable, Bernard Iremonger
In the inbound_sa_check() make sure that sa pointer stored
inside mbuf private area is not NULL.
Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
examples/ipsec-secgw/sa.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index f6271bc60..839aaca0c 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -947,10 +947,15 @@ int
inbound_sa_check(struct sa_ctx *sa_ctx, struct rte_mbuf *m, uint32_t sa_idx)
{
struct ipsec_mbuf_metadata *priv;
+ struct ipsec_sa *sa;
priv = get_priv(m);
+ sa = priv->sa;
+ if (sa != NULL)
+ return (sa_ctx->sa[sa_idx].spi == sa->spi);
- return (sa_ctx->sa[sa_idx].spi == priv->sa->spi);
+ RTE_LOG(ERR, IPSEC, "SA not saved in private data\n");
+ return 0;
}
static inline void
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued Konstantin Ananyev
@ 2019-01-02 11:44 ` Akhil Goyal
2019-01-02 13:43 ` Ananyev, Konstantin
0 siblings, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2019-01-02 11:44 UTC (permalink / raw)
To: Konstantin Ananyev, dev; +Cc: stable
On 12/28/2018 9:03 PM, Konstantin Ananyev wrote:
> In some cases crypto-ops could never be dequeued from the crypto-device.
> The easiest way to reproduce:
> start ipsec-secgw with crypto-dev and send to it less then 32 packets.
> none packets will be forwarded.
> Reason for that is that the application does dequeue() from crypto-queues
> only when new packets arrive.
> This patch makes sure it calls dequeue() on a regular basis.
>
> Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop")
> Cc: stable@dpdk.org
>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Acked-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++-----
> examples/ipsec-secgw/ipsec.c | 60 ++++++++-----
> examples/ipsec-secgw/ipsec.h | 11 +++
> 3 files changed, 165 insertions(+), 42 deletions(-)
[snip]
> +
> /* main processing loop */
> static int32_t
> main_loop(__attribute__((unused)) void *dummy)
> @@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy)
> diff_tsc = cur_tsc - prev_tsc;
>
> if (unlikely(diff_tsc > drain_tsc)) {
> - drain_buffers(qconf);
> + drain_tx_buffers(qconf);
> + drain_crypto_buffers(qconf);
> prev_tsc = cur_tsc;
> }
>
> @@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy)
> if (nb_rx > 0)
> process_pkts(qconf, pkts, nb_rx, portid);
> }
> +
> + drain_inbound_crypto_queues(qconf, &qconf->inbound);
> + drain_outbound_crypto_queues(qconf, &qconf->outbound);
drain_inbound_crypto_queues and drain_outbound_crypto_queues should be called based on diff_tsc.
moving these two lines above after drain_crypto_buffers will improve the performance drop caused due to this patch.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
2019-01-02 11:44 ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
@ 2019-01-02 13:43 ` Ananyev, Konstantin
2019-01-02 13:50 ` Akhil Goyal
0 siblings, 1 reply; 8+ messages in thread
From: Ananyev, Konstantin @ 2019-01-02 13:43 UTC (permalink / raw)
To: Akhil Goyal, dev; +Cc: stable
>
> On 12/28/2018 9:03 PM, Konstantin Ananyev wrote:
> > In some cases crypto-ops could never be dequeued from the crypto-device.
> > The easiest way to reproduce:
> > start ipsec-secgw with crypto-dev and send to it less then 32 packets.
> > none packets will be forwarded.
> > Reason for that is that the application does dequeue() from crypto-queues
> > only when new packets arrive.
> > This patch makes sure it calls dequeue() on a regular basis.
> >
> > Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Acked-by: Radu Nicolau <radu.nicolau@intel.com>
> > ---
> > examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++-----
> > examples/ipsec-secgw/ipsec.c | 60 ++++++++-----
> > examples/ipsec-secgw/ipsec.h | 11 +++
> > 3 files changed, 165 insertions(+), 42 deletions(-)
> [snip]
> > +
> > /* main processing loop */
> > static int32_t
> > main_loop(__attribute__((unused)) void *dummy)
> > @@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy)
> > diff_tsc = cur_tsc - prev_tsc;
> >
> > if (unlikely(diff_tsc > drain_tsc)) {
> > - drain_buffers(qconf);
> > + drain_tx_buffers(qconf);
> > + drain_crypto_buffers(qconf);
> > prev_tsc = cur_tsc;
> > }
> >
> > @@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy)
> > if (nb_rx > 0)
> > process_pkts(qconf, pkts, nb_rx, portid);
> > }
> > +
> > + drain_inbound_crypto_queues(qconf, &qconf->inbound);
> > + drain_outbound_crypto_queues(qconf, &qconf->outbound);
>
> drain_inbound_crypto_queues and drain_outbound_crypto_queues should be called based on diff_tsc.
> moving these two lines above after drain_crypto_buffers will improve the performance drop caused due to this patch.
Thanks, good to know.
To make what you suggest above to work properly with non-legacy mode ('-l') extra changes
would be needed...
Do you have an idea - what exactly causing a slowdown?
Just an extra function calls (drain_inbound_crypto_queues/ drain_outbound_crypto_queues)?
Or is that because we do dequeue() from crypto PMD more often then before?
Konstantin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
2019-01-02 13:43 ` Ananyev, Konstantin
@ 2019-01-02 13:50 ` Akhil Goyal
2019-01-02 15:06 ` Ananyev, Konstantin
0 siblings, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2019-01-02 13:50 UTC (permalink / raw)
To: Ananyev, Konstantin, dev; +Cc: stable
On 1/2/2019 7:13 PM, Ananyev, Konstantin wrote:
>
>> On 12/28/2018 9:03 PM, Konstantin Ananyev wrote:
>>> In some cases crypto-ops could never be dequeued from the crypto-device.
>>> The easiest way to reproduce:
>>> start ipsec-secgw with crypto-dev and send to it less then 32 packets.
>>> none packets will be forwarded.
>>> Reason for that is that the application does dequeue() from crypto-queues
>>> only when new packets arrive.
>>> This patch makes sure it calls dequeue() on a regular basis.
>>>
>>> Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>>> Acked-by: Radu Nicolau <radu.nicolau@intel.com>
>>> ---
>>> examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++-----
>>> examples/ipsec-secgw/ipsec.c | 60 ++++++++-----
>>> examples/ipsec-secgw/ipsec.h | 11 +++
>>> 3 files changed, 165 insertions(+), 42 deletions(-)
>> [snip]
>>> +
>>> /* main processing loop */
>>> static int32_t
>>> main_loop(__attribute__((unused)) void *dummy)
>>> @@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy)
>>> diff_tsc = cur_tsc - prev_tsc;
>>>
>>> if (unlikely(diff_tsc > drain_tsc)) {
>>> - drain_buffers(qconf);
>>> + drain_tx_buffers(qconf);
>>> + drain_crypto_buffers(qconf);
>>> prev_tsc = cur_tsc;
>>> }
>>>
>>> @@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy)
>>> if (nb_rx > 0)
>>> process_pkts(qconf, pkts, nb_rx, portid);
>>> }
>>> +
>>> + drain_inbound_crypto_queues(qconf, &qconf->inbound);
>>> + drain_outbound_crypto_queues(qconf, &qconf->outbound);
>> drain_inbound_crypto_queues and drain_outbound_crypto_queues should be called based on diff_tsc.
>> moving these two lines above after drain_crypto_buffers will improve the performance drop caused due to this patch.
> Thanks, good to know.
> To make what you suggest above to work properly with non-legacy mode ('-l') extra changes
> would be needed...
What changes do you see?
> Do you have an idea - what exactly causing a slowdown?
> Just an extra function calls (drain_inbound_crypto_queues/ drain_outbound_crypto_queues)?
> Or is that because we do dequeue() from crypto PMD more often then before?
I have not profiled it, but it should be because of more dequeues. On a
single call to dequeue, a burst of packets get dequeued. but now there
will be a lot more dequeues which have lesser packets than the burst
size which will deteriorate the performance as it would be wasting the
dequeue cycles.
This patch is causing around 5% drop out of the 10% that I mentioned in
the other mail.
With the change that I suggested, I am almost able to get back those 5%.
> Konstantin
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
2019-01-02 13:50 ` Akhil Goyal
@ 2019-01-02 15:06 ` Ananyev, Konstantin
0 siblings, 0 replies; 8+ messages in thread
From: Ananyev, Konstantin @ 2019-01-02 15:06 UTC (permalink / raw)
To: Akhil Goyal, dev; +Cc: stable
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Wednesday, January 2, 2019 1:51 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued
>
>
>
> On 1/2/2019 7:13 PM, Ananyev, Konstantin wrote:
> >
> >> On 12/28/2018 9:03 PM, Konstantin Ananyev wrote:
> >>> In some cases crypto-ops could never be dequeued from the crypto-device.
> >>> The easiest way to reproduce:
> >>> start ipsec-secgw with crypto-dev and send to it less then 32 packets.
> >>> none packets will be forwarded.
> >>> Reason for that is that the application does dequeue() from crypto-queues
> >>> only when new packets arrive.
> >>> This patch makes sure it calls dequeue() on a regular basis.
> >>>
> >>> Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >>> Acked-by: Radu Nicolau <radu.nicolau@intel.com>
> >>> ---
> >>> examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++-----
> >>> examples/ipsec-secgw/ipsec.c | 60 ++++++++-----
> >>> examples/ipsec-secgw/ipsec.h | 11 +++
> >>> 3 files changed, 165 insertions(+), 42 deletions(-)
> >> [snip]
> >>> +
> >>> /* main processing loop */
> >>> static int32_t
> >>> main_loop(__attribute__((unused)) void *dummy)
> >>> @@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy)
> >>> diff_tsc = cur_tsc - prev_tsc;
> >>>
> >>> if (unlikely(diff_tsc > drain_tsc)) {
> >>> - drain_buffers(qconf);
> >>> + drain_tx_buffers(qconf);
> >>> + drain_crypto_buffers(qconf);
> >>> prev_tsc = cur_tsc;
> >>> }
> >>>
> >>> @@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy)
> >>> if (nb_rx > 0)
> >>> process_pkts(qconf, pkts, nb_rx, portid);
> >>> }
> >>> +
> >>> + drain_inbound_crypto_queues(qconf, &qconf->inbound);
> >>> + drain_outbound_crypto_queues(qconf, &qconf->outbound);
> >> drain_inbound_crypto_queues and drain_outbound_crypto_queues should be called based on diff_tsc.
> >> moving these two lines above after drain_crypto_buffers will improve the performance drop caused due to this patch.
> > Thanks, good to know.
> > To make what you suggest above to work properly with non-legacy mode ('-l') extra changes
> > would be needed...
> What changes do you see?
Non-legacy mode relies on a drain_crypto_queues() to dequeuer crypto-ops.
It doesn't do that as part of process_pkts().
It is doable, but it means I have to rework my patches a bit.
> > Do you have an idea - what exactly causing a slowdown?
> > Just an extra function calls (drain_inbound_crypto_queues/ drain_outbound_crypto_queues)?
> > Or is that because we do dequeue() from crypto PMD more often then before?
> I have not profiled it, but it should be because of more dequeues. On a
> single call to dequeue, a burst of packets get dequeued. but now there
> will be a lot more dequeues which have lesser packets than the burst
> size which will deteriorate the performance as it would be wasting the
> dequeue cycles.
>
> This patch is causing around 5% drop out of the 10% that I mentioned in
> the other mail.
> With the change that I suggested, I am almost able to get back those 5%.
Great, any idea what causing other 5%?
Konstantin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-02 15:07 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1544805623-18150-2-git-send-email-konstantin.ananyev@intel.com>
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op might never get dequeued Konstantin Ananyev
2019-01-02 11:44 ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
2019-01-02 13:43 ` Ananyev, Konstantin
2019-01-02 13:50 ` Akhil Goyal
2019-01-02 15:06 ` Ananyev, Konstantin
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 04/10] examples/ipsec-secgw: fix outbound codepath for single SA Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 05/10] examples/ipsec-secgw: make local variables static Konstantin Ananyev
2018-12-28 15:33 ` [dpdk-stable] [PATCH v5 06/10] examples/ipsec-secgw: fix inbound SA checking Konstantin Ananyev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).