Re: [PATCH v2 1/2] net: macb: Remove unnecessary alignment check for TSO

[David Miller]

From: Harini Katakam <harini.katakam@xxxxxxxxxx>
Date: Mon,  3 Feb 2020 18:48:01 +0530

> The IP TSO implementation does NOT require the length to be a
> multiple of 8. That is only a requirement for UFO as per IP
> documentation.
> 
> Fixes: 1629dd4f763c ("cadence: Add LSO support.")
> Signed-off-by: Harini Katakam <harini.katakam@xxxxxxxxxx>
> ---
> v2:
> Added Fixes tag

Several problems with this.

The subject talks about alignemnt check, but you are not changing
the alignment check.  Instead you are modifying the linear buffer
check:

> @@ -1792,7 +1792,7 @@ static netdev_features_t macb_features_check(struct sk_buff *skb,
>  	/* Validate LSO compatibility */
>  
>  	/* there is only one buffer */
> -	if (!skb_is_nonlinear(skb))
> +	if (!skb_is_nonlinear(skb) || (ip_hdr(skb)->protocol != IPPROTO_UDP))
>  		return features;

So either your explanation is wrong or the code change is wrong.

Furthermore, if you add this condition then there is now dead code
below this.  The code that checks for example:

	/* length of header */
	hdrlen = skb_transport_offset(skb);
	if (ip_hdr(skb)->protocol == IPPROTO_TCP)
		hdrlen += tcp_hdrlen(skb);

will never trigger this IPPROTO_TCP condition after your change.

A lot of things about this patch do not add up.