Re: Most optimal method to dump UDP conntrack entries

[Florian Westphal <fw@strlen.de>]

Antonio Ojea <antonio.ojea.garcia@gmail.com> wrote:
> On Fri, 18 Oct 2024 at 13:33, Florian Westphal <fw@strlen.de> wrote:
> > Same as what happens now, 2nd packet follows NAT mapping of first one.
> 
> This looks like the way to go ... if you can send me a patch I can do
> some testing next week and report back

Here is a better patch, renew only when responses are seen.
This means that once either initiator or responder ceases to send
packets entry will time out.

Subject: netfilter: nf_conntrack_proto_udp: renew timeout only for bidirectional traffic

In commit e15d4cdf27cb
("netfilter: conntrack: do not renew entry stuck in tcp SYN_SENT state")
I changed TCP conntrack to let tcp entries time out in case initial SYN
packets never see SYN/ACK reply.  This allows NAT rules to be
re-evaluated again.

Do something similar for UDP and renew the timeout if the current
packet is the opposite direction as the last one.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/netfilter/nf_conntrack.h   |  3 ++-
 net/netfilter/nf_conntrack_proto_udp.c | 33 +++++++++++++++++---------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index cba3ccf03fcc..e0c098e2705d 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -25,7 +25,8 @@
 #include <net/netfilter/nf_conntrack_tuple.h>
 
 struct nf_ct_udp {
-	unsigned long	stream_ts;
+	unsigned long stream_ts;
+	enum ip_conntrack_dir last_dir;
 };
 
 /* per conntrack: protocol private data */
diff --git a/net/netfilter/nf_conntrack_proto_udp.c b/net/netfilter/nf_conntrack_proto_udp.c
index 0030fbe8885c..33fcf52eeaf4 100644
--- a/net/netfilter/nf_conntrack_proto_udp.c
+++ b/net/netfilter/nf_conntrack_proto_udp.c
@@ -80,6 +80,16 @@ static bool udp_error(struct sk_buff *skb,
 	return false;
 }
 
+static bool udp_ts_reply(struct nf_conn *ct, enum ip_conntrack_dir dir)
+{
+	bool is_reply = READ_ONCE(ct->proto.udp.last_dir) != dir;
+
+	if (is_reply)
+		WRITE_ONCE(ct->proto.udp.last_dir, dir);
+
+	return is_reply;
+}
+
 /* Returns verdict for packet, and may modify conntracktype */
 int nf_conntrack_udp_packet(struct nf_conn *ct,
 			    struct sk_buff *skb,
@@ -87,8 +97,9 @@ int nf_conntrack_udp_packet(struct nf_conn *ct,
 			    enum ip_conntrack_info ctinfo,
 			    const struct nf_hook_state *state)
 {
+	unsigned long status, extra;
+	enum ip_conntrack_dir dir;
 	unsigned int *timeouts;
-	unsigned long status;
 
 	if (udp_error(skb, dataoff, state))
 		return -NF_ACCEPT;
@@ -97,15 +108,19 @@ int nf_conntrack_udp_packet(struct nf_conn *ct,
 	if (!timeouts)
 		timeouts = udp_get_timeouts(nf_ct_net(ct));
 
+	extra = timeouts[UDP_CT_UNREPLIED];
 	status = READ_ONCE(ct->status);
-	if ((status & IPS_CONFIRMED) == 0)
+	if ((status & IPS_CONFIRMED) == 0) {
 		ct->proto.udp.stream_ts = 2 * HZ + jiffies;
+		ct->proto.udp.last_dir = IP_CT_DIR_MAX;
+	}
+
+	dir = CTINFO2DIR(ctinfo);
 
 	/* If we've seen traffic both ways, this is some kind of UDP
 	 * stream. Set Assured.
 	 */
 	if (status & IPS_SEEN_REPLY) {
-		unsigned long extra = timeouts[UDP_CT_UNREPLIED];
 		bool stream = false;
 
 		/* Still active after two seconds? Extend timeout. */
@@ -114,18 +129,14 @@ int nf_conntrack_udp_packet(struct nf_conn *ct,
 			stream = (status & IPS_ASSURED) == 0;
 		}
 
-		nf_ct_refresh_acct(ct, ctinfo, skb, extra);
-
-		/* never set ASSURED for IPS_NAT_CLASH, they time out soon */
-		if (unlikely((status & IPS_NAT_CLASH)))
-			return NF_ACCEPT;
-
 		/* Also, more likely to be important, and not a probe */
 		if (stream && !test_and_set_bit(IPS_ASSURED_BIT, &ct->status))
 			nf_conntrack_event_cache(IPCT_ASSURED, ct);
-	} else {
-		nf_ct_refresh_acct(ct, ctinfo, skb, timeouts[UDP_CT_UNREPLIED]);
 	}
+
+	if (udp_ts_reply(ct, dir))
+		nf_ct_refresh_acct(ct, ctinfo, skb, extra);
+
 	return NF_ACCEPT;
 }
 
-- 
2.45.2