DPDK patches and discussions
 help / color / mirror / Atom feed
From: Usama Nadeem <usama.nadeem@emumba.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v4] examples/l3fwd: ipv4 and udp/tcp cksum verification through software
Date: Tue, 16 Nov 2021 10:18:31 +0500	[thread overview]
Message-ID: <CABDMEv=eLForBt_DuTvR6ZaUGak+=ma3tD_S_=_WtNHfm-r9gw@mail.gmail.com> (raw)
In-Reply-To: <DM6PR11MB449171FEA4B0CC67C36B94D39A8D9@DM6PR11MB4491.namprd11.prod.outlook.com>

[-- 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 --]

  reply	other threads:[~2021-11-16  5:18 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-13 12:09 [dpdk-dev] [PATCH] examples: ipv4, udp and tcp checksum offload warning usamanadeem321
2021-09-13 15:11 ` Stephen Hemminger
2021-09-14 18:08 ` [dpdk-dev] [PATCH v2] Warns if IPv4, UDP or TCP checksum offload not available Usama Nadeem
2021-09-14 18:28   ` Stephen Hemminger
2021-09-14 22:22   ` Ananyev, Konstantin
2021-09-14 23:44     ` Stephen Hemminger
2021-09-15  8:43       ` Ananyev, Konstantin
2021-10-08 15:51   ` [dpdk-dev] [PATCH v3] ipv4 and udp/tcp cksum verification through software Usama Nadeem
2021-10-14 18:43     ` [dpdk-dev] [PATCH v4] examples/l3fwd: " Usama Nadeem
2021-11-01  8:33       ` Usama Nadeem
2021-11-04 11:11       ` Walsh, Conor
2021-11-04 16:19         ` Medvedkin, Vladimir
2021-11-16  5:20           ` Usama Nadeem
2021-11-16  5:21         ` Usama Nadeem
2023-06-30 21:50         ` Stephen Hemminger
2021-11-04 13:19       ` Ananyev, Konstantin
2021-11-16  5:18         ` Usama Nadeem [this message]
2022-01-14  9:30         ` Usama Nadeem
2022-01-14 12:05           ` Ananyev, Konstantin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CABDMEv=eLForBt_DuTvR6ZaUGak+=ma3tD_S_=_WtNHfm-r9gw@mail.gmail.com' \
    --to=usama.nadeem@emumba.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).