dccp: Bug-Fix - AWL was never updated
authorGerrit Renker <gerrit@erg.abdn.ac.uk>
Sat, 26 Jul 2008 10:59:10 +0000 (11:59 +0100)
committerGerrit Renker <gerrit@erg.abdn.ac.uk>
Sat, 26 Jul 2008 10:59:10 +0000 (11:59 +0100)
The AWL lower Ack validity window advances in proportion to GSS, the greatest
sequence number sent. Updating AWL other than at connection setup (in the
DCCP-Request sent by dccp_v{4,6}_connect()) was missing in the DCCP code.

This bug lead to syslog messages such as

 "kernel: dccp_check_seqno: DCCP: Step 6 failed for DATAACK packet, [...]
  P.ackno exists or LAWL(82947089) <= P.ackno(82948208)
                                   <= S.AWH(82948728), sending SYNC..."

The difference between AWL/AWH here is 1639 packets, while the expected value
(the Sequence Window) would have been 100 (the default).  A closer look showed
that LAWL = AWL = 82947089 equalled the ISS on the Response.

The patch now updates AWL with each increase of GSS.

Further changes:
----------------
The patch also enforces more stringent checks on the ISS sequence number:

 * AWL is initialised to ISS at connection setup and remains at this value;
 * AWH is then always set to GSS (via dccp_update_gss());
 * so on the first Request: AWL =      AWH = ISS,
   and on the n-th Request: AWL = ISS, AWH = ISS + n.

As a consequence, only Response packets that refer to Requests sent by this
host will pass, all others are discarded. This is the intention and in effect
implements the initial adjustments for AWL as specified in RFC 4340, 7.5.1.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Acked-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
net/dccp/output.c

index d19d481..d06945c 100644 (file)
@@ -53,8 +53,11 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
                                          dccp_packet_hdr_len(dcb->dccpd_type);
                int err, set_ack = 1;
                u64 ackno = dp->dccps_gsr;
-
-               dccp_inc_seqno(&dp->dccps_gss);
+               /*
+                * Increment GSS here already in case the option code needs it.
+                * Update GSS for real only if option processing below succeeds.
+                */
+               dcb->dccpd_seq = ADD48(dp->dccps_gss, 1);
 
                switch (dcb->dccpd_type) {
                case DCCP_PKT_DATA:
@@ -66,6 +69,9 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
 
                case DCCP_PKT_REQUEST:
                        set_ack = 0;
+                       /* Use ISS on the first (non-retransmitted) Request. */
+                       if (icsk->icsk_retransmits == 0)
+                               dcb->dccpd_seq = dp->dccps_iss;
                        /* fall through */
 
                case DCCP_PKT_SYNC:
@@ -84,8 +90,6 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
                        break;
                }
 
-               dcb->dccpd_seq = dp->dccps_gss;
-
                if (dccp_insert_options(sk, skb)) {
                        kfree_skb(skb);
                        return -EPROTO;
@@ -103,7 +107,7 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
                /* XXX For now we're using only 48 bits sequence numbers */
                dh->dccph_x     = 1;
 
-               dp->dccps_awh = dp->dccps_gss;
+               dccp_update_gss(sk, dcb->dccpd_seq);
                dccp_hdr_set_seq(dh, dp->dccps_gss);
                if (set_ack)
                        dccp_hdr_set_ack(dccp_hdr_ack_bits(skb), ackno);
@@ -112,6 +116,11 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb)
                case DCCP_PKT_REQUEST:
                        dccp_hdr_request(skb)->dccph_req_service =
                                                        dp->dccps_service;
+                       /*
+                        * Limit Ack window to ISS <= P.ackno <= GSS, so that
+                        * only Responses to Requests we sent are considered.
+                        */
+                       dp->dccps_awl = dp->dccps_iss;
                        break;
                case DCCP_PKT_RESET:
                        dccp_hdr_reset(skb)->dccph_reset_code =
@@ -449,19 +458,7 @@ static inline void dccp_connect_init(struct sock *sk)
 
        dccp_sync_mss(sk, dst_mtu(dst));
 
-       /*
-        * SWL and AWL are initially adjusted so that they are not less than
-        * the initial Sequence Numbers received and sent, respectively:
-        *      SWL := max(GSR + 1 - floor(W/4), ISR),
-        *      AWL := max(GSS - W' + 1, ISS).
-        * These adjustments MUST be applied only at the beginning of the
-        * connection.
-        */
-       dccp_update_gss(sk, dp->dccps_iss);
-       dccp_set_seqno(&dp->dccps_awl, max48(dp->dccps_awl, dp->dccps_iss));
-
-       /* S.GAR - greatest valid acknowledgement number received on a non-Sync;
-        *         initialized to S.ISS (sec. 8.5)                            */
+       /* Initialise GAR as per 8.5; AWL/AWH are set in dccp_transmit_skb() */
        dp->dccps_gar = dp->dccps_iss;
 
        icsk->icsk_retransmits = 0;