1. 19 May, 2017 9 commits
    • David S. Miller's avatar
      Merge branch 'net-fix-CRC32c-in-the-forwarding-path' · 5d65a16a
      David S. Miller authored
      Davide Caratti says:
      
      ====================
      net: fix CRC32c in the forwarding path
      
      Current kernel allows offloading CRC32c computation when SCTP packets
      are generated, setting skb->ip_summed to CHECKSUM_PARTIAL, if the
      underlying device features have NETIF_F_SCTP_CRC set. However, after these
      packets are forwarded, they may land on a device where CRC32c offloading is
      not available: as a consequence, transmission is done with wrong CRC32c.
      It's not possible to use sctp_compte_cksum() in the forwarding path
      and in most drivers, because it needs symbols exported by libcrc32c module.
      
      Patch 1 and 2 of this series try to solve this problem, introducing a new
      helper function, namely skb_crc32c_csum_help(), that can be used to resolve
      CHECKSUM_PARTIAL when crc32c is needed instead of Internet Checksum.
      
      Currently, we need to parse the packet headers to understand what algorithm
      is needed to resolve CHECKSUM_PARTIAL. We can speedup things by storing
      this information in the skb metadata, and use it to call an appropriate
      helper (skb_checksum_help or skb_crc32c_csum_help), or leave the packet
      unmodified when the NIC is able to offload the checksum computation.
      
      Patch 3 deprecates skb->csum_bad to free one bit in skb metadata; patch 4
      introduces skb->csum_not_inet, providing skb with an indication on the
      algorithm needed to resolve CHECKSUM_PARTIAL.
      Patch 5 and 6 fix the kernel forwarding path and openvswitch datapath,
      where skb_checksum_help was unconditionally called to resolve CHECKSUM_PARTIAL,
      thus generating wrong CRC32c in forwarded SCTP packets.
      Finally, patch 7 updates documentation to provide a better description of
      possible values of skb->ip_summed.
      
      Some further work is still possible:
      * drivers that parse the packet header to correctly resolve CHECKSUM_PARTIAL
      (e.g. ixgbe_tx_csum()) can benefit from testing skb->csum_not_inet to avoid
      calling ip_hdr(skb)->protocol or ixgbe_ipv6_csum_is_sctp(skb).
      
      * drivers that call skb_checksum_help() to resolve CHECKSUM_PARTIAL can
      call skb_csum_hwoffload_help to avoid corrupting SCTP packets.
      
      Changes v2->v3:
      - patch 1/7: more standard declaration of stub variables
      
      Changes v1->v2:
      - none
      
      Changes RFCv4->v1:
      - patch 2/7: use WARN_ON_ONCE() instead of BUG_ON(), and avoid computing
      CRC32c on the error path.
      - patch 3/7: don't invert tests on the values of same_flow and
      NAPI_GRO_CB(skb)->flush in dev_gro_receive(), it's useless and it breaks
      GRO functionality as reported by kernel test robot.
      ====================
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5d65a16a
    • Davide Caratti's avatar
      sk_buff.h: improve description of CHECKSUM_{COMPLETE, UNNECESSARY} · b4759dcd
      Davide Caratti authored
      Add FCoE to the list of protocols that can set CHECKSUM_UNNECESSARY; add a
      note to CHECKSUM_COMPLETE section to specify that it does not apply to SCTP
      and FCoE protocols.
      Suggested-by: default avatarTom Herbert <tom@herbertland.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Acked-by: default avatarTom Herbert <tom@herbertland.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b4759dcd
    • Davide Caratti's avatar
      openvswitch: more accurate checksumming in queue_userspace_packet() · 7529390d
      Davide Caratti authored
      if skb carries an SCTP packet and ip_summed is CHECKSUM_PARTIAL, it needs
      CRC32c in place of Internet Checksum: use skb_csum_hwoffload_help to avoid
      corrupting such packets while queueing them towards userspace.
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      7529390d
    • Davide Caratti's avatar
      net: more accurate checksumming in validate_xmit_skb() · 43c26a1a
      Davide Caratti authored
      skb_csum_hwoffload_help() uses netdev features and skb->csum_not_inet to
      determine if skb needs software computation of Internet Checksum or crc32c
      (or nothing, if this computation can be done by the hardware). Use it in
      place of skb_checksum_help() in validate_xmit_skb() to avoid corruption
      of non-GSO SCTP packets having skb->ip_summed equal to CHECKSUM_PARTIAL.
      
      While at it, remove references to skb_csum_off_chk* functions, since they
      are not present anymore in Linux  _ see commit cf53b1da ("Revert
       "net: Add driver helper functions to determine checksum offloadability"").
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      43c26a1a
    • Davide Caratti's avatar
      net: use skb->csum_not_inet to identify packets needing crc32c · dba00306
      Davide Caratti authored
      skb->csum_not_inet carries the indication on which algorithm is needed to
      compute checksum on skb in the transmit path, when skb->ip_summed is equal
      to CHECKSUM_PARTIAL. If skb carries a SCTP packet and crc32c hasn't been
      yet written in L4 header, skb->csum_not_inet is assigned to 1; otherwise,
      assume Internet Checksum is needed and thus set skb->csum_not_inet to 0.
      Suggested-by: default avatarTom Herbert <tom@herbertland.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Acked-by: default avatarTom Herbert <tom@herbertland.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      dba00306
    • Davide Caratti's avatar
      sk_buff: remove support for csum_bad in sk_buff · 219f1d79
      Davide Caratti authored
      This bit was introduced with commit 5a212329 ("net: Support for
      csum_bad in skbuff") to reduce the stack workload when processing RX
      packets carrying a wrong Internet Checksum. Up to now, only one driver and
      GRO core are setting it.
      Suggested-by: default avatarTom Herbert <tom@herbertland.com>
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      219f1d79
    • Davide Caratti's avatar
      net: introduce skb_crc32c_csum_help · b72b5bf6
      Davide Caratti authored
      skb_crc32c_csum_help is like skb_checksum_help, but it is designed for
      checksumming SCTP packets using crc32c (see RFC3309), provided that
      libcrc32c.ko has been loaded before. In case libcrc32c is not loaded,
      invoking skb_crc32c_csum_help on a skb results in one the following
      printouts:
      
      warn_crc32c_csum_update: attempt to compute crc32c without libcrc32c.ko
      warn_crc32c_csum_combine: attempt to compute crc32c without libcrc32c.ko
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      b72b5bf6
    • Davide Caratti's avatar
      skbuff: add stub to help computing crc32c on SCTP packets · 9617813d
      Davide Caratti authored
      sctp_compute_checksum requires crc32c symbol (provided by libcrc32c), so
      it can't be used in net core. Like it has been done previously with other
      symbols (e.g. ipv6_dst_lookup), introduce a stub struct skb_checksum_ops
      to allow computation of crc32c checksum in net core after sctp.ko (and thus
      libcrc32c) has been loaded.
      Signed-off-by: default avatarDavide Caratti <dcaratti@redhat.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9617813d
    • Soheil Hassas Yeganeh's avatar
      tcp: warn on negative reordering values · 6f5b24ee
      Soheil Hassas Yeganeh authored
      Commit bafbb9c7 ("tcp: eliminate negative reordering
      in tcp_clean_rtx_queue") fixes an issue for negative
      reordering metrics.
      
      To be resilient to such errors, warn and return
      when a negative metric is passed to tcp_update_reordering().
      Signed-off-by: default avatarSoheil Hassas Yeganeh <soheil@google.com>
      Signed-off-by: default avatarNeal Cardwell <ncardwell@google.com>
      Signed-off-by: default avatarYuchung Cheng <ycheng@google.com>
      Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6f5b24ee
  2. 18 May, 2017 31 commits