From: Davide Caratti <dcaratti@redhat.com>
To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
Cc: Alexander Duyck <alexander.duyck@gmail.com>,
Tom Herbert <tom@herbertland.com>,
David Laight <David.Laight@ACULAB.COM>,
"David S. Miller" <davem@davemloft.net>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: [PATCH net-next v3 0/7] fix CRC32c in the forwarding path
Date: Thu, 18 May 2017 13:44:36 +0000 [thread overview]
Message-ID: <cover.1495112301.git.dcaratti@redhat.com> (raw)
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.
Davide Caratti (7):
skbuff: add stub to help computing crc32c on SCTP packets
net: introduce skb_crc32c_csum_help
sk_buff: remove support for csum_bad in sk_buff
net: use skb->csum_not_inet to identify packets needing crc32c
net: more accurate checksumming in validate_xmit_skb()
openvswitch: more accurate checksumming in queue_userspace_packet()
sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY}
Documentation/networking/checksum-offloads.txt | 11 +++--
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
include/linux/netdevice.h | 8 ++--
include/linux/skbuff.h | 58 +++++++++--------------
net/bridge/netfilter/nft_reject_bridge.c | 5 +-
net/core/dev.c | 59 ++++++++++++++++++++++--
net/core/skbuff.c | 26 +++++++++++
net/ipv4/netfilter/nf_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_reject_ipv6.c | 3 --
net/openvswitch/datapath.c | 2 +-
net/sched/act_csum.c | 1 +
net/sctp/offload.c | 7 +++
net/sctp/output.c | 1 +
13 files changed, 127 insertions(+), 58 deletions(-)
--
2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: Davide Caratti <dcaratti@redhat.com>
To: linux-sctp@vger.kernel.org, netdev@vger.kernel.org
Cc: Alexander Duyck <alexander.duyck@gmail.com>,
Tom Herbert <tom@herbertland.com>,
David Laight <David.Laight@ACULAB.COM>,
"David S. Miller" <davem@davemloft.net>,
Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Subject: [PATCH net-next v3 0/7] fix CRC32c in the forwarding path
Date: Thu, 18 May 2017 15:44:36 +0200 [thread overview]
Message-ID: <cover.1495112301.git.dcaratti@redhat.com> (raw)
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.
Davide Caratti (7):
skbuff: add stub to help computing crc32c on SCTP packets
net: introduce skb_crc32c_csum_help
sk_buff: remove support for csum_bad in sk_buff
net: use skb->csum_not_inet to identify packets needing crc32c
net: more accurate checksumming in validate_xmit_skb()
openvswitch: more accurate checksumming in queue_userspace_packet()
sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY}
Documentation/networking/checksum-offloads.txt | 11 +++--
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 2 +-
include/linux/netdevice.h | 8 ++--
include/linux/skbuff.h | 58 +++++++++--------------
net/bridge/netfilter/nft_reject_bridge.c | 5 +-
net/core/dev.c | 59 ++++++++++++++++++++++--
net/core/skbuff.c | 26 +++++++++++
net/ipv4/netfilter/nf_reject_ipv4.c | 2 +-
net/ipv6/netfilter/nf_reject_ipv6.c | 3 --
net/openvswitch/datapath.c | 2 +-
net/sched/act_csum.c | 1 +
net/sctp/offload.c | 7 +++
net/sctp/output.c | 1 +
13 files changed, 127 insertions(+), 58 deletions(-)
--
2.7.4
next reply other threads:[~2017-05-18 13:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-18 13:44 Davide Caratti [this message]
2017-05-18 13:44 ` [PATCH net-next v3 0/7] fix CRC32c in the forwarding path Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 1/7] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 2/7] net: introduce skb_crc32c_csum_help Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 3/7] sk_buff: remove support for csum_bad in sk_buff Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 4/7] net: use skb->csum_not_inet to identify packets needing crc32c Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 5/7] net: more accurate checksumming in validate_xmit_skb() Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 6/7] openvswitch: more accurate checksumming in queue_userspace_packet() Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-18 13:44 ` [PATCH net-next v3 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY} Davide Caratti
2017-05-18 13:44 ` Davide Caratti
2017-05-19 23:22 ` [PATCH net-next v3 0/7] fix CRC32c in the forwarding path David Miller
2017-05-19 23:22 ` David Miller
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=cover.1495112301.git.dcaratti@redhat.com \
--to=dcaratti@redhat.com \
--cc=David.Laight@ACULAB.COM \
--cc=alexander.duyck@gmail.com \
--cc=davem@davemloft.net \
--cc=linux-sctp@vger.kernel.org \
--cc=marcelo.leitner@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=tom@herbertland.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.