Message ID | f847459dc0a0e2d8ffa1d290d06e0e4a226a6f39.1668075479.git.jamie.bainbridge@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp62613wru; Thu, 10 Nov 2022 02:29:12 -0800 (PST) X-Google-Smtp-Source: AMsMyM7Td7Ht0gyYyzrR/sc2/YieagfdFOfTVfdQ9gj7Ocnqw4Z1ud821V7zIXeAKPcvRvG5T2RF X-Received: by 2002:a63:fc4b:0:b0:46f:ed8d:7087 with SMTP id r11-20020a63fc4b000000b0046fed8d7087mr41109120pgk.135.1668076152635; Thu, 10 Nov 2022 02:29:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1668076152; cv=none; d=google.com; s=arc-20160816; b=hXE3JaDU0aNR3lSfUnwyYzccsgcCZTDe9H1GAGjeMxOws5q6GVHcSktTEGOH6BLC4O bVQKOGpK4ay7QVJntqQ/mu7f9yCD9Jyi4la+OSjzQvLK4nKdVWct4s6NVf9lyf6HqDzL JBbNjBnWBsdfu/Q3/aCoHqqBCFWYnZbIA+cMEMzUJDZZVF/8SWUF4C2G6lhEh4DER8KS XeBGczEAMm0DVo35Vy1kVRwgJgRevDpu2q4Cb6FOpg/b5Ubjfpc1p0MvcF2XKTfAzJIf 6QGslNwRPdX6+vllHQHJbmYz0RERCpRXuh2Rd3HGUhLOjkyLhAWlqrRj2pienPrSJa4k F7AA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=xIm/Oyzmfn3evXzwG3HkXqcHPzlLZjBFReRVxCcbrAk=; b=O5VrumtyhD6rpFL00l3f/8kzrYd6yZ8DwoXaAYp59ymTFuGTmXmcCSrpNN6eczu1Bs uLpMspe+iLrMHnkb1RBXS4rEtjkYaLmHlesSLV9+9oM+fprZDNDNGeJRzn19dTLZJk6R X95bGF8k1MimoyzWA8pj5Y0xThl856WyxuP7T6puoxq812A7Vuz/vQUdBYQ6XSMPCV95 RZKF8sdaJ9i8TGlSyNzPLnIxBSCS5Q9Ud3grdGwglWLCD3PiEgA7IFLNjURpUm2RRAEZ UPY5o0cvYue1u+mSpO+cj5qvocmLKsEaOXwus0+JPvGD6rpIru4eMCSMidnpe2luxr1m 4oyg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=XDhrPgcY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e11-20020a63f54b000000b0046ff7bbe583si20019855pgk.702.2022.11.10.02.28.57; Thu, 10 Nov 2022 02:29:12 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=XDhrPgcY; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229984AbiKJKVz (ORCPT <rfc822;winker.wchi@gmail.com> + 99 others); Thu, 10 Nov 2022 05:21:55 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229915AbiKJKVv (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 10 Nov 2022 05:21:51 -0500 Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3A35F13F20; Thu, 10 Nov 2022 02:21:51 -0800 (PST) Received: by mail-pg1-x52c.google.com with SMTP id e129so1364578pgc.9; Thu, 10 Nov 2022 02:21:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=xIm/Oyzmfn3evXzwG3HkXqcHPzlLZjBFReRVxCcbrAk=; b=XDhrPgcY1K4+93HyVhQPSTJ0vB/sm8roe8+cR0Hoov6m6lmmBaBQ1Cd2KwcXos6m8c R7hlFZNR4mZEwWpR62nGHfj+4rH4SSDj4lSRrqxFj80256RqdHnaBmLp0zixOHADL9+y hAIX/hXsGAcTybCd2Ig4k0sGOfMd0dmPcNAv8G/kZD8VjU3vJxtN2wULWzY/UOjvDdQZ rhtrC/RpbCF1WGKHURiUm3u2UYygoEsiIH29Czty9U8QNeRsTBZXeXrgxKArXKNjvokH +1MklRcBYNGBZwiuDHtIoJvyYZmko4vNSXqYF0QXgoVhWxtuezTrbzWFP5O51s83RQBc 3rZw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=xIm/Oyzmfn3evXzwG3HkXqcHPzlLZjBFReRVxCcbrAk=; b=L68RDvxQm3IAsJFGXhRnLV6uVNwXhACB6L5g27sKTNb7GuS66ZXntHiUnNaXuragM1 7zjTc1x9SK8T16vERVwYoHWM5+4y49xOCsYbu/JaDWnYEzODcOjJH0Y1lVEgDob3jy7s C7hEHkX+eSIID47zJmvhJxdZ3iLWIPDqUDSLjXdh4YCvH6Ek2dKCgrF/9YNzeAVOjsRF 5Y57hB7o7Up5OEPSSqeGeG8YK+pK+oEJDcf4Rsq3jsK2KkBEhTk1OlsskL+ZoP1MICWU Io36mjDIwPTDmgDF6R0GMw1GtBFw/e6otlZFQbMHvXeb7owe6IJOXYr4C3fOz8ESC/WV 2Ctg== X-Gm-Message-State: ACrzQf1wGRg4TmmAFYdRpPX6NH+/GTaOrhF2s35Znle18Q3G7j7McZOJ +mlKMEk9SlSSVVoPdhEBtww= X-Received: by 2002:aa7:9dd0:0:b0:56c:394d:c675 with SMTP id g16-20020aa79dd0000000b0056c394dc675mr2270370pfq.17.1668075710593; Thu, 10 Nov 2022 02:21:50 -0800 (PST) Received: from localhost.localdomain ([110.147.198.134]) by smtp.gmail.com with ESMTPSA id y13-20020aa78f2d000000b0056ddd2b5e9bsm9741907pfr.41.2022.11.10.02.21.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Nov 2022 02:21:50 -0800 (PST) From: Jamie Bainbridge <jamie.bainbridge@gmail.com> To: 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> Cc: Jamie Bainbridge <jamie.bainbridge@gmail.com>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] tcp: Add listening address to SYN flood message Date: Thu, 10 Nov 2022 21:21:06 +1100 Message-Id: <f847459dc0a0e2d8ffa1d290d06e0e4a226a6f39.1668075479.git.jamie.bainbridge@gmail.com> X-Mailer: git-send-email 2.38.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1749104619682077667?= X-GMAIL-MSGID: =?utf-8?q?1749104619682077667?= |
Series |
tcp: Add listening address to SYN flood message
|
|
Commit Message
Jamie Bainbridge
Nov. 10, 2022, 10:21 a.m. UTC
The SYN flood message prints the listening port number, but on a system
with many processes bound to the same port on different IPs, it's
impossible to tell which socket is the problem.
Add the listen IP address to the SYN flood message. It might have been
nicer to print the address first, but decades of monitoring tools are
watching for the string "SYN flooding on port" so don't break that.
Tested with each protcol's "any" address and a host address:
Possible SYN flooding on port 9001. IP 0.0.0.0.
Possible SYN flooding on port 9001. IP 127.0.0.1.
Possible SYN flooding on port 9001. IP ::.
Possible SYN flooding on port 9001. IP fc00::1.
Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com>
---
net/ipv4/tcp_input.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
Comments
On Thu, Nov 10, 2022 at 09:21:06PM +1100, Jamie Bainbridge wrote: > The SYN flood message prints the listening port number, but on a system > with many processes bound to the same port on different IPs, it's > impossible to tell which socket is the problem. > > Add the listen IP address to the SYN flood message. It might have been > nicer to print the address first, but decades of monitoring tools are > watching for the string "SYN flooding on port" so don't break that. > > Tested with each protcol's "any" address and a host address: > > Possible SYN flooding on port 9001. IP 0.0.0.0. > Possible SYN flooding on port 9001. IP 127.0.0.1. > Possible SYN flooding on port 9001. IP ::. > Possible SYN flooding on port 9001. IP fc00::1. > > Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com> > --- > net/ipv4/tcp_input.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > index 0640453fce54b6daae0861d948f3db075830daf6..fb86056732266fedc8ad574bbf799dbdd7a425a3 100644 > --- a/net/ipv4/tcp_input.c > +++ b/net/ipv4/tcp_input.c > @@ -6831,9 +6831,19 @@ static bool tcp_syn_flood_action(const struct sock *sk, const char *proto) > __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPREQQFULLDROP); > > if (!queue->synflood_warned && syncookies != 2 && > - xchg(&queue->synflood_warned, 1) == 0) > - net_info_ratelimited("%s: Possible SYN flooding on port %d. %s. Check SNMP counters.\n", > - proto, sk->sk_num, msg); > + xchg(&queue->synflood_warned, 1) == 0) { > +#if IS_ENABLED(CONFIG_IPV6) > + if (sk->sk_family == AF_INET6) { Can the IS_ENABLED() go inside the if? You get better build testing that way. Andrew
On Fri, 11 Nov 2022 at 00:51, Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, Nov 10, 2022 at 09:21:06PM +1100, Jamie Bainbridge wrote: > > The SYN flood message prints the listening port number, but on a system > > with many processes bound to the same port on different IPs, it's > > impossible to tell which socket is the problem. > > > > Add the listen IP address to the SYN flood message. It might have been > > nicer to print the address first, but decades of monitoring tools are > > watching for the string "SYN flooding on port" so don't break that. > > > > Tested with each protcol's "any" address and a host address: > > > > Possible SYN flooding on port 9001. IP 0.0.0.0. > > Possible SYN flooding on port 9001. IP 127.0.0.1. > > Possible SYN flooding on port 9001. IP ::. > > Possible SYN flooding on port 9001. IP fc00::1. > > > > Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com> > > --- > > net/ipv4/tcp_input.c | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > > index 0640453fce54b6daae0861d948f3db075830daf6..fb86056732266fedc8ad574bbf799dbdd7a425a3 100644 > > --- a/net/ipv4/tcp_input.c > > +++ b/net/ipv4/tcp_input.c > > @@ -6831,9 +6831,19 @@ static bool tcp_syn_flood_action(const struct sock *sk, const char *proto) > > __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPREQQFULLDROP); > > > > if (!queue->synflood_warned && syncookies != 2 && > > - xchg(&queue->synflood_warned, 1) == 0) > > - net_info_ratelimited("%s: Possible SYN flooding on port %d. %s. Check SNMP counters.\n", > > - proto, sk->sk_num, msg); > > + xchg(&queue->synflood_warned, 1) == 0) { > > +#if IS_ENABLED(CONFIG_IPV6) > > + if (sk->sk_family == AF_INET6) { > > Can the IS_ENABLED() go inside the if? You get better build testing > that way. > > Andrew Are you sure? Why would the IS_ENABLED() be inside of a condition which isn't compiled in? If IPv6 isn't compiled in then the condition would never evaluate as true, so seems pointless a pointless comparison to make? People not compiling in IPv6 have explicitly asked *not* to have their kernel filled with a bunch of "if (family == AF_INET6)" haven't they? There are many other examples of this pattern of "IS_ENABLED()" first and "if (family == AF_INET6)" inside it, but I can't see any of the inverse which I think you're suggesting, see: grep -C1 -ERHn "IS_ENABLED\(CONFIG_IPV6\)" net | grep -C1 "family == AF_INET6" Please let me know if I've misunderstood? Jamie
On Fri, Nov 11, 2022 at 08:20:18AM +1100, Jamie Bainbridge wrote: > On Fri, 11 Nov 2022 at 00:51, Andrew Lunn <andrew@lunn.ch> wrote: > > > > On Thu, Nov 10, 2022 at 09:21:06PM +1100, Jamie Bainbridge wrote: > > > The SYN flood message prints the listening port number, but on a system > > > with many processes bound to the same port on different IPs, it's > > > impossible to tell which socket is the problem. > > > > > > Add the listen IP address to the SYN flood message. It might have been > > > nicer to print the address first, but decades of monitoring tools are > > > watching for the string "SYN flooding on port" so don't break that. > > > > > > Tested with each protcol's "any" address and a host address: > > > > > > Possible SYN flooding on port 9001. IP 0.0.0.0. > > > Possible SYN flooding on port 9001. IP 127.0.0.1. > > > Possible SYN flooding on port 9001. IP ::. > > > Possible SYN flooding on port 9001. IP fc00::1. > > > > > > Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com> > > > --- > > > net/ipv4/tcp_input.c | 16 +++++++++++++--- > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > > > index 0640453fce54b6daae0861d948f3db075830daf6..fb86056732266fedc8ad574bbf799dbdd7a425a3 100644 > > > --- a/net/ipv4/tcp_input.c > > > +++ b/net/ipv4/tcp_input.c > > > @@ -6831,9 +6831,19 @@ static bool tcp_syn_flood_action(const struct sock *sk, const char *proto) > > > __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPREQQFULLDROP); > > > > > > if (!queue->synflood_warned && syncookies != 2 && > > > - xchg(&queue->synflood_warned, 1) == 0) > > > - net_info_ratelimited("%s: Possible SYN flooding on port %d. %s. Check SNMP counters.\n", > > > - proto, sk->sk_num, msg); > > > + xchg(&queue->synflood_warned, 1) == 0) { > > > +#if IS_ENABLED(CONFIG_IPV6) > > > + if (sk->sk_family == AF_INET6) { > > > > Can the IS_ENABLED() go inside the if? You get better build testing > > that way. > > > > Andrew > > Are you sure? Why would the IS_ENABLED() be inside of a condition > which isn't compiled in? If IPv6 isn't compiled in then the condition > would never evaluate as true, so seems pointless a pointless > comparison to make? People not compiling in IPv6 have explicitly asked > *not* to have their kernel filled with a bunch of "if (family == > AF_INET6)" haven't they? > > There are many other examples of this pattern of "IS_ENABLED()" first > and "if (family == AF_INET6)" inside it, but I can't see any of the > inverse which I think you're suggesting, see: > > grep -C1 -ERHn "IS_ENABLED\(CONFIG_IPV6\)" net | grep -C1 "family == AF_INET6" > > Please let me know if I've misunderstood? So what i'm suggesting is if (IS_ENABLED(CONFIG_IPV6) && 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); } The IS_ENABLED(CONFIG_IPV6) will evaluate to 0 at compile time, and the optimiser will throw away the whole lot since it can never be true. However, before the code gets to the optimiser, it first needs to compile. It will check you have the correct number of parameters for the string format, do the types match, do the structure members exist, etc. Anybody doing compile testing of a change, and they have IPV6 turned off, has a chance off getting errors reported when they have actually broken IPV6, but don't know it, because they are not compiling it. Now, IPV6 is one of those big options which i expect 0-day tests quite regularly. Using IF_ENABLED() like this brings more benefit from less used options which gets very little build testing, and so are often broke until somebody like Arnd runs builds with lots of random configs. Andrew
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.
On Thu, Nov 10, 2022 at 3:39 PM 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. Also, it is customary to use IP.port format (like most tools, see tcpdump)
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
Hi Andrew, On Thu, Nov 10, 2022 at 11:42 PM Andrew Lunn <andrew@lunn.ch> wrote: > On Fri, Nov 11, 2022 at 08:20:18AM +1100, Jamie Bainbridge wrote: > > On Fri, 11 Nov 2022 at 00:51, Andrew Lunn <andrew@lunn.ch> wrote: > > > On Thu, Nov 10, 2022 at 09:21:06PM +1100, Jamie Bainbridge wrote: > > > > The SYN flood message prints the listening port number, but on a system > > > > with many processes bound to the same port on different IPs, it's > > > > impossible to tell which socket is the problem. > > > > > > > > Add the listen IP address to the SYN flood message. It might have been > > > > nicer to print the address first, but decades of monitoring tools are > > > > watching for the string "SYN flooding on port" so don't break that. > > > > > > > > Tested with each protcol's "any" address and a host address: > > > > > > > > Possible SYN flooding on port 9001. IP 0.0.0.0. > > > > Possible SYN flooding on port 9001. IP 127.0.0.1. > > > > Possible SYN flooding on port 9001. IP ::. > > > > Possible SYN flooding on port 9001. IP fc00::1. > > > > > > > > Signed-off-by: Jamie Bainbridge <jamie.bainbridge@gmail.com> > > > > --- > > > > net/ipv4/tcp_input.c | 16 +++++++++++++--- > > > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c > > > > index 0640453fce54b6daae0861d948f3db075830daf6..fb86056732266fedc8ad574bbf799dbdd7a425a3 100644 > > > > --- a/net/ipv4/tcp_input.c > > > > +++ b/net/ipv4/tcp_input.c > > > > @@ -6831,9 +6831,19 @@ static bool tcp_syn_flood_action(const struct sock *sk, const char *proto) > > > > __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPREQQFULLDROP); > > > > > > > > if (!queue->synflood_warned && syncookies != 2 && > > > > - xchg(&queue->synflood_warned, 1) == 0) > > > > - net_info_ratelimited("%s: Possible SYN flooding on port %d. %s. Check SNMP counters.\n", > > > > - proto, sk->sk_num, msg); > > > > + xchg(&queue->synflood_warned, 1) == 0) { > > > > +#if IS_ENABLED(CONFIG_IPV6) > > > > + if (sk->sk_family == AF_INET6) { > > > > > > Can the IS_ENABLED() go inside the if? You get better build testing > > > that way. > > > > > > Andrew > > > > Are you sure? Why would the IS_ENABLED() be inside of a condition > > which isn't compiled in? If IPv6 isn't compiled in then the condition > > would never evaluate as true, so seems pointless a pointless > > comparison to make? People not compiling in IPv6 have explicitly asked > > *not* to have their kernel filled with a bunch of "if (family == > > AF_INET6)" haven't they? > > > > There are many other examples of this pattern of "IS_ENABLED()" first > > and "if (family == AF_INET6)" inside it, but I can't see any of the > > inverse which I think you're suggesting, see: > > > > grep -C1 -ERHn "IS_ENABLED\(CONFIG_IPV6\)" net | grep -C1 "family == AF_INET6" > > > > Please let me know if I've misunderstood? > > So what i'm suggesting is > > if (IS_ENABLED(CONFIG_IPV6) && 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); > } Unfortunately the IPv6-specific members are not defined if CONFIG_IPV6=n. Patch sent. https://lore.kernel.org/netdev/d1ecf500f07e063d4e8e34f4045ddca55416c686.1668507036.git.geert+renesas@glider.be Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 0640453fce54b6daae0861d948f3db075830daf6..fb86056732266fedc8ad574bbf799dbdd7a425a3 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -6831,9 +6831,19 @@ static bool tcp_syn_flood_action(const struct sock *sk, const char *proto) __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPREQQFULLDROP); if (!queue->synflood_warned && syncookies != 2 && - xchg(&queue->synflood_warned, 1) == 0) - net_info_ratelimited("%s: Possible SYN flooding on port %d. %s. Check SNMP counters.\n", - proto, sk->sk_num, msg); + 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); + } + } return want_cookie; }