Skip to content

Commit 202c265

Browse files
committed
Apply the review findings to the revoke path.
Only a created record is revoked, so the insert contract no longer calls the failed SYN-ACK an accepted trade-off; guard the backward shift against copying a slot onto itself and guard the 1 Hz delta the same way the flow-map block does.
1 parent a7f0ebf commit 202c265

7 files changed

Lines changed: 23 additions & 15 deletions

File tree

‎freebsd/netinet/tcp_syncache.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1899,11 +1899,11 @@ syncache_add(struct in_conninfo *inc, struct tcpopt *to, struct tcphdr *th,
18991899
TCPSTAT_INC(tcps_sndacks);
19001900
TCPSTAT_INC(tcps_sndtotal);
19011901
} else {
1902-
/* A1 trade-off: the four-tuple was recorded by the admission, so a
1903-
* SYN-ACK that never went out leaves a "this generation" record for
1904-
* a connection that was not established. The impact is bounded: the
1905-
* client only retransmits the pure SYN, which the dispatcher keeps
1906-
* locally anyway. */
1902+
/* The admission above recorded the four-tuple, so undo it here:
1903+
* a leftover record would claim the client\'s retransmissions for
1904+
* a connection this generation cannot complete, and the draining
1905+
* generation\'s own half-open entry would never see them. A
1906+
* duplicate four-tuple belongs to an earlier SYN and stays. */
19071907
V_syncache_synack_fail++;
19081908
if (created)
19091909
syncache_flow_map_revoke(sc);

‎lib/ff_dpdk_if.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3522,8 +3522,8 @@ ff_reload_plane_housekeeping(uint64_t now_tsc)
35223522
uint64_t synack_fail = 0, ack_mismatch = 0, inserted = 0, dup = 0;
35233523

35243524
ff_syncache_counters(&synack_fail, &ack_mismatch);
3525-
if (synack_fail > last_synack_fail
3526-
|| ack_mismatch > last_ack_mismatch) {
3525+
if (synack_fail != last_synack_fail
3526+
|| ack_mismatch != last_ack_mismatch) {
35273527
ff_flow_map_stats2(&inserted, &dup, NULL, NULL, NULL, NULL, NULL,
35283528
NULL);
35293529
ff_log(FF_LOG_WARNING, FF_LOGTYPE_FSTACK_LIB,

‎lib/ff_flow_map.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,12 @@ ff_flow_map_revoke(const struct ff_flow_key *key)
205205
d_home = (home - hole) & mask;
206206
if (d_home >= 1 && d_home <= ((j - hole) & mask))
207207
continue;
208-
memcpy(&t[hole].key, &t[j].key, sizeof(t[j].key));
209-
t[hole].hash = t[j].hash;
210-
t[hole].state = FF_FLOW_SLOT_USED;
211-
hole = j;
208+
if (hole != j) {
209+
memcpy(&t[hole].key, &t[j].key, sizeof(t[j].key));
210+
t[hole].hash = t[j].hash;
211+
t[hole].state = FF_FLOW_SLOT_USED;
212+
hole = j;
213+
}
212214
}
213215
t[hole].state = FF_FLOW_SLOT_EMPTY;
214216
g_revoked++;

‎lib/ff_flow_map.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,9 @@ int ff_flow_map_revoke(const struct ff_flow_key *key);
8888
* never takes any stack lock (it runs under NET_EPOCH with the inp/syncache
8989
* locks already dropped).
9090
* Returns 0 recorded, 1 already present, <0 on error / table full.
91-
* The entry is visible to ff_flow_map_lookup() right away: a SYN-ACK that
92-
* fails to go out therefore leaves a record for a connection that was never
93-
* established, which is the accepted trade-off of the single-phase form. */
91+
* The entry is visible to ff_flow_map_lookup() right away. A SYN-ACK that
92+
* fails to go out is undone by the caller through ff_flow_map_revoke(),
93+
* which is why admit() also reports whether it created the record. */
9494
int ff_flow_map_insert(const struct ff_flow_key *key);
9595

9696
/* Counters for ff_top / drain observability; any output pointer may be NULL. */

‎tests/integration/common/orphan_capture_analyze.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ def packets(path):
5555
ts_sec, ts_frac, caplen, _origlen = struct.unpack(
5656
endian + "IIII", blob[off:off + 16])
5757
off += 16
58+
if (caplen == 0 or off + caplen > n):
59+
break
5860
data = blob[off:off + caplen]
5961
off += caplen
6062
yield linktype, ts_sec + (ts_frac / 1e9 if nano else ts_frac / 1e6), data
@@ -117,7 +119,7 @@ def main(argv):
117119
continue
118120
cur = live.get(cport)
119121
if from_client and flags & SYN and not flags & ACK:
120-
if cur is not None and cur.isn == seq and not cur.synack:
122+
if cur is not None and cur.isn == seq:
121123
cur.retrans += 1 # the SYN was retried
122124
continue
123125
if cur is not None:

‎tests/integration/test_ff_dpdk_if_integration.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,8 @@ int ff_socket(int d, int t, int p) { (void)d;(void)t;(void)p; return -1; }
151151
int ff_socket_snd_pending(void) { return 0; }
152152
int ff_socket_drain_count(void) { return 0; }
153153
int ff_syncache_count(void) { return 0; }
154+
void ff_syncache_counters(uint64_t *a, uint64_t *b)
155+
{ if (a) *a = 0; if (b) *b = 0; }
154156
int ff_ioctl_freebsd(int f, unsigned long r, ...) { (void)f;(void)r; return -1; }
155157
int ff_close(int f) { (void)f; return 0; }
156158
int ff_rtioctl(int f, void *d, unsigned int *l, unsigned int al)

‎tests/integration/test_ff_gendir_mp_integration.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ int ff_socket(int d, int t, int p) { (void)d;(void)t;(void)p; return -1; }
194194
int ff_socket_snd_pending(void) { return 0; }
195195
int ff_socket_drain_count(void) { return 0; }
196196
int ff_syncache_count(void) { return 0; }
197+
void ff_syncache_counters(uint64_t *a, uint64_t *b)
198+
{ if (a) *a = 0; if (b) *b = 0; }
197199
int ff_ioctl_freebsd(int f, unsigned long r, ...) { (void)f;(void)r; return -1; }
198200
int ff_close(int f) { (void)f; return 0; }
199201
int ff_rtioctl(int f, void *d, unsigned int *l, unsigned int al)

0 commit comments

Comments
 (0)