Jakub Sitnicki wrote:
> If:
>
> 1) the user requested USO, but
> 2) there is not enough payload for GSO to kick in, and
> 3) the egress device doesn't offer checksum offload, then
>
> we want to compute the L4 checksum in software early on.
>
> In the case when we taking the GSO path, but it has been requested, the
What does it refers to here?
> software checksum fallback in skb_segment doesn't get a chance to compute
> the full checksum, if the egress device can't do it. As a result we end up
> sending UDP datagrams with only a partial checksum filled in, which the
> peer will discard.
>
> Fixes: 10154dbded6d ("udp: Allow GSO transmit from devices with no checksum offload")
> Reported-by: Ivan Babrou <ivan@xxxxxxxxxxxxxx>
> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> This shouldn't have fallen through the cracks. I clearly need to extend the
> net/udpgso selftests further to cover the whole TX path for software
> USO+csum case. I will follow up with that but I wanted to get the fix out
> in the meantime. Apologies for the oversight.
> ---
> net/ipv4/udp.c | 4 +++-
> net/ipv6/udp.c | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 8accbf4cb295..2849b273b131 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -951,8 +951,10 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
> cork->gso_size);
> +
> + /* Don't checksum the payload, skb will get segmented */
> + goto csum_partial;
> }
> - goto csum_partial;
The issue here is that GSO packets with CHECKSUM_NONE will get fixed
software checksummed in skb_segment, but no such fallback path is
entered for regular packets, right?
We could setup CHECKSUM_PARTIAL and rely on validate_xmit_skb. But
might as well do the software checksumming right here, as your
patch does.
If I follow this all, ACK from me. Just want to make sure.
> }
>
> if (is_udplite) /* UDP-Lite */
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 52dfbb2ff1a8..0cef8ae5d1ea 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1266,8 +1266,10 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(datalen,
> cork->gso_size);
> +
> + /* Don't checksum the payload, skb will get segmented */
> + goto csum_partial;
> }
> - goto csum_partial;
> }
>
> if (is_udplite)
>
