On 19 Jan 2022, at 14:53, Mike Pattrick wrote:
> Formerly when userspace TSO was enabled but with a non-DKDK interface
> without support IP checksum offloading, FTP NAT connections would fail
> if the packet length changed. This can happen if the packets length
> changes during L7 NAT translation, predominantly with FTP. This is why
> the "conntrack - IPv4 FTP Passive with DNAT" test would fail when run
> when run with check-system-tso.
>
> However, that isn't the only location with this type of issue. Any time
> that we had checked for dp_packet_hwol_is_ipv4() there was a possibility
> that the packet was either destined for an interface with TCP but not IP
> offload, or originating from an interface with any level of offload but
> destined for an interface with no checksum offload at all.
>
> A patch series for a more robust solution is in progress.
There is also this patch that might be of interest.
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> Reported-by: Ilya Maximets <[email protected]>
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> lib/conntrack.c | 8 +++-----
> lib/ipf.c | 36 ++++++++++++++++--------------------
> 2 files changed, 19 insertions(+), 25 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 33a1a9295..dfb2606a6 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -3402,11 +3402,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct
> conn_lookup_ctx *ctx,
> }
> if (seq_skew) {
> ip_len = ntohs(l3_hdr->ip_tot_len) + seq_skew;
> - if (!dp_packet_hwol_is_ipv4(pkt)) {
> - l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
> - l3_hdr->ip_tot_len,
> - htons(ip_len));
> - }
> + l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
> + l3_hdr->ip_tot_len,
> + htons(ip_len));
What happens if the egress interface is supporting hardware offload? Is it
ignoring the value in l3_hdr->ip_csum and just recalculating it, or is it
taking the value into account?
In DPDK I see rte_net_intel_cksum_prepare()/rte_net_intel_cksum_flags_prepare()
called by various drivers, but for mlx5 I do not see it expliocxitly set for
RTE_MBUF_F_TX_IP_CKSUM.
> l3_hdr->ip_tot_len = htons(ip_len);
> }
> }
> diff --git a/lib/ipf.c b/lib/ipf.c
> index 507db2aea..e1dee5907 100644
> --- a/lib/ipf.c
> +++ b/lib/ipf.c
> @@ -433,11 +433,9 @@ ipf_reassemble_v4_frags(struct ipf_list *ipf_list)
> len += rest_len;
> l3 = dp_packet_l3(pkt);
> ovs_be16 new_ip_frag_off = l3->ip_frag_off & ~htons(IP_MORE_FRAGMENTS);
> - if (!dp_packet_hwol_is_ipv4(pkt)) {
> - l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_frag_off,
> - new_ip_frag_off);
> - l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_tot_len, htons(len));
> - }
> + l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_frag_off,
> + new_ip_frag_off);
> + l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_tot_len, htons(len));
> l3->ip_tot_len = htons(len);
> l3->ip_frag_off = new_ip_frag_off;
> dp_packet_set_l2_pad_size(pkt, 0);
> @@ -1185,21 +1183,19 @@ ipf_post_execute_reass_pkts(struct ipf *ipf,
> } else {
> struct ip_header *l3_frag =
> dp_packet_l3(frag_i->pkt);
> struct ip_header *l3_reass = dp_packet_l3(pkt);
> - if (!dp_packet_hwol_is_ipv4(frag_i->pkt)) {
> - ovs_be32 reass_ip =
> - get_16aligned_be32(&l3_reass->ip_src);
> - ovs_be32 frag_ip =
> - get_16aligned_be32(&l3_frag->ip_src);
> -
> - l3_frag->ip_csum =
> recalc_csum32(l3_frag->ip_csum,
> - frag_ip,
> - reass_ip);
> - reass_ip = get_16aligned_be32(&l3_reass->ip_dst);
> - frag_ip = get_16aligned_be32(&l3_frag->ip_dst);
> - l3_frag->ip_csum =
> recalc_csum32(l3_frag->ip_csum,
> - frag_ip,
> - reass_ip);
> - }
> + ovs_be32 reass_ip =
> + get_16aligned_be32(&l3_reass->ip_src);
> + ovs_be32 frag_ip =
> + get_16aligned_be32(&l3_frag->ip_src);
> +
> + l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
> + frag_ip,
> + reass_ip);
> + reass_ip = get_16aligned_be32(&l3_reass->ip_dst);
> + frag_ip = get_16aligned_be32(&l3_frag->ip_dst);
> + l3_frag->ip_csum = recalc_csum32(l3_frag->ip_csum,
> + frag_ip,
> + reass_ip);
>
> l3_frag->ip_src = l3_reass->ip_src;
> l3_frag->ip_dst = l3_reass->ip_dst;
> --
> 2.27.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
