tcp_buffer_tuner: remove tracing for cookie v[46] check

include/bpftune/libbpftune.h

@@ -352,6 +352,7 @@ int bpftune_sysctl_read(int netns_fd, const char *name, long *values);
 int bpftune_sysctl_write(int netns_fd, const char *name, __u8 num_values, long *values);
 long long bpftune_ksym_addr(char type, const char *name);
 int bpftune_snmpstat_read(unsigned long netns_cookie, int family, const char *name, long *value);
+int bpftune_netstat_read(unsigned long netns_cookie, int family, const char *name, long *value);
 int bpftune_sched_wait_run_percent_read(void);
 bool bpftune_netns_cookie_supported(void);
 int bpftune_netns_set(int fd, int *orig_fd, bool quiet);

src/libbpftune.c

@@ -1134,24 +1134,13 @@ long long bpftune_ksym_addr(char type, const char *name)
 	return ret;
 }
 
-int bpftune_snmpstat_read(unsigned long netns_cookie, int family,
-			  const char *name, long *value)
+static int bpftune_nstat_read(unsigned long netns_cookie, int family,
+			      const char *file, const char *name, long *value)
 {
 	int err, netns_fd = 0, orig_netns_fd = 0, stat_index = 0;
-	const char *file;
 	char line[1024];
 	FILE *fp = NULL;
 
-	switch (family) {
-	case AF_INET:
-		file = "/proc/net/snmp";
-		break;
-	case AF_INET6:
-		file = "/proc/net/snmp6";
-		break;
-	default:
-		return -EINVAL;
-	}
 	err = bpftune_cap_add();
 	if (err)
 		return err;
@@ -1217,6 +1206,22 @@ int bpftune_snmpstat_read(unsigned long netns_cookie, int family,
 	return err;
 }
 
+int bpftune_snmpstat_read(unsigned long netns_cookie, int family,
+                          const char *name, long *value)
+{
+	return bpftune_nstat_read(netns_cookie, family,
+				  family == AF_INET ? "/proc/net/snmp" :
+				 		      "/proc/net/snmp6",
+				  name, value);
+}
+
+int bpftune_netstat_read(unsigned long netns_cookie, int family,
+			 const char *name, long *value)
+{
+	return bpftune_nstat_read(netns_cookie, family, "/proc/net/netstat",
+				  name, value);
+}
+
 /* return % of overall wait/run time on all cpus gathered from
  * /proc/schedstat ; see https://docs.kernel.org/scheduler/sched-stats.html
  * Usually > 100%.

src/libbpftune.map

@@ -56,6 +56,7 @@ LIBBPFTUNE_0.1.1 {
 		bpftune_sysctl_read;
 		bpftune_sysctl_write;
 		bpftune_ksym_addr;
+		bpftune_netstat_read;
 		bpftune_snmpstat_read;
 		bpftune_sched_wait_run_percent_read;
 		bpftune_netns_init_all;

src/tcp_buffer_tuner.bpf.c

@@ -298,10 +298,6 @@ BPF_FENTRY(tcp_init_sock, struct sock *sk)
 
 __u64 tcp_syn_flood_count;
 __u64 tcp_established_count;
-__u64 tcp_bad_syncookies;
-__u64 tcp_good_syncookies;
-
-long tcp_syncookies;
 
 struct bpftune_sample syn_flood_action_sample = { };
 
@@ -326,9 +322,6 @@ BPF_FENTRY(tcp_syn_flood_action, struct sock *sk, const char *proto)
 	if (state != TCP_LISTEN)
 		return 0;
 
-	if (tcp_syncookies > 0)
-		return 0;
-
 	syn_backlog[0] = BPFTUNE_CORE_READ(net, ipv4.sysctl_max_syn_backlog);
 
 	syn_backlog_new[0] = BPFTUNE_GROW_BY_DELTA(syn_backlog[0]);
@@ -348,31 +341,3 @@ BPF_FENTRY(tcp_init_transfer, struct sock *sk, int bpf_op)
 		tcp_established_count++;
 	return 0;
 }
-
-#ifdef BPFTUNE_LEGACY
-SEC("kretprobe/__cookie_v4_check")
-int BPF_KRETPROBE(bpftune__cookie_v4_check, int ret)
-#else
-SEC("fexit/__cookie_v4_check")
-int BPF_PROG(bpftune__cookie_v4_check, const struct iphdr *iph, const struct tcphdr *th, u32 cookie, int ret)
-#endif
-{
-	if (ret == 0)
-		tcp_bad_syncookies++;
-	tcp_good_syncookies++;
-	return 0;
-}
-
-#ifdef BPFTUNE_LEGACY
-SEC("kretprobe/__cookie_v6_check")
-int BPF_KRETPROBE(bpftune__cookie_v6_check, int ret)
-#else
-SEC("fexit/__cookie_v6_check")
-int BPF_PROG(bpftune__cookie_v6_check, const struct ipv6hdr *iph, const struct tcphdr *th, __u32 cookie, int ret)
-#endif
-{
-	if (ret == 0)
-		tcp_bad_syncookies++;
-	tcp_good_syncookies++;
-	return 0;
-}

src/tcp_buffer_tuner.c

@@ -168,8 +168,7 @@ long long nr_free_buffer_pages(bool initial)
 int init(struct bpftuner *tuner)
 {
 	/* on some platforms, this function is inlined */
-	const char *optionals[] = { "entry__tcp_sndbuf_expand", "bpftune__cookie_v4_check",
-				    "bpftune__cookie_v6_check", NULL };
+	const char *optionals[] = { "entry__tcp_sndbuf_expand", NULL };
 	int pagesize;
 	int err;
 
@@ -271,6 +270,7 @@ void event_handler(struct bpftuner *tuner,
 	const char *lowmem = "normal memory conditions";
 	const char *reason = "unknown reason";
 	int scenario = event->scenario_id;
+	long goodcookies, badcookies;
 	bool prev_lowmem = false;
 	struct corr c = { 0 };
 	long double corr = 0;
@@ -372,23 +372,24 @@ void event_handler(struct bpftuner *tuner,
 		if (scenario != TCP_MAX_SYN_BACKLOG_INCREASE)
 			break;
 		t = bpftuner_tunable(tuner, TCP_BUFFER_TCP_SYNCOOKIES);
-		if (t && t->current_values[0] > 0) {
-			__u64 good = bpftuner_bpf_var_get(tcp_buffer, tuner, tcp_good_syncookies);
-			__u64 bad = bpftuner_bpf_var_get(tcp_buffer, tuner, tcp_bad_syncookies);
+		if (t && t->current_values[0] > 0 &&
+		    !bpftune_netstat_read(event->netns_cookie, AF_INET,
+					  "SyncookiesRecv", &goodcookies) &&
+		    !bpftune_netstat_read(event->netns_cookie, AF_INET,
+					  "SyncookiesFailed", &badcookies)) {
 
 			/* syncookies are enabled; are they effective? compare good/bad counts.
 			 * If none are good, syncookies are not really effective and we would
 			 * do better to rely on syn backlog increases.
 			 */
-			if (bad >= TCP_SYNCOOKIES_BAD_COUNT && !good) {
-				bpftuner_bpf_var_set(tcp_buffer, tuner, tcp_good_syncookies, 0);
-				bpftuner_bpf_var_set(tcp_buffer, tuner, tcp_bad_syncookies, 0);
-
+			if (badcookies >= TCP_SYNCOOKIES_BAD_COUNT &&
+			    !goodcookies) {
 				new[0] = 0;
 				bpftuner_tunable_sysctl_write(tuner, TCP_BUFFER_TCP_SYNCOOKIES,
 							      TCP_SYNCOOKIES_DISABLE,
 							      event->netns_cookie, 1, new,
 "Due to receiving %d invalid syncookies and no valid ones, disable '%s' as they are ineffective under current network conditions\n",
+							      badcookies,
 							      t->desc.name);
 				break;
 

test/many_netns_legacy_test.sh

@@ -52,7 +52,7 @@ for FAMILY in ipv4 ipv6 ; do
 	;;
    esac
 
-   test_start "$0|wmem test to $ADDR:$PORT $FAMILY opts $CLIENT_OPTS $LATENCY"
+   test_start "$0|many netns legacy test to $ADDR:$PORT $FAMILY opts $CLIENT_OPTS $LATENCY"
 
    wmem_orig=($(sysctl -n net.ipv4.tcp_wmem))
 
@@ -96,6 +96,17 @@ for FAMILY in ipv4 ipv6 ; do
 	sleep $SLEEPTIME
    done
 
+   fds=$($LSOF -p $(pgrep bpftune) 2>/dev/null|wc -l)
+   # if we have 20 more than the original number of fds open, likely a leak
+   fdsX=${fds_orig}+20
+   if [[ "$fds" -gt $fdsX ]]; then
+        echo "bpftune has $fds open versus original $fds_orig; fd leak? files:"
+        $LSOF -p $(pgrep bpftune)
+        test_cleanup
+   fi
+   echo "found $fds fds open for bpftune"
+   pkill -TERM bpftune
+
    wmem_post=($(sysctl -n net.ipv4.tcp_wmem))
    wmem_post_netns=($(ip netns exec $NETNS sysctl -n net.ipv4.tcp_wmem))
    sysctl -w net.ipv4.tcp_wmem="${wmem_orig[0]} ${wmem_orig[1]} ${wmem_orig[2]}"
@@ -140,16 +151,6 @@ for FAMILY in ipv4 ipv6 ; do
 	fi
    done 
 
-   fds=$($LSOF -p $(pgrep bpftune) 2>/dev/null|wc -l)
-   # if we have 20 more than the original number of fds open, likely a leak
-   fdsX=${fds_orig}+20
-   if [[ "$fds" -gt $fdsX ]]; then
-        echo "bpftune has $fds open versus original $fds_orig; fd leak? files:"
-	$LSOF -p $(pgrep bpftune)
-        test_cleanup
-   fi
-   echo "found $fds fds open for bpftune"
-
    test_pass
 
    test_cleanup

test/many_netns_test.sh

@@ -52,7 +52,7 @@ for FAMILY in ipv4 ipv6 ; do
 	;;
    esac
 
-   test_start "$0|wmem test to $ADDR:$PORT $FAMILY opts $CLIENT_OPTS $LATENCY"
+   test_start "$0|many netns test to $ADDR:$PORT $FAMILY opts $CLIENT_OPTS $LATENCY"
 
    wmem_orig=($(sysctl -n net.ipv4.tcp_wmem))
 
@@ -96,6 +96,17 @@ for FAMILY in ipv4 ipv6 ; do
 	sleep $SLEEPTIME
    done
 
+   fds=$($LSOF -p $(pgrep bpftune) 2>/dev/null|wc -l)
+   # if we have 20 more than the original number of fds open, likely a leak
+   fdsX=${fds_orig}+20
+   if [[ "$fds" -gt $fdsX ]]; then
+        echo "bpftune has $fds open versus original $fds_orig; fd leak? files:"
+        $LSOF -p $(pgrep bpftune)
+        test_cleanup
+   fi
+   echo "found $fds fds open for bpftune"
+   pkill -TERM bpftune
+
    wmem_post=($(sysctl -n net.ipv4.tcp_wmem))
    wmem_post_netns=($(ip netns exec $NETNS sysctl -n net.ipv4.tcp_wmem))
    sysctl -w net.ipv4.tcp_wmem="${wmem_orig[0]} ${wmem_orig[1]} ${wmem_orig[2]}"
@@ -140,16 +151,6 @@ for FAMILY in ipv4 ipv6 ; do
 	fi
    done 
 
-   fds=$($LSOF -p $(pgrep bpftune) 2>/dev/null|wc -l)
-   # if we have 20 more than the original number of fds open, likely a leak
-   fdsX=${fds_orig}+20
-   if [[ "$fds" -gt $fdsX ]]; then
-        echo "bpftune has $fds open versus original $fds_orig; fd leak? files:"
-	$LSOF -p $(pgrep bpftune)
-        test_cleanup
-   fi
-   echo "found $fds fds open for bpftune"
-
    test_pass
 
    test_cleanup