tcp: fix syncookie regression
authorEric Dumazet <eric.dumazet@gmail.com>
Sat, 10 Mar 2012 09:20:21 +0000 (09:20 +0000)
committerDavid S. Miller <davem@davemloft.net>
Sun, 11 Mar 2012 22:52:12 +0000 (15:52 -0700)
commit ea4fc0d619 (ipv4: Don't use rt->rt_{src,dst} in ip_queue_xmit())
added a serious regression on synflood handling.

Simon Kirby discovered a successful connection was delayed by 20 seconds
before being responsive.

In my tests, I discovered that xmit frames were lost, and needed ~4
retransmits and a socket dst rebuild before being really sent.

In case of syncookie initiated connection, we use a different path to
initialize the socket dst, and inet->cork.fl.u.ip4 is left cleared.

As ip_queue_xmit() now depends on inet flow being setup, fix this by
copying the temp flowi4 we use in cookie_v4_check().

Reported-by: Simon Kirby <sim@netnation.com>
Bisected-by: Simon Kirby <sim@netnation.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Tested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv4/syncookies.c
net/ipv4/tcp_ipv4.c

index 51fdbb4..eab2a7f 100644 (file)
@@ -278,6 +278,7 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
        struct rtable *rt;
        __u8 rcv_wscale;
        bool ecn_ok = false;
+       struct flowi4 fl4;
 
        if (!sysctl_tcp_syncookies || !th->ack || th->rst)
                goto out;
@@ -346,20 +347,16 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
         * hasn't changed since we received the original syn, but I see
         * no easy way to do this.
         */
-       {
-               struct flowi4 fl4;
-
-               flowi4_init_output(&fl4, 0, sk->sk_mark, RT_CONN_FLAGS(sk),
-                                  RT_SCOPE_UNIVERSE, IPPROTO_TCP,
-                                  inet_sk_flowi_flags(sk),
-                                  (opt && opt->srr) ? opt->faddr : ireq->rmt_addr,
-                                  ireq->loc_addr, th->source, th->dest);
-               security_req_classify_flow(req, flowi4_to_flowi(&fl4));
-               rt = ip_route_output_key(sock_net(sk), &fl4);
-               if (IS_ERR(rt)) {
-                       reqsk_free(req);
-                       goto out;
-               }
+       flowi4_init_output(&fl4, 0, sk->sk_mark, RT_CONN_FLAGS(sk),
+                          RT_SCOPE_UNIVERSE, IPPROTO_TCP,
+                          inet_sk_flowi_flags(sk),
+                          (opt && opt->srr) ? opt->faddr : ireq->rmt_addr,
+                          ireq->loc_addr, th->source, th->dest);
+       security_req_classify_flow(req, flowi4_to_flowi(&fl4));
+       rt = ip_route_output_key(sock_net(sk), &fl4);
+       if (IS_ERR(rt)) {
+               reqsk_free(req);
+               goto out;
        }
 
        /* Try to redo what tcp_v4_send_synack did. */
@@ -373,5 +370,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb,
        ireq->rcv_wscale  = rcv_wscale;
 
        ret = get_cookie_sock(sk, skb, req, &rt->dst);
+       /* ip_queue_xmit() depends on our flow being setup
+        * Normal sockets get it right from inet_csk_route_child_sock()
+        */
+       if (ret)
+               inet_sk(ret)->cork.fl.u.ip4 = fl4;
 out:   return ret;
 }
index 94d683a..fd54c5f 100644 (file)
@@ -1466,9 +1466,13 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
                inet_csk(newsk)->icsk_ext_hdr_len = inet_opt->opt.optlen;
        newinet->inet_id = newtp->write_seq ^ jiffies;
 
-       if (!dst && (dst = inet_csk_route_child_sock(sk, newsk, req)) == NULL)
-               goto put_and_exit;
-
+       if (!dst) {
+               dst = inet_csk_route_child_sock(sk, newsk, req);
+               if (!dst)
+                       goto put_and_exit;
+       } else {
+               /* syncookie case : see end of cookie_v4_check() */
+       }
        sk_setup_caps(newsk, dst);
 
        tcp_mtup_init(newsk);