examples/ipsec-secgw: fix width of variables
PATCH 1/2 examples/ipsec-secgw: fix width of variables
'rte_eth_rx_burst' returns uint16_t. The same value need to be passed to 'process_packets' functions which performs further processing. Having this function use 'uint8_t' can result in issues when MAX_PKT_BURST is larger.
The route functions (route4_pkts & route6_pkts) take uint8_t as the argument. The caller can pass larger values as the field that is passed is of type uint32_t. And the function can work with uint32_t as it loops through the packets and sends it out. Using uint8_t can result in silent packet drops.
--- examples/ipsec-secgw/ipsec-secgw.c | 5 ++--- examples/ipsec-secgw/ipsec_worker.h | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-)
examples/ipsec-secgw/ipsec-secgw.c +2 -3
@@ -568,7 +568,7 @@ process_pkts_outbound_nosp(struct ipsec_ctx *ipsec_ctx,static inline void process_pkts (struct lcore_conf *qconf,struct rte_mbuf **pkts, -uint8_t nb_pkts,uint16_t portid,void *ctx) +uint16_t nb_pkts,uint16_t portid,void *ctx) {struct ipsec_traffic traffic;
@@ -695,8 +695,7 @@ ipsec_poll_mode_worker(void)struct rte_mbuf *pkts[MAX_PKT_BURST ];uint32_t lcore_id;uint64_t prev_tsc, diff_tsc, cur_tsc; -int32_t i, nb_rx; -uint16_t portid; +uint16_t i, nb_rx, portid;uint8_t queueid;struct lcore_conf *qconf;int32_t rc, socket_id;
examples/ipsec-secgw/ipsec_worker.h +2 -2
@@ -469,7 +469,7 @@ get_hop_for_offload_pkt(struct rte_mbuf *pkt, int is_ipv6) static __rte_always_inline voidroute4_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[], -uint8_t nb_pkts,uint64_t tx_offloads,bool ip_cksum) +uint32_t nb_pkts,uint64_t tx_offloads,bool ip_cksum) {uint32_t hop[MAX_PKT_BURST *2 ];uint32_t dst_ip[MAX_PKT_BURST *2 ];
@@ -557,7 +557,7 @@ route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], } static __rte_always_inline void -route6_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[],uint8_t nb_pkts) +route6_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[],uint32_t nb_pkts) {int32_t hop[MAX_PKT_BURST *2 ];uint8_t dst_ip[MAX_PKT_BURST *2 ][16 ];
Signature · 1 line
2.25.1
PATCH 2/2 examples/ipsec-secgw: update stats when freeing packets
Instead of freeing directly, use commonly used function which also updates stats.
--- examples/ipsec-secgw/ipsec_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
examples/ipsec-secgw/ipsec_process.c +1 -1
@@ -22,7 +22,7 @@ free_cops(struct rte_crypto_op *cop[], uint32_t n)uint32_t i;for (i =0 ; i != n; i++) -rte_pktmbuf_free (cop[i]->sym->m_src); +free_pkts (&cop[i]->sym->m_src,1 ); }/* helper routine to enqueue bulk of crypto ops */
Signature · 1 line
2.25.1
Re: [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets
Quoted 13 lines · On Tue, 19 Dec 2023 10:59:23 +0530 Anoob Joseph <[email protected]> wrote:
Instead of freeing directly, use commonly used function which also updates stats.
Signed-off-by Anoob Joseph <[email protected]>--- examples/ipsec-secgw/ipsec_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)examples/ipsec-secgw/ipsec_process.c +0
-0
examples/ipsec-secgw/ipsec_process.c free_cops(struct rte_crypto_op *cop[], uint32_t n) · line 25
Quoted 5 lines
examples/ipsec-secgw/ipsec_process.c +1
-1@@ -22,7 +22,7 @@ free_cops(struct rte_crypto_op *cop[], uint32_t n)uint32_t i;for (i =0 ; i != n; i++) -rte_pktmbuf_free (cop[i]->sym->m_src); +free_pkts (&cop[i]->sym->m_src,1 );
Also, free_pkts is using a loop and should be using rte_pktmbuf_free_bulk() instead.
RE: [EXT] Re: [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets
Hi Stephen,
Please see inline.
Thanks,
Anoob
Quoted 33 lines
-----Original Message-----
From: Stephen Hemminger <[email protected]>
Sent: Tuesday, December 19, 2023 10:49 PM
To: Anoob Joseph <[email protected]>
Cc: Radu Nicolau <[email protected]>; Akhil Goyal <[email protected]>; Konstantin Ananyev <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; [email protected]
Subject: [EXT] Re: [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packetsExternal Email
----------------------------------------------------------------------
Quoted 16 lines · On Tue, 19 Dec 2023 10:59:23 +0530 Anoob Joseph <[email protected]> wrote:
Instead of freeing directly, use commonly used function which also updates stats.
Signed-off-by Anoob Joseph <[email protected]>--- examples/ipsec-secgw/ipsec_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)examples/ipsec-secgw/ipsec_process.c +0
-0@@ -22,7 +22,7 @@ free_cops(struct rte_crypto_op *cop[], uint32_t n)uint32_t i;
Quoted 6 lines
examples/ipsec-secgw/ipsec_process.c free_cops(struct rte_crypto_op *cop[], uint32_t n) · line 25
Quoted 3 lines
examples/ipsec-secgw/ipsec_process.c +1
-1for (i =0 ; i != n; i++) -rte_pktmbuf_free (cop[i]->sym->m_src); +free_pkts (&cop[i]->sym->m_src,1 );Also,
free_pktsis using a loop and should be usingrte_pktmbuf_free_bulk()instead.
[Anoob] Indeed. Will push a separate patch for addressing the same. Thanks for pointing out.
Quoted 57 lines
'
rte_eth_rx_burst' returnsuint16_t. The same value need to be passed to 'process_packets' functions which performs further processing. Having this function use 'uint8_t' can result in issues whenMAX_PKT_BURSTis larger.The route functions (
route4_pkts&route6_pkts) takeuint8_tas the argument. The caller can pass larger values as the field that is passed is of typeuint32_t. And the function can work withuint32_tas it loops through the packets and sends it out. Usinguint8_tcan result in silent packet drops.Fixes 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during route lookup")Signed-off-by Anoob Joseph <[email protected]>--- examples/ipsec-secgw/ipsec-secgw.c | 5 ++--- examples/ipsec-secgw/ipsec_worker.h | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-)examples/ipsec-secgw/ipsec-secgw.c +2
-3@@ -568,7 +568,7 @@ process_pkts_outbound_nosp(struct ipsec_ctx *ipsec_ctx,static inline void process_pkts (struct lcore_conf *qconf,struct rte_mbuf **pkts, -uint8_t nb_pkts,uint16_t portid,void *ctx) +uint16_t nb_pkts,uint16_t portid,void *ctx) {struct ipsec_traffic traffic;@@ -695,8 +695,7 @@ ipsec_poll_mode_worker(void)struct rte_mbuf *pkts[MAX_PKT_BURST ];uint32_t lcore_id;uint64_t prev_tsc, diff_tsc, cur_tsc; -int32_t i, nb_rx; -uint16_t portid; +uint16_t i, nb_rx, portid;uint8_t queueid;struct lcore_conf *qconf;int32_t rc, socket_id;examples/ipsec-secgw/ipsec_worker.h +1
-1@@ -469,7 +469,7 @@ get_hop_for_offload_pkt(struct rte_mbuf *pkt, int is_ipv6) static __rte_always_inline voidroute4_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[], -uint8_t nb_pkts,uint64_t tx_offloads,bool ip_cksum) +uint32_t nb_pkts,uint64_t tx_offloads,bool ip_cksum) {uint32_t hop[MAX_PKT_BURST *2 ];uint32_t dst_ip[MAX_PKT_BURST *2 ];
examples/ipsec-secgw/ipsec_worker.h route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[],
Quoted 9 lines
examples/ipsec-secgw/ipsec_worker.h +1
-2@@ -557,7 +557,7 @@ route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], } static __rte_always_inline void -route6_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[],uint8_t nb_pkts) +route6_pkts (struct rt_ctx *rt_ctx,struct rte_mbuf *pkts[],uint32_t nb_pkts) {int32_t hop[MAX_PKT_BURST *2 ];uint8_t dst_ip[MAX_PKT_BURST *2 ][16 ]; --
Quoted 1 line
2.25.1
Quoted 6 lines
Quoted 6 lines
'
rte_eth_rx_burst' returnsuint16_t. The same value need to be passed to 'process_packets' functions which performs further processing. Having this function use 'uint8_t' can result in issues whenMAX_PKT_BURSTis larger.
Quoted 8 lines
Quoted 7 lines
The route functions (
route4_pkts&route6_pkts) takeuint8_tas the argument. The caller can pass larger values as the field that is passed is of typeuint32_t. And the function can work withuint32_tas it loops through the packets and sends it out. Usinguint8_tcan result in silent packet drops.Fixes 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during routelookup")
Quoted 2 lines
Quoted 2 lines
Signed-off-by Anoob Joseph <[email protected]>
Applied to dpdk-next-crypto
Quoted 1 line
Email Id shows warning in mailmap
Contributor name/email mismatch with .mailmap:
Konstantin Ananyev <[email protected]> is not the primary email address
I believe Konstantin is using both email ids, but mailmap is taking the first one as primary one. Konstantin,
Is it possible for you to use a single email ID and update mailmap accordingly?