Hello Mike,
On Fri, Oct 21, 2022 at 5:37 AM Mike Pattrick <[email protected]> wrote:
>
> From: Flavio Leitner <[email protected]>
>
> The netdev receiving packets is supposed to provide the flags
> indicating if the L4 csum 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 csum 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 <[email protected]>
> Co-authored-by: Mike Pattrick <[email protected]>
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> lib/conntrack.c | 15 +--
> lib/dp-packet.c | 24 ++++
> 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, 579 insertions(+), 257 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 973f6cdb0..5b20a10dc 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;
> @@ -3449,8 +3448,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 5609c2397..19b4e23be 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,25 @@ dp_packet_ol_send_prepare(struct dp_packet *p, const
> uint64_t flags)
> }
> dp_packet_hwol_reset_tx_ip_csum(p);
> }
> +
> + if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(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_ol_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_ol_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_ol_reset_tx_l4_csum(p);
> + }
> }
Same as my comment on patch 3, if OVS knows the l4 checksum is good,
it's better not to request hw offloading.
if (dp_packet_l4_checksum_good(p) || !dp_packet_hwol_tx_l4_checksum(p)) {
+ dp_packet_ol_reset_tx_l4_csum(p);
return;
}
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index f60618716..a2c450682 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; /* Offset to place checksum. */
> union {
> struct pkt_metadata md;
> uint64_t data[DP_PACKET_CONTEXT_SIZE / 8];
> @@ -995,6 +997,13 @@ dp_packet_hwol_is_ipv4(const struct dp_packet *b)
> return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_IPV4);
> }
>
> +/* Returns 'true' if packet 'p' is marked as IPv6. */
> +static inline bool
> +dp_packet_ol_tx_ipv6(const struct dp_packet *p)
For consistency: dp_packet_hwol_is_ipv6.
> +{
> + return !!(*dp_packet_ol_flags_ptr(p) & DP_PACKET_OL_TX_IPV6);
> +}
> +
> /* Returns 'true' if packet 'b' is marked for TCP checksum offloading. */
> static inline bool
> dp_packet_hwol_l4_is_tcp(const struct dp_packet *b)
> @@ -1019,18 +1028,26 @@ dp_packet_hwol_l4_is_sctp(struct dp_packet *b)
> DP_PACKET_OL_TX_SCTP_CKSUM;
> }
>
> -/* Mark packet 'b' for IPv4 checksum offloading. */
> static inline void
> -dp_packet_hwol_set_tx_ipv4(struct dp_packet *b)
> +dp_packet_ol_reset_tx_l4_csum(struct dp_packet *p)
> +{
> + *dp_packet_ol_flags_ptr(p) &= ~DP_PACKET_OL_TX_L4_MASK;
> +}
For consistency, dp_packet_hwol_reset_tx_l4_csum.
The rest lgtm.
I'll do a final pass on next revision.
--
David Marchand
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
