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

[Usama Nadeem <usama.nadeem@emumba.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 --]