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

[David Miller]

From: Marcelo Ricardo Leitner <marcelo.leit...@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.