Subject: Re: [Patch net] net_sched: fix mirrored packets checksum

From: Daniel Borkmann <[email-masked]>
Date: Thu, 30 Jun 2016 21:50:59 +0200

Hi Cong,

On 06/30/2016 07:15 PM, Cong Wang wrote:
> Similar to commit 9b368814b336 ("net: fix bridge multicast packet checksum validation")
> we need to fixup the checksum for CHECKSUM_COMPLETE when
> pushing skb on RX path. Otherwise we get similar splats.
>
> Cc: Jamal Hadi Salim <[email-masked]>
> Cc: Tom Herbert <[email-masked]>
> Signed-off-by: Cong Wang <[email-masked]>
> ---
>   include/linux/skbuff.h | 19 +++++++++++++++++++
>   net/core/skbuff.c      | 18 ------------------
>   net/sched/act_mirred.c |  2 +-
>   3 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index ee38a41..61ab566 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2870,6 +2870,25 @@ static inline void skb_postpush_rcsum(struct sk_buff *skb,
>   }
>
>   /**
> + *	skb_push_rcsum - push skb and update receive checksum
> + *	@skb: buffer to update
> + *	@len: length of data pulled
> + *
> + *	This function performs an skb_push on the packet and updates
> + *	the CHECKSUM_COMPLETE checksum.  It should be used on
> + *	receive path processing instead of skb_push unless you know
> + *	that the checksum difference is zero (e.g., a valid IP header)
> + *	or you are setting ip_summed to CHECKSUM_NONE.
> + */
> +static inline unsigned char *skb_push_rcsum(struct sk_buff *skb,
> +					    unsigned int len)
> +{
> +	skb_push(skb, len);
> +	skb_postpush_rcsum(skb, skb->data, len);
> +	return skb->data;
> +}
> +
> +/**
>    *	pskb_trim_rcsum - trim received skb and update checksum
>    *	@skb: buffer to trim
>    *	@len: new length
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index f2b77e5..eb12d21 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3016,24 +3016,6 @@ int skb_append_pagefrags(struct sk_buff *skb, struct page *page,
>   EXPORT_SYMBOL_GPL(skb_append_pagefrags);
>
>   /**
> - *	skb_push_rcsum - push skb and update receive checksum
> - *	@skb: buffer to update
> - *	@len: length of data pulled
> - *
> - *	This function performs an skb_push on the packet and updates
> - *	the CHECKSUM_COMPLETE checksum.  It should be used on
> - *	receive path processing instead of skb_push unless you know
> - *	that the checksum difference is zero (e.g., a valid IP header)
> - *	or you are setting ip_summed to CHECKSUM_NONE.
> - */
> -static unsigned char *skb_push_rcsum(struct sk_buff *skb, unsigned len)
> -{
> -	skb_push(skb, len);
> -	skb_postpush_rcsum(skb, skb->data, len);
> -	return skb->data;
> -}
> -
> -/**
>    *	skb_pull_rcsum - pull skb and update receive checksum
>    *	@skb: buffer to update
>    *	@len: length of data pulled

Fix looks good to me, just a minor comment.

Maybe makes sense to move skb_push_rcsum() but /also/ skb_pull_rcsum()
to the header then? Both seem similarly small at least (could be split
f.e into two patches then, first for the move, second for the actual fix).

Thanks,
Daniel