Date: Sun, 06 Nov 2022 12:19:18 -0800 From: Ravi Pokala <rpokala@freebsd.org> To: Richard Scheffenegger <rscheff@FreeBSD.org>, <src-committers@FreeBSD.org>, <dev-commits-src-all@FreeBSD.org>, <dev-commits-src-main@FreeBSD.org> Subject: Re: b1258b76435a - main - tcp: add conservative d.cep accounting algorithm Message-ID: <5D60182C-42AF-428F-80A2-1E22438D8D1D@panasas.com> In-Reply-To: <202211061110.2A6BAqJL010414@gitrepo.freebsd.org> References: <202211061110.2A6BAqJL010414@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@)
=EF=BB=BF-----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-c=
ommits-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=3Db1258b76435ac370ddd0f814a3=
51779ddb267f6f
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, in=
t pkts, int iptos)
{
- int delta_ace =3D 0;
+ int delta_cep =3D 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 t=
hflags, int iptos)
if ((iptos & IPTOS_ECN_MASK) =3D=3D IPTOS_ECN_CE)
tp->t_rcep +=3D 1;
if (tp->t_flags2 & TF2_ECN_PERMIT) {
- delta_ace =3D (tcp_ecn_get_ace(thflags) + 8 -
- (tp->t_scep & 0x07)) & 0x07;
- tp->t_scep +=3D delta_ace;
+ delta_cep =3D (tcp_ecn_get_ace(thflags) + 8 -
+ (tp->t_scep & 7)) & 7;
+ if (delta_cep < pkts)
+ delta_cep =3D pkts -
+ ((pkts - delta_cep) & 7);
+ tp->t_scep +=3D delta_cep;
} else {
/*
* process the final ACK of the 3WHS
@@ -326,7 +329,7 @@ tcp_ecn_input_segment(struct tcpcb *tp, uint16_t th=
flags, int iptos)
} else {
/* RFC3168 ECN handling */
if ((thflags & (TH_SYN | TH_ECE)) =3D=3D TH_ECE) {
- delta_ace =3D 1;
+ delta_cep =3D 1;
tp->t_scep++;
}
if (thflags & TH_CWR) {
@@ -341,7 +344,7 @@ tcp_ecn_input_segment(struct tcpcb *tp, uint16_t th=
flags, 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/rac=
k.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 =3D 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_outpu=
t_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) !=3D 0) ? 1 : 0));
+}
/*
* Flags for the t_oobflags field.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5D60182C-42AF-428F-80A2-1E22438D8D1D>