[dpdk-dev] [PATCH] examples: ipv4, udp and tcp checksum offload warning

[dpdk-dev] [PATCH] examples: ipv4, udp and tcp checksum offload warning

[usamanadeem321]

Closes gracefully if IPV4 checksum offload is not available.
Gives warning if UDP or TCP checksum offloads are not available.

Signed-off-by: usamanadeem321 <usama.nadeem@emumba.com>
---
 examples/l3fwd/main.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 00ac267af1..81e605700e 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -123,7 +123,7 @@ static struct rte_eth_conf port_conf = {
 		.mq_mode = ETH_MQ_RX_RSS,
 		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
-		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
+
 	},
 	.rx_adv_conf = {
 		.rss_conf = {
@@ -1039,6 +1039,30 @@ l3fwd_poll_resource_setup(void)
 			local_port_conf.txmode.offloads |=
 				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			 DEV_RX_OFFLOAD_IPV4_CKSUM;
+		else {
+			rte_exit(EXIT_FAILURE,
+				"IPV4 Checksum offload not available. (port %u) ",
+				portid);
+		}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_UDP_CKSUM;
+
+		else
+			printf("WARNING: UDP Checksum offload not available.\n");
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_TCP_CKSUM;
+
+		else
+			printf("WARNING: TCP Checksum offload not available.\n");
+
+
 		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 			dev_info.flow_type_rss_offloads;
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH] examples: ipv4, udp and tcp checksum offload warning

[Stephen Hemminger]

On Mon, 13 Sep 2021 17:09:51 +0500
usamanadeem321 <usama.nadeem@emumba.com> wrote:

>  
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			 DEV_RX_OFFLOAD_IPV4_CKSUM;
> +		else {
> +			rte_exit(EXIT_FAILURE,
> +				"IPV4 Checksum offload not available. (port %u) ",
> +				portid);
> +		}
> +

Why not just do it in software if not available. The IPv4 checksum
is so cheap many operating systems just always do it in software.

[dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available

[Usama Nadeem]

From: usamanadeem321 <usama.nadeem@emumba.com>

Checks if IPV4, UDP and TCP Checksum offloads are available.
If not available, prints a warning message.

Bugzilla ID: 545
Signed-off-by: usamanadeem321 <usama.nadeem@emumba.com>
---
 examples/l3fwd/main.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 00ac267af1..ae62bc570d 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -123,7 +123,6 @@ static struct rte_eth_conf port_conf = {
 		.mq_mode = ETH_MQ_RX_RSS,
 		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
-		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
 	},
 	.rx_adv_conf = {
 		.rss_conf = {
@@ -1039,6 +1038,27 @@ l3fwd_poll_resource_setup(void)
 			local_port_conf.txmode.offloads |=
 				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			 DEV_RX_OFFLOAD_IPV4_CKSUM;
+		else {
+			printf("WARNING: IPV4 Checksum offload not available.\n");
+		}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_UDP_CKSUM;
+
+		else
+			printf("WARNING: UDP Checksum offload not available.\n");
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_TCP_CKSUM;
+
+		else
+			printf("WARNING: TCP Checksum offload not available.\n");
+
 		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 			dev_info.flow_type_rss_offloads;
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available

[Stephen Hemminger]

On Tue, 14 Sep 2021 23:08:27 +0500
Usama Nadeem <usama.nadeem@emumba.com> wrote:

> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_UDP_CKSUM;
> +
> +		else
> +			printf("WARNING: UDP Checksum offload not available.\n");
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_TCP_CKSUM;
> +
> +		else
> +			printf("WARNING: TCP Checksum offload not available.\n");

Why does l3fwd care about L4 checksum offload?
The application should really be just a simple L3 router. But it
seems to have become a test for ptype and depends on TCP/UDP.

Re: [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available

[Ananyev, Konstantin]

> 
> From: usamanadeem321 <usama.nadeem@emumba.com>
> 
> Checks if IPV4, UDP and TCP Checksum offloads are available.
> If not available, prints a warning message.
> 
> Bugzilla ID: 545
> Signed-off-by: usamanadeem321 <usama.nadeem@emumba.com>
> ---
>  examples/l3fwd/main.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 00ac267af1..ae62bc570d 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -123,7 +123,6 @@ static struct rte_eth_conf port_conf = {
>  		.mq_mode = ETH_MQ_RX_RSS,
>  		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
>  		.split_hdr_size = 0,
> -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
>  	},
>  	.rx_adv_conf = {
>  		.rss_conf = {
> @@ -1039,6 +1038,27 @@ l3fwd_poll_resource_setup(void)
>  			local_port_conf.txmode.offloads |=
>  				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			 DEV_RX_OFFLOAD_IPV4_CKSUM;
> +		else {
> +			printf("WARNING: IPV4 Checksum offload not available.\n");
> +		}
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_UDP_CKSUM;
> +
> +		else
> +			printf("WARNING: UDP Checksum offload not available.\n");
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_TCP_CKSUM;
> +
> +		else
> +			printf("WARNING: TCP Checksum offload not available.\n");
> +

Sorry, but I didn't get the logic:
Application expects some offloads to be supported by HW.
You add the code that checks for offloads, but if they are not supported just prints warning
and continues, as if everything is ok. Doesn't look like correct behaviour to me.
I think, it should either terminate with error message or be prepared to work properly
on HW without these offloads (check cksums in SW if necessary).
In fact I don't see what was wrong with original behaviour, one thing that probably
was missing - more descriptive error message. 

>  		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>  			dev_info.flow_type_rss_offloads;
> 
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available

[Stephen Hemminger]

On Tue, 14 Sep 2021 22:22:04 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> > 
> > From: usamanadeem321 <usama.nadeem@emumba.com>
> > 
> > Checks if IPV4, UDP and TCP Checksum offloads are available.
> > If not available, prints a warning message.
> > 
> > Bugzilla ID: 545
> > Signed-off-by: usamanadeem321 <usama.nadeem@emumba.com>
> > ---
> >  examples/l3fwd/main.c | 22 +++++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > index 00ac267af1..ae62bc570d 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -123,7 +123,6 @@ static struct rte_eth_conf port_conf = {
> >  		.mq_mode = ETH_MQ_RX_RSS,
> >  		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> >  		.split_hdr_size = 0,
> > -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
> >  	},
> >  	.rx_adv_conf = {
> >  		.rss_conf = {
> > @@ -1039,6 +1038,27 @@ l3fwd_poll_resource_setup(void)
> >  			local_port_conf.txmode.offloads |=
> >  				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> > 
> > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> > +			local_port_conf.rxmode.offloads |=
> > +			 DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +		else {
> > +			printf("WARNING: IPV4 Checksum offload not available.\n");
> > +		}
> > +
> > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> > +			local_port_conf.rxmode.offloads |=
> > +			DEV_RX_OFFLOAD_UDP_CKSUM;
> > +
> > +		else
> > +			printf("WARNING: UDP Checksum offload not available.\n");
> > +
> > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> > +			local_port_conf.rxmode.offloads |=
> > +			DEV_RX_OFFLOAD_TCP_CKSUM;
> > +
> > +		else
> > +			printf("WARNING: TCP Checksum offload not available.\n");
> > +  
> 
> Sorry, but I didn't get the logic:
> Application expects some offloads to be supported by HW.

The application is expecting more offloads than is necessary for basic
IP level forwarding which is all the example is documented to do.

  "The application performs L3 forwarding."

> You add the code that checks for offloads, but if they are not supported just prints warning
> and continues, as if everything is ok. Doesn't look like correct behaviour to me.
> I think, it should either terminate with error message or be prepared to work properly
> on HW without these offloads (check cksums in SW if necessary).
> In fact I don't see what was wrong with original behaviour, one thing that probably
> was missing - more descriptive error message. 

It is not a problem with your patch, it is fine.

It is a problem in how l3fwd has grown and changed and no longer really what
was intended in the original version. There is no reason that the application
should be looking at L4 data. In fact, it shouldn't care if it gets TCP, UDP, SCP or DCCP;
but the application now depends on ptype.

It should be possible to do L3 forwarding independent of packet type.
The application only needs to look at Ether type and do IPv4 or IPv6 based on that.

Re: [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available

[Ananyev, Konstantin]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Wednesday, September 15, 2021 12:44 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Usama Nadeem <usama.nadeem@emumba.com>; thomas@monjalon.net; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available
> 
> On Tue, 14 Sep 2021 22:22:04 +0000
> "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> 
> > >
> > > From: usamanadeem321 <usama.nadeem@emumba.com>
> > >
> > > Checks if IPV4, UDP and TCP Checksum offloads are available.
> > > If not available, prints a warning message.
> > >
> > > Bugzilla ID: 545
> > > Signed-off-by: usamanadeem321 <usama.nadeem@emumba.com>
> > > ---
> > >  examples/l3fwd/main.c | 22 +++++++++++++++++++++-
> > >  1 file changed, 21 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > > index 00ac267af1..ae62bc570d 100644
> > > --- a/examples/l3fwd/main.c
> > > +++ b/examples/l3fwd/main.c
> > > @@ -123,7 +123,6 @@ static struct rte_eth_conf port_conf = {
> > >  		.mq_mode = ETH_MQ_RX_RSS,
> > >  		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> > >  		.split_hdr_size = 0,
> > > -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
> > >  	},
> > >  	.rx_adv_conf = {
> > >  		.rss_conf = {
> > > @@ -1039,6 +1038,27 @@ l3fwd_poll_resource_setup(void)
> > >  			local_port_conf.txmode.offloads |=
> > >  				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> > >
> > > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> > > +			local_port_conf.rxmode.offloads |=
> > > +			 DEV_RX_OFFLOAD_IPV4_CKSUM;
> > > +		else {
> > > +			printf("WARNING: IPV4 Checksum offload not available.\n");
> > > +		}
> > > +
> > > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> > > +			local_port_conf.rxmode.offloads |=
> > > +			DEV_RX_OFFLOAD_UDP_CKSUM;
> > > +
> > > +		else
> > > +			printf("WARNING: UDP Checksum offload not available.\n");
> > > +
> > > +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> > > +			local_port_conf.rxmode.offloads |=
> > > +			DEV_RX_OFFLOAD_TCP_CKSUM;
> > > +
> > > +		else
> > > +			printf("WARNING: TCP Checksum offload not available.\n");
> > > +
> >
> > Sorry, but I didn't get the logic:
> > Application expects some offloads to be supported by HW.
> 
> The application is expecting more offloads than is necessary for basic
> IP level forwarding which is all the example is documented to do.
> 
>   "The application performs L3 forwarding."
> 
> > You add the code that checks for offloads, but if they are not supported just prints warning
> > and continues, as if everything is ok. Doesn't look like correct behaviour to me.
> > I think, it should either terminate with error message or be prepared to work properly
> > on HW without these offloads (check cksums in SW if necessary).
> > In fact I don't see what was wrong with original behaviour, one thing that probably
> > was missing - more descriptive error message.
> 
> It is not a problem with your patch, it is fine.
> 
> It is a problem in how l3fwd has grown and changed and no longer really what
> was intended in the original version. There is no reason that the application
> should be looking at L4 data. In fact, it shouldn't care if it gets TCP, UDP, SCP or DCCP;
> but the application now depends on ptype.
> 
> It should be possible to do L3 forwarding independent of packet type.
> The application only needs to look at Ether type and do IPv4 or IPv6 based on that.
> 

As I remember l3fwd cares about L4 headers (chan cksums) because it can do FWD decisions
based on 5-tuple (exact-macth mode).
I presume that's the reason L4 cksum offloads was enabled at first place.
For LPM/FIB I believe ipv4 cksum check should be sufficient.
If we believe that some offloads are excessive,
then I think right way is to simply remove them
(with updating docs and source in a proper way etc.).
Just printing warnings and continuing seems wrong to me.
  

[dpdk-dev] [PATCH v3] ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

checks if ipv4 and udptcp cksum offload capability available
If not available, cksum is verified through software
If cksum is corrupt, packet is dropped, rest of the packets
are forwarded back.

Bugzilla ID:545
Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
---
 examples/l3fwd/l3fwd.h     |  6 ++++
 examples/l3fwd/l3fwd_lpm.c | 71 ++++++++++++++++++++++++++++++++++++--
 examples/l3fwd/main.c      | 32 +++++++++++++++--
 3 files changed, 103 insertions(+), 6 deletions(-)

diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
index a808d60247..b0b6a906d4 100644
--- a/examples/l3fwd/l3fwd.h
+++ b/examples/l3fwd/l3fwd.h
@@ -55,6 +55,8 @@
 #define L3FWD_HASH_ENTRIES		(1024*1024*1)
 #endif
 #define HASH_ENTRY_NUMBER_DEFAULT	4
+bool l3_sft_cksum;
+bool l4_sft_cksum;
 
 struct mbuf_table {
 	uint16_t len;
@@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
 int
 lpm_main_loop(__rte_unused void *dummy);
 
+int
+check_software_cksum(struct rte_mbuf **pkts_burst,
+struct rte_mbuf **pkts_burst_to_send, int nb_rx);
+
 int
 fib_main_loop(__rte_unused void *dummy);
 
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 232b606b54..e237ca6bc4 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -26,6 +26,7 @@
 #include <rte_udp.h>
 #include <rte_lpm.h>
 #include <rte_lpm6.h>
+#include <rte_net.h>
 
 #include "l3fwd.h"
 #include "l3fwd_event.h"
@@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf *qconf, struct rte_mbuf *pkt,
 #include "l3fwd_lpm.h"
 #endif
 
+
+int check_software_cksum(struct rte_mbuf **pkts_burst,
+struct rte_mbuf **pkts_burst_to_send, int nb_rx)
+{
+	int j;
+	int i = 0;
+	struct rte_net_hdr_lens hdr_lens;
+	struct rte_ipv4_hdr *ipv4_hdr;
+	void *l3_hdr;
+	void *l4_hdr;
+	rte_be16_t prev_cksum;
+	int dropped_pkts_udp_tcp = 0;
+	int dropped_pkts_ipv4 = 0;
+	bool dropped;
+	for (j = 0; j < nb_rx; j++) {
+		dropped = false;
+		rte_net_get_ptype(pkts_burst[j], &hdr_lens, RTE_PTYPE_ALL_MASK);
+		l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
+		void *, hdr_lens.l2_len);
+		l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
+		void *, hdr_lens.l2_len + hdr_lens.l3_len);
+		ipv4_hdr = l3_hdr;
+		prev_cksum = ipv4_hdr->hdr_checksum;
+		ipv4_hdr->hdr_checksum = 0;
+		ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
+
+		if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
+			rte_pktmbuf_free(pkts_burst[j]);
+			dropped_pkts_ipv4++;
+			dropped = true;
+		} else if (l4_sft_cksum &&
+				rte_ipv4_udptcp_cksum_verify
+				(l3_hdr, l4_hdr) != 0) {
+
+			rte_pktmbuf_free(pkts_burst[j]);
+			dropped_pkts_udp_tcp++;
+			dropped = true;
+		}
+		if (dropped == false) {
+			pkts_burst_to_send[i] = pkts_burst[j];
+			i++;
+		}
+
+	}
+	return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
+}
+
 /* main processing loop */
 int
 lpm_main_loop(__rte_unused void *dummy)
 {
 	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
+	struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
 	unsigned lcore_id;
 	uint64_t prev_tsc, diff_tsc, cur_tsc;
 	int i, nb_rx;
 	uint16_t portid;
 	uint8_t queueid;
+	int dropped;
 	struct lcore_conf *qconf;
 	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
 		US_PER_S * BURST_TX_DRAIN_US;
@@ -208,20 +258,35 @@ lpm_main_loop(__rte_unused void *dummy)
 				MAX_PKT_BURST);
 			if (nb_rx == 0)
 				continue;
+			if (l3_sft_cksum || l4_sft_cksum) {
+				dropped = check_software_cksum(pkts_burst,
+				pkts_burst_to_send,	nb_rx);
+
+				nb_rx = nb_rx-dropped;
+			}
+
 
 #if defined RTE_ARCH_X86 || defined __ARM_NEON \
 			 || defined RTE_ARCH_PPC_64
+		if (l3_sft_cksum == false && l4_sft_cksum == false)
 			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
 						portid, qconf);
+		else
+			l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
+						portid, qconf);
+
 #else
-			l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
+			if (l3_sft_cksum == false && l4_sft_cksum == false)
+				l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
 							portid, qconf);
+			else
+				l3fwd_lpm_no_opt_send_packets(nb_rx,
+				pkts_burst_to_send, portid, qconf);
+
 #endif /* X86 */
 		}
-
 		cur_tsc = rte_rdtsc();
 	}
-
 	return 0;
 }
 
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 00ac267af1..68248fd189 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -123,7 +123,6 @@ static struct rte_eth_conf port_conf = {
 		.mq_mode = ETH_MQ_RX_RSS,
 		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
-		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
 	},
 	.rx_adv_conf = {
 		.rss_conf = {
@@ -981,6 +980,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t queueid)
 	return 0;
 }
 
+
 static void
 l3fwd_poll_resource_setup(void)
 {
@@ -993,7 +993,8 @@ l3fwd_poll_resource_setup(void)
 	unsigned int nb_ports;
 	unsigned int lcore_id;
 	int ret;
-
+	l3_sft_cksum = false;
+	l4_sft_cksum = false;
 	if (check_lcore_params() < 0)
 		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
 
@@ -1034,11 +1035,36 @@ l3fwd_poll_resource_setup(void)
 			rte_exit(EXIT_FAILURE,
 				"Error during getting device (port %u) info: %s\n",
 				portid, strerror(-ret));
-
 		if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
 			local_port_conf.txmode.offloads |=
 				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_IPV4_CKSUM;
+		else {
+			l3_sft_cksum = true;
+			printf("WARNING: IPV4 Checksum offload not available.\n");
+			}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_UDP_CKSUM;
+
+		else {
+			l4_sft_cksum = true;
+			printf("WARNING: UDP Checksum offload not available.\n");
+			}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_TCP_CKSUM;
+
+		else {
+			l4_sft_cksum = true;
+			printf("WARNING: TCP Checksum offload not available.\n");
+			}
+
 		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 			dev_info.flow_type_rss_offloads;
 
-- 
2.17.1

[dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

checks if ipv4 and udptcp cksum offload capability available
If not available, cksum is verified through software
If cksum is corrupt, packet is dropped, rest of the packets
are forwarded back.

Bugzilla ID:545
Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
---
 examples/l3fwd/l3fwd.h     |  6 ++++
 examples/l3fwd/l3fwd_lpm.c | 72 ++++++++++++++++++++++++++++++++++++--
 examples/l3fwd/main.c      | 33 +++++++++++++++--
 3 files changed, 105 insertions(+), 6 deletions(-)

diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
index a808d60247..c2c21a91fb 100644
--- a/examples/l3fwd/l3fwd.h
+++ b/examples/l3fwd/l3fwd.h
@@ -55,6 +55,8 @@
 #define L3FWD_HASH_ENTRIES		(1024*1024*1)
 #endif
 #define HASH_ENTRY_NUMBER_DEFAULT	4
+extern bool l3_sft_cksum;
+extern bool l4_sft_cksum;
 
 struct mbuf_table {
 	uint16_t len;
@@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
 int
 lpm_main_loop(__rte_unused void *dummy);
 
+int
+check_software_cksum(struct rte_mbuf **pkts_burst,
+struct rte_mbuf **pkts_burst_to_send, int nb_rx);
+
 int
 fib_main_loop(__rte_unused void *dummy);
 
diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
index 232b606b54..ecaf323943 100644
--- a/examples/l3fwd/l3fwd_lpm.c
+++ b/examples/l3fwd/l3fwd_lpm.c
@@ -26,6 +26,7 @@
 #include <rte_udp.h>
 #include <rte_lpm.h>
 #include <rte_lpm6.h>
+#include <rte_net.h>
 
 #include "l3fwd.h"
 #include "l3fwd_event.h"
@@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf *qconf, struct rte_mbuf *pkt,
 #include "l3fwd_lpm.h"
 #endif
 
+
+int check_software_cksum(struct rte_mbuf **pkts_burst,
+struct rte_mbuf **pkts_burst_to_send, int nb_rx)
+{
+	int j;
+	int i = 0;
+	struct rte_net_hdr_lens hdr_lens;
+	struct rte_ipv4_hdr *ipv4_hdr;
+	void *l3_hdr;
+	void *l4_hdr;
+	rte_be16_t prev_cksum;
+	int dropped_pkts_udp_tcp = 0;
+	int dropped_pkts_ipv4 = 0;
+	bool dropped;
+	for (j = 0; j < nb_rx; j++) {
+		dropped = false;
+		rte_net_get_ptype(pkts_burst[j], &hdr_lens, RTE_PTYPE_ALL_MASK);
+		l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
+		void *, hdr_lens.l2_len);
+		l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
+		void *, hdr_lens.l2_len + hdr_lens.l3_len);
+		ipv4_hdr = l3_hdr;
+		prev_cksum = ipv4_hdr->hdr_checksum;
+		ipv4_hdr->hdr_checksum = 0;
+		ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
+
+		if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
+			rte_pktmbuf_free(pkts_burst[j]);
+			dropped_pkts_ipv4++;
+			dropped = true;
+		} else if (l4_sft_cksum &&
+				rte_ipv4_udptcp_cksum_verify
+				(l3_hdr, l4_hdr) != 0) {
+
+			rte_pktmbuf_free(pkts_burst[j]);
+			dropped_pkts_udp_tcp++;
+			dropped = true;
+		}
+		if (dropped == false) {
+			pkts_burst_to_send[i] = pkts_burst[j];
+			i++;
+		}
+
+	}
+	return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
+}
+
 /* main processing loop */
 int
 lpm_main_loop(__rte_unused void *dummy)
 {
 	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
+	struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
 	unsigned lcore_id;
 	uint64_t prev_tsc, diff_tsc, cur_tsc;
 	int i, nb_rx;
 	uint16_t portid;
 	uint8_t queueid;
+	int dropped;
 	struct lcore_conf *qconf;
 	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
 		US_PER_S * BURST_TX_DRAIN_US;
@@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
 			if (nb_rx == 0)
 				continue;
 
+			if (l3_sft_cksum || l4_sft_cksum) {
+				dropped = check_software_cksum(pkts_burst,
+				pkts_burst_to_send,	nb_rx);
+
+				nb_rx = nb_rx-dropped;
+			}
+
+
 #if defined RTE_ARCH_X86 || defined __ARM_NEON \
 			 || defined RTE_ARCH_PPC_64
+		if (l3_sft_cksum == false && l4_sft_cksum == false)
 			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
 						portid, qconf);
+		else
+			l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
+						portid, qconf);
+
 #else
-			l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
+			if (l3_sft_cksum == false && l4_sft_cksum == false)
+				l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
 							portid, qconf);
+			else
+				l3fwd_lpm_no_opt_send_packets(nb_rx,
+				pkts_burst_to_send, portid, qconf);
+
 #endif /* X86 */
 		}
-
 		cur_tsc = rte_rdtsc();
 	}
-
 	return 0;
 }
 
diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
index 00ac267af1..a54aca070d 100644
--- a/examples/l3fwd/main.c
+++ b/examples/l3fwd/main.c
@@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
 /**< Ports set in promiscuous mode off by default. */
 static int promiscuous_on;
 
+bool l3_sft_cksum;
+bool l4_sft_cksum;
+
 /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
 enum L3FWD_LOOKUP_MODE {
 	L3FWD_LOOKUP_DEFAULT,
@@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
 		.mq_mode = ETH_MQ_RX_RSS,
 		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
 		.split_hdr_size = 0,
-		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
 	},
 	.rx_adv_conf = {
 		.rss_conf = {
@@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t queueid)
 	return 0;
 }
 
+
 static void
 l3fwd_poll_resource_setup(void)
 {
@@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
 	unsigned int nb_ports;
 	unsigned int lcore_id;
 	int ret;
-
+	l3_sft_cksum = false;
+	l4_sft_cksum = false;
 	if (check_lcore_params() < 0)
 		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
 
@@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
 			rte_exit(EXIT_FAILURE,
 				"Error during getting device (port %u) info: %s\n",
 				portid, strerror(-ret));
-
 		if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
 			local_port_conf.txmode.offloads |=
 				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
 
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
+			local_port_conf.rxmode.offloads |=
+			DEV_RX_OFFLOAD_IPV4_CKSUM;
+		else {
+			l3_sft_cksum = true;
+			printf("WARNING: IPV4 checksum offload not available.\n");
+			}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_UDP_CKSUM;
+		else {
+			l4_sft_cksum = true;
+			printf("WARNING: UDP checksum offload not available.\n");
+		}
+
+		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
+			local_port_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_TCP_CKSUM;
+		else {
+			l4_sft_cksum = true;
+			printf("WARNING: TCP checksum offload not available.\n");
+		}
+
 		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
 			dev_info.flow_type_rss_offloads;
 
-- 
2.25.1

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

Hi,
Please have a look at this patch, submitted around 2 weeks ago. Let me know
if any further changes are required in this patch.

Thanks
-usama

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Walsh, Conor]

> From: dev <dev-bounces@dpdk.org> On Behalf Of Usama Nadeem
> Sent: Thursday 14 October 2021 19:43
> To: thomas@monjalon.net
> Cc: dev@dpdk.org; Usama Nadeem <usama.nadeem@emumba.com>
> Subject: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum
> verification through software
> 
> checks if ipv4 and udptcp cksum offload capability available
> If not available, cksum is verified through software
> If cksum is corrupt, packet is dropped, rest of the packets
> are forwarded back.
> 
> Bugzilla ID:545
> Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> ---

Hi Usama,

This should be done in a generic way that allows all the lookup methods to support it not just LPM.
check_software_cksum should go in a common file and be called from LPM, FIB and possibly EM.

Thanks,
Conor.

>  examples/l3fwd/l3fwd.h     |  6 ++++
>  examples/l3fwd/l3fwd_lpm.c | 72
> ++++++++++++++++++++++++++++++++++++--
>  examples/l3fwd/main.c      | 33 +++++++++++++++--
>  3 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> index a808d60247..c2c21a91fb 100644
> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -55,6 +55,8 @@
>  #define L3FWD_HASH_ENTRIES		(1024*1024*1)
>  #endif
>  #define HASH_ENTRY_NUMBER_DEFAULT	4
> +extern bool l3_sft_cksum;
> +extern bool l4_sft_cksum;
> 
>  struct mbuf_table {
>  	uint16_t len;
> @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
>  int
>  lpm_main_loop(__rte_unused void *dummy);
> 
> +int
> +check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> +
>  int
>  fib_main_loop(__rte_unused void *dummy);
> 
> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> index 232b606b54..ecaf323943 100644
> --- a/examples/l3fwd/l3fwd_lpm.c
> +++ b/examples/l3fwd/l3fwd_lpm.c
> @@ -26,6 +26,7 @@
>  #include <rte_udp.h>
>  #include <rte_lpm.h>
>  #include <rte_lpm6.h>
> +#include <rte_net.h>
> 
>  #include "l3fwd.h"
>  #include "l3fwd_event.h"
> @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct
> lcore_conf *qconf, struct rte_mbuf *pkt,
>  #include "l3fwd_lpm.h"
>  #endif
> 
> +
> +int check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> +{
> +	int j;
> +	int i = 0;
> +	struct rte_net_hdr_lens hdr_lens;
> +	struct rte_ipv4_hdr *ipv4_hdr;
> +	void *l3_hdr;
> +	void *l4_hdr;
> +	rte_be16_t prev_cksum;
> +	int dropped_pkts_udp_tcp = 0;
> +	int dropped_pkts_ipv4 = 0;
> +	bool dropped;
> +	for (j = 0; j < nb_rx; j++) {
> +		dropped = false;
> +		rte_net_get_ptype(pkts_burst[j], &hdr_lens,
> RTE_PTYPE_ALL_MASK);
> +		l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +		void *, hdr_lens.l2_len);
> +		l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +		void *, hdr_lens.l2_len + hdr_lens.l3_len);
> +		ipv4_hdr = l3_hdr;
> +		prev_cksum = ipv4_hdr->hdr_checksum;
> +		ipv4_hdr->hdr_checksum = 0;
> +		ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> +
> +		if (l3_sft_cksum && prev_cksum != ipv4_hdr-
> >hdr_checksum) {
> +			rte_pktmbuf_free(pkts_burst[j]);
> +			dropped_pkts_ipv4++;
> +			dropped = true;
> +		} else if (l4_sft_cksum &&
> +				rte_ipv4_udptcp_cksum_verify
> +				(l3_hdr, l4_hdr) != 0) {
> +
> +			rte_pktmbuf_free(pkts_burst[j]);
> +			dropped_pkts_udp_tcp++;
> +			dropped = true;
> +		}
> +		if (dropped == false) {
> +			pkts_burst_to_send[i] = pkts_burst[j];
> +			i++;
> +		}
> +
> +	}
> +	return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> +}
> +
>  /* main processing loop */
>  int
>  lpm_main_loop(__rte_unused void *dummy)
>  {
>  	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> +	struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
>  	unsigned lcore_id;
>  	uint64_t prev_tsc, diff_tsc, cur_tsc;
>  	int i, nb_rx;
>  	uint16_t portid;
>  	uint8_t queueid;
> +	int dropped;
>  	struct lcore_conf *qconf;
>  	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
>  		US_PER_S * BURST_TX_DRAIN_US;
> @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
>  			if (nb_rx == 0)
>  				continue;
> 
> +			if (l3_sft_cksum || l4_sft_cksum) {
> +				dropped =
> check_software_cksum(pkts_burst,
> +				pkts_burst_to_send,	nb_rx);
> +
> +				nb_rx = nb_rx-dropped;
> +			}
> +
> +
>  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
>  			 || defined RTE_ARCH_PPC_64
> +		if (l3_sft_cksum == false && l4_sft_cksum == false)
>  			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
>  						portid, qconf);
> +		else
> +			l3fwd_lpm_send_packets(nb_rx,
> pkts_burst_to_send,
> +						portid, qconf);
> +
>  #else
> -			l3fwd_lpm_no_opt_send_packets(nb_rx,
> pkts_burst,
> +			if (l3_sft_cksum == false && l4_sft_cksum == false)
> +				l3fwd_lpm_no_opt_send_packets(nb_rx,
> pkts_burst,
>  							portid, qconf);
> +			else
> +				l3fwd_lpm_no_opt_send_packets(nb_rx,
> +				pkts_burst_to_send, portid, qconf);
> +
>  #endif /* X86 */
>  		}
> -
>  		cur_tsc = rte_rdtsc();
>  	}
> -
>  	return 0;
>  }
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 00ac267af1..a54aca070d 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
>  /**< Ports set in promiscuous mode off by default. */
>  static int promiscuous_on;
> 
> +bool l3_sft_cksum;
> +bool l4_sft_cksum;
> +
>  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
>  enum L3FWD_LOOKUP_MODE {
>  	L3FWD_LOOKUP_DEFAULT,
> @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
>  		.mq_mode = ETH_MQ_RX_RSS,
>  		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
>  		.split_hdr_size = 0,
> -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
>  	},
>  	.rx_adv_conf = {
>  		.rss_conf = {
> @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
> queueid)
>  	return 0;
>  }
> 
> +
>  static void
>  l3fwd_poll_resource_setup(void)
>  {
> @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
>  	unsigned int nb_ports;
>  	unsigned int lcore_id;
>  	int ret;
> -
> +	l3_sft_cksum = false;
> +	l4_sft_cksum = false;
>  	if (check_lcore_params() < 0)
>  		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> 
> @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
>  			rte_exit(EXIT_FAILURE,
>  				"Error during getting device (port %u) info:
> %s\n",
>  				portid, strerror(-ret));
> -
>  		if (dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>  			local_port_conf.txmode.offloads |=
>  				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> +		if (dev_info.rx_offload_capa &
> DEV_RX_OFFLOAD_IPV4_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_IPV4_CKSUM;
> +		else {
> +			l3_sft_cksum = true;
> +			printf("WARNING: IPV4 checksum offload not
> available.\n");
> +			}
> +
> +		if (dev_info.rx_offload_capa &
> DEV_RX_OFFLOAD_UDP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_UDP_CKSUM;
> +		else {
> +			l4_sft_cksum = true;
> +			printf("WARNING: UDP checksum offload not
> available.\n");
> +		}
> +
> +		if (dev_info.rx_offload_capa &
> DEV_RX_OFFLOAD_TCP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_TCP_CKSUM;
> +		else {
> +			l4_sft_cksum = true;
> +			printf("WARNING: TCP checksum offload not
> available.\n");
> +		}
> +
>  		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>  			dev_info.flow_type_rss_offloads;
> 
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Ananyev, Konstantin]

> checks if ipv4 and udptcp cksum offload capability available
> If not available, cksum is verified through software
> If cksum is corrupt, packet is dropped, rest of the packets
> are forwarded back.

From what I see right now l3fwd:
   a) enables HW RX cksum offload 
   b) simply ignores HW provided cksum status 
Which came as a real surprise to me. 
Feel free to correct me if I missed something obvious here.

So, I think first we need to add missing check first,
even though it might cause some perf drop. 
Then make changes to actually check provided by HW status and
when HW doesn't provide such offload do check in SW.

Another alternative would be to remove request for HW offloads
and document l3fwd that it doesn't check checksums at all,
but I don't think it is a good way.

> Bugzilla ID:545
> Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> ---
>  examples/l3fwd/l3fwd.h     |  6 ++++
>  examples/l3fwd/l3fwd_lpm.c | 72 ++++++++++++++++++++++++++++++++++++--
>  examples/l3fwd/main.c      | 33 +++++++++++++++--
>  3 files changed, 105 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> index a808d60247..c2c21a91fb 100644
> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -55,6 +55,8 @@
>  #define L3FWD_HASH_ENTRIES		(1024*1024*1)
>  #endif
>  #define HASH_ENTRY_NUMBER_DEFAULT	4
> +extern bool l3_sft_cksum;
> +extern bool l4_sft_cksum;

About the approach itself.
We have similar issue for HW PTYPE recognition - some HW doesn't support it.
So we check HW capabilities and if required we setup SW RX callbacks to do
determine PTYPE in SW. Note that for EM/LPM we have different callbacks.
I think for cksum checks we can do the same:
check HW capabilities, if they are missing add a new callback that would
calculate/check cksum and set  RTE_MBUF_F_RX_*_CKSUM_* flags.
That way it will HW/SW cksum will be transparent for the rest of l3fwd code.  

About cksums required: for LPM/FIB mode just IPv4 cksum seems enough.
For EM we probably need L4 cksum too, though not sure is it really needed.
Wonder what other people think here? 

 >  struct mbuf_table {
>  	uint16_t len;
> @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
>  int
>  lpm_main_loop(__rte_unused void *dummy);
> 
> +int
> +check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> +
>  int
>  fib_main_loop(__rte_unused void *dummy);
> 
> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> index 232b606b54..ecaf323943 100644
> --- a/examples/l3fwd/l3fwd_lpm.c
> +++ b/examples/l3fwd/l3fwd_lpm.c
> @@ -26,6 +26,7 @@
>  #include <rte_udp.h>
>  #include <rte_lpm.h>
>  #include <rte_lpm6.h>
> +#include <rte_net.h>
> 
>  #include "l3fwd.h"
>  #include "l3fwd_event.h"
> @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf *qconf, struct rte_mbuf *pkt,
>  #include "l3fwd_lpm.h"
>  #endif
> 
> +
> +int check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> +{
> +	int j;
> +	int i = 0;
> +	struct rte_net_hdr_lens hdr_lens;
> +	struct rte_ipv4_hdr *ipv4_hdr;
> +	void *l3_hdr;
> +	void *l4_hdr;
> +	rte_be16_t prev_cksum;
> +	int dropped_pkts_udp_tcp = 0;
> +	int dropped_pkts_ipv4 = 0;
> +	bool dropped;
> +	for (j = 0; j < nb_rx; j++) {
> +		dropped = false;
> +		rte_net_get_ptype(pkts_burst[j], &hdr_lens, RTE_PTYPE_ALL_MASK);
> +		l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +		void *, hdr_lens.l2_len);
> +		l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +		void *, hdr_lens.l2_len + hdr_lens.l3_len);
> +		ipv4_hdr = l3_hdr;
> +		prev_cksum = ipv4_hdr->hdr_checksum;
> +		ipv4_hdr->hdr_checksum = 0;
> +		ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> +
> +		if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
> +			rte_pktmbuf_free(pkts_burst[j]);
> +			dropped_pkts_ipv4++;
> +			dropped = true;
> +		} else if (l4_sft_cksum &&
> +				rte_ipv4_udptcp_cksum_verify
> +				(l3_hdr, l4_hdr) != 0) {
> +
> +			rte_pktmbuf_free(pkts_burst[j]);
> +			dropped_pkts_udp_tcp++;
> +			dropped = true;
> +		}
> +		if (dropped == false) {
> +			pkts_burst_to_send[i] = pkts_burst[j];
> +			i++;
> +		}
> +
> +	}
> +	return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> +}
> +
>  /* main processing loop */
>  int
>  lpm_main_loop(__rte_unused void *dummy)
>  {
>  	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> +	struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
>  	unsigned lcore_id;
>  	uint64_t prev_tsc, diff_tsc, cur_tsc;
>  	int i, nb_rx;
>  	uint16_t portid;
>  	uint8_t queueid;
> +	int dropped;
>  	struct lcore_conf *qconf;
>  	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
>  		US_PER_S * BURST_TX_DRAIN_US;
> @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
>  			if (nb_rx == 0)
>  				continue;
> 
> +			if (l3_sft_cksum || l4_sft_cksum) {
> +				dropped = check_software_cksum(pkts_burst,
> +				pkts_burst_to_send,	nb_rx);
> +
> +				nb_rx = nb_rx-dropped;
> +			}
> +
> +
>  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
>  			 || defined RTE_ARCH_PPC_64
> +		if (l3_sft_cksum == false && l4_sft_cksum == false)
>  			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
>  						portid, qconf);
> +		else
> +			l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
> +						portid, qconf);
> +
>  #else
> -			l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
> +			if (l3_sft_cksum == false && l4_sft_cksum == false)
> +				l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
>  							portid, qconf);
> +			else
> +				l3fwd_lpm_no_opt_send_packets(nb_rx,
> +				pkts_burst_to_send, portid, qconf);
> +
>  #endif /* X86 */
>  		}
> -
>  		cur_tsc = rte_rdtsc();
>  	}
> -
>  	return 0;
>  }
> 
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 00ac267af1..a54aca070d 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
>  /**< Ports set in promiscuous mode off by default. */
>  static int promiscuous_on;
> 
> +bool l3_sft_cksum;
> +bool l4_sft_cksum;
> +
>  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
>  enum L3FWD_LOOKUP_MODE {
>  	L3FWD_LOOKUP_DEFAULT,
> @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
>  		.mq_mode = ETH_MQ_RX_RSS,
>  		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
>  		.split_hdr_size = 0,
> -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
>  	},
>  	.rx_adv_conf = {
>  		.rss_conf = {
> @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t queueid)
>  	return 0;
>  }
> 
> +
>  static void
>  l3fwd_poll_resource_setup(void)
>  {
> @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
>  	unsigned int nb_ports;
>  	unsigned int lcore_id;
>  	int ret;
> -
> +	l3_sft_cksum = false;
> +	l4_sft_cksum = false;
>  	if (check_lcore_params() < 0)
>  		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> 
> @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
>  			rte_exit(EXIT_FAILURE,
>  				"Error during getting device (port %u) info: %s\n",
>  				portid, strerror(-ret));
> -
>  		if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>  			local_port_conf.txmode.offloads |=
>  				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +			DEV_RX_OFFLOAD_IPV4_CKSUM;
> +		else {
> +			l3_sft_cksum = true;
> +			printf("WARNING: IPV4 checksum offload not available.\n");
> +			}
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_UDP_CKSUM;
> +		else {
> +			l4_sft_cksum = true;
> +			printf("WARNING: UDP checksum offload not available.\n");
> +		}
> +
> +		if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> +			local_port_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_TCP_CKSUM;
> +		else {
> +			l4_sft_cksum = true;
> +			printf("WARNING: TCP checksum offload not available.\n");
> +		}
> +
>  		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>  			dev_info.flow_type_rss_offloads;
> 
> --
> 2.25.1

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Medvedkin, Vladimir]

Hi Usama,

On 04/11/2021 12:11, Walsh, Conor wrote:
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Usama Nadeem
>> Sent: Thursday 14 October 2021 19:43
>> To: thomas@monjalon.net
>> Cc: dev@dpdk.org; Usama Nadeem <usama.nadeem@emumba.com>
>> Subject: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum
>> verification through software
>>
>> checks if ipv4 and udptcp cksum offload capability available
>> If not available, cksum is verified through software
>> If cksum is corrupt, packet is dropped, rest of the packets
>> are forwarded back.
>>
>> Bugzilla ID:545
>> Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
>> ---
> 
> Hi Usama,
> 
> This should be done in a generic way that allows all the lookup methods to support it not just LPM.
> check_software_cksum should go in a common file and be called from LPM, FIB and possibly EM.
> 
> Thanks,
> Conor.
> 
>>   examples/l3fwd/l3fwd.h     |  6 ++++
>>   examples/l3fwd/l3fwd_lpm.c | 72
>> ++++++++++++++++++++++++++++++++++++--
>>   examples/l3fwd/main.c      | 33 +++++++++++++++--
>>   3 files changed, 105 insertions(+), 6 deletions(-)
>>
>> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
>> index a808d60247..c2c21a91fb 100644
>> --- a/examples/l3fwd/l3fwd.h
>> +++ b/examples/l3fwd/l3fwd.h
>> @@ -55,6 +55,8 @@
>>   #define L3FWD_HASH_ENTRIES		(1024*1024*1)
>>   #endif
>>   #define HASH_ENTRY_NUMBER_DEFAULT	4
>> +extern bool l3_sft_cksum;
>> +extern bool l4_sft_cksum;
>>
>>   struct mbuf_table {
>>   	uint16_t len;
>> @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
>>   int
>>   lpm_main_loop(__rte_unused void *dummy);
>>
>> +int
>> +check_software_cksum(struct rte_mbuf **pkts_burst,
>> +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
>> +
>>   int
>>   fib_main_loop(__rte_unused void *dummy);
>>
>> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
>> index 232b606b54..ecaf323943 100644
>> --- a/examples/l3fwd/l3fwd_lpm.c
>> +++ b/examples/l3fwd/l3fwd_lpm.c
>> @@ -26,6 +26,7 @@
>>   #include <rte_udp.h>
>>   #include <rte_lpm.h>
>>   #include <rte_lpm6.h>
>> +#include <rte_net.h>
>>
>>   #include "l3fwd.h"
>>   #include "l3fwd_event.h"
>> @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct
>> lcore_conf *qconf, struct rte_mbuf *pkt,
>>   #include "l3fwd_lpm.h"
>>   #endif
>>
>> +
>> +int check_software_cksum(struct rte_mbuf **pkts_burst,
>> +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
>> +{
>> +	int j;
>> +	int i = 0;
>> +	struct rte_net_hdr_lens hdr_lens;
>> +	struct rte_ipv4_hdr *ipv4_hdr;
>> +	void *l3_hdr;
>> +	void *l4_hdr;
>> +	rte_be16_t prev_cksum;
>> +	int dropped_pkts_udp_tcp = 0;
>> +	int dropped_pkts_ipv4 = 0;

Why do you need two separate counters if you eventually summing them up?

>> +	bool dropped;
>> +	for (j = 0; j < nb_rx; j++) {
>> +		dropped = false;
>> +		rte_net_get_ptype(pkts_burst[j], &hdr_lens,
>> RTE_PTYPE_ALL_MASK);
>> +		l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
>> +		void *, hdr_lens.l2_len);
>> +		l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
>> +		void *, hdr_lens.l2_len + hdr_lens.l3_len);

here hdr_lens.l3_len could be non initialized, for example in case of 
MPLS packet.

>> +		ipv4_hdr = l3_hdr;
>> +		prev_cksum = ipv4_hdr->hdr_checksum;

it could be non IPv4 packet.

>> +		ipv4_hdr->hdr_checksum = 0;
>> +		ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);

same here and below, it can be IPv6 for example.

>> +
>> +		if (l3_sft_cksum && prev_cksum != ipv4_hdr-
>>> hdr_checksum) {
>> +			rte_pktmbuf_free(pkts_burst[j]);
>> +			dropped_pkts_ipv4++;
>> +			dropped = true;

Do you need "dropped" value + the the final if statement at all? Maybe 
it's better to just
...
     continue;
}
here...

>> +		} else if (l4_sft_cksum &&
>> +				rte_ipv4_udptcp_cksum_verify
>> +				(l3_hdr, l4_hdr) != 0) {
>> +
>> +			rte_pktmbuf_free(pkts_burst[j]);
>> +			dropped_pkts_udp_tcp++;
>> +			dropped = true;
>> +		}
>> +		if (dropped == false) { >> +			pkts_burst_to_send[i] = pkts_burst[j];
>> +			i++;

...and execute this code unconditionally?

>> +		}
>> +
>> +	}
>> +	return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
>> +}
>> +
>>   /* main processing loop */
>>   int
>>   lpm_main_loop(__rte_unused void *dummy)
>>   {
>>   	struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
>> +	struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
>>   	unsigned lcore_id;
>>   	uint64_t prev_tsc, diff_tsc, cur_tsc;
>>   	int i, nb_rx;
>>   	uint16_t portid;
>>   	uint8_t queueid;
>> +	int dropped;
>>   	struct lcore_conf *qconf;
>>   	const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
>>   		US_PER_S * BURST_TX_DRAIN_US;
>> @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
>>   			if (nb_rx == 0)
>>   				continue;
>>
>> +			if (l3_sft_cksum || l4_sft_cksum) {
>> +				dropped =
>> check_software_cksum(pkts_burst,

You are calling this function fight after rte_eth_rx_burst(), so 
pkts_burst[] can have any possible packet proto, but current 
check_software_cksum() implementation if purely IPv4.

>> +				pkts_burst_to_send,	nb_rx);
>> +
>> +				nb_rx = nb_rx-dropped;
>> +			}
>> +
>> +
>>   #if defined RTE_ARCH_X86 || defined __ARM_NEON \
>>   			 || defined RTE_ARCH_PPC_64
>> +		if (l3_sft_cksum == false && l4_sft_cksum == false)
>>   			l3fwd_lpm_send_packets(nb_rx, pkts_burst,
>>   						portid, qconf);
>> +		else
>> +			l3fwd_lpm_send_packets(nb_rx,
>> pkts_burst_to_send,
>> +						portid, qconf);
>> +
>>   #else
>> -			l3fwd_lpm_no_opt_send_packets(nb_rx,
>> pkts_burst,
>> +			if (l3_sft_cksum == false && l4_sft_cksum == false)

While those if statements are perfectly predictable, it is still better 
to avoid branching in hot path if possible. You can implement 
check_software_cksum() to work with a single pkts_burst[] modifying it 
in place and throw away pkts_burst_to_send[] and corresponding branches.

>> +				l3fwd_lpm_no_opt_send_packets(nb_rx,
>> pkts_burst,
>>   							portid, qconf);
>> +			else
>> +				l3fwd_lpm_no_opt_send_packets(nb_rx,
>> +				pkts_burst_to_send, portid, qconf);
>> +
>>   #endif /* X86 */
>>   		}
>> -
>>   		cur_tsc = rte_rdtsc();
>>   	}
>> -
>>   	return 0;
>>   }
>>
>> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
>> index 00ac267af1..a54aca070d 100644
>> --- a/examples/l3fwd/main.c
>> +++ b/examples/l3fwd/main.c
>> @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
>>   /**< Ports set in promiscuous mode off by default. */
>>   static int promiscuous_on;
>>
>> +bool l3_sft_cksum;
>> +bool l4_sft_cksum;
>> +
>>   /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
>>   enum L3FWD_LOOKUP_MODE {
>>   	L3FWD_LOOKUP_DEFAULT,
>> @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
>>   		.mq_mode = ETH_MQ_RX_RSS,
>>   		.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
>>   		.split_hdr_size = 0,
>> -		.offloads = DEV_RX_OFFLOAD_CHECKSUM,
>>   	},
>>   	.rx_adv_conf = {
>>   		.rss_conf = {
>> @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
>> queueid)
>>   	return 0;
>>   }
>>
>> +
>>   static void
>>   l3fwd_poll_resource_setup(void)
>>   {
>> @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
>>   	unsigned int nb_ports;
>>   	unsigned int lcore_id;
>>   	int ret;
>> -
>> +	l3_sft_cksum = false;
>> +	l4_sft_cksum = false;
>>   	if (check_lcore_params() < 0)
>>   		rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
>>
>> @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
>>   			rte_exit(EXIT_FAILURE,
>>   				"Error during getting device (port %u) info:
>> %s\n",
>>   				portid, strerror(-ret));
>> -
>>   		if (dev_info.tx_offload_capa &
>> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>>   			local_port_conf.txmode.offloads |=
>>   				DEV_TX_OFFLOAD_MBUF_FAST_FREE;
>>
>> +		if (dev_info.rx_offload_capa &
>> DEV_RX_OFFLOAD_IPV4_CKSUM)
>> +			local_port_conf.rxmode.offloads |=
>> +			DEV_RX_OFFLOAD_IPV4_CKSUM;
>> +		else {
>> +			l3_sft_cksum = true;
>> +			printf("WARNING: IPV4 checksum offload not
>> available.\n");
>> +			}
>> +
>> +		if (dev_info.rx_offload_capa &
>> DEV_RX_OFFLOAD_UDP_CKSUM)
>> +			local_port_conf.rxmode.offloads |=
>> +				DEV_RX_OFFLOAD_UDP_CKSUM;
>> +		else {
>> +			l4_sft_cksum = true;
>> +			printf("WARNING: UDP checksum offload not
>> available.\n");
>> +		}
>> +
>> +		if (dev_info.rx_offload_capa &
>> DEV_RX_OFFLOAD_TCP_CKSUM)
>> +			local_port_conf.rxmode.offloads |=
>> +				DEV_RX_OFFLOAD_TCP_CKSUM;
>> +		else {
>> +			l4_sft_cksum = true;
>> +			printf("WARNING: TCP checksum offload not
>> available.\n");
>> +		}
>> +
>>   		local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>>   			dev_info.flow_type_rss_offloads;
>>
>> --
>> 2.25.1
> 

generalizing:
1. As Conor said earlier, make it generic, not only LPM and not only for 
run-to-completion mode, it should be done for event mode as well.
2. Function must work not only with IPv4.
3. There should be no performance degradation if NIC supports CSUM offload.

-- 
Regards,
Vladimir

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

[-- Attachment #1: Type: text/plain, Size: 11011 bytes --]

Hi Ananyev, Konstantin
<https://patches.dpdk.org/project/dpdk/list/?submitter=33>,

Yes, you understand it right.

The approach you discussed, we have been doing the same thing in our patch.
At first, we check the HW offload capabilities.

> + if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)

> + local_port_conf.rxmode.offloads |=

> + DEV_RX_OFFLOAD_IPV4_CKSUM;

> + else {

> + l3_sft_cksum = true;

> + printf("WARNING: IPV4 checksum offload not available.\n");

> + }

If cksum HW offload capability is missing, we call SFT cksum function, by
setting a bool variable to true. We will look into setting up the SFT
callbacks in the next version of patch.


On Thu, Nov 4, 2021 at 6:19 PM Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:

> > checks if ipv4 and udptcp cksum offload capability available
> > If not available, cksum is verified through software
> > If cksum is corrupt, packet is dropped, rest of the packets
> > are forwarded back.
>
> From what I see right now l3fwd:
>    a) enables HW RX cksum offload
>    b) simply ignores HW provided cksum status
> Which came as a real surprise to me.
> Feel free to correct me if I missed something obvious here.
>
> So, I think first we need to add missing check first,
> even though it might cause some perf drop.
> Then make changes to actually check provided by HW status and
> when HW doesn't provide such offload do check in SW.
>
> Another alternative would be to remove request for HW offloads
> and document l3fwd that it doesn't check checksums at all,
> but I don't think it is a good way.
>
> > Bugzilla ID:545
> > Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> > ---
> >  examples/l3fwd/l3fwd.h     |  6 ++++
> >  examples/l3fwd/l3fwd_lpm.c | 72 ++++++++++++++++++++++++++++++++++++--
> >  examples/l3fwd/main.c      | 33 +++++++++++++++--
> >  3 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> > index a808d60247..c2c21a91fb 100644
> > --- a/examples/l3fwd/l3fwd.h
> > +++ b/examples/l3fwd/l3fwd.h
> > @@ -55,6 +55,8 @@
> >  #define L3FWD_HASH_ENTRIES           (1024*1024*1)
> >  #endif
> >  #define HASH_ENTRY_NUMBER_DEFAULT    4
> > +extern bool l3_sft_cksum;
> > +extern bool l4_sft_cksum;
>
> About the approach itself.
> We have similar issue for HW PTYPE recognition - some HW doesn't support
> it.
> So we check HW capabilities and if required we setup SW RX callbacks to do
> determine PTYPE in SW. Note that for EM/LPM we have different callbacks.
> I think for cksum checks we can do the same:
> check HW capabilities, if they are missing add a new callback that would
> calculate/check cksum and set  RTE_MBUF_F_RX_*_CKSUM_* flags.
> That way it will HW/SW cksum will be transparent for the rest of l3fwd
> code.
>
> About cksums required: for LPM/FIB mode just IPv4 cksum seems enough.
> For EM we probably need L4 cksum too, though not sure is it really needed.
> Wonder what other people think here?
>
>  >  struct mbuf_table {
> >       uint16_t len;
> > @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
> >  int
> >  lpm_main_loop(__rte_unused void *dummy);
> >
> > +int
> > +check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> > +
> >  int
> >  fib_main_loop(__rte_unused void *dummy);
> >
> > diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> > index 232b606b54..ecaf323943 100644
> > --- a/examples/l3fwd/l3fwd_lpm.c
> > +++ b/examples/l3fwd/l3fwd_lpm.c
> > @@ -26,6 +26,7 @@
> >  #include <rte_udp.h>
> >  #include <rte_lpm.h>
> >  #include <rte_lpm6.h>
> > +#include <rte_net.h>
> >
> >  #include "l3fwd.h"
> >  #include "l3fwd_event.h"
> > @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf
> *qconf, struct rte_mbuf *pkt,
> >  #include "l3fwd_lpm.h"
> >  #endif
> >
> > +
> > +int check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> > +{
> > +     int j;
> > +     int i = 0;
> > +     struct rte_net_hdr_lens hdr_lens;
> > +     struct rte_ipv4_hdr *ipv4_hdr;
> > +     void *l3_hdr;
> > +     void *l4_hdr;
> > +     rte_be16_t prev_cksum;
> > +     int dropped_pkts_udp_tcp = 0;
> > +     int dropped_pkts_ipv4 = 0;
> > +     bool dropped;
> > +     for (j = 0; j < nb_rx; j++) {
> > +             dropped = false;
> > +             rte_net_get_ptype(pkts_burst[j], &hdr_lens,
> RTE_PTYPE_ALL_MASK);
> > +             l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len);
> > +             l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len + hdr_lens.l3_len);
> > +             ipv4_hdr = l3_hdr;
> > +             prev_cksum = ipv4_hdr->hdr_checksum;
> > +             ipv4_hdr->hdr_checksum = 0;
> > +             ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> > +
> > +             if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_ipv4++;
> > +                     dropped = true;
> > +             } else if (l4_sft_cksum &&
> > +                             rte_ipv4_udptcp_cksum_verify
> > +                             (l3_hdr, l4_hdr) != 0) {
> > +
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_udp_tcp++;
> > +                     dropped = true;
> > +             }
> > +             if (dropped == false) {
> > +                     pkts_burst_to_send[i] = pkts_burst[j];
> > +                     i++;
> > +             }
> > +
> > +     }
> > +     return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> > +}
> > +
> >  /* main processing loop */
> >  int
> >  lpm_main_loop(__rte_unused void *dummy)
> >  {
> >       struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> > +     struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
> >       unsigned lcore_id;
> >       uint64_t prev_tsc, diff_tsc, cur_tsc;
> >       int i, nb_rx;
> >       uint16_t portid;
> >       uint8_t queueid;
> > +     int dropped;
> >       struct lcore_conf *qconf;
> >       const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
> >               US_PER_S * BURST_TX_DRAIN_US;
> > @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
> >                       if (nb_rx == 0)
> >                               continue;
> >
> > +                     if (l3_sft_cksum || l4_sft_cksum) {
> > +                             dropped = check_software_cksum(pkts_burst,
> > +                             pkts_burst_to_send,     nb_rx);
> > +
> > +                             nb_rx = nb_rx-dropped;
> > +                     }
> > +
> > +
> >  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
> >                        || defined RTE_ARCH_PPC_64
> > +             if (l3_sft_cksum == false && l4_sft_cksum == false)
> >                       l3fwd_lpm_send_packets(nb_rx, pkts_burst,
> >                                               portid, qconf);
> > +             else
> > +                     l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
> > +                                             portid, qconf);
> > +
> >  #else
> > -                     l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
> > +                     if (l3_sft_cksum == false && l4_sft_cksum == false)
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> pkts_burst,
> >                                                       portid, qconf);
> > +                     else
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> > +                             pkts_burst_to_send, portid, qconf);
> > +
> >  #endif /* X86 */
> >               }
> > -
> >               cur_tsc = rte_rdtsc();
> >       }
> > -
> >       return 0;
> >  }
> >
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > index 00ac267af1..a54aca070d 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
> >  /**< Ports set in promiscuous mode off by default. */
> >  static int promiscuous_on;
> >
> > +bool l3_sft_cksum;
> > +bool l4_sft_cksum;
> > +
> >  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
> >  enum L3FWD_LOOKUP_MODE {
> >       L3FWD_LOOKUP_DEFAULT,
> > @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
> >               .mq_mode = ETH_MQ_RX_RSS,
> >               .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> >               .split_hdr_size = 0,
> > -             .offloads = DEV_RX_OFFLOAD_CHECKSUM,
> >       },
> >       .rx_adv_conf = {
> >               .rss_conf = {
> > @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
> queueid)
> >       return 0;
> >  }
> >
> > +
> >  static void
> >  l3fwd_poll_resource_setup(void)
> >  {
> > @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
> >       unsigned int nb_ports;
> >       unsigned int lcore_id;
> >       int ret;
> > -
> > +     l3_sft_cksum = false;
> > +     l4_sft_cksum = false;
> >       if (check_lcore_params() < 0)
> >               rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> >
> > @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
> >                       rte_exit(EXIT_FAILURE,
> >                               "Error during getting device (port %u)
> info: %s\n",
> >                               portid, strerror(-ret));
> > -
> >               if (dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> >                       local_port_conf.txmode.offloads |=
> >                               DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> >
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                     DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +             else {
> > +                     l3_sft_cksum = true;
> > +                     printf("WARNING: IPV4 checksum offload not
> available.\n");
> > +                     }
> > +
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_UDP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: UDP checksum offload not
> available.\n");
> > +             }
> > +
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_TCP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: TCP checksum offload not
> available.\n");
> > +             }
> > +
> >               local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> >                       dev_info.flow_type_rss_offloads;
> >
> > --
> > 2.25.1
>
>

-- 
-Usama

[-- Attachment #2: Type: text/html, Size: 24240 bytes --]

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

[-- Attachment #1: Type: text/plain, Size: 11620 bytes --]

Hi Medvedkin, Vladimir,


Thank you for your suggestions. Two counters aren't really necessary. We
also don't need the "dropped" variable. "continue" can also be used to
implement logic. We will update the logic in the next patch version.

This patch supports only IPV4 packets in LPM. It does not support other
lookup methods, neither does it support IPV6 packets for now. If the
current patch is satisfactory, we intend to begin work on those as well.

Regarding perf drop, I will submit a new version of the patch, containing
the callback.


On Thu, Nov 4, 2021 at 9:19 PM Medvedkin, Vladimir <
vladimir.medvedkin@intel.com> wrote:

> Hi Usama,
>
> On 04/11/2021 12:11, Walsh, Conor wrote:
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Usama Nadeem
> >> Sent: Thursday 14 October 2021 19:43
> >> To: thomas@monjalon.net
> >> Cc: dev@dpdk.org; Usama Nadeem <usama.nadeem@emumba.com>
> >> Subject: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum
> >> verification through software
> >>
> >> checks if ipv4 and udptcp cksum offload capability available
> >> If not available, cksum is verified through software
> >> If cksum is corrupt, packet is dropped, rest of the packets
> >> are forwarded back.
> >>
> >> Bugzilla ID:545
> >> Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> >> ---
> >
> > Hi Usama,
> >
> > This should be done in a generic way that allows all the lookup methods
> to support it not just LPM.
> > check_software_cksum should go in a common file and be called from LPM,
> FIB and possibly EM.
> >
> > Thanks,
> > Conor.
> >
> >>   examples/l3fwd/l3fwd.h     |  6 ++++
> >>   examples/l3fwd/l3fwd_lpm.c | 72
> >> ++++++++++++++++++++++++++++++++++++--
> >>   examples/l3fwd/main.c      | 33 +++++++++++++++--
> >>   3 files changed, 105 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> >> index a808d60247..c2c21a91fb 100644
> >> --- a/examples/l3fwd/l3fwd.h
> >> +++ b/examples/l3fwd/l3fwd.h
> >> @@ -55,6 +55,8 @@
> >>   #define L3FWD_HASH_ENTRIES         (1024*1024*1)
> >>   #endif
> >>   #define HASH_ENTRY_NUMBER_DEFAULT  4
> >> +extern bool l3_sft_cksum;
> >> +extern bool l4_sft_cksum;
> >>
> >>   struct mbuf_table {
> >>      uint16_t len;
> >> @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
> >>   int
> >>   lpm_main_loop(__rte_unused void *dummy);
> >>
> >> +int
> >> +check_software_cksum(struct rte_mbuf **pkts_burst,
> >> +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> >> +
> >>   int
> >>   fib_main_loop(__rte_unused void *dummy);
> >>
> >> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> >> index 232b606b54..ecaf323943 100644
> >> --- a/examples/l3fwd/l3fwd_lpm.c
> >> +++ b/examples/l3fwd/l3fwd_lpm.c
> >> @@ -26,6 +26,7 @@
> >>   #include <rte_udp.h>
> >>   #include <rte_lpm.h>
> >>   #include <rte_lpm6.h>
> >> +#include <rte_net.h>
> >>
> >>   #include "l3fwd.h"
> >>   #include "l3fwd_event.h"
> >> @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct
> >> lcore_conf *qconf, struct rte_mbuf *pkt,
> >>   #include "l3fwd_lpm.h"
> >>   #endif
> >>
> >> +
> >> +int check_software_cksum(struct rte_mbuf **pkts_burst,
> >> +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> >> +{
> >> +    int j;
> >> +    int i = 0;
> >> +    struct rte_net_hdr_lens hdr_lens;
> >> +    struct rte_ipv4_hdr *ipv4_hdr;
> >> +    void *l3_hdr;
> >> +    void *l4_hdr;
> >> +    rte_be16_t prev_cksum;
> >> +    int dropped_pkts_udp_tcp = 0;
> >> +    int dropped_pkts_ipv4 = 0;
>
> Why do you need two separate counters if you eventually summing them up?
>
> >> +    bool dropped;
> >> +    for (j = 0; j < nb_rx; j++) {
> >> +            dropped = false;
> >> +            rte_net_get_ptype(pkts_burst[j], &hdr_lens,
> >> RTE_PTYPE_ALL_MASK);
> >> +            l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> >> +            void *, hdr_lens.l2_len);
> >> +            l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> >> +            void *, hdr_lens.l2_len + hdr_lens.l3_len);
>
> here hdr_lens.l3_len could be non initialized, for example in case of
> MPLS packet.
>
> >> +            ipv4_hdr = l3_hdr;
> >> +            prev_cksum = ipv4_hdr->hdr_checksum;
>
> it could be non IPv4 packet.
>
> >> +            ipv4_hdr->hdr_checksum = 0;
> >> +            ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
>
> same here and below, it can be IPv6 for example.
>
> >> +
> >> +            if (l3_sft_cksum && prev_cksum != ipv4_hdr-
> >>> hdr_checksum) {
> >> +                    rte_pktmbuf_free(pkts_burst[j]);
> >> +                    dropped_pkts_ipv4++;
> >> +                    dropped = true;
>
> Do you need "dropped" value + the the final if statement at all? Maybe
> it's better to just
> ...
>      continue;
> }
> here...
>
> >> +            } else if (l4_sft_cksum &&
> >> +                            rte_ipv4_udptcp_cksum_verify
> >> +                            (l3_hdr, l4_hdr) != 0) {
> >> +
> >> +                    rte_pktmbuf_free(pkts_burst[j]);
> >> +                    dropped_pkts_udp_tcp++;
> >> +                    dropped = true;
> >> +            }
> >> +            if (dropped == false) { >> +
> pkts_burst_to_send[i] = pkts_burst[j];
> >> +                    i++;
>
> ...and execute this code unconditionally?
>
> >> +            }
> >> +
> >> +    }
> >> +    return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> >> +}
> >> +
> >>   /* main processing loop */
> >>   int
> >>   lpm_main_loop(__rte_unused void *dummy)
> >>   {
> >>      struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> >> +    struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
> >>      unsigned lcore_id;
> >>      uint64_t prev_tsc, diff_tsc, cur_tsc;
> >>      int i, nb_rx;
> >>      uint16_t portid;
> >>      uint8_t queueid;
> >> +    int dropped;
> >>      struct lcore_conf *qconf;
> >>      const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
> >>              US_PER_S * BURST_TX_DRAIN_US;
> >> @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
> >>                      if (nb_rx == 0)
> >>                              continue;
> >>
> >> +                    if (l3_sft_cksum || l4_sft_cksum) {
> >> +                            dropped =
> >> check_software_cksum(pkts_burst,
>
> You are calling this function fight after rte_eth_rx_burst(), so
> pkts_burst[] can have any possible packet proto, but current
> check_software_cksum() implementation if purely IPv4.
>
> >> +                            pkts_burst_to_send,     nb_rx);
> >> +
> >> +                            nb_rx = nb_rx-dropped;
> >> +                    }
> >> +
> >> +
> >>   #if defined RTE_ARCH_X86 || defined __ARM_NEON \
> >>                       || defined RTE_ARCH_PPC_64
> >> +            if (l3_sft_cksum == false && l4_sft_cksum == false)
> >>                      l3fwd_lpm_send_packets(nb_rx, pkts_burst,
> >>                                              portid, qconf);
> >> +            else
> >> +                    l3fwd_lpm_send_packets(nb_rx,
> >> pkts_burst_to_send,
> >> +                                            portid, qconf);
> >> +
> >>   #else
> >> -                    l3fwd_lpm_no_opt_send_packets(nb_rx,
> >> pkts_burst,
> >> +                    if (l3_sft_cksum == false && l4_sft_cksum == false)
>
> While those if statements are perfectly predictable, it is still better
> to avoid branching in hot path if possible. You can implement
> check_software_cksum() to work with a single pkts_burst[] modifying it
> in place and throw away pkts_burst_to_send[] and corresponding branches.
>
> >> +                            l3fwd_lpm_no_opt_send_packets(nb_rx,
> >> pkts_burst,
> >>                                                      portid, qconf);
> >> +                    else
> >> +                            l3fwd_lpm_no_opt_send_packets(nb_rx,
> >> +                            pkts_burst_to_send, portid, qconf);
> >> +
> >>   #endif /* X86 */
> >>              }
> >> -
> >>              cur_tsc = rte_rdtsc();
> >>      }
> >> -
> >>      return 0;
> >>   }
> >>
> >> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> >> index 00ac267af1..a54aca070d 100644
> >> --- a/examples/l3fwd/main.c
> >> +++ b/examples/l3fwd/main.c
> >> @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
> >>   /**< Ports set in promiscuous mode off by default. */
> >>   static int promiscuous_on;
> >>
> >> +bool l3_sft_cksum;
> >> +bool l4_sft_cksum;
> >> +
> >>   /* Select Longest-Prefix, Exact match or Forwarding Information Base.
> */
> >>   enum L3FWD_LOOKUP_MODE {
> >>      L3FWD_LOOKUP_DEFAULT,
> >> @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
> >>              .mq_mode = ETH_MQ_RX_RSS,
> >>              .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> >>              .split_hdr_size = 0,
> >> -            .offloads = DEV_RX_OFFLOAD_CHECKSUM,
> >>      },
> >>      .rx_adv_conf = {
> >>              .rss_conf = {
> >> @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
> >> queueid)
> >>      return 0;
> >>   }
> >>
> >> +
> >>   static void
> >>   l3fwd_poll_resource_setup(void)
> >>   {
> >> @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
> >>      unsigned int nb_ports;
> >>      unsigned int lcore_id;
> >>      int ret;
> >> -
> >> +    l3_sft_cksum = false;
> >> +    l4_sft_cksum = false;
> >>      if (check_lcore_params() < 0)
> >>              rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> >>
> >> @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
> >>                      rte_exit(EXIT_FAILURE,
> >>                              "Error during getting device (port %u)
> info:
> >> %s\n",
> >>                              portid, strerror(-ret));
> >> -
> >>              if (dev_info.tx_offload_capa &
> >> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> >>                      local_port_conf.txmode.offloads |=
> >>                              DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> >>
> >> +            if (dev_info.rx_offload_capa &
> >> DEV_RX_OFFLOAD_IPV4_CKSUM)
> >> +                    local_port_conf.rxmode.offloads |=
> >> +                    DEV_RX_OFFLOAD_IPV4_CKSUM;
> >> +            else {
> >> +                    l3_sft_cksum = true;
> >> +                    printf("WARNING: IPV4 checksum offload not
> >> available.\n");
> >> +                    }
> >> +
> >> +            if (dev_info.rx_offload_capa &
> >> DEV_RX_OFFLOAD_UDP_CKSUM)
> >> +                    local_port_conf.rxmode.offloads |=
> >> +                            DEV_RX_OFFLOAD_UDP_CKSUM;
> >> +            else {
> >> +                    l4_sft_cksum = true;
> >> +                    printf("WARNING: UDP checksum offload not
> >> available.\n");
> >> +            }
> >> +
> >> +            if (dev_info.rx_offload_capa &
> >> DEV_RX_OFFLOAD_TCP_CKSUM)
> >> +                    local_port_conf.rxmode.offloads |=
> >> +                            DEV_RX_OFFLOAD_TCP_CKSUM;
> >> +            else {
> >> +                    l4_sft_cksum = true;
> >> +                    printf("WARNING: TCP checksum offload not
> >> available.\n");
> >> +            }
> >> +
> >>              local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> >>                      dev_info.flow_type_rss_offloads;
> >>
> >> --
> >> 2.25.1
> >
>
> generalizing:
> 1. As Conor said earlier, make it generic, not only LPM and not only for
> run-to-completion mode, it should be done for event mode as well.
> 2. Function must work not only with IPv4.
> 3. There should be no performance degradation if NIC supports CSUM offload.
>
> --
> Regards,
> Vladimir
>


-- 
-Usama

[-- Attachment #2: Type: text/html, Size: 17599 bytes --]

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

[-- Attachment #1: Type: text/plain, Size: 9880 bytes --]

Hi Walsh, Conor <https://patches.dpdk.org/project/dpdk/list/?submitter=1935>
,

This, I agree, should be done for LPM, FIB, and EM. Only LPM is completed
in this patch. For the time being, you can think about this one. I will
look into FIB and EM in separate patches.

Thanks
-usama

On Thu, Nov 4, 2021 at 4:11 PM Walsh, Conor <conor.walsh@intel.com> wrote:

> > From: dev <dev-bounces@dpdk.org> On Behalf Of Usama Nadeem
> > Sent: Thursday 14 October 2021 19:43
> > To: thomas@monjalon.net
> > Cc: dev@dpdk.org; Usama Nadeem <usama.nadeem@emumba.com>
> > Subject: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum
> > verification through software
> >
> > checks if ipv4 and udptcp cksum offload capability available
> > If not available, cksum is verified through software
> > If cksum is corrupt, packet is dropped, rest of the packets
> > are forwarded back.
> >
> > Bugzilla ID:545
> > Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> > ---
>
> Hi Usama,
>
> This should be done in a generic way that allows all the lookup methods to
> support it not just LPM.
> check_software_cksum should go in a common file and be called from LPM,
> FIB and possibly EM.
>
> Thanks,
> Conor.
>
> >  examples/l3fwd/l3fwd.h     |  6 ++++
> >  examples/l3fwd/l3fwd_lpm.c | 72
> > ++++++++++++++++++++++++++++++++++++--
> >  examples/l3fwd/main.c      | 33 +++++++++++++++--
> >  3 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> > index a808d60247..c2c21a91fb 100644
> > --- a/examples/l3fwd/l3fwd.h
> > +++ b/examples/l3fwd/l3fwd.h
> > @@ -55,6 +55,8 @@
> >  #define L3FWD_HASH_ENTRIES           (1024*1024*1)
> >  #endif
> >  #define HASH_ENTRY_NUMBER_DEFAULT    4
> > +extern bool l3_sft_cksum;
> > +extern bool l4_sft_cksum;
> >
> >  struct mbuf_table {
> >       uint16_t len;
> > @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
> >  int
> >  lpm_main_loop(__rte_unused void *dummy);
> >
> > +int
> > +check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> > +
> >  int
> >  fib_main_loop(__rte_unused void *dummy);
> >
> > diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> > index 232b606b54..ecaf323943 100644
> > --- a/examples/l3fwd/l3fwd_lpm.c
> > +++ b/examples/l3fwd/l3fwd_lpm.c
> > @@ -26,6 +26,7 @@
> >  #include <rte_udp.h>
> >  #include <rte_lpm.h>
> >  #include <rte_lpm6.h>
> > +#include <rte_net.h>
> >
> >  #include "l3fwd.h"
> >  #include "l3fwd_event.h"
> > @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct
> > lcore_conf *qconf, struct rte_mbuf *pkt,
> >  #include "l3fwd_lpm.h"
> >  #endif
> >
> > +
> > +int check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> > +{
> > +     int j;
> > +     int i = 0;
> > +     struct rte_net_hdr_lens hdr_lens;
> > +     struct rte_ipv4_hdr *ipv4_hdr;
> > +     void *l3_hdr;
> > +     void *l4_hdr;
> > +     rte_be16_t prev_cksum;
> > +     int dropped_pkts_udp_tcp = 0;
> > +     int dropped_pkts_ipv4 = 0;
> > +     bool dropped;
> > +     for (j = 0; j < nb_rx; j++) {
> > +             dropped = false;
> > +             rte_net_get_ptype(pkts_burst[j], &hdr_lens,
> > RTE_PTYPE_ALL_MASK);
> > +             l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len);
> > +             l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len + hdr_lens.l3_len);
> > +             ipv4_hdr = l3_hdr;
> > +             prev_cksum = ipv4_hdr->hdr_checksum;
> > +             ipv4_hdr->hdr_checksum = 0;
> > +             ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> > +
> > +             if (l3_sft_cksum && prev_cksum != ipv4_hdr-
> > >hdr_checksum) {
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_ipv4++;
> > +                     dropped = true;
> > +             } else if (l4_sft_cksum &&
> > +                             rte_ipv4_udptcp_cksum_verify
> > +                             (l3_hdr, l4_hdr) != 0) {
> > +
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_udp_tcp++;
> > +                     dropped = true;
> > +             }
> > +             if (dropped == false) {
> > +                     pkts_burst_to_send[i] = pkts_burst[j];
> > +                     i++;
> > +             }
> > +
> > +     }
> > +     return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> > +}
> > +
> >  /* main processing loop */
> >  int
> >  lpm_main_loop(__rte_unused void *dummy)
> >  {
> >       struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> > +     struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
> >       unsigned lcore_id;
> >       uint64_t prev_tsc, diff_tsc, cur_tsc;
> >       int i, nb_rx;
> >       uint16_t portid;
> >       uint8_t queueid;
> > +     int dropped;
> >       struct lcore_conf *qconf;
> >       const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
> >               US_PER_S * BURST_TX_DRAIN_US;
> > @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
> >                       if (nb_rx == 0)
> >                               continue;
> >
> > +                     if (l3_sft_cksum || l4_sft_cksum) {
> > +                             dropped =
> > check_software_cksum(pkts_burst,
> > +                             pkts_burst_to_send,     nb_rx);
> > +
> > +                             nb_rx = nb_rx-dropped;
> > +                     }
> > +
> > +
> >  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
> >                        || defined RTE_ARCH_PPC_64
> > +             if (l3_sft_cksum == false && l4_sft_cksum == false)
> >                       l3fwd_lpm_send_packets(nb_rx, pkts_burst,
> >                                               portid, qconf);
> > +             else
> > +                     l3fwd_lpm_send_packets(nb_rx,
> > pkts_burst_to_send,
> > +                                             portid, qconf);
> > +
> >  #else
> > -                     l3fwd_lpm_no_opt_send_packets(nb_rx,
> > pkts_burst,
> > +                     if (l3_sft_cksum == false && l4_sft_cksum == false)
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> > pkts_burst,
> >                                                       portid, qconf);
> > +                     else
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> > +                             pkts_burst_to_send, portid, qconf);
> > +
> >  #endif /* X86 */
> >               }
> > -
> >               cur_tsc = rte_rdtsc();
> >       }
> > -
> >       return 0;
> >  }
> >
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > index 00ac267af1..a54aca070d 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
> >  /**< Ports set in promiscuous mode off by default. */
> >  static int promiscuous_on;
> >
> > +bool l3_sft_cksum;
> > +bool l4_sft_cksum;
> > +
> >  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
> >  enum L3FWD_LOOKUP_MODE {
> >       L3FWD_LOOKUP_DEFAULT,
> > @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
> >               .mq_mode = ETH_MQ_RX_RSS,
> >               .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> >               .split_hdr_size = 0,
> > -             .offloads = DEV_RX_OFFLOAD_CHECKSUM,
> >       },
> >       .rx_adv_conf = {
> >               .rss_conf = {
> > @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
> > queueid)
> >       return 0;
> >  }
> >
> > +
> >  static void
> >  l3fwd_poll_resource_setup(void)
> >  {
> > @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
> >       unsigned int nb_ports;
> >       unsigned int lcore_id;
> >       int ret;
> > -
> > +     l3_sft_cksum = false;
> > +     l4_sft_cksum = false;
> >       if (check_lcore_params() < 0)
> >               rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> >
> > @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
> >                       rte_exit(EXIT_FAILURE,
> >                               "Error during getting device (port %u)
> info:
> > %s\n",
> >                               portid, strerror(-ret));
> > -
> >               if (dev_info.tx_offload_capa &
> > DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> >                       local_port_conf.txmode.offloads |=
> >                               DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> >
> > +             if (dev_info.rx_offload_capa &
> > DEV_RX_OFFLOAD_IPV4_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                     DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +             else {
> > +                     l3_sft_cksum = true;
> > +                     printf("WARNING: IPV4 checksum offload not
> > available.\n");
> > +                     }
> > +
> > +             if (dev_info.rx_offload_capa &
> > DEV_RX_OFFLOAD_UDP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_UDP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: UDP checksum offload not
> > available.\n");
> > +             }
> > +
> > +             if (dev_info.rx_offload_capa &
> > DEV_RX_OFFLOAD_TCP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_TCP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: TCP checksum offload not
> > available.\n");
> > +             }
> > +
> >               local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> >                       dev_info.flow_type_rss_offloads;
> >
> > --
> > 2.25.1
>
>

-- 
-Usama

[-- Attachment #2: Type: text/html, Size: 14560 bytes --]

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Usama Nadeem]

[-- Attachment #1: Type: text/plain, Size: 11005 bytes --]

Hello, and thank you for the recommendations. I did investigate the
alternative options offered. I have a question about it. After verifying
mbuf's cksum, we may set the RTE MBUF F RX * CKSUM_* flags. I was just
wondering if it should be done at the application level or in the driver
code.
If we add this feature to driver code, do we have to include it in all
drivers that don't support sw-based cksums, and will it vary per driver?
Is it possible to accomplish it in a generic way? As an example, how about
at the application level. If we set the RTE MBUF F RX * CKSUM_* flags on
the application level, it will be independent of the individual drivers.
Thanks

On Thu, Nov 4, 2021 at 6:19 PM Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:

> > checks if ipv4 and udptcp cksum offload capability available
> > If not available, cksum is verified through software
> > If cksum is corrupt, packet is dropped, rest of the packets
> > are forwarded back.
>
> From what I see right now l3fwd:
>    a) enables HW RX cksum offload
>    b) simply ignores HW provided cksum status
> Which came as a real surprise to me.
> Feel free to correct me if I missed something obvious here.
>
> So, I think first we need to add missing check first,
> even though it might cause some perf drop.
> Then make changes to actually check provided by HW status and
> when HW doesn't provide such offload do check in SW.
>
> Another alternative would be to remove request for HW offloads
> and document l3fwd that it doesn't check checksums at all,
> but I don't think it is a good way.
>
> > Bugzilla ID:545
> > Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> > ---
> >  examples/l3fwd/l3fwd.h     |  6 ++++
> >  examples/l3fwd/l3fwd_lpm.c | 72 ++++++++++++++++++++++++++++++++++++--
> >  examples/l3fwd/main.c      | 33 +++++++++++++++--
> >  3 files changed, 105 insertions(+), 6 deletions(-)
> >
> > diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> > index a808d60247..c2c21a91fb 100644
> > --- a/examples/l3fwd/l3fwd.h
> > +++ b/examples/l3fwd/l3fwd.h
> > @@ -55,6 +55,8 @@
> >  #define L3FWD_HASH_ENTRIES           (1024*1024*1)
> >  #endif
> >  #define HASH_ENTRY_NUMBER_DEFAULT    4
> > +extern bool l3_sft_cksum;
> > +extern bool l4_sft_cksum;
>
> About the approach itself.
> We have similar issue for HW PTYPE recognition - some HW doesn't support
> it.
> So we check HW capabilities and if required we setup SW RX callbacks to do
> determine PTYPE in SW. Note that for EM/LPM we have different callbacks.
> I think for cksum checks we can do the same:
> check HW capabilities, if they are missing add a new callback that would
> calculate/check cksum and set  RTE_MBUF_F_RX_*_CKSUM_* flags.
> That way it will HW/SW cksum will be transparent for the rest of l3fwd
> code.
>
> About cksums required: for LPM/FIB mode just IPv4 cksum seems enough.
> For EM we probably need L4 cksum too, though not sure is it really needed.
> Wonder what other people think here?
>
>  >  struct mbuf_table {
> >       uint16_t len;
> > @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
> >  int
> >  lpm_main_loop(__rte_unused void *dummy);
> >
> > +int
> > +check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> > +
> >  int
> >  fib_main_loop(__rte_unused void *dummy);
> >
> > diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> > index 232b606b54..ecaf323943 100644
> > --- a/examples/l3fwd/l3fwd_lpm.c
> > +++ b/examples/l3fwd/l3fwd_lpm.c
> > @@ -26,6 +26,7 @@
> >  #include <rte_udp.h>
> >  #include <rte_lpm.h>
> >  #include <rte_lpm6.h>
> > +#include <rte_net.h>
> >
> >  #include "l3fwd.h"
> >  #include "l3fwd_event.h"
> > @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf
> *qconf, struct rte_mbuf *pkt,
> >  #include "l3fwd_lpm.h"
> >  #endif
> >
> > +
> > +int check_software_cksum(struct rte_mbuf **pkts_burst,
> > +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> > +{
> > +     int j;
> > +     int i = 0;
> > +     struct rte_net_hdr_lens hdr_lens;
> > +     struct rte_ipv4_hdr *ipv4_hdr;
> > +     void *l3_hdr;
> > +     void *l4_hdr;
> > +     rte_be16_t prev_cksum;
> > +     int dropped_pkts_udp_tcp = 0;
> > +     int dropped_pkts_ipv4 = 0;
> > +     bool dropped;
> > +     for (j = 0; j < nb_rx; j++) {
> > +             dropped = false;
> > +             rte_net_get_ptype(pkts_burst[j], &hdr_lens,
> RTE_PTYPE_ALL_MASK);
> > +             l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len);
> > +             l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> > +             void *, hdr_lens.l2_len + hdr_lens.l3_len);
> > +             ipv4_hdr = l3_hdr;
> > +             prev_cksum = ipv4_hdr->hdr_checksum;
> > +             ipv4_hdr->hdr_checksum = 0;
> > +             ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> > +
> > +             if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_ipv4++;
> > +                     dropped = true;
> > +             } else if (l4_sft_cksum &&
> > +                             rte_ipv4_udptcp_cksum_verify
> > +                             (l3_hdr, l4_hdr) != 0) {
> > +
> > +                     rte_pktmbuf_free(pkts_burst[j]);
> > +                     dropped_pkts_udp_tcp++;
> > +                     dropped = true;
> > +             }
> > +             if (dropped == false) {
> > +                     pkts_burst_to_send[i] = pkts_burst[j];
> > +                     i++;
> > +             }
> > +
> > +     }
> > +     return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> > +}
> > +
> >  /* main processing loop */
> >  int
> >  lpm_main_loop(__rte_unused void *dummy)
> >  {
> >       struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> > +     struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
> >       unsigned lcore_id;
> >       uint64_t prev_tsc, diff_tsc, cur_tsc;
> >       int i, nb_rx;
> >       uint16_t portid;
> >       uint8_t queueid;
> > +     int dropped;
> >       struct lcore_conf *qconf;
> >       const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
> >               US_PER_S * BURST_TX_DRAIN_US;
> > @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
> >                       if (nb_rx == 0)
> >                               continue;
> >
> > +                     if (l3_sft_cksum || l4_sft_cksum) {
> > +                             dropped = check_software_cksum(pkts_burst,
> > +                             pkts_burst_to_send,     nb_rx);
> > +
> > +                             nb_rx = nb_rx-dropped;
> > +                     }
> > +
> > +
> >  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
> >                        || defined RTE_ARCH_PPC_64
> > +             if (l3_sft_cksum == false && l4_sft_cksum == false)
> >                       l3fwd_lpm_send_packets(nb_rx, pkts_burst,
> >                                               portid, qconf);
> > +             else
> > +                     l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
> > +                                             portid, qconf);
> > +
> >  #else
> > -                     l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
> > +                     if (l3_sft_cksum == false && l4_sft_cksum == false)
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> pkts_burst,
> >                                                       portid, qconf);
> > +                     else
> > +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> > +                             pkts_burst_to_send, portid, qconf);
> > +
> >  #endif /* X86 */
> >               }
> > -
> >               cur_tsc = rte_rdtsc();
> >       }
> > -
> >       return 0;
> >  }
> >
> > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> > index 00ac267af1..a54aca070d 100644
> > --- a/examples/l3fwd/main.c
> > +++ b/examples/l3fwd/main.c
> > @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
> >  /**< Ports set in promiscuous mode off by default. */
> >  static int promiscuous_on;
> >
> > +bool l3_sft_cksum;
> > +bool l4_sft_cksum;
> > +
> >  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
> >  enum L3FWD_LOOKUP_MODE {
> >       L3FWD_LOOKUP_DEFAULT,
> > @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
> >               .mq_mode = ETH_MQ_RX_RSS,
> >               .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> >               .split_hdr_size = 0,
> > -             .offloads = DEV_RX_OFFLOAD_CHECKSUM,
> >       },
> >       .rx_adv_conf = {
> >               .rss_conf = {
> > @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t
> queueid)
> >       return 0;
> >  }
> >
> > +
> >  static void
> >  l3fwd_poll_resource_setup(void)
> >  {
> > @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
> >       unsigned int nb_ports;
> >       unsigned int lcore_id;
> >       int ret;
> > -
> > +     l3_sft_cksum = false;
> > +     l4_sft_cksum = false;
> >       if (check_lcore_params() < 0)
> >               rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
> >
> > @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
> >                       rte_exit(EXIT_FAILURE,
> >                               "Error during getting device (port %u)
> info: %s\n",
> >                               portid, strerror(-ret));
> > -
> >               if (dev_info.tx_offload_capa &
> DEV_TX_OFFLOAD_MBUF_FAST_FREE)
> >                       local_port_conf.txmode.offloads |=
> >                               DEV_TX_OFFLOAD_MBUF_FAST_FREE;
> >
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                     DEV_RX_OFFLOAD_IPV4_CKSUM;
> > +             else {
> > +                     l3_sft_cksum = true;
> > +                     printf("WARNING: IPV4 checksum offload not
> available.\n");
> > +                     }
> > +
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_UDP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: UDP checksum offload not
> available.\n");
> > +             }
> > +
> > +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> > +                     local_port_conf.rxmode.offloads |=
> > +                             DEV_RX_OFFLOAD_TCP_CKSUM;
> > +             else {
> > +                     l4_sft_cksum = true;
> > +                     printf("WARNING: TCP checksum offload not
> available.\n");
> > +             }
> > +
> >               local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
> >                       dev_info.flow_type_rss_offloads;
> >
> > --
> > 2.25.1
>
>

-- 
-Usama

[-- Attachment #2: Type: text/html, Size: 14240 bytes --]

RE: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Ananyev, Konstantin]

[-- Attachment #1: Type: text/plain, Size: 11465 bytes --]

Hi Usama,

AFAIK, all drivers that support DEV_RX_OFFLOAD_*_CKSUM will
set RTE_MBUF_F_RX_*_CKSUM_*  properly
(when  DEV_RX_OFFLOAD_*_CKSUM was requested by user off-course).
The problem is that not all PMDs support checksum offload.
For such cases my suggestion was to install RX callback that would
verify packet checksum and set RTE_MBUF_F_RX_*_CKSUM_*  for it.

From: Usama Nadeem <usama.nadeem@emumba.com>
Sent: Friday, January 14, 2022 9:30 AM
To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
Cc: thomas@monjalon.net; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

Hello, and thank you for the recommendations. I did investigate the alternative options offered. I have a question about it. After verifying mbuf's cksum, we may set the RTE MBUF F RX * CKSUM_* flags. I was just wondering if it should be done at the application level or in the driver code.
If we add this feature to driver code, do we have to include it in all drivers that don't support sw-based cksums, and will it vary per driver?
Is it possible to accomplish it in a generic way? As an example, how about at the application level. If we set the RTE MBUF F RX * CKSUM_* flags on the application level, it will be independent of the individual drivers.
Thanks

On Thu, Nov 4, 2021 at 6:19 PM Ananyev, Konstantin <konstantin.ananyev@intel.com<mailto:konstantin.ananyev@intel.com>> wrote:
> checks if ipv4 and udptcp cksum offload capability available
> If not available, cksum is verified through software
> If cksum is corrupt, packet is dropped, rest of the packets
> are forwarded back.

From what I see right now l3fwd:
   a) enables HW RX cksum offload
   b) simply ignores HW provided cksum status
Which came as a real surprise to me.
Feel free to correct me if I missed something obvious here.

So, I think first we need to add missing check first,
even though it might cause some perf drop.
Then make changes to actually check provided by HW status and
when HW doesn't provide such offload do check in SW.

Another alternative would be to remove request for HW offloads
and document l3fwd that it doesn't check checksums at all,
but I don't think it is a good way.

> Bugzilla ID:545
> Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com<mailto:usama.nadeem@emumba.com>>
> ---
>  examples/l3fwd/l3fwd.h     |  6 ++++
>  examples/l3fwd/l3fwd_lpm.c | 72 ++++++++++++++++++++++++++++++++++++--
>  examples/l3fwd/main.c      | 33 +++++++++++++++--
>  3 files changed, 105 insertions(+), 6 deletions(-)
>
> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> index a808d60247..c2c21a91fb 100644
> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -55,6 +55,8 @@
>  #define L3FWD_HASH_ENTRIES           (1024*1024*1)
>  #endif
>  #define HASH_ENTRY_NUMBER_DEFAULT    4
> +extern bool l3_sft_cksum;
> +extern bool l4_sft_cksum;

About the approach itself.
We have similar issue for HW PTYPE recognition - some HW doesn't support it.
So we check HW capabilities and if required we setup SW RX callbacks to do
determine PTYPE in SW. Note that for EM/LPM we have different callbacks.
I think for cksum checks we can do the same:
check HW capabilities, if they are missing add a new callback that would
calculate/check cksum and set  RTE_MBUF_F_RX_*_CKSUM_* flags.
That way it will HW/SW cksum will be transparent for the rest of l3fwd code.

About cksums required: for LPM/FIB mode just IPv4 cksum seems enough.
For EM we probably need L4 cksum too, though not sure is it really needed.
Wonder what other people think here?

 >  struct mbuf_table {
>       uint16_t len;
> @@ -210,6 +212,10 @@ em_main_loop(__rte_unused void *dummy);
>  int
>  lpm_main_loop(__rte_unused void *dummy);
>
> +int
> +check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx);
> +
>  int
>  fib_main_loop(__rte_unused void *dummy);
>
> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> index 232b606b54..ecaf323943 100644
> --- a/examples/l3fwd/l3fwd_lpm.c
> +++ b/examples/l3fwd/l3fwd_lpm.c
> @@ -26,6 +26,7 @@
>  #include <rte_udp.h>
>  #include <rte_lpm.h>
>  #include <rte_lpm6.h>
> +#include <rte_net.h>
>
>  #include "l3fwd.h"
>  #include "l3fwd_event.h"
> @@ -139,16 +140,65 @@ lpm_get_dst_port_with_ipv4(const struct lcore_conf *qconf, struct rte_mbuf *pkt,
>  #include "l3fwd_lpm.h"
>  #endif
>
> +
> +int check_software_cksum(struct rte_mbuf **pkts_burst,
> +struct rte_mbuf **pkts_burst_to_send, int nb_rx)
> +{
> +     int j;
> +     int i = 0;
> +     struct rte_net_hdr_lens hdr_lens;
> +     struct rte_ipv4_hdr *ipv4_hdr;
> +     void *l3_hdr;
> +     void *l4_hdr;
> +     rte_be16_t prev_cksum;
> +     int dropped_pkts_udp_tcp = 0;
> +     int dropped_pkts_ipv4 = 0;
> +     bool dropped;
> +     for (j = 0; j < nb_rx; j++) {
> +             dropped = false;
> +             rte_net_get_ptype(pkts_burst[j], &hdr_lens, RTE_PTYPE_ALL_MASK);
> +             l3_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +             void *, hdr_lens.l2_len);
> +             l4_hdr = rte_pktmbuf_mtod_offset(pkts_burst[j],
> +             void *, hdr_lens.l2_len + hdr_lens.l3_len);
> +             ipv4_hdr = l3_hdr;
> +             prev_cksum = ipv4_hdr->hdr_checksum;
> +             ipv4_hdr->hdr_checksum = 0;
> +             ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> +
> +             if (l3_sft_cksum && prev_cksum != ipv4_hdr->hdr_checksum) {
> +                     rte_pktmbuf_free(pkts_burst[j]);
> +                     dropped_pkts_ipv4++;
> +                     dropped = true;
> +             } else if (l4_sft_cksum &&
> +                             rte_ipv4_udptcp_cksum_verify
> +                             (l3_hdr, l4_hdr) != 0) {
> +
> +                     rte_pktmbuf_free(pkts_burst[j]);
> +                     dropped_pkts_udp_tcp++;
> +                     dropped = true;
> +             }
> +             if (dropped == false) {
> +                     pkts_burst_to_send[i] = pkts_burst[j];
> +                     i++;
> +             }
> +
> +     }
> +     return dropped_pkts_udp_tcp+dropped_pkts_ipv4;
> +}
> +
>  /* main processing loop */
>  int
>  lpm_main_loop(__rte_unused void *dummy)
>  {
>       struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> +     struct rte_mbuf *pkts_burst_to_send[MAX_PKT_BURST];
>       unsigned lcore_id;
>       uint64_t prev_tsc, diff_tsc, cur_tsc;
>       int i, nb_rx;
>       uint16_t portid;
>       uint8_t queueid;
> +     int dropped;
>       struct lcore_conf *qconf;
>       const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) /
>               US_PER_S * BURST_TX_DRAIN_US;
> @@ -209,19 +259,35 @@ lpm_main_loop(__rte_unused void *dummy)
>                       if (nb_rx == 0)
>                               continue;
>
> +                     if (l3_sft_cksum || l4_sft_cksum) {
> +                             dropped = check_software_cksum(pkts_burst,
> +                             pkts_burst_to_send,     nb_rx);
> +
> +                             nb_rx = nb_rx-dropped;
> +                     }
> +
> +
>  #if defined RTE_ARCH_X86 || defined __ARM_NEON \
>                        || defined RTE_ARCH_PPC_64
> +             if (l3_sft_cksum == false && l4_sft_cksum == false)
>                       l3fwd_lpm_send_packets(nb_rx, pkts_burst,
>                                               portid, qconf);
> +             else
> +                     l3fwd_lpm_send_packets(nb_rx, pkts_burst_to_send,
> +                                             portid, qconf);
> +
>  #else
> -                     l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
> +                     if (l3_sft_cksum == false && l4_sft_cksum == false)
> +                             l3fwd_lpm_no_opt_send_packets(nb_rx, pkts_burst,
>                                                       portid, qconf);
> +                     else
> +                             l3fwd_lpm_no_opt_send_packets(nb_rx,
> +                             pkts_burst_to_send, portid, qconf);
> +
>  #endif /* X86 */
>               }
> -
>               cur_tsc = rte_rdtsc();
>       }
> -
>       return 0;
>  }
>
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 00ac267af1..a54aca070d 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -61,6 +61,9 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
>  /**< Ports set in promiscuous mode off by default. */
>  static int promiscuous_on;
>
> +bool l3_sft_cksum;
> +bool l4_sft_cksum;
> +
>  /* Select Longest-Prefix, Exact match or Forwarding Information Base. */
>  enum L3FWD_LOOKUP_MODE {
>       L3FWD_LOOKUP_DEFAULT,
> @@ -123,7 +126,6 @@ static struct rte_eth_conf port_conf = {
>               .mq_mode = ETH_MQ_RX_RSS,
>               .max_rx_pkt_len = RTE_ETHER_MAX_LEN,
>               .split_hdr_size = 0,
> -             .offloads = DEV_RX_OFFLOAD_CHECKSUM,
>       },
>       .rx_adv_conf = {
>               .rss_conf = {
> @@ -981,6 +983,7 @@ prepare_ptype_parser(uint16_t portid, uint16_t queueid)
>       return 0;
>  }
>
> +
>  static void
>  l3fwd_poll_resource_setup(void)
>  {
> @@ -993,7 +996,8 @@ l3fwd_poll_resource_setup(void)
>       unsigned int nb_ports;
>       unsigned int lcore_id;
>       int ret;
> -
> +     l3_sft_cksum = false;
> +     l4_sft_cksum = false;
>       if (check_lcore_params() < 0)
>               rte_exit(EXIT_FAILURE, "check_lcore_params failed\n");
>
> @@ -1034,11 +1038,34 @@ l3fwd_poll_resource_setup(void)
>                       rte_exit(EXIT_FAILURE,
>                               "Error during getting device (port %u) info: %s\n",
>                               portid, strerror(-ret));
> -
>               if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
>                       local_port_conf.txmode.offloads |=
>                               DEV_TX_OFFLOAD_MBUF_FAST_FREE;
>
> +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_IPV4_CKSUM)
> +                     local_port_conf.rxmode.offloads |=
> +                     DEV_RX_OFFLOAD_IPV4_CKSUM;
> +             else {
> +                     l3_sft_cksum = true;
> +                     printf("WARNING: IPV4 checksum offload not available.\n");
> +                     }
> +
> +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_UDP_CKSUM)
> +                     local_port_conf.rxmode.offloads |=
> +                             DEV_RX_OFFLOAD_UDP_CKSUM;
> +             else {
> +                     l4_sft_cksum = true;
> +                     printf("WARNING: UDP checksum offload not available.\n");
> +             }
> +
> +             if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_TCP_CKSUM)
> +                     local_port_conf.rxmode.offloads |=
> +                             DEV_RX_OFFLOAD_TCP_CKSUM;
> +             else {
> +                     l4_sft_cksum = true;
> +                     printf("WARNING: TCP checksum offload not available.\n");
> +             }
> +
>               local_port_conf.rx_adv_conf.rss_conf.rss_hf &=
>                       dev_info.flow_type_rss_offloads;
>
> --
> 2.25.1


--
-Usama

[-- Attachment #2: Type: text/html, Size: 23668 bytes --]

Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software

[Stephen Hemminger]

On Thu, 4 Nov 2021 11:11:02 +0000
"Walsh, Conor" <conor.walsh@intel.com> wrote:

> > checks if ipv4 and udptcp cksum offload capability available
> > If not available, cksum is verified through software
> > If cksum is corrupt, packet is dropped, rest of the packets
> > are forwarded back.
> > 
> > Bugzilla ID:545
> > Signed-off-by: Usama Nadeem <usama.nadeem@emumba.com>
> > ---  
> 
> Hi Usama,
> 
> This should be done in a generic way that allows all the lookup methods to support it not just LPM.
> check_software_cksum should go in a common file and be called from LPM, FIB and possibly EM.
> 
> Thanks,
> Conor.

Agree.
This is a real bug in l3fwd-XXX examples.
It needs to be done in a more general way so that applications can use this
design pattern as a template.

Please submit a new version