Re: b1258b76435a - main - tcp: add conservative d.cep accounting algorithm
- In reply to: Richard Scheffenegger : "git: b1258b76435a - main - tcp: add conservative d.cep accounting algorithm"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sun, 06 Nov 2022 20:19:18 UTC
Hi Richard,
This change adds 'int tlen' and 'int pkts' as an arguments to tcp_ecn_input_segment(). However, while the function uses 'pkts', it does not use 'tlen'. Is it supposed to? Was the argument added now for planned use in the future? If it is not currently being use, it should be marked '__unused', right?
Thanks,
Ravi (rpokala@)
-----Original Message-----
From: <owner-src-committers@freebsd.org> on behalf of Richard Scheffenegger <rscheff@FreeBSD.org>
Date: 2022-11-06, Sunday at 03:10
To: <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org>
Subject: git: b1258b76435a - main - tcp: add conservative d.cep accounting algorithm
The branch main has been updated by rscheff:
URL: https://cgit.FreeBSD.org/src/commit/?id=b1258b76435ac370ddd0f814a351779ddb267f6f
commit b1258b76435ac370ddd0f814a351779ddb267f6f
Author: Richard Scheffenegger <rscheff@FreeBSD.org>
AuthorDate: 2022-11-06 10:59:55 +0000
Commit: Richard Scheffenegger <rscheff@FreeBSD.org>
CommitDate: 2022-11-06 11:05:22 +0000
tcp: add conservative d.cep accounting algorithm
Accurate ECN asks to conservatively estimate, when the
ACE counter may have wrapped due to a single ACK covering a larger
number of segments. This is described in Annex A.2 of the
accurate-ecn draft.
Event: IETF 115 Hackathon
Reviewed By: tuexen, #transport
Sponsored by: NetApp, Inc.
Differential Revision: https://reviews.freebsd.org/D37281
---
sys/netinet/tcp_ecn.c | 17 ++++++++++-------
sys/netinet/tcp_ecn.h | 2 +-
sys/netinet/tcp_input.c | 4 +++-
sys/netinet/tcp_stacks/rack.c | 10 +++++++---
sys/netinet/tcp_var.h | 6 ++++++
5 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/sys/netinet/tcp_ecn.c b/sys/netinet/tcp_ecn.c
index 1d693944ac40..40791172e55f 100644
--- a/sys/netinet/tcp_ecn.c
+++ b/sys/netinet/tcp_ecn.c
@@ -271,9 +271,9 @@ tcp_ecn_input_parallel_syn(struct tcpcb *tp, uint16_t thflags, int iptos)
* TCP ECN processing.
*/
int
-tcp_ecn_input_segment(struct tcpcb *tp, uint16_t thflags, int iptos)
+tcp_ecn_input_segment(struct tcpcb *tp, uint16_t thflags, int tlen, int pkts, int iptos)
{
- int delta_ace = 0;
+ int delta_cep = 0;
if (tp->t_flags2 & (TF2_ECN_PERMIT | TF2_ACE_PERMIT)) {
switch (iptos & IPTOS_ECN_MASK) {
@@ -292,9 +292,12 @@ tcp_ecn_input_segment(struct tcpcb *tp, uint16_t thflags, int iptos)
if ((iptos & IPTOS_ECN_MASK) == IPTOS_ECN_CE)
tp->t_rcep += 1;
if (tp->t_flags2 & TF2_ECN_PERMIT) {
- delta_ace = (tcp_ecn_get_ace(thflags) + 8 -
- (tp->t_scep & 0x07)) & 0x07;
- tp->t_scep += delta_ace;
+ delta_cep = (tcp_ecn_get_ace(thflags) + 8 -
+ (tp->t_scep & 7)) & 7;
+ if (delta_cep < pkts)
+ delta_cep = pkts -
+ ((pkts - delta_cep) & 7);
+ tp->t_scep += delta_cep;
} else {
/*
* process the final ACK of the 3WHS
@@ -326,7 +329,7 @@ tcp_ecn_input_segment(struct tcpcb *tp, uint16_t thflags, int iptos)
} else {
/* RFC3168 ECN handling */
if ((thflags & (TH_SYN | TH_ECE)) == TH_ECE) {
- delta_ace = 1;
+ delta_cep = 1;
tp->t_scep++;
}
if (thflags & TH_CWR) {
@@ -341,7 +344,7 @@ tcp_ecn_input_segment(struct tcpcb *tp, uint16_t thflags, int iptos)
cc_ecnpkt_handler_flags(tp, thflags, iptos);
}
- return delta_ace;
+ return delta_cep;
}
/*
diff --git a/sys/netinet/tcp_ecn.h b/sys/netinet/tcp_ecn.h
index deade12b75d1..3cc7715b9562 100644
--- a/sys/netinet/tcp_ecn.h
+++ b/sys/netinet/tcp_ecn.h
@@ -43,7 +43,7 @@
void tcp_ecn_input_syn_sent(struct tcpcb *, uint16_t, int);
void tcp_ecn_input_parallel_syn(struct tcpcb *, uint16_t, int);
-int tcp_ecn_input_segment(struct tcpcb *, uint16_t, int);
+int tcp_ecn_input_segment(struct tcpcb *, uint16_t, int, int, int);
uint16_t tcp_ecn_output_syn_sent(struct tcpcb *);
int tcp_ecn_output_established(struct tcpcb *, uint16_t *, int, bool);
void tcp_ecn_syncache_socket(struct tcpcb *, struct syncache *);
diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c
index 84574aaa00ae..a1787a0f93db 100644
--- a/sys/netinet/tcp_input.c
+++ b/sys/netinet/tcp_input.c
@@ -1627,7 +1627,9 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so,
/*
* TCP ECN processing.
*/
- if (tcp_ecn_input_segment(tp, thflags, iptos))
+ if (tcp_ecn_input_segment(tp, thflags, tlen,
+ tcp_packets_this_ack(tp, th->th_ack),
+ iptos))
cc_cong_signal(tp, th, CC_ECN);
/*
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index fdac23d0c5cc..e99f104bfa29 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -13528,8 +13528,10 @@ rack_do_compressed_ack_processing(struct tcpcb *tp, struct socket *so, struct mb
rack_cc_after_idle(rack, tp);
}
tp->t_rcvtime = ticks;
- /* Now what about ECN? */
- if (tcp_ecn_input_segment(tp, ae->flags, ae->codepoint))
+ /* Now what about ECN of a chain of pure ACKs? */
+ if (tcp_ecn_input_segment(tp, ae->flags, 0,
+ tcp_packets_this_ack(tp, ae->ack),
+ ae->codepoint))
rack_cong_signal(tp, CC_ECN, ae->ack, __LINE__);
#ifdef TCP_ACCOUNTING
/* Count for the specific type of ack in */
@@ -14320,7 +14322,9 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so,
* TCP ECN processing. XXXJTL: If we ever use ECN, we need to move
* this to occur after we've validated the segment.
*/
- if (tcp_ecn_input_segment(tp, thflags, iptos))
+ if (tcp_ecn_input_segment(tp, thflags, tlen,
+ tcp_packets_this_ack(tp, th->th_ack),
+ iptos))
rack_cong_signal(tp, CC_ECN, th->th_ack, __LINE__);
/*
diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h
index d115a18d66d5..01deeaad58cf 100644
--- a/sys/netinet/tcp_var.h
+++ b/sys/netinet/tcp_var.h
@@ -551,6 +551,12 @@ tcp_unlock_or_drop(struct tcpcb *tp, int tcp_output_retval)
#endif
#define BYTES_THIS_ACK(tp, th) (th->th_ack - tp->snd_una)
+static int inline
+tcp_packets_this_ack(struct tcpcb *tp, tcp_seq ack)
+{
+ return ((ack - tp->snd_una) / tp->t_maxseg +
+ ((((ack - tp->snd_una) % tp->t_maxseg) != 0) ? 1 : 0));
+}
/*
* Flags for the t_oobflags field.