Message ID | 1510623446-9115-1-git-send-email-subashab@codeaurora.org |
---|---|
State | Changes Requested |
Delegated to: | Pablo Neira |
Headers | show |
Series | [nf-next] netfilter: nf_defrag_ipv4: Skip defrag if NOTRACK is set | expand |
Hi Subash, Sorry it took a while, but I've been discussing this with Jozsef too, he's now on Cc. On Mon, Nov 13, 2017 at 06:37:26PM -0700, Subash Abhinov Kasiviswanathan wrote: > 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. > > Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> > --- > include/uapi/linux/netfilter_ipv4.h | 3 ++- > net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++--- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/include/uapi/linux/netfilter_ipv4.h b/include/uapi/linux/netfilter_ipv4.h > index e6b1a84..cedf7b4 100644 > --- a/include/uapi/linux/netfilter_ipv4.h > +++ b/include/uapi/linux/netfilter_ipv4.h > @@ -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! -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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! 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?
On Sat, Dec 09, 2017 at 07:06:14PM -0700, Subash Abhinov Kasiviswanathan wrote: > >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! > > 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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.
diff --git a/include/uapi/linux/netfilter_ipv4.h b/include/uapi/linux/netfilter_ipv4.h index e6b1a84..cedf7b4 100644 --- a/include/uapi/linux/netfilter_ipv4.h +++ b/include/uapi/linux/netfilter_ipv4.h @@ -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, diff --git a/net/ipv4/netfilter/nf_defrag_ipv4.c b/net/ipv4/netfilter/nf_defrag_ipv4.c index 37fe1616..6496890 100644 --- a/net/ipv4/netfilter/nf_defrag_ipv4.c +++ b/net/ipv4/netfilter/nf_defrag_ipv4.c @@ -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, }, };
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. Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org> --- include/uapi/linux/netfilter_ipv4.h | 3 ++- net/ipv4/netfilter/nf_defrag_ipv4.c | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-)