On Fri, Mar 31, 2023 at 10:31 PM Fei Cheng <chenwei.0515@xxxxxxxxxxxxx> wrote:
>
> From: "chenwei.0515" <chenwei.0515@xxxxxxxxxxxxx>
>
> If vxlan-dev enable tx csum offload, there are two case of CHECKSUM_PARTIAL,
> but udp->check donot have the both meanings.
>
> 1. vxlan-dev disable tx csum offload, udp->check is just pseudo hdr.
> 2. vxlan-dev enable tx csum offload, udp->check is pseudo hdr and
> csum from outter l4 to innner l4.
>
> Unfortunately if there is a nat process after vxlan tx，udp_manip_pkt just use
> CSUM_PARTIAL to re csum PKT, which is just right on vxlan tx csum disable offload.
The issue is that for encapsulated traffic with local checksum offload,
netfilter incorrectly recomputes the outer UDP checksum as if it is an
unencapsulated CHECKSUM_PARTIAL packet, correct?
So the underlying issue is that the two types of packets are
indistinguishable after udp_set_csum:
} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
uh->check = 0;
uh->check = udp_v4_check(len, saddr, daddr, lco_csum(skb));
if (uh->check == 0)
uh->check = CSUM_MANGLED_0;
} else {
skb->ip_summed = CHECKSUM_PARTIAL;
skb->csum_start = skb_transport_header(skb) - skb->head;
skb->csum_offset = offsetof(struct udphdr, check);
uh->check = ~udp_v4_check(len, saddr, daddr, 0);
}
Clearly their ip_summed will be the same.
>
> This patch use skb->csum_local flag to identify two case, which will csum lco_csum if valid.
>
> Signed-off-by: chenwei.0515 <chenwei.0515@xxxxxxxxxxxxx>
> ---
> include/linux/skbuff.h | 1 +
> net/ipv4/udp.c | 1 +
> net/netfilter/nf_nat_proto.c | 9 +++++++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index ff7ad331fb82..62996d8d0b4d 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -990,6 +990,7 @@ struct sk_buff {
> __u8 slow_gro:1;
> __u8 csum_not_inet:1;
> __u8 scm_io_uring:1;
> + __u8 csum_local:1;
sk_buff are in space constrained.
I hope we can disambiguate the packets somehow without having
to resort to a new flag.
>
> #ifdef CONFIG_NET_SCHED
> __u16 tc_index; /* traffic control index */
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index c605d171eb2d..86bad0bbb76e 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -889,6 +889,7 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
> uh->check = udp_v4_check(len, saddr, daddr, lco_csum(skb));
> if (uh->check == 0)
> uh->check = CSUM_MANGLED_0;
> + skb->csum_local = 1;
> } else {
> skb->ip_summed = CHECKSUM_PARTIAL;
> skb->csum_start = skb_transport_header(skb) - skb->head;
> diff --git a/net/netfilter/nf_nat_proto.c b/net/netfilter/nf_nat_proto.c
> index 48cc60084d28..a0261fe2d932 100644
> --- a/net/netfilter/nf_nat_proto.c
> +++ b/net/netfilter/nf_nat_proto.c
> @@ -25,6 +25,7 @@
> #include <net/ip6_route.h>
> #include <net/xfrm.h>
> #include <net/ipv6.h>
> +#include <net/udp.h>
>
> #include <net/netfilter/nf_conntrack_core.h>
> #include <net/netfilter/nf_conntrack.h>
> @@ -75,6 +76,14 @@ static bool udp_manip_pkt(struct sk_buff *skb,
> hdr = (struct udphdr *)(skb->data + hdroff);
> __udp_manip_pkt(skb, iphdroff, hdr, tuple, maniptype, !!hdr->check);
>
> + if (skb->csum_local) {
> + hdr->check = 0;
> + hdr->check = udp_v4_check(htons(hdr->len), tuple->src.u3.ip, tuple->dst.u3.ip,
> + lco_csum(skb));
> + if (hdr->check == 0)
> + hdr->check = CSUM_MANGLED_0;
> + }
> +
> return true;
> }
>
> --
> 2.11.0
>
