On Tue, 2019-05-28 at 12:22 -0600, Sean Tranchetti wrote:
> Currently, when resegmenting an unexpected UDP GRO packet, the full UDP
> checksum will be calculated for every new SKB created by skb_segment()
> because the netdev features passed in by udp_rcv_segment() lack any
> information about checksum offload capabilities.
>
> Usually, we have no need to perform this calculation again, as
> 1) The GRO implementation guarantees that any packets making it to the
> udp_rcv_segment() function had correct checksums, and, more
> importantly,
> 2) Upon the successful return of udp_rcv_segment(), we immediately pull
> the UDP header off and either queue the segment to the socket or
> hand it off to a new protocol handler.
>
> Unless userspace has set the IP_CHECKSUM sockopt to indicate that they
> want the final checksum values, we can pass the needed netdev feature
> flags to __skb_gso_segment() to avoid checksumming each segment in
> skb_segment().
>
> Fixes: cf329aa42b66 ("udp: cope with UDP GRO packet misdirection")
> Cc: Paolo Abeni <pab...@redhat.com>
> Cc: Subash Abhinov Kasiviswanathan <subas...@codeaurora.org>
> Signed-off-by: Sean Tranchetti <stran...@codeaurora.org>
> ---
> include/net/udp.h | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/udp.h b/include/net/udp.h
> index d8ce937..dbe030d 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -471,12 +471,19 @@ struct udp_iter_state {
> static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
> struct sk_buff *skb, bool ipv4)
> {
> + netdev_features_t features = NETIF_F_SG;
> struct sk_buff *segs;
>
> + /* Avoid csum recalculation by skb_segment unless userspace explicitly
> + * asks for the final checksum values
> + */
> + if (!inet_get_convert_csum(sk))
> + features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> +
> /* the GSO CB lays after the UDP one, no need to save and restore any
> * CB fragment
> */
> - segs = __skb_gso_segment(skb, NETIF_F_SG, false);
> + segs = __skb_gso_segment(skb, features, false);
> if (unlikely(IS_ERR_OR_NULL(segs))) {
> int segs_nr = skb_shinfo(skb)->gso_segs;
The patch itself LGTM, thanks.
Acked-by: Paolo Abeni <pab...@redhat.com>
Possibly this can target the 'net' tree as the relevant commit is
already there and the patch itself is not very invasive.
Paolo
