[ovs-dev] [PATCH v9 4/4] userspace: Enable L4 checksum offloading by default.
From: Flavio Leitner The netdev receiving packets is supposed to provide the flags indicating if the L4 checksum was verified and it is OK or BAD, otherwise the stack will check when appropriate by software. If the packet comes with good checksum, then postpone the checksum calculation to the egress device if needed. When encapsulate a packet with that flag, set the checksum of the inner L4 header since that is not yet supported. Calculate the L4 checksum when the packet is going to be sent over a device that doesn't support the feature. Linux tap devices allows enabling L3 and L4 offload, so this patch enables the feature. However, Linux socket interface remains disabled because the API doesn't allow enabling those two features without enabling TSO too. Signed-off-by: Flavio Leitner Co-authored-by: Mike Pattrick Signed-off-by: Mike Pattrick --- lib/conntrack.c | 15 +-- lib/dp-packet.c | 25 lib/dp-packet.h | 78 - lib/flow.c | 23 lib/netdev-dpdk.c | 188 -- lib/netdev-linux.c | 252 ++-- lib/netdev-native-tnl.c | 32 + lib/netdev.c| 46 ++-- lib/packets.c | 175 ++-- lib/packets.h | 3 + 10 files changed, 580 insertions(+), 257 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 12194cce8..57e6a55e0 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2118,13 +2118,12 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, } if (ok) { -bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt); -if (!hwol_bad_l4_csum) { -bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt) - || dp_packet_hwol_tx_l4_checksum(pkt); +if (!dp_packet_l4_checksum_bad(pkt)) { /* Validate the checksum only when hwol is not supported. */ if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), - &ctx->icmp_related, l3, !hwol_good_l4_csum, + &ctx->icmp_related, l3, + !dp_packet_l4_checksum_good(pkt) && + !dp_packet_hwol_tx_l4_checksum(pkt), NULL)) { ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); return true; @@ -3453,8 +3452,10 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, adj_seqnum(&th->tcp_seq, ec->seq_skew); } -th->tcp_csum = 0; -if (!dp_packet_hwol_tx_l4_checksum(pkt)) { +if (dp_packet_hwol_tx_l4_checksum(pkt)) { +dp_packet_ol_reset_l4_csum_good(pkt); +} else { +th->tcp_csum = 0; if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { th->tcp_csum = packet_csum_upperlayer6(nh6, th, ctx->key.nw_proto, dp_packet_l4_size(pkt)); diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 90ef85de3..2cfaf5274 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -38,6 +38,9 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so dp_packet_init_specific(b); /* By default assume the packet type to be Ethernet. */ b->packet_type = htonl(PT_ETH); +/* Reset csum start and offset. */ +b->csum_start = 0; +b->csum_offset = 0; } static void @@ -544,4 +547,26 @@ dp_packet_ol_send_prepare(struct dp_packet *p, const uint64_t flags) dp_packet_ol_set_ip_csum_good(p); dp_packet_hwol_reset_tx_ip_csum(p); } + +if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) { +dp_packet_hwol_reset_tx_l4_csum(p); +return; +} + +if (dp_packet_hwol_l4_is_tcp(p) +&& !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) { +packet_tcp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} else if (dp_packet_hwol_l4_is_udp(p) +&& !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) { +packet_udp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} else if (!(flags & NETDEV_TX_OFFLOAD_SCTP_CKSUM) +&& dp_packet_hwol_l4_is_sctp(p)) { +packet_sctp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} } diff --git a/lib/dp-packet.h b/lib/dp-packet.h index f60618716..d550b099c 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -140,6 +140,8 @@ struct dp_packet { or UINT16_MAX. */ uint32_t cutlen; /* length in bytes to cut from the end. */ ovs_be32 packet_type; /* Packet type as defined in OpenFlow */ +uint16_t csum_start; /* Position to start checksumming from. */ +uint16_t csum_offset; /* Offse
Re: [ovs-dev] [PATCH v9 4/4] userspace: Enable L4 checksum offloading by default.
On Thu, Nov 24, 2022 at 6:31 AM Mike Pattrick wrote: > > From: Flavio Leitner > > The netdev receiving packets is supposed to provide the flags > indicating if the L4 checksum was verified and it is OK or BAD, > otherwise the stack will check when appropriate by software. > > If the packet comes with good checksum, then postpone the > checksum calculation to the egress device if needed. > > When encapsulate a packet with that flag, set the checksum > of the inner L4 header since that is not yet supported. > > Calculate the L4 checksum when the packet is going to be sent > over a device that doesn't support the feature. > > Linux tap devices allows enabling L3 and L4 offload, so this > patch enables the feature. However, Linux socket interface > remains disabled because the API doesn't allow enabling > those two features without enabling TSO too. > > Signed-off-by: Flavio Leitner > Co-authored-by: Mike Pattrick > Signed-off-by: Mike Pattrick I tested tcp traffic in various setups (with mlx5 nic as physical port): - external host <-> ovs <-> kernel host - external host <-> ovs <-> kernel vm - kernel vm <-> ovs <-> kernel host - kernel vm1 <-> ovs <-> kernel vm2 The changes may need more eyes on the netdev-linux parts, but the rest looks good to me. Reviewed-by: David Marchand -- David Marchand ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v9 4/4] userspace: Enable L4 checksum offloading by default.
From: Ilya Maximets On 11/24/22 06:30, Mike Pattrick wrote: > From: Flavio Leitner > > The netdev receiving packets is supposed to provide the flags > indicating if the L4 checksum was verified and it is OK or BAD, > otherwise the stack will check when appropriate by software. > > If the packet comes with good checksum, then postpone the > checksum calculation to the egress device if needed. > > When encapsulate a packet with that flag, set the checksum > of the inner L4 header since that is not yet supported. > > Calculate the L4 checksum when the packet is going to be sent > over a device that doesn't support the feature. > > Linux tap devices allows enabling L3 and L4 offload, so this > patch enables the feature. However, Linux socket interface > remains disabled because the API doesn't allow enabling > those two features without enabling TSO too. > > Signed-off-by: Flavio Leitner > Co-authored-by: Mike Pattrick > Signed-off-by: Mike Pattrick > --- Didn't test this as well. Only visual review. Should we enable checksum offloading in CONFIGURE_VETH_OFFLOADS for check-system-userspace testsuite since support is enabled by default? More comments inline. Best regards, Ilya Maximets. > lib/conntrack.c | 15 +-- > lib/dp-packet.c | 25 > lib/dp-packet.h | 78 - > lib/flow.c | 23 > lib/netdev-dpdk.c | 188 -- > lib/netdev-linux.c | 252 ++-- > lib/netdev-native-tnl.c | 32 + > lib/netdev.c| 46 ++-- > lib/packets.c | 175 ++-- > lib/packets.h | 3 + > 10 files changed, 580 insertions(+), 257 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 12194cce8..57e6a55e0 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -2118,13 +2118,12 @@ conn_key_extract(struct conntrack *ct, struct > dp_packet *pkt, ovs_be16 dl_type, > } > > if (ok) { > -bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt); > -if (!hwol_bad_l4_csum) { > -bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt) > - || dp_packet_hwol_tx_l4_checksum(pkt); > +if (!dp_packet_l4_checksum_bad(pkt)) { > /* Validate the checksum only when hwol is not supported. */ > if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), > - &ctx->icmp_related, l3, !hwol_good_l4_csum, > + &ctx->icmp_related, l3, > + !dp_packet_l4_checksum_good(pkt) && > + !dp_packet_hwol_tx_l4_checksum(pkt), > NULL)) { > ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); > return true; > @@ -3453,8 +3452,10 @@ handle_ftp_ctl(struct conntrack *ct, const struct > conn_lookup_ctx *ctx, > adj_seqnum(&th->tcp_seq, ec->seq_skew); > } > > -th->tcp_csum = 0; > -if (!dp_packet_hwol_tx_l4_checksum(pkt)) { > +if (dp_packet_hwol_tx_l4_checksum(pkt)) { > +dp_packet_ol_reset_l4_csum_good(pkt); > +} else { > +th->tcp_csum = 0; > if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { > th->tcp_csum = packet_csum_upperlayer6(nh6, th, > ctx->key.nw_proto, > dp_packet_l4_size(pkt)); > diff --git a/lib/dp-packet.c b/lib/dp-packet.c > index 90ef85de3..2cfaf5274 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -38,6 +38,9 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, > enum dp_packet_source so > dp_packet_init_specific(b); > /* By default assume the packet type to be Ethernet. */ > b->packet_type = htonl(PT_ETH); > +/* Reset csum start and offset. */ > +b->csum_start = 0; > +b->csum_offset = 0; > } > > static void > @@ -544,4 +547,26 @@ dp_packet_ol_send_prepare(struct dp_packet *p, const > uint64_t flags) > dp_packet_ol_set_ip_csum_good(p); > dp_packet_hwol_reset_tx_ip_csum(p); > } > + > +if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) { > +dp_packet_hwol_reset_tx_l4_csum(p); > +return; > +} > + > +if (dp_packet_hwol_l4_is_tcp(p) > +&& !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) { > +packet_tcp_complete_csum(p); > +dp_packet_ol_set_l4_csum_good(p); > +dp_packet_hwol_reset_tx_l4_csum(p); > +} else if (dp_packet_hwol_l4_is_udp(p) > +&& !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) { Indentation. > +packet_udp_complete_csum(p); > +dp_packet_ol_set_l4_csum_good(p); > +dp_packet_hwol_reset_tx_l4_csum(p); > +} else if (!(flags & NETDEV_TX_OFFLOAD_SCTP_CKSUM) > +&& dp_packet_hwol_l4_is_sctp(p)) { Indentation. > +packet_sctp_complete_csum(p); > +dp_packet_ol_set_l4_csum_go
Re: [ovs-dev] [PATCH v9 4/4] userspace: Enable L4 checksum offloading by default.
Please ignore this email. fbl On 7/7/23 16:21, Flavio Leitner wrote: From: Ilya Maximets On 11/24/22 06:30, Mike Pattrick wrote: From: Flavio Leitner The netdev receiving packets is supposed to provide the flags indicating if the L4 checksum was verified and it is OK or BAD, otherwise the stack will check when appropriate by software. If the packet comes with good checksum, then postpone the checksum calculation to the egress device if needed. When encapsulate a packet with that flag, set the checksum of the inner L4 header since that is not yet supported. Calculate the L4 checksum when the packet is going to be sent over a device that doesn't support the feature. Linux tap devices allows enabling L3 and L4 offload, so this patch enables the feature. However, Linux socket interface remains disabled because the API doesn't allow enabling those two features without enabling TSO too. Signed-off-by: Flavio Leitner Co-authored-by: Mike Pattrick Signed-off-by: Mike Pattrick --- Didn't test this as well. Only visual review. Should we enable checksum offloading in CONFIGURE_VETH_OFFLOADS for check-system-userspace testsuite since support is enabled by default? More comments inline. Best regards, Ilya Maximets. lib/conntrack.c | 15 +-- lib/dp-packet.c | 25 lib/dp-packet.h | 78 - lib/flow.c | 23 lib/netdev-dpdk.c | 188 -- lib/netdev-linux.c | 252 ++-- lib/netdev-native-tnl.c | 32 + lib/netdev.c| 46 ++-- lib/packets.c | 175 ++-- lib/packets.h | 3 + 10 files changed, 580 insertions(+), 257 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 12194cce8..57e6a55e0 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -2118,13 +2118,12 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, } if (ok) { -bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt); -if (!hwol_bad_l4_csum) { -bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt) - || dp_packet_hwol_tx_l4_checksum(pkt); +if (!dp_packet_l4_checksum_bad(pkt)) { /* Validate the checksum only when hwol is not supported. */ if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), - &ctx->icmp_related, l3, !hwol_good_l4_csum, + &ctx->icmp_related, l3, + !dp_packet_l4_checksum_good(pkt) && + !dp_packet_hwol_tx_l4_checksum(pkt), NULL)) { ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); return true; @@ -3453,8 +3452,10 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, adj_seqnum(&th->tcp_seq, ec->seq_skew); } -th->tcp_csum = 0; -if (!dp_packet_hwol_tx_l4_checksum(pkt)) { +if (dp_packet_hwol_tx_l4_checksum(pkt)) { +dp_packet_ol_reset_l4_csum_good(pkt); +} else { +th->tcp_csum = 0; if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { th->tcp_csum = packet_csum_upperlayer6(nh6, th, ctx->key.nw_proto, dp_packet_l4_size(pkt)); diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 90ef85de3..2cfaf5274 100644 --- a/lib/dp-packet.c +++ b/lib/dp-packet.c @@ -38,6 +38,9 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source so dp_packet_init_specific(b); /* By default assume the packet type to be Ethernet. */ b->packet_type = htonl(PT_ETH); +/* Reset csum start and offset. */ +b->csum_start = 0; +b->csum_offset = 0; } static void @@ -544,4 +547,26 @@ dp_packet_ol_send_prepare(struct dp_packet *p, const uint64_t flags) dp_packet_ol_set_ip_csum_good(p); dp_packet_hwol_reset_tx_ip_csum(p); } + +if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) { +dp_packet_hwol_reset_tx_l4_csum(p); +return; +} + +if (dp_packet_hwol_l4_is_tcp(p) +&& !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) { +packet_tcp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} else if (dp_packet_hwol_l4_is_udp(p) +&& !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) { Indentation. +packet_udp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} else if (!(flags & NETDEV_TX_OFFLOAD_SCTP_CKSUM) +&& dp_packet_hwol_l4_is_sctp(p)) { Indentation. +packet_sctp_complete_csum(p); +dp_packet_ol_set_l4_csum_good(p); +dp_packet_hwol_reset_tx_l4_csum(p); +} } diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
Re: [ovs-dev] [PATCH v9 4/4] userspace: Enable L4 checksum offloading by default.
On 11/24/22 06:30, Mike Pattrick wrote: > From: Flavio Leitner > > The netdev receiving packets is supposed to provide the flags > indicating if the L4 checksum was verified and it is OK or BAD, > otherwise the stack will check when appropriate by software. > > If the packet comes with good checksum, then postpone the > checksum calculation to the egress device if needed. > > When encapsulate a packet with that flag, set the checksum > of the inner L4 header since that is not yet supported. > > Calculate the L4 checksum when the packet is going to be sent > over a device that doesn't support the feature. > > Linux tap devices allows enabling L3 and L4 offload, so this > patch enables the feature. However, Linux socket interface > remains disabled because the API doesn't allow enabling > those two features without enabling TSO too. > > Signed-off-by: Flavio Leitner > Co-authored-by: Mike Pattrick > Signed-off-by: Mike Pattrick > --- Didn't test this as well. Only visual review. Should we enable checksum offloading in CONFIGURE_VETH_OFFLOADS for check-system-userspace testsuite since support is enabled by default? More comments inline. Best regards, Ilya Maximets. > lib/conntrack.c | 15 +-- > lib/dp-packet.c | 25 > lib/dp-packet.h | 78 - > lib/flow.c | 23 > lib/netdev-dpdk.c | 188 -- > lib/netdev-linux.c | 252 ++-- > lib/netdev-native-tnl.c | 32 + > lib/netdev.c| 46 ++-- > lib/packets.c | 175 ++-- > lib/packets.h | 3 + > 10 files changed, 580 insertions(+), 257 deletions(-) > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 12194cce8..57e6a55e0 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -2118,13 +2118,12 @@ conn_key_extract(struct conntrack *ct, struct > dp_packet *pkt, ovs_be16 dl_type, > } > > if (ok) { > -bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt); > -if (!hwol_bad_l4_csum) { > -bool hwol_good_l4_csum = dp_packet_l4_checksum_good(pkt) > - || dp_packet_hwol_tx_l4_checksum(pkt); > +if (!dp_packet_l4_checksum_bad(pkt)) { > /* Validate the checksum only when hwol is not supported. */ > if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), > - &ctx->icmp_related, l3, !hwol_good_l4_csum, > + &ctx->icmp_related, l3, > + !dp_packet_l4_checksum_good(pkt) && > + !dp_packet_hwol_tx_l4_checksum(pkt), > NULL)) { > ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); > return true; > @@ -3453,8 +3452,10 @@ handle_ftp_ctl(struct conntrack *ct, const struct > conn_lookup_ctx *ctx, > adj_seqnum(&th->tcp_seq, ec->seq_skew); > } > > -th->tcp_csum = 0; > -if (!dp_packet_hwol_tx_l4_checksum(pkt)) { > +if (dp_packet_hwol_tx_l4_checksum(pkt)) { > +dp_packet_ol_reset_l4_csum_good(pkt); > +} else { > +th->tcp_csum = 0; > if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { > th->tcp_csum = packet_csum_upperlayer6(nh6, th, > ctx->key.nw_proto, > dp_packet_l4_size(pkt)); > diff --git a/lib/dp-packet.c b/lib/dp-packet.c > index 90ef85de3..2cfaf5274 100644 > --- a/lib/dp-packet.c > +++ b/lib/dp-packet.c > @@ -38,6 +38,9 @@ dp_packet_init__(struct dp_packet *b, size_t allocated, > enum dp_packet_source so > dp_packet_init_specific(b); > /* By default assume the packet type to be Ethernet. */ > b->packet_type = htonl(PT_ETH); > +/* Reset csum start and offset. */ > +b->csum_start = 0; > +b->csum_offset = 0; > } > > static void > @@ -544,4 +547,26 @@ dp_packet_ol_send_prepare(struct dp_packet *p, const > uint64_t flags) > dp_packet_ol_set_ip_csum_good(p); > dp_packet_hwol_reset_tx_ip_csum(p); > } > + > +if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) { > +dp_packet_hwol_reset_tx_l4_csum(p); > +return; > +} > + > +if (dp_packet_hwol_l4_is_tcp(p) > +&& !(flags & NETDEV_TX_OFFLOAD_TCP_CKSUM)) { > +packet_tcp_complete_csum(p); > +dp_packet_ol_set_l4_csum_good(p); > +dp_packet_hwol_reset_tx_l4_csum(p); > +} else if (dp_packet_hwol_l4_is_udp(p) > +&& !(flags & NETDEV_TX_OFFLOAD_UDP_CKSUM)) { Indentation. > +packet_udp_complete_csum(p); > +dp_packet_ol_set_l4_csum_good(p); > +dp_packet_hwol_reset_tx_l4_csum(p); > +} else if (!(flags & NETDEV_TX_OFFLOAD_SCTP_CKSUM) > +&& dp_packet_hwol_l4_is_sctp(p)) { Indentation. > +packet_sctp_complete_csum(p); > +dp_packet_ol_set_l4_csum_good(p); > +dp_p