[ovs-dev] [PATCH] conntrack : Use Rx checksum offload feature on DPDK ports for conntrack.

Fischetti, Antonio antonio.fischetti at intel.com
Fri May 26 10:04:31 UTC 2017


Hi Sugesh,
it looks good to me, it makes sense to leverage the csum info when present.

I've tested it with the firewall rules - see below for details - I saw a ~+3% improvement in my testbench with 10 UDP connections.

Traffic Gen: IXIA IxExplorer
10 UDP different flows, 64B pkts

Original OvS:   3.0 Mpps 
With this Patch: 3.1 Mpps


Below some details of my testbench.

===================================

BUILD
-----
make -j 28 CFLAGS="-O2 -march=native -g"

#I didn't use intrinsics, I expect in that case the benefit will be smaller.

FLOW DUMP
---------
NXST_FLOW reply (xid=0x4):
 cookie=0x0, duration=0.064s, table=0, n_packets=0, n_bytes=0, idle_age=0, priority=100,ct_state=-trk,ip actions=ct(table=1)
 cookie=0x0, duration=0.075s, table=0, n_packets=0, n_bytes=0, idle_age=0, priority=10,arp actions=NORMAL
 cookie=0x0, duration=0.085s, table=0, n_packets=0, n_bytes=0, idle_age=0, priority=1 actions=drop
 cookie=0x0, duration=0.054s, table=1, n_packets=0, n_bytes=0, idle_age=0, ct_state=+new+trk,ip,in_port=1 actions=ct(commit),output:2
 cookie=0x0, duration=0.033s, table=1, n_packets=0, n_bytes=0, idle_age=0, ct_state=+new+trk,ip,in_port=2 actions=drop
 cookie=0x0, duration=0.043s, table=1, n_packets=0, n_bytes=0, idle_age=0, ct_state=+est+trk,ip,in_port=1 actions=output:2
 cookie=0x0, duration=0.023s, table=1, n_packets=0, n_bytes=0, idle_age=0, ct_state=+est+trk,ip,in_port=2 actions=output:1

HugePages_Total:   20480
HugePages_Free:    20480
HugePages_Rsvd:        0
HugePages_Surp:        0
_______________________________________________
DPDK: HEAD detached at v16.11
OvS:  On my local branch ConnTrack_01
_______________________________________________

   PID PSR COMMAND         %CPU
 20509   0 ovsdb-server     0.0
 20522   2 ovs-vswitchd    78.1
 20522   4 pmd62           80.8
 20522   5 pmd61           71.6

PDM threads:  2

configured_tx_queues=3,
configured_tx_queues=3,
========================

Regards,
Antonio


Acked-by: Antonio Fischetti <antonio.fischetti at intel.com>


> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of Sugesh Chandran
> Sent: Thursday, May 25, 2017 10:11 PM
> To: ovs-dev at openvswitch.org
> Subject: [ovs-dev] [PATCH] conntrack : Use Rx checksum offload feature on
> DPDK ports for conntrack.
> 
> Avoiding checksum validation in conntrack module if it is already verified
> in DPDK physical NIC ports.
> 
> Signed-off-by: Sugesh Chandran <sugesh.chandran at intel.com>
> ---
>  lib/conntrack.c | 58 ++++++++++++++++++++++++++++++++++++----------------
> -----
>  1 file changed, 37 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index cb30ac7..af6a372 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -642,10 +642,13 @@ extract_l3_ipv6(struct conn_key *key, const void
> *data, size_t size,
> 
>  static inline bool
>  checksum_valid(const struct conn_key *key, const void *data, size_t size,
> -               const void *l3)
> +               const void *l3, bool validate_checksum)
>  {
>      uint32_t csum = 0;
> 
> +    if (!validate_checksum) {
> +       return true;
> +    }
>      if (key->dl_type == htons(ETH_TYPE_IP)) {
>          csum = packet_csum_pseudoheader(l3);
>      } else if (key->dl_type == htons(ETH_TYPE_IPV6)) {
> @@ -661,7 +664,7 @@ checksum_valid(const struct conn_key *key, const void
> *data, size_t size,
> 
>  static inline bool
>  check_l4_tcp(const struct conn_key *key, const void *data, size_t size,
> -             const void *l3)
> +             const void *l3, bool validate_checksum)
>  {
>      const struct tcp_header *tcp = data;
>      if (size < sizeof *tcp) {
> @@ -673,12 +676,12 @@ check_l4_tcp(const struct conn_key *key, const void
> *data, size_t size,
>          return false;
>      }
> 
> -    return checksum_valid(key, data, size, l3);
> +    return checksum_valid(key, data, size, l3, validate_checksum);
>  }
> 
>  static inline bool
>  check_l4_udp(const struct conn_key *key, const void *data, size_t size,
> -             const void *l3)
> +             const void *l3, bool validate_checksum)
>  {
>      const struct udp_header *udp = data;
>      if (size < sizeof *udp) {
> @@ -692,20 +695,23 @@ check_l4_udp(const struct conn_key *key, const void
> *data, size_t size,
> 
>      /* Validation must be skipped if checksum is 0 on IPv4 packets */
>      return (udp->udp_csum == 0 && key->dl_type == htons(ETH_TYPE_IP))
> -           || checksum_valid(key, data, size, l3);
> +           || checksum_valid(key, data, size, l3, validate_checksum);
>  }
> 
>  static inline bool
> -check_l4_icmp(const void *data, size_t size)
> +check_l4_icmp(const void *data, size_t size, bool validate_checksum)
>  {
> -    return csum(data, size) == 0;
> +    if (validate_checksum) {
> +        return csum(data, size) == 0;
> +    }
> +    return true;
>  }
> 
>  static inline bool
>  check_l4_icmp6(const struct conn_key *key, const void *data, size_t size,
> -               const void *l3)
> +               const void *l3, bool validate_checksum)
>  {
> -    return checksum_valid(key, data, size, l3);
> +    return checksum_valid(key, data, size, l3, validate_checksum);
>  }
> 
>  static inline bool
> @@ -741,7 +747,8 @@ extract_l4_udp(struct conn_key *key, const void *data,
> size_t size)
>  }
> 
>  static inline bool extract_l4(struct conn_key *key, const void *data,
> -                              size_t size, bool *related, const void
> *l3);
> +                              size_t size, bool *related, const void *l3,
> +                              bool validate_checksum);
> 
>  static uint8_t
>  reverse_icmp_type(uint8_t type)
> @@ -830,7 +837,7 @@ extract_l4_icmp(struct conn_key *key, const void
> *data, size_t size,
>          key->dst = inner_key.dst;
>          key->nw_proto = inner_key.nw_proto;
> 
> -        ok = extract_l4(key, l4, tail - l4, NULL, l3);
> +        ok = extract_l4(key, l4, tail - l4, NULL, l3, false);
>          if (ok) {
>              conn_key_reverse(key);
>              *related = true;
> @@ -920,7 +927,7 @@ extract_l4_icmp6(struct conn_key *key, const void
> *data, size_t size,
>          key->dst = inner_key.dst;
>          key->nw_proto = inner_key.nw_proto;
> 
> -        ok = extract_l4(key, l4, tail - l4, NULL, l3);
> +        ok = extract_l4(key, l4, tail - l4, NULL, l3, false);
>          if (ok) {
>              conn_key_reverse(key);
>              *related = true;
> @@ -945,21 +952,22 @@ extract_l4_icmp6(struct conn_key *key, const void
> *data, size_t size,
>   * in an ICMP error.  In this case, we skip checksum and length
> validation. */
>  static inline bool
>  extract_l4(struct conn_key *key, const void *data, size_t size, bool
> *related,
> -           const void *l3)
> +           const void *l3, bool validate_checksum)
>  {
>      if (key->nw_proto == IPPROTO_TCP) {
> -        return (!related || check_l4_tcp(key, data, size, l3))
> -               && extract_l4_tcp(key, data, size);
> +        return (!related || check_l4_tcp(key, data, size, l3,
> +                validate_checksum)) && extract_l4_tcp(key, data, size);
>      } else if (key->nw_proto == IPPROTO_UDP) {
> -        return (!related || check_l4_udp(key, data, size, l3))
> -               && extract_l4_udp(key, data, size);
> +        return (!related || check_l4_udp(key, data, size, l3,
> +                validate_checksum)) && extract_l4_udp(key, data, size);
>      } else if (key->dl_type == htons(ETH_TYPE_IP)
>                 && key->nw_proto == IPPROTO_ICMP) {
> -        return (!related || check_l4_icmp(data, size))
> +        return (!related || check_l4_icmp(data, size, validate_checksum))
>                 && extract_l4_icmp(key, data, size, related);
>      } else if (key->dl_type == htons(ETH_TYPE_IPV6)
>                 && key->nw_proto == IPPROTO_ICMPV6) {
> -        return (!related || check_l4_icmp6(key, data, size, l3))
> +        return (!related || check_l4_icmp6(key, data, size, l3,
> +                validate_checksum))
>                 && extract_l4_icmp6(key, data, size, related);
>      } else {
>          return false;
> @@ -975,6 +983,8 @@ conn_key_extract(struct conntrack *ct, struct
> dp_packet *pkt, ovs_be16 dl_type,
>      const char *l4 = dp_packet_l4(pkt);
>      const char *tail = dp_packet_tail(pkt);
>      bool ok;
> +    bool valid_l4_csum = 0;
> +    bool  valid_l3_csum = 0;
> 
>      memset(ctx, 0, sizeof *ctx);
> 
> @@ -1018,7 +1028,10 @@ conn_key_extract(struct conntrack *ct, struct
> dp_packet *pkt, ovs_be16 dl_type,
>       */
>      ctx->key.dl_type = dl_type;
>      if (ctx->key.dl_type == htons(ETH_TYPE_IP)) {
> -        ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL,
> true);
> +        valid_l3_csum = dp_packet_ip_checksum_valid(pkt);
> +        /* Validate the checksum only when the csum is invalid */
> +        ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL,
> +                             !valid_l3_csum);
>      } else if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
>          ok = extract_l3_ipv6(&ctx->key, l3, tail - (char *) l3, NULL);
>      } else {
> @@ -1026,7 +1039,10 @@ conn_key_extract(struct conntrack *ct, struct
> dp_packet *pkt, ovs_be16 dl_type,
>      }
> 
>      if (ok) {
> -        if (extract_l4(&ctx->key, l4, tail - l4, &ctx->related, l3)) {
> +        valid_l4_csum = dp_packet_l4_checksum_valid(pkt);
> +        /* Validate the ckecksum only when the checksum is not
> valid/unset */
> +        if (extract_l4(&ctx->key, l4, tail - l4, &ctx->related, l3,
> +            !valid_l4_csum)) {
>              ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis);
>              return true;
>          }
> --
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list