Commit d2c72630 authored by Gerrit Renker's avatar Gerrit Renker

dccp ccid-3: remove buggy RTT-sampling history lookup

This removes the RTT-sampling function tfrc_tx_hist_rtt(), since

 1. it suffered from complex passing of return values (the return value both
    indicated successful lookup while the value doubled as RTT sample);

 2. when for some odd reason the sample value equalled 0, this triggered a bug
    warning about "bogus Ack", due to the ambiguity of the return value;

 3. on a passive host which has not sent anything the TX history is empty and
    thus will lead to unwanted "bogus Ack" warnings such as
    ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-28197148
    ccid3_hc_tx_packet_recv: server(e7b7d518): DATAACK with bogus ACK-26641606.

The fix is to replace the implicit encoding by performing the steps manually.

Furthermore, the "bogus Ack" warning has been removed, since it can actually be
triggered due to several reasons (network reordering, old packet, (3) above),
hence it is not very useful.
Signed-off-by: default avatarGerrit Renker <gerrit@erg.abdn.ac.uk>
parent 20cbd3e1
...@@ -370,6 +370,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) ...@@ -370,6 +370,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
{ {
struct ccid3_hc_tx_sock *hc = ccid3_hc_tx_sk(sk); struct ccid3_hc_tx_sock *hc = ccid3_hc_tx_sk(sk);
struct ccid3_options_received *opt_recv = &hc->tx_options_received; struct ccid3_options_received *opt_recv = &hc->tx_options_received;
struct tfrc_tx_hist_entry *acked;
ktime_t now; ktime_t now;
unsigned long t_nfb; unsigned long t_nfb;
u32 pinv, r_sample; u32 pinv, r_sample;
...@@ -383,17 +384,24 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) ...@@ -383,17 +384,24 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
hc->tx_state != TFRC_SSTATE_NO_FBACK) hc->tx_state != TFRC_SSTATE_NO_FBACK)
return; return;
now = ktime_get_real(); /*
* Locate the acknowledged packet in the TX history.
/* Estimate RTT from history if ACK number is valid */ *
r_sample = tfrc_tx_hist_rtt(hc->tx_hist, * Returning "entry not found" here can for instance happen when
DCCP_SKB_CB(skb)->dccpd_ack_seq, now); * - the host has not sent out anything (e.g. a passive server),
if (r_sample == 0) { * - the Ack is outdated (packet with higher Ack number was received),
DCCP_WARN("%s(%p): %s with bogus ACK-%llu\n", dccp_role(sk), sk, * - it is a bogus Ack (for a packet not sent on this connection).
dccp_packet_name(DCCP_SKB_CB(skb)->dccpd_type), */
(unsigned long long)DCCP_SKB_CB(skb)->dccpd_ack_seq); acked = tfrc_tx_hist_find_entry(hc->tx_hist, dccp_hdr_ack_seq(skb));
if (acked == NULL)
return; return;
} /* For the sake of RTT sampling, ignore/remove all older entries */
tfrc_tx_hist_purge(&acked->next);
/* Update the moving average for the RTT estimate (RFC 3448, 4.3) */
now = ktime_get_real();
r_sample = dccp_sample_rtt(sk, ktime_us_delta(now, acked->stamp));
hc->tx_rtt = tfrc_ewma(hc->tx_rtt, r_sample, 9);
/* Update receive rate in units of 64 * bytes/second */ /* Update receive rate in units of 64 * bytes/second */
hc->tx_x_recv = opt_recv->ccid3or_receive_rate; hc->tx_x_recv = opt_recv->ccid3or_receive_rate;
...@@ -405,11 +413,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) ...@@ -405,11 +413,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
hc->tx_p = 0; hc->tx_p = 0;
else /* can not exceed 100% */ else /* can not exceed 100% */
hc->tx_p = scaled_div(1, pinv); hc->tx_p = scaled_div(1, pinv);
/*
* Validate new RTT sample and update moving average
*/
r_sample = dccp_sample_rtt(sk, r_sample);
hc->tx_rtt = tfrc_ewma(hc->tx_rtt, r_sample, 9);
/* /*
* Update allowed sending rate X as per draft rfc3448bis-00, 4.2/3 * Update allowed sending rate X as per draft rfc3448bis-00, 4.2/3
*/ */
......
...@@ -38,18 +38,6 @@ ...@@ -38,18 +38,6 @@
#include "packet_history.h" #include "packet_history.h"
#include "../../dccp.h" #include "../../dccp.h"
/**
* tfrc_tx_hist_entry - Simple singly-linked TX history list
* @next: next oldest entry (LIFO order)
* @seqno: sequence number of this entry
* @stamp: send time of packet with sequence number @seqno
*/
struct tfrc_tx_hist_entry {
struct tfrc_tx_hist_entry *next;
u64 seqno;
ktime_t stamp;
};
/* /*
* Transmitter History Routines * Transmitter History Routines
*/ */
...@@ -71,15 +59,6 @@ void tfrc_tx_packet_history_exit(void) ...@@ -71,15 +59,6 @@ void tfrc_tx_packet_history_exit(void)
} }
} }
static struct tfrc_tx_hist_entry *
tfrc_tx_hist_find_entry(struct tfrc_tx_hist_entry *head, u64 seqno)
{
while (head != NULL && head->seqno != seqno)
head = head->next;
return head;
}
int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno) int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno)
{ {
struct tfrc_tx_hist_entry *entry = kmem_cache_alloc(tfrc_tx_hist_slab, gfp_any()); struct tfrc_tx_hist_entry *entry = kmem_cache_alloc(tfrc_tx_hist_slab, gfp_any());
...@@ -107,24 +86,6 @@ void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp) ...@@ -107,24 +86,6 @@ void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp)
*headp = NULL; *headp = NULL;
} }
u32 tfrc_tx_hist_rtt(struct tfrc_tx_hist_entry *head, const u64 seqno,
const ktime_t now)
{
u32 rtt = 0;
struct tfrc_tx_hist_entry *packet = tfrc_tx_hist_find_entry(head, seqno);
if (packet != NULL) {
rtt = ktime_us_delta(now, packet->stamp);
/*
* Garbage-collect older (irrelevant) entries:
*/
tfrc_tx_hist_purge(&packet->next);
}
return rtt;
}
/* /*
* Receiver History Routines * Receiver History Routines
*/ */
......
...@@ -40,12 +40,28 @@ ...@@ -40,12 +40,28 @@
#include <linux/slab.h> #include <linux/slab.h>
#include "tfrc.h" #include "tfrc.h"
struct tfrc_tx_hist_entry; /**
* tfrc_tx_hist_entry - Simple singly-linked TX history list
* @next: next oldest entry (LIFO order)
* @seqno: sequence number of this entry
* @stamp: send time of packet with sequence number @seqno
*/
struct tfrc_tx_hist_entry {
struct tfrc_tx_hist_entry *next;
u64 seqno;
ktime_t stamp;
};
static inline struct tfrc_tx_hist_entry *
tfrc_tx_hist_find_entry(struct tfrc_tx_hist_entry *head, u64 seqno)
{
while (head != NULL && head->seqno != seqno)
head = head->next;
return head;
}
extern int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno); extern int tfrc_tx_hist_add(struct tfrc_tx_hist_entry **headp, u64 seqno);
extern void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp); extern void tfrc_tx_hist_purge(struct tfrc_tx_hist_entry **headp);
extern u32 tfrc_tx_hist_rtt(struct tfrc_tx_hist_entry *head,
const u64 seqno, const ktime_t now);
/* Subtraction a-b modulo-16, respects circular wrap-around */ /* Subtraction a-b modulo-16, respects circular wrap-around */
#define SUB16(a, b) (((a) + 16 - (b)) & 0xF) #define SUB16(a, b) (((a) + 16 - (b)) & 0xF)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment