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

Darrell Ball dball at vmware.com
Fri Jul 7 00:19:56 UTC 2017


This has been updated and I will be reviewing the changes

Thanks Darrell

On 7/6/17, 4:59 PM, "Ben Pfaff" <blp at ovn.org> wrote:

    Do you still intend to look closer?
    
    On Tue, Jun 06, 2017 at 04:40:14PM +0000, Darrell Ball wrote:
    > I did a first pass, but I will look thru. it more carefully and test it.
    > The benefit is smaller than I had hoped, even in a simple case, but the code delta is also simple.
    > 
    > Thanks Darrell
    > 
    > On 6/6/17, 3:45 AM, "ovs-dev-bounces at openvswitch.org on behalf of Chandran, Sugesh" <ovs-dev-bounces at openvswitch.org on behalf of sugesh.chandran at intel.com> wrote:
    > 
    >     Ping!. 
    >     Any other comments on this patch??
    >     
    >     
    >     Regards
    >     _Sugesh
    >     
    >     
    >     > -----Original Message-----
    >     > From: Fischetti, Antonio
    >     > Sent: Friday, May 26, 2017 11:05 AM
    >     > To: Chandran, Sugesh <sugesh.chandran at intel.com>; ovs-
    >     > dev at openvswitch.org
    >     > Subject: RE: [ovs-dev] [PATCH] conntrack : Use Rx checksum offload feature
    >     > on DPDK ports for conntrack.
    >     > 
    >     > 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://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=kVLpddWgvkgbLp3fpJuodSguI0Q073H7dUQnYuL7eY0&s=9JGLm-HbUQv0mvfEWEEiiIEBHsRBn-i2IVfCH8mzbhI&e= 
    >     _______________________________________________
    >     dev mailing list
    >     dev at openvswitch.org
    >     https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=kVLpddWgvkgbLp3fpJuodSguI0Q073H7dUQnYuL7eY0&s=9JGLm-HbUQv0mvfEWEEiiIEBHsRBn-i2IVfCH8mzbhI&e= 
    >     
    > 
    > _______________________________________________
    > dev mailing list
    > dev at openvswitch.org
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwIBAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=PttJsCkFn16WMCzEaJn4v6l5zONxHUnPbcfG4FLR94E&s=8li16gpsqoZh5uhKmVEBbvkY94ZRG-bWn5bGV-Gfq0c&e= 
    



More information about the dev mailing list