Message ID | 20110918194818.GB1641@x4.trippels.de |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
Le dimanche 18 septembre 2011 à 21:48 +0200, Markus Trippelsdorf a écrit : > On 2011.09.18 at 21:46 +0200, Eric Dumazet wrote: > > Le dimanche 18 septembre 2011 à 21:23 +0200, Markus Trippelsdorf a > > écrit : > > > On 2011.09.18 at 11:06 -0700, Linus Torvalds wrote: > > > > 2011/9/17 David Miller <davem@davemloft.net>: > > > > > > > > > > dpward (2): > > > > > net: Make flow cache namespace-aware > > > > > net: Handle different key sizes between address families in flow cache > > > > > > > > > > nhorman (1): > > > > > net: don't clear IFF_XMIT_DST_RELEASE in ether_setup > > > > > > > > > > rajan.aggarwal85@gmail.com (1): > > > > > net/can/af_can.c: Change del_timer to del_timer_sync > > > > > > > > Guys, if somebody has such a broken email setup that they don't even > > > > show their own name, don't take patches from them. > > > > > > > > If you cannot even set up email sanely, there is zero reason to > > > > believe that the patch should be good. And if the patch is trivial and > > > > you want to take it despite the source of the patch being crap, please > > > > spend the five seconds to fix it up. > > > > > > > > Proper names are part of the commit message. Don't make it look like > > > > crap. I get ugly flashbacks to SVN or CVS when I see stuff like this. > > > > Don't do it. > > > > > > Plus commit 946cedccbd73874 breaks the build: > > > > > > LD init/built-in.o > > > LD .tmp_vmlinux1 > > > net/built-in.o:sysctl_net.c:function tcp_v4_conn_request: error: undefined reference to 'cookie_v4_init_sequence' > > > make: *** [.tmp_vmlinux1] Error 1 > > > > > > commit 946cedccbd7387488d2cee5da92cdfeb28d2e670 > > > Author: Eric Dumazet <eric.dumazet@gmail.com> > > > Date: Tue Aug 30 03:21:44 2011 +0000 > > > > > > tcp: Change possible SYN flooding messages > > > > > > "Possible SYN flooding on port xxxx " messages can fill logs on servers. > > > > > > Change logic to log the message only once per listener, and add two new > > > SNMP counters to track : > > > > > > TCPReqQFullDoCookies : number of times a SYNCOOKIE was replied to client > > > > > > TCPReqQFullDrop : number of times a SYN request was dropped because > > > syncookies were not enabled. > > > > > > Based on a prior patch from Tom Herbert, and suggestions from David. > > > > > > > > > > Oh well, trying to remove those ugly #ifdef was not so easy. > > I'll cook a patch, thanks for the report > > The following works for me: > > > diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c > index c34f015..ef9dd55 100644 > --- a/net/ipv4/tcp_ipv4.c > +++ b/net/ipv4/tcp_ipv4.c > @@ -1264,7 +1264,9 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) > * evidently real one. > */ > if (inet_csk_reqsk_queue_is_full(sk) && !isn) { > +#ifdef CONFIG_SYN_COOKIES > want_cookie = tcp_syn_flood_action(sk, skb, "TCP"); > +#endif > if (!want_cookie) > goto drop; > } > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c > index 3c9fa61..7ffc3b1 100644 > --- a/net/ipv6/tcp_ipv6.c > +++ b/net/ipv6/tcp_ipv6.c > @@ -1174,7 +1174,9 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) > goto drop; > > if (inet_csk_reqsk_queue_is_full(sk) && !isn) { > +#ifdef CONFIG_SYN_COOKIES > want_cookie = tcp_syn_flood_action(sk, skb, "TCPv6"); > +#endif > if (!want_cookie) > goto drop; > } > Dont do that, we _really_ want to call tcp_syn_flood_action() -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 18, 2011 at 12:48 PM, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > > The following works for me: No it doesn't. It may *compile* for you, but it doesn't work for you. It avoids all the other stuff that tcp_syn_flood_action() also does (notably the printout). The real fix looks to be either: - make an empty (inline/macro) cookie_v4_init_sequence() for the non-syncookie config case OR - change tcp_syn_flood_action() to have an inline wrapper that always returns 0 for the non-syncookie config case so that the compiler can see statically that when syncookies are disabled, it will always return zero. Or something like that. Tssk. David, linux-next may not be fully operational, but by -rc6 you shouldn't have sent me stuff like this that was even *remotely* complex anyway. Stop it. Linus -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index c34f015..ef9dd55 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1264,7 +1264,9 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb) * evidently real one. */ if (inet_csk_reqsk_queue_is_full(sk) && !isn) { +#ifdef CONFIG_SYN_COOKIES want_cookie = tcp_syn_flood_action(sk, skb, "TCP"); +#endif if (!want_cookie) goto drop; } diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 3c9fa61..7ffc3b1 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1174,7 +1174,9 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb) goto drop; if (inet_csk_reqsk_queue_is_full(sk) && !isn) { +#ifdef CONFIG_SYN_COOKIES want_cookie = tcp_syn_flood_action(sk, skb, "TCPv6"); +#endif if (!want_cookie) goto drop; }