DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] examples/ipsec-secgw: fix width of variables
@ 2023-12-19  5:29 Anoob Joseph
  2023-12-19  5:29 ` [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets Anoob Joseph
  2023-12-19  9:51 ` [PATCH 1/2] examples/ipsec-secgw: fix width of variables Konstantin Ananyev
  0 siblings, 2 replies; 6+ messages in thread
From: Anoob Joseph @ 2023-12-19  5:29 UTC (permalink / raw)
  To: Radu Nicolau, Akhil Goyal; +Cc: Konstantin Ananyev, Jerin Jacob, dev

'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.

Fixes: 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during route lookup")

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 examples/ipsec-secgw/ipsec-secgw.c  | 5 ++---
 examples/ipsec-secgw/ipsec_worker.h | 4 ++--
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index bf98d2618b..a61bea400a 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -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;
diff --git a/examples/ipsec-secgw/ipsec_worker.h b/examples/ipsec-secgw/ipsec_worker.h
index ac980b8bcf..8e937fda3e 100644
--- a/examples/ipsec-secgw/ipsec_worker.h
+++ b/examples/ipsec-secgw/ipsec_worker.h
@@ -469,7 +469,7 @@ get_hop_for_offload_pkt(struct rte_mbuf *pkt, int is_ipv6)
 
 static __rte_always_inline void
 route4_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];
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets
  2023-12-19  5:29 [PATCH 1/2] examples/ipsec-secgw: fix width of variables Anoob Joseph
@ 2023-12-19  5:29 ` Anoob Joseph
  2023-12-19 17:18   ` Stephen Hemminger
  2023-12-19  9:51 ` [PATCH 1/2] examples/ipsec-secgw: fix width of variables Konstantin Ananyev
  1 sibling, 1 reply; 6+ messages in thread
From: Anoob Joseph @ 2023-12-19  5:29 UTC (permalink / raw)
  To: Radu Nicolau, Akhil Goyal; +Cc: Konstantin Ananyev, Jerin Jacob, dev

Instead of freeing directly, use commonly used function which also
updates stats.

Signed-off-by: Anoob Joseph <anoobj@marvell.com>
---
 examples/ipsec-secgw/ipsec_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index b0cece3ad1..ddbe30745b 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -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 */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/2] examples/ipsec-secgw: fix width of variables
  2023-12-19  5:29 [PATCH 1/2] examples/ipsec-secgw: fix width of variables Anoob Joseph
  2023-12-19  5:29 ` [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets Anoob Joseph
@ 2023-12-19  9:51 ` Konstantin Ananyev
  2024-01-09 17:38   ` Akhil Goyal
  1 sibling, 1 reply; 6+ messages in thread
From: Konstantin Ananyev @ 2023-12-19  9:51 UTC (permalink / raw)
  To: Anoob Joseph, Radu Nicolau, Akhil Goyal
  Cc: Konstantin Ananyev, Jerin Jacob, dev


> '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.
> 
> Fixes: 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during route lookup")
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c  | 5 ++---
>  examples/ipsec-secgw/ipsec_worker.h | 4 ++--
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index bf98d2618b..a61bea400a 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -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;
> diff --git a/examples/ipsec-secgw/ipsec_worker.h b/examples/ipsec-secgw/ipsec_worker.h
> index ac980b8bcf..8e937fda3e 100644
> --- a/examples/ipsec-secgw/ipsec_worker.h
> +++ b/examples/ipsec-secgw/ipsec_worker.h
> @@ -469,7 +469,7 @@ get_hop_for_offload_pkt(struct rte_mbuf *pkt, int is_ipv6)
> 
>  static __rte_always_inline void
>  route4_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];
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
 

> 2.25.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets
  2023-12-19  5:29 ` [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets Anoob Joseph
@ 2023-12-19 17:18   ` Stephen Hemminger
  2023-12-20 13:10     ` [EXT] " Anoob Joseph
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2023-12-19 17:18 UTC (permalink / raw)
  To: Anoob Joseph
  Cc: Radu Nicolau, Akhil Goyal, Konstantin Ananyev, Jerin Jacob, dev

On Tue, 19 Dec 2023 10:59:23 +0530
Anoob Joseph <anoobj@marvell.com> wrote:

> Instead of freeing directly, use commonly used function which also
> updates stats.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec_process.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
> index b0cece3ad1..ddbe30745b 100644
> --- a/examples/ipsec-secgw/ipsec_process.c
> +++ b/examples/ipsec-secgw/ipsec_process.c
> @@ -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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [EXT] Re: [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets
  2023-12-19 17:18   ` Stephen Hemminger
@ 2023-12-20 13:10     ` Anoob Joseph
  0 siblings, 0 replies; 6+ messages in thread
From: Anoob Joseph @ 2023-12-20 13:10 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Radu Nicolau, Akhil Goyal, Konstantin Ananyev,
	Jerin Jacob Kollanukkaran, dev

Hi Stephen,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, December 19, 2023 10:49 PM
> To: Anoob Joseph <anoobj@marvell.com>
> Cc: Radu Nicolau <radu.nicolau@intel.com>; Akhil Goyal
> <gakhil@marvell.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [PATCH 2/2] examples/ipsec-secgw: update stats when
> freeing packets
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Tue, 19 Dec 2023 10:59:23 +0530
> Anoob Joseph <anoobj@marvell.com> wrote:
> 
> > Instead of freeing directly, use commonly used function which also
> > updates stats.
> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> > ---
> >  examples/ipsec-secgw/ipsec_process.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/examples/ipsec-secgw/ipsec_process.c
> > b/examples/ipsec-secgw/ipsec_process.c
> > index b0cece3ad1..ddbe30745b 100644
> > --- a/examples/ipsec-secgw/ipsec_process.c
> > +++ b/examples/ipsec-secgw/ipsec_process.c
> > @@ -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.

[Anoob] Indeed. Will push a separate patch for addressing the same. Thanks for pointing out.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/2] examples/ipsec-secgw: fix width of variables
  2023-12-19  9:51 ` [PATCH 1/2] examples/ipsec-secgw: fix width of variables Konstantin Ananyev
@ 2024-01-09 17:38   ` Akhil Goyal
  0 siblings, 0 replies; 6+ messages in thread
From: Akhil Goyal @ 2024-01-09 17:38 UTC (permalink / raw)
  To: Konstantin Ananyev, Anoob Joseph, Radu Nicolau
  Cc: Konstantin Ananyev, Jerin Jacob Kollanukkaran, dev, stable

> 
> > '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.
> >
> > Fixes: 4fbfa6c7c921 ("examples/ipsec-secgw: update eth header during route
> lookup")

Cc: stable@dpdk.org

> >
> > Signed-off-by: Anoob Joseph <anoobj@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>

Applied to dpdk-next-crypto
 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
Email Id shows warning in mailmap
Contributor name/email mismatch with .mailmap:
        Konstantin Ananyev <konstantin.ananyev@huawei.com> 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?



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-01-09 17:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-19  5:29 [PATCH 1/2] examples/ipsec-secgw: fix width of variables Anoob Joseph
2023-12-19  5:29 ` [PATCH 2/2] examples/ipsec-secgw: update stats when freeing packets Anoob Joseph
2023-12-19 17:18   ` Stephen Hemminger
2023-12-20 13:10     ` [EXT] " Anoob Joseph
2023-12-19  9:51 ` [PATCH 1/2] examples/ipsec-secgw: fix width of variables Konstantin Ananyev
2024-01-09 17:38   ` Akhil Goyal

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).