Delete nfcache use in ip_table
Subsystems (2)
PATCH 1/1 Delete nfcache use in ip_table
Hi Patrick,
Since we've removed the use of nfcache in iptables, I think that it should be remove in ip_tables.c as well because:
a) there's no utility on it
b) it can be source of interferences for the conntrack-event-api, and thus for conntrack_netlink and ct_sync.
Please see the email labeled as:
[Re: BUG/CONFLICT conntrack with preroute/postroute mangle table]
I could keep this as a seperate patch in conntrack-event-api pom-ng, but I prefer kill it now and forget about it. If you've got no reason to object, please then pass it to davem.
--
Pablo
net/ipv4/netfilter/ip_tables.c (mode:100644) renamed from net/ipv4/netfilter/ip_tables.c (mode:100644 sha1:8a54f92b8496a4218a243c35ab2b55ec659e7d18) +0 -1
@@ -313,7 +313,6 @@ do { IP_NF_ASSERT(e); IP_NF_ASSERT(back); - (*pskb)->nfcache |= e->nfcache; if (ip_packet_match(ip, indev, outdev, &e->ip, offset)) { struct ipt_entry_target *t;
Quoted 3 lines · Pablo Neira
I could keep this as a seperate patch in conntrack-event-api pom-ng, but I prefer kill it now and forget about it. If you've got no reason to object, please then pass it to davem.
No objections, but please remove it in all files under net/ipv4/netfilter.
Regards
Patrick
Hi Pablo,
Quoted 6 lines · Pablo Neira
Since we've removed the use of nfcache in iptables, I think that it should be remove in
ip_tables.cas well because:a) there's no utility on it
b) it can be source of interferences for the conntrack-event-api, and thus forconntrack_netlinkandct_sync.
Wow, thanks a lot for this fix. Indeed, it seems this bug might have
been the source of a ct_sync problem experienced by some of the testers.
--
Regards,
Krisztian Kovacs
Quoted 9 lines · Patrick McHardy
Quoted 3 lines · Pablo Neira
I could keep this as a seperate patch in conntrack-event-api pom-ng, but I prefer kill it now and forget about it. If you've got no reason to object, please then pass it to davem.
No objections, but please remove it in all files under
net/ipv4/netfilter.
I've deleted all references to nfcache found in ipv4 and ipv6.
While I was at it, I thought about killing NFC_* stuff in include/linux/netfilter[_ipv4].h, then I realized that old iptables versions won't compile if I do so. Those will have to remain there to ensure backward compatibility compilation.
--
Pablo
net/ipv4/netfilter/ip_conntrack_core.c (mode:100644) renamed from net/ipv4/netfilter/ip_conntrack_core.c (mode:100644 sha1:28d9425d5c390dac7601953e65043e973833e7aa) +0 -4
@@ -627,9 +627,6 @@ return NF_DROP; } - /* FIXME: Do this right please. --RR */ - (*pskb)->nfcache |= NFC_UNKNOWN; - /* Doesn't cover locally-generated broadcast, so not worth it. */ #if 0 /* Ignore broadcast: no `connection'. */
@@ -966,7 +963,6 @@ } ip_send_check(skb->nh.iph); - skb->nfcache |= NFC_ALTERED; #ifdef CONFIG_NETFILTER_DEBUG /* Packet path as if nothing had happened. */ skb->nf_debug = olddebug;
net/ipv4/netfilter/ip_nat_core.c (mode:100644) renamed from net/ipv4/netfilter/ip_nat_core.c (mode:100644 sha1:9fc6f93af0dd15de8933a14e053bf8cdf41dfb69) +0 -1
@@ -321,7 +321,6 @@ { struct iphdr *iph; - (*pskb)->nfcache |= NFC_ALTERED; if (!skb_ip_make_writable(pskb, iphdroff + sizeof(*iph))) return 0;
net/ipv4/netfilter/ip_nat_standalone.c (mode:100644) renamed from net/ipv4/netfilter/ip_nat_standalone.c (mode:100644 sha1:79f56f662b336bb1a48298af7c0d03c1e1d73b89) +0 -2
@@ -73,8 +73,6 @@ IP_NF_ASSERT(!((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET))); - (*pskb)->nfcache |= NFC_UNKNOWN; - /* If we had a hardware checksum before, it's now invalid */ if ((*pskb)->ip_summed == CHECKSUM_HW) if (skb_checksum_help(*pskb, (out == NULL)))
net/ipv4/netfilter/ip_queue.c (mode:100644) renamed from net/ipv4/netfilter/ip_queue.c (mode:100644 sha1:9e40dffc204f3438e0dbabc1ed89be0031f13449) +0 -1
@@ -375,7 +375,6 @@ if (!skb_ip_make_writable(&e->skb, v->data_len)) return -ENOMEM; memcpy(e->skb->data, v->payload, v->data_len); - e->skb->nfcache |= NFC_ALTERED; /* * Extra routing may needed on local out, as the QUEUE target never
net/ipv4/netfilter/ip_tables.c (mode:100644) renamed from net/ipv4/netfilter/ip_tables.c (mode:100644 sha1:8a54f92b8496a4218a243c35ab2b55ec659e7d18) +0 -1
@@ -313,7 +313,6 @@ do { IP_NF_ASSERT(e); IP_NF_ASSERT(back); - (*pskb)->nfcache |= e->nfcache; if (ip_packet_match(ip, indev, outdev, &e->ip, offset)) { struct ipt_entry_target *t;
net/ipv4/netfilter/ipt_CLASSIFY.c (mode:100644) renamed from net/ipv4/netfilter/ipt_CLASSIFY.c (mode:100644 sha1:9842e6e231845c7aec4e42dafe4585f03fca4def) +1 -3
@@ -32,10 +32,8 @@ { const struct ipt_classify_target_info *clinfo = targinfo; - if((*pskb)->priority != clinfo->priority) { + if((*pskb)->priority != clinfo->priority) (*pskb)->priority = clinfo->priority; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }
net/ipv4/netfilter/ipt_CONNMARK.c (mode:100644) renamed from net/ipv4/netfilter/ipt_CONNMARK.c (mode:100644 sha1:30ddd3e18eb747184b80eea693778058a85c14ec) +1 -3
@@ -61,10 +61,8 @@ case IPT_CONNMARK_RESTORE: nfmark = (*pskb)->nfmark; diff = (ct->mark ^ nfmark) & markinfo->mask; - if (diff != 0) { + if (diff != 0) (*pskb)->nfmark = nfmark ^ diff; - (*pskb)->nfcache |= NFC_ALTERED; - } break; } }
net/ipv4/netfilter/ipt_DSCP.c (mode:100644) renamed from net/ipv4/netfilter/ipt_DSCP.c (mode:100644 sha1:3ea4509099f907ec15fe0975e3dc9d8def7f78eb) +0 -1
@@ -51,7 +51,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^ 0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return IPT_CONTINUE; }
net/ipv4/netfilter/ipt_ECN.c (mode:100644) renamed from net/ipv4/netfilter/ipt_ECN.c (mode:100644 sha1:ada9911118e9a7ec2ee841db8b3f250e48bfb71c) +0 -2
@@ -43,7 +43,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return 1; }
@@ -86,7 +85,6 @@ else if (skb_checksum_help(*pskb, inward)) return 0; - (*pskb)->nfcache |= NFC_ALTERED; return 1; }
net/ipv4/netfilter/ipt_MARK.c (mode:100644) renamed from net/ipv4/netfilter/ipt_MARK.c (mode:100644 sha1:33c6f9b63b8d7af0d0fa36c74158b0fc633f2e37) +2 -6
@@ -29,10 +29,8 @@ { const struct ipt_mark_target_info *markinfo = targinfo; - if((*pskb)->nfmark != markinfo->mark) { + if((*pskb)->nfmark != markinfo->mark) (*pskb)->nfmark = markinfo->mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }
@@ -61,10 +59,8 @@ break; } - if((*pskb)->nfmark != mark) { + if((*pskb)->nfmark != mark) (*pskb)->nfmark = mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }
net/ipv4/netfilter/ipt_REJECT.c (mode:100644) renamed from net/ipv4/netfilter/ipt_REJECT.c (mode:100644 sha1:266d64979286dd039245355d108af146e8e5ba8d) +0 -1
@@ -145,7 +145,6 @@ /* This packet will not be the same as the other: clear nf fields */ nf_reset(nskb); - nskb->nfcache = 0; nskb->nfmark = 0; #ifdef CONFIG_BRIDGE_NETFILTER nf_bridge_put(nskb->nf_bridge);
net/ipv4/netfilter/ipt_TCPMSS.c (mode:100644) renamed from net/ipv4/netfilter/ipt_TCPMSS.c (mode:100644 sha1:1049050b2bfbc0a5123662548b37ac382e1cb11f) +0 -1
@@ -189,7 +189,6 @@ /* We never hw checksum SYN packets. */ BUG_ON((*pskb)->ip_summed == CHECKSUM_HW); - (*pskb)->nfcache |= NFC_UNKNOWN | NFC_ALTERED; return IPT_CONTINUE; }
net/ipv4/netfilter/ipt_TOS.c (mode:100644) renamed from net/ipv4/netfilter/ipt_TOS.c (mode:100644 sha1:85c70d240f8bf433e6e7b59d7cc9712371a5769b) +0 -1
@@ -46,7 +46,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return IPT_CONTINUE; }
net/ipv6/netfilter/ip6_queue.c (mode:100644) renamed from net/ipv6/netfilter/ip6_queue.c (mode:100644 sha1:c54830b895939ed78f099978841d48a0d1d4a457) +0 -1
@@ -379,7 +379,6 @@ if (!skb_ip_make_writable(&e->skb, v->data_len)) return -ENOMEM; memcpy(e->skb->data, v->payload, v->data_len); - e->skb->nfcache |= NFC_ALTERED; /* * Extra routing may needed on local out, as the QUEUE target never
net/ipv6/netfilter/ip6t_MARK.c (mode:100644) renamed from net/ipv6/netfilter/ip6t_MARK.c (mode:100644 sha1:d09ceb05013a90346f916a44a5c77830164a4250) +1 -3
@@ -28,10 +28,8 @@ { const struct ip6t_mark_target_info *markinfo = targinfo; - if((*pskb)->nfmark != markinfo->mark) { + if((*pskb)->nfmark != markinfo->mark) (*pskb)->nfmark = markinfo->mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IP6T_CONTINUE; }
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoting Pablo Neira on 2005-05-06 00:25 UTC:
Quoted 4 lines
While I was at it, I thought about killing NFC_* stuff in include/linux/netfilter[_ipv4].h, then I realized that old iptables versions won't compile if I do so. Those will have to remain there to ensure backward compatibility compilation.
Would it be possible (and would it make sense) using #ifdefs to only have those NFC_* visible to userspace? Or would that break something too?
- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQFCexUdxyF48ZTvn+4RAgSSAKCThQsBMaX+WSaqnA/l3Ljyok+QUwCgsrcn tXAeFh2XKpgg5BnPrgC7XEw=
=ZmxL
-----END PGP SIGNATURE-----
Quoted 6 lines · Pablo Neira
Quoted 2 lines · Patrick McHardy
No objections, but please remove it in all files under
net/ipv4/netfilter.I've deleted all references to nfcache found in ipv4 and ipv6.
Thanks. I thought we couldn't remove them in ipv6/ because ip6_maybe_reroute() reroutes packets with NFC_ALTERED, but the flag wasn't set on any change that could affect routing, so it should be fine. I've removed ip6_maybe_reroute() and killed one more NFC_ALTERED in net/bridge.
Quoted 4 lines
While I was at it, I thought about killing NFC_* stuff in include/linux/netfilter[_ipv4].h, then I realized that old iptables versions won't compile if I do so. Those will have to remain there to ensure backward compatibility compilation.
Yes, we can kill them once iptables versions that still include them don't work anymore for other reasons.
BTW: Please also sign off resends. I've used the line from your first patch.
Regards
Patrick
Quoted 2 lines · Jonas Berlin
Would it be possible (and would it make sense) using #ifdefs to only have those NFC_* visible to userspace? Or would that break something too?
Possible sure (#ifndef __KERNEL__), but what would the gain be?
Regards
Patrick
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoting Patrick McHardy on 2005-05-06 15:24 UTC:
Quoted 3 lines
Quoted 2 lines
Would it be possible (and would it make sense) using #ifdefs to only have those NFC_* visible to userspace? Or would that break something too?
Possible sure (#ifndef
__KERNEL__), but what would the gain be?
I assumed the point by the removal was to make sure nobody used them [in the kernel] anymore.. But maybe the point is just to phase it out completely.. :)
- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
iD8DBQFCfEfIxyF48ZTvn+4RAoMUAKCdCS032rSUAM9AEcpdEfNkuxw22QCgiCrn bGScaDZ3IReiUdQHhakbVc0=
=NAx/
-----END PGP SIGNATURE-----
This patch does not seem to have made it into 2.6.12-git or 2.6.12.2 from kernel.org
Am I alone in thinking that it ought to be sent upstream?
Azez
Quoted 254 lines · Pablo Neira
Quoted 10 lines · Patrick McHardy
Quoted 3 lines · Pablo Neira
I could keep this as a seperate patch in conntrack-event-api pom-ng, but I prefer kill it now and forget about it. If you've got no reason to object, please then pass it to davem.
No objections, but please remove it in all files under
net/ipv4/netfilter.I've deleted all references to nfcache found in ipv4 and ipv6.
While I was at it, I thought about killing NFC_* stuff in include/linux/netfilter[_ipv4].h, then I realized that old iptables versions won't compile if I do so. Those will have to remain there to ensure backward compatibility compilation.
Signature · 7 lines
Pablo ------------------------------------------------------------------------ Index: net/ipv4/netfilter/ip_conntrack_core.c ===================================================================net/ipv4/netfilter/ip_conntrack_core.c (mode:100644) renamed from net/ipv4/netfilter/ip_conntrack_core.c (mode:100644 sha1:28d9425d5c390dac7601953e65043e973833e7aa) +0
-4@@ -627,9 +627,6 @@ return NF_DROP; } - /* FIXME: Do this right please. --RR */ - (*pskb)->nfcache |= NFC_UNKNOWN; - /* Doesn't cover locally-generated broadcast, so not worth it. */ #if 0 /* Ignore broadcast: no `connection'. */@@ -966,7 +963,6 @@ } ip_send_check(skb->nh.iph); - skb->nfcache |= NFC_ALTERED; #ifdef CONFIG_NETFILTER_DEBUG /* Packet path as if nothing had happened. */ skb->nf_debug = olddebug;Index:
net/ipv4/netfilter/ip_nat_core.c
===================================================================net/ipv4/netfilter/ip_nat_core.c (mode:100644) renamed from net/ipv4/netfilter/ip_nat_core.c (mode:100644 sha1:9fc6f93af0dd15de8933a14e053bf8cdf41dfb69) +0
-1@@ -321,7 +321,6 @@ { struct iphdr *iph; - (*pskb)->nfcache |= NFC_ALTERED; if (!skb_ip_make_writable(pskb, iphdroff + sizeof(*iph))) return 0;Index:
net/ipv4/netfilter/ip_nat_standalone.c
===================================================================net/ipv4/netfilter/ip_nat_standalone.c (mode:100644) renamed from net/ipv4/netfilter/ip_nat_standalone.c (mode:100644 sha1:79f56f662b336bb1a48298af7c0d03c1e1d73b89) +0
-2@@ -73,8 +73,6 @@ IP_NF_ASSERT(!((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET))); - (*pskb)->nfcache |= NFC_UNKNOWN; - /* If we had a hardware checksum before, it's now invalid */ if ((*pskb)->ip_summed == CHECKSUM_HW) if (skb_checksum_help(*pskb, (out == NULL)))Index:
net/ipv4/netfilter/ip_queue.c
===================================================================net/ipv4/netfilter/ip_queue.c (mode:100644) renamed from net/ipv4/netfilter/ip_queue.c (mode:100644 sha1:9e40dffc204f3438e0dbabc1ed89be0031f13449) +0
-1@@ -375,7 +375,6 @@ if (!skb_ip_make_writable(&e->skb, v->data_len)) return -ENOMEM; memcpy(e->skb->data, v->payload, v->data_len); - e->skb->nfcache |= NFC_ALTERED; /* * Extra routing may needed on local out, as the QUEUE target neverIndex:
net/ipv4/netfilter/ip_tables.c
===================================================================net/ipv4/netfilter/ip_tables.c (mode:100644) renamed from net/ipv4/netfilter/ip_tables.c (mode:100644 sha1:8a54f92b8496a4218a243c35ab2b55ec659e7d18) +0
-1@@ -313,7 +313,6 @@ do { IP_NF_ASSERT(e); IP_NF_ASSERT(back); - (*pskb)->nfcache |= e->nfcache; if (ip_packet_match(ip, indev, outdev, &e->ip, offset)) { struct ipt_entry_target *t;Index:
net/ipv4/netfilter/ipt_CLASSIFY.c
===================================================================net/ipv4/netfilter/ipt_CLASSIFY.c (mode:100644) renamed from net/ipv4/netfilter/ipt_CLASSIFY.c (mode:100644 sha1:9842e6e231845c7aec4e42dafe4585f03fca4def) +1
-3@@ -32,10 +32,8 @@ { const struct ipt_classify_target_info *clinfo = targinfo; - if((*pskb)->priority != clinfo->priority) { + if((*pskb)->priority != clinfo->priority) (*pskb)->priority = clinfo->priority; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }Index:
net/ipv4/netfilter/ipt_CONNMARK.c
===================================================================net/ipv4/netfilter/ipt_CONNMARK.c (mode:100644) renamed from net/ipv4/netfilter/ipt_CONNMARK.c (mode:100644 sha1:30ddd3e18eb747184b80eea693778058a85c14ec) +1
-3@@ -61,10 +61,8 @@ case IPT_CONNMARK_RESTORE: nfmark = (*pskb)->nfmark; diff = (ct->mark ^ nfmark) & markinfo->mask; - if (diff != 0) { + if (diff != 0) (*pskb)->nfmark = nfmark ^ diff; - (*pskb)->nfcache |= NFC_ALTERED; - } break; } }Index:
net/ipv4/netfilter/ipt_DSCP.c
===================================================================net/ipv4/netfilter/ipt_DSCP.c (mode:100644) renamed from net/ipv4/netfilter/ipt_DSCP.c (mode:100644 sha1:3ea4509099f907ec15fe0975e3dc9d8def7f78eb) +0
-1@@ -51,7 +51,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^ 0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return IPT_CONTINUE; }Index:
net/ipv4/netfilter/ipt_ECN.c
===================================================================net/ipv4/netfilter/ipt_ECN.c (mode:100644) renamed from net/ipv4/netfilter/ipt_ECN.c (mode:100644 sha1:ada9911118e9a7ec2ee841db8b3f250e48bfb71c) +0
-2@@ -43,7 +43,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return 1; }@@ -86,7 +85,6 @@ else if (skb_checksum_help(*pskb, inward)) return 0; - (*pskb)->nfcache |= NFC_ALTERED; return 1; }Index:
net/ipv4/netfilter/ipt_MARK.c
===================================================================net/ipv4/netfilter/ipt_MARK.c (mode:100644) renamed from net/ipv4/netfilter/ipt_MARK.c (mode:100644 sha1:33c6f9b63b8d7af0d0fa36c74158b0fc633f2e37) +2
-6@@ -29,10 +29,8 @@ { const struct ipt_mark_target_info *markinfo = targinfo; - if((*pskb)->nfmark != markinfo->mark) { + if((*pskb)->nfmark != markinfo->mark) (*pskb)->nfmark = markinfo->mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }@@ -61,10 +59,8 @@ break; } - if((*pskb)->nfmark != mark) { + if((*pskb)->nfmark != mark) (*pskb)->nfmark = mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IPT_CONTINUE; }Index:
net/ipv4/netfilter/ipt_REJECT.c
===================================================================net/ipv4/netfilter/ipt_REJECT.c (mode:100644) renamed from net/ipv4/netfilter/ipt_REJECT.c (mode:100644 sha1:266d64979286dd039245355d108af146e8e5ba8d) +0
-1@@ -145,7 +145,6 @@ /* This packet will not be the same as the other: clear nf fields */ nf_reset(nskb); - nskb->nfcache = 0; nskb->nfmark = 0; #ifdef CONFIG_BRIDGE_NETFILTER nf_bridge_put(nskb->nf_bridge);Index:
net/ipv4/netfilter/ipt_TCPMSS.c
===================================================================net/ipv4/netfilter/ipt_TCPMSS.c (mode:100644) renamed from net/ipv4/netfilter/ipt_TCPMSS.c (mode:100644 sha1:1049050b2bfbc0a5123662548b37ac382e1cb11f) +0
-1@@ -189,7 +189,6 @@ /* We never hw checksum SYN packets. */ BUG_ON((*pskb)->ip_summed == CHECKSUM_HW); - (*pskb)->nfcache |= NFC_UNKNOWN | NFC_ALTERED; return IPT_CONTINUE; }Index:
net/ipv4/netfilter/ipt_TOS.c
===================================================================net/ipv4/netfilter/ipt_TOS.c (mode:100644) renamed from net/ipv4/netfilter/ipt_TOS.c (mode:100644 sha1:85c70d240f8bf433e6e7b59d7cc9712371a5769b) +0
-1@@ -46,7 +46,6 @@ sizeof(diffs), (*pskb)->nh.iph->check ^0xFFFF)); - (*pskb)->nfcache |= NFC_ALTERED; } return IPT_CONTINUE; }Index:
net/ipv6/netfilter/ip6_queue.c
===================================================================net/ipv6/netfilter/ip6_queue.c (mode:100644) renamed from net/ipv6/netfilter/ip6_queue.c (mode:100644 sha1:c54830b895939ed78f099978841d48a0d1d4a457) +0
-1@@ -379,7 +379,6 @@ if (!skb_ip_make_writable(&e->skb, v->data_len)) return -ENOMEM; memcpy(e->skb->data, v->payload, v->data_len); - e->skb->nfcache |= NFC_ALTERED; /* * Extra routing may needed on local out, as the QUEUE target neverIndex:
net/ipv6/netfilter/ip6t_MARK.c
===================================================================net/ipv6/netfilter/ip6t_MARK.c (mode:100644) renamed from net/ipv6/netfilter/ip6t_MARK.c (mode:100644 sha1:d09ceb05013a90346f916a44a5c77830164a4250) +1
-3@@ -28,10 +28,8 @@ { const struct ip6t_mark_target_info *markinfo = targinfo; - if((*pskb)->nfmark != markinfo->mark) { + if((*pskb)->nfmark != markinfo->mark) (*pskb)->nfmark = markinfo->mark; - (*pskb)->nfcache |= NFC_ALTERED; - } return IP6T_CONTINUE; }