Re: [PATCH net v3] tcp: warn on bogus MSS and try to amend it

[David Miller <davem@davemloft.net>]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Fri,  2 Dec 2016 20:51:51 -0200

> @@ -144,7 +144,21 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
>  	 */
>  	len = skb_shinfo(skb)->gso_size ? : skb->len;
>  	if (len >= icsk->icsk_ack.rcv_mss) {
> -		icsk->icsk_ack.rcv_mss = len;
> +		static bool __once __read_mostly;
> +
> +		icsk->icsk_ack.rcv_mss = min_t(unsigned int, len,
> +					       tcp_sk(sk)->advmss);
> +		if (icsk->icsk_ack.rcv_mss != len && !__once) {
> +			struct net_device *dev;
> +
> +			__once = true;
> +
> +			rcu_read_lock();
> +			dev = dev_get_by_index_rcu(sock_net(sk), skb->skb_iif);
> +			pr_warn_once("%s: Driver has suspect GRO implementation, TCP performance may be compromised.\n",
> +				     dev ? dev->name : "Unknown driver");
> +			rcu_read_unlock();
> +		}

This is almost ready to go.

Since you are doing the 'once' logic by hand, using pr_warn_once() is
redundant.  And while you're at it, why not split this into a helper
function:

static void tcp_gro_dev_warn(struct sock *sk, const struct sk_buff *skb)
{
	static bool __once __read_mostly;

	if (!__once) {
		__once = true;

		rcu_read_lock();
		dev = dev_get_by_index_rcu(sock_net(sk), skb->skb_iif);
		pr_warn("%s: Driver has suspect GRO implementation, TCP performance may be compromised.\n",
			dev ? dev->name : "Unknown driver");
		rcu_read_unlock();
	}
}

And then call that when icsk->icsk_ack.rcv_mss != len, you can even
put an unlikely() around the condition as well.