| Message ID | 20240315151722.119628-5-atenart@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | gro: various fixes related to UDP tunnels | expand |
Antoine Tenart wrote: > GRO has a fundamental issue with UDP tunnel packets as it can't detect > those in a foolproof way and GRO could happen before they reach the > tunnel endpoint. Previous commits have fixed issues when UDP tunnel > packets come from a remote host, but if those packets are issued locally > they could run into checksum issues. > > If the inner packet has a partial checksum the information will be lost > in the GRO logic, either in udp4/6_gro_complete or in > udp_gro_complete_segment and packets will have an invalid checksum when > leaving the host. Before the previous patch, the tunnel code would convert ip_summed to CHECKSUM_UNNECESSARY. After that patch CHECKSUM_PARTIAL is preserved. Are the tunneled packets still corrupted once forwarded to the egress path? In principle CHECKSUM partial with tunnel and GSO should work, whether built as such or arrived at through GRO. > Prevent local UDP tunnel packets from ever being GROed at the outer UDP > level. > > Due to skb->encapsulation being wrongly used in some drivers this is > actually only preventing UDP tunnel packets with a partial checksum to > be GROed (see iptunnel_handle_offloads) but those were also the packets > triggering issues so in practice this should be sufficient. Because of this in iptunnel_handle_offloads: if (skb->ip_summed != CHECKSUM_PARTIAL) { skb->ip_summed = CHECKSUM_NONE; /* We clear encapsulation here to prevent badly-written * drivers potentially deciding to offload an inner checksum * if we set CHECKSUM_PARTIAL on the outer header. * This should go away when the drivers are all fixed. */ skb->encapsulation = 0; } > Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") > Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets") > Suggested-by: Paolo Abeni <pabeni@redhat.com> > Signed-off-by: Antoine Tenart <atenart@kernel.org> Choosing not to coalesce certain edge case packets that cause problems is safe and reasonable. Reviewed-by: Willem de Bruijn <willemb@google.com>
Hello, Quoting Willem de Bruijn (2024-03-16 16:43:26) > Antoine Tenart wrote: > > GRO has a fundamental issue with UDP tunnel packets as it can't detect > > those in a foolproof way and GRO could happen before they reach the > > tunnel endpoint. Previous commits have fixed issues when UDP tunnel > > packets come from a remote host, but if those packets are issued locally > > they could run into checksum issues. > > > > If the inner packet has a partial checksum the information will be lost > > in the GRO logic, either in udp4/6_gro_complete or in > > udp_gro_complete_segment and packets will have an invalid checksum when > > leaving the host. > > Before the previous patch, the tunnel code would convert ip_summed to > CHECKSUM_UNNECESSARY. After that patch CHECKSUM_PARTIAL is preserved. > Are the tunneled packets still corrupted once forwarded to the egress > path? In principle CHECKSUM partial with tunnel and GSO should work, > whether built as such or arrived at through GRO. Previous patch removed the partial -> unnecessary conversion for fraglist only; but packets GROed by rx-udp-gro-forwarding can hit udp_gro_complete_segment and the partial info would be overwritten there in case of an UDP tunnel packet GROed at the UDP level with the inner csum being partial. Thanks! Antoine
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index 3263ebcaa3f4..4ea72bd4f6d7 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -559,6 +559,12 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb, */ NAPI_GRO_CB(skb)->is_flist = 0; if (!sk || !udp_sk(sk)->gro_receive) { + /* If the packet was locally encapsulated in a UDP tunnel that + * wasn't detected above, do not GRO. + */ + if (skb->encapsulation) + goto out; + if (skb->dev->features & NETIF_F_GRO_FRAGLIST) NAPI_GRO_CB(skb)->is_flist = sk ? !udp_test_bit(GRO_ENABLED, sk) : 1;
GRO has a fundamental issue with UDP tunnel packets as it can't detect those in a foolproof way and GRO could happen before they reach the tunnel endpoint. Previous commits have fixed issues when UDP tunnel packets come from a remote host, but if those packets are issued locally they could run into checksum issues. If the inner packet has a partial checksum the information will be lost in the GRO logic, either in udp4/6_gro_complete or in udp_gro_complete_segment and packets will have an invalid checksum when leaving the host. Prevent local UDP tunnel packets from ever being GROed at the outer UDP level. Due to skb->encapsulation being wrongly used in some drivers this is actually only preventing UDP tunnel packets with a partial checksum to be GROed (see iptunnel_handle_offloads) but those were also the packets triggering issues so in practice this should be sufficient. Fixes: 9fd1ff5d2ac7 ("udp: Support UDP fraglist GRO/GSO.") Fixes: 36707061d6ba ("udp: allow forwarding of plain (non-fraglisted) UDP GRO packets") Suggested-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Antoine Tenart <atenart@kernel.org> --- net/ipv4/udp_offload.c | 6 ++++++ 1 file changed, 6 insertions(+)