Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <f847459dc0a0e2d8ffa1d290d06e0e4a226a6f39.1668075479.git.jamie.bainbridge@gmail.com>
 <20221110153901.7daa86e1@hermes.local>
In-Reply-To: <20221110153901.7daa86e1@hermes.local>
From:   Jamie Bainbridge <jamie.bainbridge@gmail.com>
Date:   Fri, 11 Nov 2022 10:38:48 +1000
Message-ID: <CAAvyFNiojSM-8euLLuuuZ5bDi0CKmoevncs3OQVLzrao1-Q4Jg@mail.gmail.com>
Subject: Re: [PATCH] tcp: Add listening address to SYN flood message
To:     Stephen Hemminger <stephen@networkplumber.org>
Cc:     Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>,
        David Ahern <dsahern@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 11 Nov 2022 at 09:39, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 10 Nov 2022 21:21:06 +1100
> Jamie Bainbridge <jamie.bainbridge@gmail.com> wrote:
>
> > +         xchg(&queue->synflood_warned, 1) == 0) {
> > +#if IS_ENABLED(CONFIG_IPV6)
> > +             if (sk->sk_family == AF_INET6) {
> > +                     net_info_ratelimited("%s: Possible SYN flooding on port %d. IP %pI6c. %s.  Check SNMP counters.\n",
> > +                                     proto, sk->sk_num,
> > +                                     &sk->sk_v6_rcv_saddr, msg);
> > +             } else
> > +#endif
> > +             {
> > +                     net_info_ratelimited("%s: Possible SYN flooding on port %d. IP %pI4. %s.  Check SNMP counters.\n",
> > +                                     proto, sk->sk_num, &sk->sk_rcv_saddr, msg);
> > +             }
> > +     }
> >
>
> Port number is unsigned not signed.
> Message also seems overly wordy to me.

Thanks for bringing this up. I agree with you.

I'd like to remove "Check SNMP counters" as it's not helpful to users.

How do they do that? (note userspace has changed from net-tools
"netstat -s" to iproute "nstat" since this message was added). Check
counters for what? If they even figure out the LISTEN stats are
growing, there's still troubleshooting to determine if the SYNs are
genuine or malicious, check/increase somaxconn and the socket listen()
backlog, check/improve application accept() performance, etc...

This is way too much to describe in a kernel log message, and it's the
job of the log message to be "descriptive" of what happened, not
"prescriptive" of policy to follow and cover every troubleshooting
possibility.

I will re-submit with a second patch removing this phrase.

Jamie