Skip to contentSkip to conversation

netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

PATCH Superseded Subash Abhinov Kasiviswanathan 2 participants 5 messages Nov 14, 2017 – Dec 11, 2017

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set

conntrack defrag is needed only if some module like CONNTRACK or NAT explicitly requests it. For plain forwarding scenarios, defrag is not needed and can be skipped if NOTRACK is set in a rule.

Since conntrack defrag is currently higher priority than raw table, setting NOTRACK is not sufficient. We need to introduce a lower priority rule and deprecate the existing rule.

---
 include/uapi/linux/netfilter_ipv4.h | 3 ++-
 net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)
include/uapi/linux/netfilter_ipv4.h +2 -1
index e6b1a84..cedf7b4 100644
@@ -57,9 +57,10 @@
 
 enum nf_ip_hook_priorities {
 	NF_IP_PRI_FIRST = INT_MIN,
-	NF_IP_PRI_CONNTRACK_DEFRAG = -400,
+	NF_IP_PRI_CONNTRACK_DEFRAG = -400, /* deprecated */
 	NF_IP_PRI_RAW = -300,
 	NF_IP_PRI_SELINUX_FIRST = -225,
+	NF_IP_PRI_CONNTRACK_DEFRAG_V1 = -210,
 	NF_IP_PRI_CONNTRACK = -200,
 	NF_IP_PRI_MANGLE = -150,
 	NF_IP_PRI_NAT_DST = -100,
net/ipv4/netfilter/nf_defrag_ipv4.c +3 -3
index 37fe1616..6496890 100644
@@ -80,7 +80,7 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
 #endif
 #endif
 	/* Gather fragments. */
-	if (ip_is_fragment(ip_hdr(skb))) {
+	if (skb->_nfct != IP_CT_UNTRACKED && ip_is_fragment(ip_hdr(skb))) {
 		enum ip_defrag_users user =
 			nf_ct_defrag_user(state->hook, skb);
 
@@ -95,13 +95,13 @@ static unsigned int ipv4_conntrack_defrag(void *priv,
 		.hook		= ipv4_conntrack_defrag,
 		.pf		= NFPROTO_IPV4,
 		.hooknum	= NF_INET_PRE_ROUTING,
-		.priority	= NF_IP_PRI_CONNTRACK_DEFRAG,
+		.priority	= NF_IP_PRI_CONNTRACK_DEFRAG_V1,
 	},
 	{
 		.hook           = ipv4_conntrack_defrag,
 		.pf             = NFPROTO_IPV4,
 		.hooknum        = NF_INET_LOCAL_OUT,
-		.priority       = NF_IP_PRI_CONNTRACK_DEFRAG,
+		.priority       = NF_IP_PRI_CONNTRACK_DEFRAG_V1,
 	},
 };
 
Signature · 1 line
1.9.1
#1 lore raw

Hi Subash,

Sorry it took a while, but I've been discussing this with Jozsef too, he's now on Cc.

Quoted 19 lines · Subash Abhinov Kasiviswanathan

conntrack defrag is needed only if some module like CONNTRACK or NAT explicitly requests it. For plain forwarding scenarios, defrag is not needed and can be skipped if NOTRACK is set in a rule.

Since conntrack defrag is currently higher priority than raw table, setting NOTRACK is not sufficient. We need to introduce a lower priority rule and deprecate the existing rule.

---
 include/uapi/linux/netfilter_ipv4.h | 3 ++-
 net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++---
 2 files changed, 5 insertions(+), 4 deletions(-)
include/uapi/linux/netfilter_ipv4.h +0 -0
index e6b1a84..cedf7b4 100644

include/uapi/linux/netfilter_ipv4.h line 63

Quoted 8 lines
include/uapi/linux/netfilter_ipv4.h +2 -1
@@ -57,9 +57,10 @@
 
 enum nf_ip_hook_priorities {
 	NF_IP_PRI_FIRST = INT_MIN,
-	NF_IP_PRI_CONNTRACK_DEFRAG = -400,
+	NF_IP_PRI_CONNTRACK_DEFRAG = -400, /* deprecated */
 	NF_IP_PRI_RAW = -300,
 	NF_IP_PRI_SELINUX_FIRST = -225,
+	NF_IP_PRI_CONNTRACK_DEFRAG_V1 = -210,

Would it work for you if this is specific via global modparam? I'm telling this because:

1) This is changing the default behaviour, which is always tricky.
2) This is already solved in nftables, so whatever solution that we apply, it should be iptables specific.

If modparam is fine, just placing a line into /etc/modprobe.d/options.conf (or similar) should be good enough to store that you're requesting raw hook registration before defrag.

Let me know,
Thanks!

#2 lore raw
Quoted 7 lines

Would it work for you if this is specific via global modparam? I'm telling this because:

1) This is changing the default behaviour, which is always tricky.
2) This is already solved in nftables, so whatever solution that we apply, it should be iptables specific.

Quoted 6 lines

If modparam is fine, just placing a line into /etc/modprobe.d/options.conf (or similar) should be good enough to store that you're requesting raw hook registration before defrag.

Let me know,
Thanks!

Hi Pablo

Can you explain a bit more about the /etc/modprobe.d/ option and how it would be configured for this? /etc/modprobe.d/ doesnt exist on Android based Linux systems so it might be a problem for me.

Would it be an acceptable solution to create a kernel config for this particular feature instead?

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#3 lore raw
Quoted 14 lines · Subash Abhinov Kasiviswanathan
Quoted 7 lines

Would it work for you if this is specific via global modparam? I'm telling this because:

1) This is changing the default behaviour, which is always tricky.
2) This is already solved in nftables, so whatever solution that we apply, it should be iptables specific.

Quoted 6 lines

If modparam is fine, just placing a line into /etc/modprobe.d/options.conf (or similar) should be good enough to store that you're requesting raw hook registration before defrag.

Let me know,
Thanks!

Quoted 8 lines

Hi Pablo

Can you explain a bit more about the /etc/modprobe.d/ option and how it would be configured for this? /etc/modprobe.d/ doesnt exist on Android based Linux systems so it might be a problem for me.

Would it be an acceptable solution to create a kernel config for this particular feature instead?

I'm actually refering to module_param(), that is specified at modprobe time. Such parameter would set an alternative hook priority for the raw table, ie. before the defrag hook. I guess there must be a way to store these module parameters in Android, so whenever modprobe is invoked, either explicitly or via module autoload, this module parameter is passed to the iptable_raw module.

#4 lore raw
Quoted 6 lines

I'm actually refering to module_param(), that is specified at modprobe time. Such parameter would set an alternative hook priority for the raw table, ie. before the defrag hook. I guess there must be a way to store these module parameters in Android, so whenever modprobe is invoked, either explicitly or via module autoload, this module parameter is passed to the iptable_raw module.

Thanks for the clarification Pablo. I'll try this and send a new patch.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project