> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.go...@nxp.com]
> Sent: Friday, December 21, 2018 1:57 PM
> To: Ananyev, Konstantin <konstantin.anan...@intel.com>; dev@dpdk.org
> Cc: Nicolau, Radu <radu.nico...@intel.com>; Horton, Remy
> <remy.hor...@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/9] examples/ipsec-secgw: avoid to request
> unused TX offloads
>
> Hi Konstantin,
>
> On 12/14/2018 10:10 PM, Konstantin Ananyev wrote:
> > ipsec-secgw always enables TX offloads
> > (DEV_TX_OFFLOAD_MULTI_SEGS, DEV_TX_OFFLOAD_SECURITY),
> > even when they are not requested by the config.
> > That causes many PMD to choose full-featured TX function,
> > which in many cases is much slower then one without offloads.
> > That patch adds checks to enabled extra HW offloads, only when
> > they were requested.
> > Plus it enables DEV_TX_OFFLOAD_IPV4_CKSUM,
> > only when other HW TX ofloads are going to be enabled.
> > Otherwise SW version of ip cksum calculation is used.
> > That allows to use vector TX function, when inline-ipsec is not
> > requested.
> >
> > Signed-off-by: Remy Horton <remy.hor...@intel.com>
> > Signed-off-by: Konstantin Ananyev <konstantin.anan...@intel.com>
> > Acked-by: Radu Nicolau <radu.nico...@intel.com>
> > ---
> > examples/ipsec-secgw/ipsec-secgw.c | 44 +++++++++++++++--------
> > examples/ipsec-secgw/ipsec.h | 6 ++++
> > examples/ipsec-secgw/sa.c | 56 ++++++++++++++++++++++++++++++
> > 3 files changed, 91 insertions(+), 15 deletions(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > b/examples/ipsec-secgw/ipsec-secgw.c
> > index 1bc0b5b50..cfc2b05e5 100644
> > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > @@ -208,8 +208,6 @@ static struct rte_eth_conf port_conf = {
> > },
> > .txmode = {
> > .mq_mode = ETH_MQ_TX_NONE,
> > - .offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
> > - DEV_TX_OFFLOAD_MULTI_SEGS),
> I believe this is disabling checksum offload for all cases and then
> enabling only for inline crypto and inline proto.
Yes.
> This is breaking lookaside proto and lookaside none cases. Please
> correct me if I am wrong.
Why breaking?
For cases when HW cksum offload is disabled, IPv4 cksum calculation
will be done in SW, see below:
prepare_tx_pkt(...)
{
...
+
+ /* calculate IPv4 cksum in SW */
+ if ((pkt->ol_flags & PKT_TX_IP_CKSUM) == 0)
+ ip->ip_sum = rte_ipv4_cksum((struct ipv4_hdr *)ip);
We tested lookaside-none case quite extensively - all works well,
in fact on Intel NICs it became even a bit faster because of that change
(though not much).
Disabling HW offloads when they are not really required has 2 benefits:
1) allows app to be run on NICs without HW offloads support.
2) allows dev_configure() for TX path to select simple/vector TX functions
which for many NICs are significantly faster.
Konstantin
> So a NACK for this if my understanding is correct.
> > },
> > };
> >
> > @@ -315,7 +313,8 @@ prepare_traffic(struct rte_mbuf **pkts, struct
> > ipsec_traffic *t,
> > }
> >
> > static inline void
> > -prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port)
> > +prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
> > + const struct lcore_conf *qconf)
> > {
> > struct ip *ip;
> > struct ether_hdr *ethhdr;
> > @@ -325,14 +324,19 @@ prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port)
> > ethhdr = (struct ether_hdr *)rte_pktmbuf_prepend(pkt, ETHER_HDR_LEN);
> >
> > if (ip->ip_v == IPVERSION) {
> > - pkt->ol_flags |= PKT_TX_IP_CKSUM | PKT_TX_IPV4;
> > + pkt->ol_flags |= qconf->outbound.ipv4_offloads;
> > pkt->l3_len = sizeof(struct ip);
> > pkt->l2_len = ETHER_HDR_LEN;
> >
> > ip->ip_sum = 0;
> > +
> > + /* calculate IPv4 cksum in SW */
> > + if ((pkt->ol_flags & PKT_TX_IP_CKSUM) == 0)
> > + ip->ip_sum = rte_ipv4_cksum((struct ipv4_hdr *)ip);
> > +
> > ethhdr->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
> > } else {
> > - pkt->ol_flags |= PKT_TX_IPV6;
> > + pkt->ol_flags |= qconf->outbound.ipv6_offloads;
> > pkt->l3_len = sizeof(struct ip6_hdr);
> > pkt->l2_len = ETHER_HDR_LEN;
> >
> > @@ -346,18 +350,19 @@ prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port)
> > }
> >
> > static inline void
> > -prepare_tx_burst(struct rte_mbuf *pkts[], uint16_t nb_pkts, uint16_t port)
> > +prepare_tx_burst(struct rte_mbuf *pkts[], uint16_t nb_pkts, uint16_t port,
> > + const struct lcore_conf *qconf)
> > {
> > int32_t i;
> > const int32_t prefetch_offset = 2;
> >
> > for (i = 0; i < (nb_pkts - prefetch_offset); i++) {
> > rte_mbuf_prefetch_part2(pkts[i + prefetch_offset]);
> > - prepare_tx_pkt(pkts[i], port);
> > + prepare_tx_pkt(pkts[i], port, qconf);
> > }
> > /* Process left packets */
> > for (; i < nb_pkts; i++)
> > - prepare_tx_pkt(pkts[i], port);
> > + prepare_tx_pkt(pkts[i], port, qconf);
> > }
> >
> > /* Send burst of packets on an output interface */
> > @@ -371,7 +376,7 @@ send_burst(struct lcore_conf *qconf, uint16_t n,
> > uint16_t port)
> > queueid = qconf->tx_queue_id[port];
> > m_table = (struct rte_mbuf **)qconf->tx_mbufs[port].m_table;
> >
> > - prepare_tx_burst(m_table, n, port);
> > + prepare_tx_burst(m_table, n, port, qconf);
> >
> > ret = rte_eth_tx_burst(port, queueid, m_table, n);
> > if (unlikely(ret < n)) {
> > @@ -1543,7 +1548,7 @@ cryptodevs_init(void)
> > }
> >
> > static void
> > -port_init(uint16_t portid)
> > +port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t
> > req_tx_offloads)
> > {
> > struct rte_eth_dev_info dev_info;
> > struct rte_eth_txconf *txconf;
> > @@ -1584,10 +1589,10 @@ port_init(uint16_t portid)
> > local_port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> > }
> >
> > - if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_SECURITY)
> > - local_port_conf.rxmode.offloads |= DEV_RX_OFFLOAD_SECURITY;
> > - if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
> > - local_port_conf.txmode.offloads |= DEV_TX_OFFLOAD_SECURITY;
> > + /* Capabilities will already have been checked.. */
> > + local_port_conf.rxmode.offloads |= req_rx_offloads;
> > + local_port_conf.txmode.offloads |= req_tx_offloads;
> > +
> > if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> > local_port_conf.txmode.offloads |=
> > DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> > @@ -1639,6 +1644,13 @@ port_init(uint16_t portid)
> >
> > qconf = &lcore_conf[lcore_id];
> > qconf->tx_queue_id[portid] = tx_queueid;
> > +
> > + /* Pre-populate pkt offloads based on capabilities */
> > + qconf->outbound.ipv4_offloads = PKT_TX_IPV4;
> > + qconf->outbound.ipv6_offloads = PKT_TX_IPV6;
> > + if (req_tx_offloads & DEV_TX_OFFLOAD_IPV4_CKSUM)
> > + qconf->outbound.ipv4_offloads |= PKT_TX_IP_CKSUM;
> > +
> > tx_queueid++;
> >
> > /* init RX queues */
> > @@ -1749,6 +1761,7 @@ main(int32_t argc, char **argv)
> > uint32_t lcore_id;
> > uint8_t socket_id;
> > uint16_t portid;
> > + uint64_t req_rx_offloads, req_tx_offloads;
> >
> > /* init EAL */
> > ret = rte_eal_init(argc, argv);
> > @@ -1804,7 +1817,8 @@ main(int32_t argc, char **argv)
> > if ((enabled_port_mask & (1 << portid)) == 0)
> > continue;
> >
> > - port_init(portid);
> > + sa_check_offloads(portid, &req_rx_offloads, &req_tx_offloads);
> > + port_init(portid, req_rx_offloads, req_tx_offloads);
> > }
> >
> > cryptodevs_init();
> > diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
> > index c998c8076..9b1586f52 100644
> > --- a/examples/ipsec-secgw/ipsec.h
> > +++ b/examples/ipsec-secgw/ipsec.h
> > @@ -146,6 +146,8 @@ struct ipsec_ctx {
> > struct rte_mempool *session_pool;
> > struct rte_mbuf *ol_pkts[MAX_PKT_BURST] __rte_aligned(sizeof(void *));
> > uint16_t ol_pkts_cnt;
> > + uint64_t ipv4_offloads;
> > + uint64_t ipv6_offloads;
> > };
> >
> > struct cdev_key {
> > @@ -239,4 +241,8 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id);
> > void
> > rt_init(struct socket_ctx *ctx, int32_t socket_id);
> >
> > +int
> > +sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
> > + uint64_t *tx_offloads);
> > +
> > #endif /* __IPSEC_H__ */
> > diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> > index d2d3550a4..ff8c4b829 100644
> > --- a/examples/ipsec-secgw/sa.c
> > +++ b/examples/ipsec-secgw/sa.c
> > @@ -1017,3 +1017,59 @@ outbound_sa_lookup(struct sa_ctx *sa_ctx, uint32_t
> > sa_idx[],
> > for (i = 0; i < nb_pkts; i++)
> > sa[i] = &sa_ctx->sa[sa_idx[i]];
> > }
> > +
> > +/*
> > + * Select HW offloads to be used.
> > + */
> > +int
> > +sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
> > + uint64_t *tx_offloads)
> > +{
> > + struct ipsec_sa *rule;
> > + uint32_t idx_sa;
> > + struct rte_eth_dev_info dev_info;
> > +
> > + rte_eth_dev_info_get(port_id, &dev_info);
> > +
> > + *rx_offloads = 0;
> > + *tx_offloads = 0;
> > +
> > + /* Check for inbound rules that use offloads and use this port */
> > + for (idx_sa = 0; idx_sa < nb_sa_in; idx_sa++) {
> > + rule = &sa_in[idx_sa];
> > + if ((rule->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
> > + rule->type ==
> > + RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
> > + && rule->portid == port_id) {
> > + if ((dev_info.rx_offload_capa & DEV_RX_OFFLOAD_SECURITY)
> > + == 0) {
> > + RTE_LOG(WARNING, PORT,
> > + "HW RX IPSec is not supported\n");
> > + return -EINVAL;
> > + }
> > + *rx_offloads |= DEV_RX_OFFLOAD_SECURITY;
> > + }
> > + }
> > +
> > + /* Check for outbound rules that use offloads and use this port */
> > + for (idx_sa = 0; idx_sa < nb_sa_out; idx_sa++) {
> > + rule = &sa_out[idx_sa];
> > + if ((rule->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
> > + rule->type ==
> > + RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
> > + && rule->portid == port_id) {
> > + if ((dev_info.tx_offload_capa & DEV_TX_OFFLOAD_SECURITY)
> > + == 0) {
> > + RTE_LOG(WARNING, PORT,
> > + "HW TX IPSec is not supported\n");
> > + return -EINVAL;
> > + }
> > + *tx_offloads |= DEV_TX_OFFLOAD_SECURITY;
> > + /* Enable HW IPv4 cksum as well, if it is available */
> > + if (dev_info.tx_offload_capa &
> > + DEV_TX_OFFLOAD_IPV4_CKSUM)
> > + *tx_offloads |= DEV_TX_OFFLOAD_IPV4_CKSUM;
> > + }
> > + }
> > + return 0;
> > +}
