dccp: ensure gap does not become unsigned -1
Subsystems (2)
PATCH 1/1 dccp: ensure gap does not become unsigned -1
Is this maybe necessary? ------------------------------>8-------------8<--------------------------------- if packets is 0, becomes -1, but since gap is unsigned, the test 'gap > 0' does not work.
---
net/dccp/ackvec.c +5 -1
@@ -207,7 +207,11 @@ static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av,if (av->av_vec_len + packets >DCCP_MAX_ACKVEC_LEN )return -ENOBUFS ; - gap = packets -1 ; +if (packets >0 ) + gap = packets -1 ; +else + gap =0 ; + new_head = av->av_buf_head - packets;if (new_head <0 ) {
[added missing copy to netdev@vger]
Quoted 5 lines
Is this maybe necessary? ------------------------------>8-------------8<--------------------------------- if packets is 0, becomes -1, but since gap is unsigned, the test 'gap > 0' does not work.
Thank you for looking through. It is not necessary, see below.
net/dccp/ackvec.c static inline int · line 211
Quoted 9 lines
net/dccp/ackvec.c +1
-1@@ -207,7 +207,11 @@ static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, if (av->av_vec_len + packets > DCCP_MAX_ACKVEC_LEN) return -ENOBUFS; - gap = packets -1 ; +if (packets >0 )
Why 'gap < 0' <=> packets < 0 can not happen:
/*
* If several packets are missing, the HC-Receiver may prefer to enter
multiple
* bytes with run length 0, rather than a single byte with a larger run
length;
* this simplifies table updates if one of the missing packets arrives. */
static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av,
const unsigned int packets, const unsigned char state)
This is a static routine which is called in one place only in net/dccp/ackvec.c, in dccp_ackvec_add():
//...
} else if (after48(ackno, av->av_buf_ackno)) {
const u64 delta = dccp_delta_seqno(av->av_buf_ackno, ackno);
/*
* Look if the state of this packet is the same as the
* previous ackno and if so if we can bump the head len.
*/
if (delta = 1 &&
dccp_ackvec_state(av, av->av_buf_head) = state &&
dccp_ackvec_len(av, av->av_buf_head) <
DCCP_ACKVEC_LEN_MASK)
av->av_buf[av->av_buf_head]++;
else if (dccp_ackvec_set_buf_head_state(av, delta, state))
return -ENOBUFS;=> We thus have:
1) after48(a, b) => sequence number a' is after' b
2) sequence number a' is after' `b' => dccp_delta_seqno(b, a) > 0
3) hence delta > 0
4) hence delta - 1 = gap >= 0
5) The subsequent `else' in dccp_ackvec_add() catches the case of !after48(a, b), note the reversed order of arguments to dccp_delta_seqno, so that a positive value is returned:
} else {
/*
* A.1.2. Old Packets
*
* When a packet with Sequence Number S <= buf_ackno
* arrives, the HC-Receiver will scan the table for
* the byte corresponding to S. (Indexing structures
* could reduce the complexity of this scan.)
*/
u64 delta = dccp_delta_seqno(ackno, av->av_buf_ackno);
=> But this is in an entirely different block of code outside the
one to which this patch was meant.
Hence it is not necessary.