Skip to contentSkip to conversation

: Netfilter IPsec support

PATCH In review Patrick McHardy 5 participants 54 messages Nov 20, 2005 – Dec 4, 2005 4 messages not in the archive

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

cover : Netfilter IPsec support

This is the latest netfilter/IPsec patchset. Its purpose is to make IPsec look as much as a normal tunnel device to netfilter as possible and to enable NAT support.

It consists of basically five parts:

- output hooks:

Currently on the output path netfilter sees the plain text packet in LOCAL_OUT and FORWARD and the encapsulated packet in POST_ROUTING. For connection tracking and NAT the plain text packets need to be visible on POST_ROUTING and the encapsulated packets on LOCAL_OUT as well. The patchset adds two new functions, ip_dst_output and ip6_dst_output that call the appropriate netfilter hooks for the plain text packet before encapsulation and for the encapsulated packets once for each tunnel mode transform.

- input hooks:

The input path is already mostly symetrical to the output path with the new output hooks, except for one case, if the innermost transform uses transport mode the decapsulated packets will not hit netfilter again. New hooks are added to xfrm{4,6}_input to handle this case. The hooks are only called if the _last_ transform is transport mode, otherwise decapsulated transport mode packets are not visible to netfilter.

- policy lookups after NAT:

When NAT changes a packet it already calls ip_route_me_harder, which reroutes the packet and does a new policy lookup. It only looks at the IP addresses however, changing the port numbers require a new policy lookup as well. It also doesn't reroute in POST_ROUTING, since the packet has already been routed. To behave more like a regular tunnel device a policy lookup is now also done after SNAT and the packet is passed to dst_output again if the lookup yielded a new policy.

- policy checks after NAT:

Policy checks look up the policy of the decapsulated packet and check that all decapsulations match what has been specified by the policy. If the packet has been NATed before policy checks the policy lookup might return a different policy from what was actually used. To handle this a new function nf_nat_decode_session reconstructs a struct flowi for the original packet which is then used for policy lookups.

- policy match:

To allow matching on the policy or the decapsulations done on the input path a new match is added. It can be used to replace rules like "-i ipsec0" or "-o ipsec0" which were commonly used with KLIPS, but can also be used for more fine-grained filtering.

Changes this last post:

- updated to apply to latest kernel
- the defered fragmentation patch has been replaced by a new patch which moves the POST_ROUTING hook before fragmentation
- added missing EXPORT_SYMBOL(xfrm_decode_session) for IPv6
- moving nf_reset from ip_local_deliver_finish to the upper protocols has been split into a seperate patch, unnecessary nf_reset's on paths were the packet is dropped have been removed and a missing nf_reset before icmp_send in ip_local_deliver_finish has been added.

The patches are now in a state in which I think they could be merged in the net-2.6.16 tree. Unfortunately cloning the tree fails for me, so they are still based on Linus's tree, but I don't think there are any changes in net-2.6.16 yet which conflict.

The patches are also available in a git-tree:

http://people.netfilter.org/kaber/nf-2.6-xfrm.git/

[NETFILTER]: Remove okfn usage in ip_vs_core.c
[NETFILTER]: Call POST_ROUTING hook before fragmentation [IPV4]: Replace dst_output by ip_dst_output
[IPV6]: Replace dst_output by ip6_dst_output
[IPV4/6]: Netfilter IPsec output hooks
[IPV4/6]: Netfilter IPsec input hooks
[NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder [NETFILTER]: Use conntrack information to determine if packet was NATed [NETFILTER]: Redo policy lookups after NAT when neccessary [NETFILTER]: Keep the conntrack reference until after policy checks [NETFILTER]: Handle NAT in IPsec policy checks
[NETFILTER]: Export ip6_masked_addrcmp, don't pass IPv6 addresses on stack [NETFILTER]: Add ipt_policy/ip6t_policy matches

#1 lore raw

PATCH 1/13 : [NETFILTER]: Remove okfn usage in ip_vs_core.c

[NETFILTER]: Remove okfn usage in ip_vs_core.c

okfn should only be used from different contexts to avoid deep call stacks, i.e. by nf_queue.

---
commit ebb0baec0a5e909d4acf16a15601f013093fefb3
tree da877af1ad83666e8ccd54dd94da485444e6b37b
parent b286e39207237e2f6929959372bf66d9a8d05a82
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:48:33 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:48:33 +0100

 net/ipv4/ipvs/ip_vs_core.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)
net/ipv4/ipvs/ip_vs_core.c +1 -4
index 1a0843c..b63bb28 100644
@@ -532,11 +532,8 @@ static unsigned int ip_vs_post_routing(u
 {
 	if (!((*pskb)->ipvs_property))
 		return NF_ACCEPT;
-
 	/* The packet was sent from IPVS, exit this chain */
-	(*okfn)(*pskb);
-
-	return NF_STOLEN;
+	return NF_STOP;
 }
 
 u16 ip_vs_checksum_complete(struct sk_buff *skb, int offset)
#2 lore raw

PATCH 2/13 : [NETFILTER]: Call POST_ROUTING hook before fragmentation

In reply to Patrick McHardy

[NETFILTER]: Call POST_ROUTING hook before fragmentation

Call POST_ROUTING hook before fragmentation to get rid of the okfn use in ip_refrag and save the useless fragmentation/defragmentation step when NAT is used.

The patch introduces one user-visible change, the POSTROUTING chain in the mangle table gets entire packets, not fragments, which should simplify use of the MARK and CLASSIFY targets for queueing as a nice side-effect.

---
commit d3c70d774e32c4d6f4cc6b8b0b73678aa14a9932
tree 16b82a0bedbb29b6a709140a7047ce2d52bb776f
parent ebb0baec0a5e909d4acf16a15601f013093fefb3
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:11 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:11 +0100

 include/net/ip.h                               |    1 -
 net/ipv4/ip_output.c                           |   30 +++++++++++-------------
 net/ipv4/netfilter/ip_conntrack_standalone.c   |   26 +--------------------
 net/ipv4/netfilter/ip_nat_standalone.c         |   17 --------------
 net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c |   26 +--------------------
 5 files changed, 16 insertions(+), 84 deletions(-)
include/net/ip.h +0 -1
index e4563bb..9f09882 100644
@@ -310,7 +310,6 @@ enum ip_defrag_users
 	IP_DEFRAG_CALL_RA_CHAIN,
 	IP_DEFRAG_CONNTRACK_IN,
 	IP_DEFRAG_CONNTRACK_OUT,
-	IP_DEFRAG_NAT_OUT,
 	IP_DEFRAG_VS_IN,
 	IP_DEFRAG_VS_OUT,
 	IP_DEFRAG_VS_FWD
net/ipv4/ip_output.c +14 -16
index 11c2f68..946e812 100644
@@ -202,13 +202,11 @@ static inline int ip_finish_output2(stru
 
 static inline int ip_finish_output(struct sk_buff *skb)
 {
-	struct net_device *dev = skb->dst->dev;
-
-	skb->dev = dev;
-	skb->protocol = htons(ETH_P_IP);
-
-	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, dev,
-		       ip_finish_output2);
+	if (skb->len > dst_mtu(skb->dst) &&
+	    !(skb_shinfo(skb)->ufo_size || skb_shinfo(skb)->tso_size))
+		return ip_fragment(skb, ip_finish_output2);
+	else
+		return ip_finish_output2(skb);
 }
 
 int ip_mc_output(struct sk_buff *skb)
@@ -265,21 +263,21 @@ int ip_mc_output(struct sk_buff *skb)
 				newskb->dev, ip_dev_loopback_xmit);
 	}
 
-	if (skb->len > dst_mtu(&rt->u.dst))
-		return ip_fragment(skb, ip_finish_output);
-	else
-		return ip_finish_output(skb);
+	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, skb->dev,
+		       ip_finish_output);
 }
 
 int ip_output(struct sk_buff *skb)
 {
+	struct net_device *dev = skb->dst->dev;
+
 	IP_INC_STATS(IPSTATS_MIB_OUTREQUESTS);
 
-	if (skb->len > dst_mtu(skb->dst) &&
-		!(skb_shinfo(skb)->ufo_size || skb_shinfo(skb)->tso_size))
-		return ip_fragment(skb, ip_finish_output);
-	else
-		return ip_finish_output(skb);
+	skb->dev = dev;
+	skb->protocol = htons(ETH_P_IP);
+
+	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, dev,
+		       ip_finish_output);
 }
 
 int ip_queue_xmit(struct sk_buff *skb, int ipfragok)
net/ipv4/netfilter/ip_conntrack_standalone.c +1 -25
index dd476b1..13ed18a 100644
@@ -450,30 +450,6 @@ static unsigned int ip_conntrack_defrag(
 	return NF_ACCEPT;
 }
 
-static unsigned int ip_refrag(unsigned int hooknum,
-			      struct sk_buff **pskb,
-			      const struct net_device *in,
-			      const struct net_device *out,
-			      int (*okfn)(struct sk_buff *))
-{
-	struct rtable *rt = (struct rtable *)(*pskb)->dst;
-
-	/* We've seen it coming out the other side: confirm */
-	if (ip_confirm(hooknum, pskb, in, out, okfn) != NF_ACCEPT)
-		return NF_DROP;
-
-	/* Local packets are never produced too large for their
-	   interface.  We degfragment them at LOCAL_OUT, however,
-	   so we have to refragment them here. */
-	if ((*pskb)->len > dst_mtu(&rt->u.dst) &&
-	    !skb_shinfo(*pskb)->tso_size) {
-		/* No hook can be after us, so this should be OK. */
-		ip_fragment(*pskb, okfn);
-		return NF_STOLEN;
-	}
-	return NF_ACCEPT;
-}
-
 static unsigned int ip_conntrack_local(unsigned int hooknum,
 				       struct sk_buff **pskb,
 				       const struct net_device *in,
@@ -543,7 +519,7 @@ static struct nf_hook_ops ip_conntrack_h
 
 /* Refragmenter; last chance. */
 static struct nf_hook_ops ip_conntrack_out_ops = {
-	.hook		= ip_refrag,
+	.hook		= ip_confirm,
 	.owner		= THIS_MODULE,
 	.pf		= PF_INET,
 	.hooknum	= NF_IP_POST_ROUTING,
net/ipv4/netfilter/ip_nat_standalone.c +0 -17
index 30cd4e1..f04111f 100644
@@ -190,23 +190,6 @@ ip_nat_out(unsigned int hooknum,
 	    || (*pskb)->nh.iph->ihl * 4 < sizeof(struct iphdr))
 		return NF_ACCEPT;
 
-	/* We can hit fragment here; forwarded packets get
-	   defragmented by connection tracking coming in, then
-	   fragmented (grr) by the forward code.
-
-	   In future: If we have nfct != NULL, AND we have NAT
-	   initialized, AND there is no helper, then we can do full
-	   NAPT on the head, and IP-address-only NAT on the rest.
-
-	   I'm starting to have nightmares about fragments.  */
-
-	if ((*pskb)->nh.iph->frag_off & htons(IP_MF|IP_OFFSET)) {
-		*pskb = ip_ct_gather_frags(*pskb, IP_DEFRAG_NAT_OUT);
-
-		if (!*pskb)
-			return NF_STOLEN;
-	}
-
 	return ip_nat_fn(hooknum, pskb, in, out, okfn);
 }
 
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c +1 -25
index 8202c1c..818ba72 100644
@@ -180,30 +180,6 @@ static unsigned int ipv4_conntrack_defra
 	return NF_ACCEPT;
 }
 
-static unsigned int ipv4_refrag(unsigned int hooknum,
-				struct sk_buff **pskb,
-				const struct net_device *in,
-				const struct net_device *out,
-				int (*okfn)(struct sk_buff *))
-{
-	struct rtable *rt = (struct rtable *)(*pskb)->dst;
-
-	/* We've seen it coming out the other side: confirm */
-	if (ipv4_confirm(hooknum, pskb, in, out, okfn) != NF_ACCEPT)
-		return NF_DROP;
-
-	/* Local packets are never produced too large for their
-	   interface.  We degfragment them at LOCAL_OUT, however,
-	   so we have to refragment them here. */
-	if ((*pskb)->len > dst_mtu(&rt->u.dst) &&
-	    !skb_shinfo(*pskb)->tso_size) {
-		/* No hook can be after us, so this should be OK. */
-		ip_fragment(*pskb, okfn);
-		return NF_STOLEN;
-	}
-	return NF_ACCEPT;
-}
-
 static unsigned int ipv4_conntrack_in(unsigned int hooknum,
 				      struct sk_buff **pskb,
 				      const struct net_device *in,
@@ -283,7 +259,7 @@ static struct nf_hook_ops ipv4_conntrack
 
 /* Refragmenter; last chance. */
 static struct nf_hook_ops ipv4_conntrack_out_ops = {
-	.hook		= ipv4_refrag,
+	.hook		= ipv4_confirm,
 	.owner		= THIS_MODULE,
 	.pf		= PF_INET,
 	.hooknum	= NF_IP_POST_ROUTING,
#3 lore raw

PATCH 3/13 : [IPV4]: Replace dst_output by ip_dst_output

In reply to Patrick McHardy

[IPV4]: Replace dst_output by ip_dst_output

Preparation for netfilter IPsec support.

---
commit 4eb320a6444a9035da8a83e4886b3691a2ea98f7
tree d31f7b331e06e1e598593c4095be7713e6fd3ba0
parent d3c70d774e32c4d6f4cc6b8b0b73678aa14a9932
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:28 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:28 +0100

 include/net/dst.h               |    2 ++
 include/net/ipip.h              |    2 +-
 net/ipv4/igmp.c                 |    4 ++--
 net/ipv4/ip_forward.c           |    2 +-
 net/ipv4/ip_output.c            |    6 +++---
 net/ipv4/ipmr.c                 |    2 +-
 net/ipv4/ipvs/ip_vs_xmit.c      |    2 +-
 net/ipv4/netfilter/ipt_REJECT.c |    2 +-
 net/ipv4/raw.c                  |    2 +-
 9 files changed, 13 insertions(+), 11 deletions(-)
include/net/dst.h +2 -0
index 6c196a5..07f552b 100644
@@ -236,6 +236,8 @@ static inline int dst_output(struct sk_b
 	}
 }
 
+#define ip_dst_output	dst_output
+
 /* Input packet from network to transport.  */
 static inline int dst_input(struct sk_buff *skb)
 {
include/net/ipip.h +1 -1
index f490c3c..b267496 100644
@@ -34,7 +34,7 @@ struct ip_tunnel
 	ip_select_ident(iph, &rt->u.dst, NULL);				\
 	ip_send_check(iph);						\
 									\
-	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev, dst_output);\
+	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev, ip_dst_output);\
 	if (err == NET_XMIT_SUCCESS || err == NET_XMIT_CN) {		\
 		stats->tx_bytes += pkt_len;				\
 		stats->tx_packets++;					\
net/ipv4/igmp.c +2 -2
index c04607b..55779dc 100644
@@ -343,7 +343,7 @@ static int igmpv3_sendpack(struct sk_buf
 	pig->csum = ip_compute_csum((void *)skb->h.igmph, igmplen);
 
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dev,
-		       dst_output);
+		       ip_dst_output);
 }
 
 static int grec_size(struct ip_mc_list *pmc, int type, int gdel, int sdel)
@@ -674,7 +674,7 @@ static int igmp_send_report(struct in_de
 	ih->csum=ip_compute_csum((void *)ih, sizeof(struct igmphdr));
 
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		       dst_output);
+		       ip_dst_output);
 }
 
 static void igmp_gq_timer_expire(unsigned long data)
net/ipv4/ip_forward.c +1 -1
index 0923add..486355d 100644
@@ -51,7 +51,7 @@ static inline int ip_forward_finish(stru
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
-	return dst_output(skb);
+	return ip_dst_output(skb);
 }
 
 int ip_forward(struct sk_buff *skb)
net/ipv4/ip_output.c +3 -3
index 946e812..2c91f03 100644
@@ -155,7 +155,7 @@ int ip_build_and_send_pkt(struct sk_buff
 
 	/* Send it out. */
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		       dst_output);
+		       ip_dst_output);
 }
 
 EXPORT_SYMBOL_GPL(ip_build_and_send_pkt);
@@ -360,7 +360,7 @@ packet_routed:
 	skb->priority = sk->sk_priority;
 
 	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		       dst_output);
+		       ip_dst_output);
 
 no_route:
 	IP_INC_STATS(IPSTATS_MIB_OUTNOROUTES);
@@ -1251,7 +1251,7 @@ int ip_push_pending_frames(struct sock *
 
 	/* Netfilter gets whole the not fragmented skb. */
 	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, 
-		      skb->dst->dev, dst_output);
+		      skb->dst->dev, ip_dst_output);
 	if (err) {
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
net/ipv4/ipmr.c +1 -1
index 302b7eb..40af27f 100644
@@ -1125,7 +1125,7 @@ static inline int ipmr_forward_finish(st
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
-	return dst_output(skb);
+	return ip_dst_output(skb);
 }
 
 /*
net/ipv4/ipvs/ip_vs_xmit.c +1 -1
index 3b87482..b66c3da 100644
@@ -130,7 +130,7 @@ do {							\
 	(skb)->ipvs_property = 1;			\
 	(skb)->ip_summed = CHECKSUM_NONE;		\
 	NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, (skb), NULL,	\
-		(rt)->u.dst.dev, dst_output);		\
+		(rt)->u.dst.dev, ip_dst_output);	\
 } while (0)
 
 
net/ipv4/netfilter/ipt_REJECT.c +1 -1
index f057025..55b601b 100644
@@ -220,7 +220,7 @@ static void send_reset(struct sk_buff *o
 	nf_ct_attach(nskb, oldskb);
 
 	NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, nskb, NULL, nskb->dst->dev,
-		dst_output);
+		ip_dst_output);
 	return;
 
  free_nskb:
net/ipv4/raw.c +1 -1
index 4b0d7e4..421538a 100644
@@ -313,7 +313,7 @@ static int raw_send_hdrinc(struct sock *
 	}
 
 	err = NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		      dst_output);
+		      ip_dst_output);
 	if (err > 0)
 		err = inet->recverr ? net_xmit_errno(err) : 0;
 	if (err)
#4 lore raw

PATCH 4/13 : [IPV6]: Replace dst_output by ip6_dst_output

In reply to Patrick McHardy

[IPV6]: Replace dst_output by ip6_dst_output

Preparation for netfilter IPsec support.

---
commit 73f59ffcebcd0a08f6a405c8522074e8b5892b73
tree 4be1e3bb174f611fa57ee6e1b8d9187e784c85ad
parent 4eb320a6444a9035da8a83e4886b3691a2ea98f7
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:40 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:40 +0100

 include/net/dst.h                |    1 +
 net/ipv6/ip6_input.c             |    4 ++--
 net/ipv6/ip6_output.c            |    7 ++++---
 net/ipv6/ip6_tunnel.c            |    2 +-
 net/ipv6/ndisc.c                 |    8 ++++----
 net/ipv6/netfilter/ip6t_REJECT.c |    2 +-
 net/ipv6/raw.c                   |    2 +-
 7 files changed, 14 insertions(+), 12 deletions(-)
include/net/dst.h +1 -0
index 07f552b..4886f25 100644
@@ -237,6 +237,7 @@ static inline int dst_output(struct sk_b
 }
 
 #define ip_dst_output	dst_output
+#define ip6_dst_output	dst_output
 
 /* Input packet from network to transport.  */
 static inline int dst_input(struct sk_buff *skb)
net/ipv6/ip6_input.c +2 -2
index a6026d2..33d3b0e 100644
@@ -255,9 +255,9 @@ int ip6_mc_input(struct sk_buff *skb)
 			
 			if (deliver) {
 				skb2 = skb_clone(skb, GFP_ATOMIC);
-				dst_output(skb2);
+				ip6_dst_output(skb2);
 			} else {
-				dst_output(skb);
+				ip6_dst_output(skb);
 				return 0;
 			}
 		}
net/ipv6/ip6_output.c +4 -3
index c1fa693..c270798 100644
@@ -230,7 +230,7 @@ int ip6_xmit(struct sock *sk, struct sk_
 	if ((skb->len <= mtu) || ipfragok) {
 		IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);
 		return NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev,
-				dst_output);
+				ip6_dst_output);
 	}
 
 	if (net_ratelimit())
@@ -308,7 +308,7 @@ static int ip6_call_ra_chain(struct sk_b
 
 static inline int ip6_forward_finish(struct sk_buff *skb)
 {
-	return dst_output(skb);
+	return ip6_dst_output(skb);
 }
 
 int ip6_forward(struct sk_buff *skb)
@@ -1181,7 +1181,8 @@ int ip6_push_pending_frames(struct sock 
 
 	skb->dst = dst_clone(&rt->u.dst);
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);	
-	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, skb->dst->dev, dst_output);
+	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, skb->dst->dev,
+	              ip6_dst_output);
 	if (err) {
 		if (err > 0)
 			err = np->recverr ? net_xmit_errno(err) : 0;
net/ipv6/ip6_tunnel.c +1 -1
index e315d0f..7c01eaf 100644
@@ -746,7 +746,7 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, str
 	nf_reset(skb);
 	pkt_len = skb->len;
 	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, 
-		      skb->dst->dev, dst_output);
+		      skb->dst->dev, ip6_dst_output);
 
 	if (err == NET_XMIT_SUCCESS || err == NET_XMIT_CN) {
 		stats->tx_bytes += pkt_len;
net/ipv6/ndisc.c +4 -4
index 305d9ee..287170f 100644
@@ -499,7 +499,7 @@ static void ndisc_send_na(struct net_dev
 	skb->dst = dst;
 	idev = in6_dev_get(dst->dev);
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);
-	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, dst_output);
+	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, ip6_dst_output);
 	if (!err) {
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTNEIGHBORADVERTISEMENTS);
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTMSGS);
@@ -582,7 +582,7 @@ void ndisc_send_ns(struct net_device *de
 	skb->dst = dst;
 	idev = in6_dev_get(dst->dev);
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);
-	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, dst_output);
+	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, ip6_dst_output);
 	if (!err) {
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTNEIGHBORSOLICITS);
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTMSGS);
@@ -654,7 +654,7 @@ void ndisc_send_rs(struct net_device *de
 	skb->dst = dst;
 	idev = in6_dev_get(dst->dev);
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);	
-	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, dst_output);
+	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev, ip6_dst_output);
 	if (!err) {
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTROUTERSOLICITS);
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTMSGS);
@@ -1438,7 +1438,7 @@ void ndisc_send_redirect(struct sk_buff 
 	buff->dst = dst;
 	idev = in6_dev_get(dst->dev);
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);
-	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, buff, NULL, dst->dev, dst_output);
+	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, buff, NULL, dst->dev, ip6_dst_output);
 	if (!err) {
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTREDIRECTS);
 		ICMP6_INC_STATS(idev, ICMP6_MIB_OUTMSGS);
net/ipv6/netfilter/ip6t_REJECT.c +1 -1
index b03e87a..8579eaa 100644
@@ -161,7 +161,7 @@ static void send_reset(struct sk_buff *o
 						   sizeof(struct tcphdr), 0));
 
 	NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, nskb, NULL, nskb->dst->dev,
-		dst_output);
+		ip6_dst_output);
 }
 
 static inline void
net/ipv6/raw.c +1 -1
index 8e9628f..e7fd031 100644
@@ -571,7 +571,7 @@ static int rawv6_send_hdrinc(struct sock
 
 	IP6_INC_STATS(IPSTATS_MIB_OUTREQUESTS);		
 	err = NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, rt->u.dst.dev,
-		      dst_output);
+		      ip6_dst_output);
 	if (err > 0)
 		err = np->recverr ? net_xmit_errno(err) : 0;
 	if (err)
#5 lore raw

PATCH 5/13 : [IPV4/6]: Netfilter IPsec output hooks

In reply to Patrick McHardy

[IPV4/6]: Netfilter IPsec output hooks

Add alternative ip_dst_output/ip6_dst_output functions to call netfilter hooks between xfrm transforms. Packets visit the FORWARD/LOCAL_OUT and POST_ROUTING hook before encapsulation and the LOCAL_OUT and POST_ROUTING hook after each tunnel mode transform.

---
commit b847425c693f43a63137d18e36e5c8cf0187c175
tree a811e8c573150bc279a9df53958270f25cb531bc
parent 73f59ffcebcd0a08f6a405c8522074e8b5892b73
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:58 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:49:58 +0100

 include/net/dst.h       |    5 +++++
 net/ipv4/netfilter.c    |   31 ++++++++++++++++++++++++++++++-
 net/ipv4/xfrm4_output.c |    1 +
 net/ipv6/netfilter.c    |   29 +++++++++++++++++++++++++++++
 net/ipv6/xfrm6_output.c |    1 +
 5 files changed, 66 insertions(+), 1 deletions(-)
include/net/dst.h +5 -0
index 4886f25..7eadd0c 100644
@@ -236,8 +236,13 @@ static inline int dst_output(struct sk_b
 	}
 }
 
+#if defined(CONFIG_XFRM) && defined(CONFIG_NETFILTER)
+extern int ip_dst_output(struct sk_buff *skb);
+extern int ip6_dst_output(struct sk_buff *skb);
+#else
 #define ip_dst_output	dst_output
 #define ip6_dst_output	dst_output
+#endif
 
 /* Input packet from network to transport.  */
 static inline int dst_input(struct sk_buff *skb)
net/ipv4/netfilter.c +30 -1
index ae0779d..b93e7cd 100644
@@ -10,8 +10,9 @@
 #include <linux/tcp.h>
 #include <linux/udp.h>
 #include <linux/icmp.h>
-#include <net/route.h>
 #include <linux/ip.h>
+#include <net/route.h>
+#include <net/xfrm.h>
 
 /* route_me_harder function, used by iptable_nat, iptable_mangle + ip_queue */
 int ip_route_me_harder(struct sk_buff **pskb)
@@ -78,6 +79,34 @@ int ip_route_me_harder(struct sk_buff **
 }
 EXPORT_SYMBOL(ip_route_me_harder);
 
+#ifdef CONFIG_XFRM
+static inline int __ip_dst_output(struct sk_buff *skb)
+{
+	int err;
+
+	do {
+		err = skb->dst->output(skb);
+
+		if (likely(err == 0))
+			return err;
+		if (unlikely(err != NET_XMIT_BYPASS))
+			return err;
+	} while (skb->dst->xfrm && !skb->dst->xfrm->props.mode);
+
+	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dst->dev,
+	               ip_dst_output);
+}
+
+int ip_dst_output(struct sk_buff *skb)
+{
+	if (skb->dst->xfrm != NULL)
+		return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL,
+		               skb->dst->dev, __ip_dst_output);
+	return dst_output(skb);
+}
+EXPORT_SYMBOL(ip_dst_output);
+#endif /* CONFIG_XFRM */
+
 /*
  * Extra routing may needed on local out, as the QUEUE target never
  * returns control to the table.
net/ipv4/xfrm4_output.c +1 -0
index 66620a9..c135746 100644
@@ -133,6 +133,7 @@ int xfrm4_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	nf_reset(skb);
 	err = NET_XMIT_BYPASS;
 
 out_exit:
net/ipv6/netfilter.c +29 -0
index f8626eb..06b275e 100644
@@ -10,6 +10,7 @@
 #include <net/dst.h>
 #include <net/ipv6.h>
 #include <net/ip6_route.h>
+#include <net/xfrm.h>
 
 int ip6_route_me_harder(struct sk_buff *skb)
 {
@@ -41,6 +42,34 @@ int ip6_route_me_harder(struct sk_buff *
 }
 EXPORT_SYMBOL(ip6_route_me_harder);
 
+#ifdef CONFIG_XFRM
+static inline int __ip6_dst_output(struct sk_buff *skb)
+{
+	int err;
+	
+	do {
+		err = skb->dst->output(skb);
+
+		if (likely(err == 0))
+			return err;
+		if (unlikely(err != NET_XMIT_BYPASS))
+			return err;
+	} while (skb->dst->xfrm && !skb->dst->xfrm->props.mode);
+
+	return NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, skb->dst->dev,
+	               ip6_dst_output);
+}
+
+int ip6_dst_output(struct sk_buff *skb)
+{
+	if (skb->dst->xfrm != NULL)
+		return NF_HOOK(PF_INET6, NF_IP6_POST_ROUTING, skb, NULL,
+		               skb->dst->dev, __ip6_dst_output);
+	return dst_output(skb);
+}
+EXPORT_SYMBOL(ip6_dst_output);
+#endif /* CONFIG_XFRM */
+
 /*
  * Extra routing may needed on local out, as the QUEUE target never
  * returns control to the table.
net/ipv6/xfrm6_output.c +1 -0
index 6b98677..a566d25 100644
@@ -132,6 +132,7 @@ int xfrm6_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	nf_reset(skb);
 	err = NET_XMIT_BYPASS;
 
 out_exit:
maintainer
#6 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 83

Quoted 11 lines · Patrick McHardy
net/ipv4/netfilter.c +2 -0
index ae0779d..b93e7cd 100644
@@ -78,6 +79,34 @@ int ip_route_me_harder(struct sk_buff **
 }
 EXPORT_SYMBOL(ip_route_me_harder);
 
+#ifdef CONFIG_XFRM
+static inline int __ip_dst_output(struct sk_buff *skb)

I'd like to suggest an alternative way of doing this that

1) Keeps this XFRM stuff in xfrm*.c.
2) Removes the need for ip_dst_output.

Please see the attached patch.

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 94

Quoted 11 lines
net/ipv4/netfilter.c +11 -0
+	do {
+		err = skb->dst->output(skb);
+
+		if (likely(err == 0))
+			return err;
+		if (unlikely(err != NET_XMIT_BYPASS))
+			return err;
+	} while (skb->dst->xfrm && !skb->dst->xfrm->props.mode);
+
+	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dst->dev,
+	               ip_dst_output);

The idea is simply to put this stuff in xfrm[46]_output directly. So for your patch you would simply need to add the two NF_HOOK calls at the beginning and end of xfrm[46]_output once they've been modified in the way I outline below.

net/ipv4/xfrm4_output.c int xfrm4_output(struct sk_buff *skb) · line 137

Quoted 10 lines
net/ipv4/xfrm4_output.c +1 -0
index 66620a9..c135746 100644
@@ -133,6 +133,7 @@ int xfrm4_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	nf_reset(skb);
 	err = NET_XMIT_BYPASS;

Shouldn't this sit after POST_ROUTING, i.e., once the connection has been confirmed?

Cheers,

Signature · 5 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
include/net/ip.h +0 -0
net/ipv4/xfrm4_output.c +22 -17
@@ -113,26 +113,31 @@ int xfrm4_output(struct sk_buff *skb)
 			goto error_nolock;
 	}
 
-	spin_lock_bh(&x->lock);
-	err = xfrm_state_check(x, skb);
-	if (err)
-		goto error;
-
-	xfrm4_encap(skb);
-
-	err = x->type->output(x, skb);
-	if (err)
-		goto error;
+	do {
+		spin_lock_bh(&x->lock);
+		err = xfrm_state_check(x, skb);
+		if (err)
+			goto error;
+
+		xfrm4_encap(skb);
+
+		err = x->type->output(x, skb);
+		if (err)
+			goto error;
 
-	x->curlft.bytes += skb->len;
-	x->curlft.packets++;
+		x->curlft.bytes += skb->len;
+		x->curlft.packets++;
 
-	spin_unlock_bh(&x->lock);
+		spin_unlock_bh(&x->lock);
 	
-	if (!(skb->dst = dst_pop(dst))) {
-		err = -EHOSTUNREACH;
-		goto error_nolock;
-	}
+		if (!(skb->dst = dst_pop(dst))) {
+			err = -EHOSTUNREACH;
+			goto error_nolock;
+		}
+		dst = skb->dst;
+		x = dst->xfrm;
+	} while (x && !x->props.mode);
+
 	err = NET_XMIT_BYPASS;
 
 out_exit:
net/ipv6/xfrm6_output.c +26 -21

54 lines not on this page · raw message

#7 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 31 lines · Herbert Xu

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 83

Quoted 10 lines · Patrick McHardy
net/ipv4/netfilter.c +2 -0
index ae0779d..b93e7cd 100644
@@ -78,6 +79,34 @@ int ip_route_me_harder(struct sk_buff **
 }
 EXPORT_SYMBOL(ip_route_me_harder);
 
+#ifdef CONFIG_XFRM
+static inline int __ip_dst_output(struct sk_buff *skb)

I'd like to suggest an alternative way of doing this that

1) Keeps this XFRM stuff in xfrm*.c.
2) Removes the need for ip_dst_output.

Please see the attached patch.

Quoted 9 lines
net/ipv4/netfilter.c +9 -0
+	do {
+		err = skb->dst->output(skb);
+
+		if (likely(err == 0))
+			return err;
+		if (unlikely(err != NET_XMIT_BYPASS))
+			return err;
+	} while (skb->dst->xfrm && !skb->dst->xfrm->props.mode);
+
Quoted 8 lines

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 94

Quoted 2 lines
net/ipv4/netfilter.c +2 -0
+	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dst->dev,
+	               ip_dst_output);

The idea is simply to put this stuff in xfrm[46]_output directly. So for your patch you would simply need to add the two NF_HOOK calls at the beginning and end of xfrm[46]_output once they've been modified in the way I outline below.

This looks nice, but placing the hooks at the end of the xfrm[46] functions doesn't work with queueing without recursively calling dst_output (as okfn) since we have to provide an okfn but also have to return ownership of the skb back to dst_output.

Quoted 6 lines
Quoted 6 lines
net/ipv4/xfrm4_output.c +0 -0
index 66620a9..c135746 100644
@@ -133,6 +133,7 @@ int xfrm4_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
Quoted 8 lines

net/ipv4/xfrm4_output.c int xfrm4_output(struct sk_buff *skb) · line 141

Quoted 4 lines
net/ipv4/xfrm4_output.c +1 -0
 		goto error_nolock;
 	}
+	nf_reset(skb);
 	err = NET_XMIT_BYPASS;

Shouldn't this sit after POST_ROUTING, i.e., once the connection has been confirmed?

This is after POST_ROUTING :) POST_ROUTING is called before the transforms and LOCAL_OUT afterwards. But it could be moved to the ip/ip6_dst_output functions to avoid unnecessarily trying to reset the skb for transport mode transforms.

#8 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 45 lines · Patrick McHardy
Quoted 33 lines · Herbert Xu

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 83

Quoted 10 lines · Patrick McHardy
net/ipv4/netfilter.c +2 -0
index ae0779d..b93e7cd 100644
@@ -78,6 +79,34 @@ int ip_route_me_harder(struct sk_buff **
 }
 EXPORT_SYMBOL(ip_route_me_harder);
 
+#ifdef CONFIG_XFRM
+static inline int __ip_dst_output(struct sk_buff *skb)

I'd like to suggest an alternative way of doing this that

1) Keeps this XFRM stuff in xfrm*.c.
2) Removes the need for ip_dst_output.

Please see the attached patch.

Quoted 10 lines
net/ipv4/netfilter.c +10 -0
+    do {
+        err = skb->dst->output(skb);
+
+        if (likely(err == 0))
+            return err;
+        if (unlikely(err != NET_XMIT_BYPASS))
+            return err;
+    } while (skb->dst->xfrm && !skb->dst->xfrm->props.mode);
+
+    return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, skb->dst->dev,
Quoted 8 lines

net/ipv4/netfilter.c int ip_route_me_harder(struct sk_buff ** · line 94

Quoted 1 line
net/ipv4/netfilter.c +1 -0
+                   ip_dst_output);

The idea is simply to put this stuff in xfrm[46]_output directly. So for your patch you would simply need to add the two NF_HOOK calls at the beginning and end of xfrm[46]_output once they've been modified in the way I outline below.

Quoted 4 lines

This looks nice, but placing the hooks at the end of the xfrm[46] functions doesn't work with queueing without recursively calling dst_output (as okfn) since we have to provide an okfn but also have to return ownership of the skb back to dst_output.

I should add, the same affects ip_dst_output/__ip_dst_output of course, which is why they do call themselves recursively. But since __ip_dst_output is an inline function and is not called through the function pointer except from a different context (ip_queue), the compiler does a good job at eliminating the recursion for the inlined version. This probably wouldn't work if we used a recursive dst_output call in xfrm[46]_output. But your patch looks like a nice idea anyway, so how about we move the looping to xfrm[46]_input and keep ip/ip6_dst_output for the hooks?

maintainer
#9 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

In reply to Patrick McHardy
Quoted 5 lines · Patrick McHardy

This looks nice, but placing the hooks at the end of the xfrm[46] functions doesn't work with queueing without recursively calling dst_output (as okfn) since we have to provide an okfn but also have to return ownership of the skb back to dst_output.

This patch (on top of the last one) is what we could do to eliminate the need to return control to dst_output. The only reason for dst_output to exist is to handle compilers that can't optimise away tail calls. So if we are going to rely on the compiler to do away with tail calls (ip_dst_output <-> __ip_dst_output), then we might as well get rid of the loop in dst_output.

BTW, I killed the corresponding inline (which would have gone onto xfrm[46]_output_finish) because the compiler should be able to optimise it into a tail call. If it doesn't, then we're in trouble anyway since it won't be able to optimise away the call to dst_output.

Quoted 4 lines

This is after POST_ROUTING :) POST_ROUTING is called before the transforms and LOCAL_OUT afterwards. But it could be moved to the ip/ip6_dst_output functions to avoid unnecessarily trying to reset the skb for transport mode transforms.

You're absolute right. I somehow mistook LOCAL_OUT for POST_ROUTING :)

Cheers,

Signature · 5 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
include/net/dst.h +1 -10
@@ -224,16 +224,7 @@ static inline void dst_set_expires(struc
 /* Output packet to network from transport.  */
 static inline int dst_output(struct sk_buff *skb)
 {
-	int err;
-
-	for (;;) {
-		err = skb->dst->output(skb);
-
-		if (likely(err == 0))
-			return err;
-		if (unlikely(err != NET_XMIT_BYPASS))
-			return err;
-	}
+	return skb->dst->output(skb);
 }
 
 /* Input packet from network to transport.  */
net/ipv4/xfrm4_output.c +13 -5
@@ -10,6 +10,7 @@
 
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
+#include <linux/netfilter_ipv4.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -95,7 +96,7 @@ out:
 	return ret;
 }
 
-int xfrm4_output(struct sk_buff *skb)
+static int xfrm4_output_finish(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -138,13 +139,20 @@ int xfrm4_output(struct sk_buff *skb)
 		x = dst->xfrm;
 	} while (x && !x->props.mode);
 
-	err = NET_XMIT_BYPASS;
+	nf_reset(skb);
+	
+	return NF_HOOK(PF_INET, NF_IP_LOCAL_OUT, skb, NULL, dst->dev,
+		       dst_output);
 
-out_exit:
-	return err;
 error:
 	spin_unlock_bh(&x->lock);
 error_nolock:
 	kfree_skb(skb);
-	goto out_exit;
+	return err;
+}
+
+int xfrm4_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm4_output_finish);
 }
net/ipv6/xfrm6_output.c +13 -5
@@ -12,6 +12,7 @@
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
 #include <linux/icmpv6.h>
+#include <linux/netfilter_ipv6.h>
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/ipv6.h>
@@ -92,7 +93,7 @@ static int xfrm6_tunnel_check_size(struc
 	return ret;
 }
 
-int xfrm6_output(struct sk_buff *skb)
+static int xfrm6_output_finish(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -137,13 +138,20 @@ int xfrm6_output(struct sk_buff *skb)
 		x = dst->xfrm;
 	} while (x && !x->props.mode);
 
-	err = NET_XMIT_BYPASS;
+	nf_reset(skb);
+	
+	return NF_HOOK(PF_INET6, NF_IP6_LOCAL_OUT, skb, NULL, dst->dev,
+		       dst_output);
 
-out_exit:
-	return err;
 error:
 	spin_unlock_bh(&x->lock);
 error_nolock:
 	kfree_skb(skb);
-	goto out_exit;
+	return err;
+}
+
+int xfrm6_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET6, NF_IP6_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm6_output_finish);
 }
maintainer
#10 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 6 lines · herbert

the need to return control to dst_output. The only reason for dst_output to exist is to handle compilers that can't optimise away tail calls. So if we are going to rely on the compiler to do away with tail calls (ip_dst_output <-> __ip_dst_output), then we might as well get rid of the loop in dst_output.

Unfortunately it looks like gcc 3.3.5 at least is too dumb to optimise it away. I think we'll need a better strategy.

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#11 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 3 lines · herbert

Unfortunately it looks like gcc 3.3.5 at least is too dumb to optimise it away. I think we'll need a better strategy.

OK, the idea is still the same: Move the loop from dst_output into xfrm4_output/xfrm6_output since they're the only ones who need to it.

In order to avoid the tail call issue, I've added the inline function nf_hook which is nf_hook_slow plus the empty list check.

Cheers,

Signature · 5 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
include/linux/netfilter.h +37 -24
@@ -168,6 +168,37 @@ void nf_log_packet(int pf,
 		   const struct net_device *out,
 		   struct nf_loginfo *li,
 		   const char *fmt, ...);
+
+int nf_hook_slow(int pf, unsigned int hook, struct sk_buff **pskb,
+		 struct net_device *indev, struct net_device *outdev,
+		 int (*okfn)(struct sk_buff *), int thresh);
+
+/**
+ *	nf_hook_thresh - call a netfilter hook
+ *	
+ *	Returns 1 if the hook has allowed the packet to pass.  The function
+ *	okfn must be invoked by the caller in this case.  Any other return
+ *	value indicates the packet has been consumed by the hook.
+ */
+static inline int nf_hook_thresh(int pf, unsigned int hook,
+				 struct sk_buff **pskb,
+				 struct net_device *indev,
+				 struct net_device *outdev,
+				 int (*okfn)(struct sk_buff *), int thresh)
+{
+#ifndef CONFIG_NETFILTER_DEBUG
+	if (list_empty(&nf_hooks[pf][hook]))
+		return 1;
+#endif
+	return nf_hook_slow(pf, hook, pskb, indev, outdev, okfn, thresh);
+}
+
+static inline int nf_hook(int pf, unsigned int hook, struct sk_buff **pskb,
+			  struct net_device *indev, struct net_device *outdev,
+			  int (*okfn)(struct sk_buff *))
+{
+	return nf_hook_thresh(pf, hook, pskb, indev, outdev, okfn, INT_MIN);
+}
                    
 /* Activate hook; either okfn or kfree_skb called, unless a hook
    returns NF_STOLEN (in which case, it's up to the hook to deal with
@@ -188,35 +219,17 @@ void nf_log_packet(int pf,
 
 /* This is gross, but inline doesn't cut it for avoiding the function
    call in fast path: gcc doesn't inline (needs value tracking?). --RR */
-#ifdef CONFIG_NETFILTER_DEBUG
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			       \
-({int __ret;								       \
-if ((__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, INT_MIN)) == 1) \
-	__ret = (okfn)(skb);						       \
-__ret;})
-#define NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, thresh)	       \
-({int __ret;								       \
-if ((__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)  \
-	__ret = (okfn)(skb);						       \
-__ret;})
-#else
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			       \
-({int __ret;								       \
-if (list_empty(&nf_hooks[pf][hook]) ||					       \
-    (__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, INT_MIN)) == 1) \
-	__ret = (okfn)(skb);						       \
-__ret;})
+
+/* HX: It's slightly less gross now. */
+
 #define NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, thresh)	       \
 ({int __ret;								       \
-if (list_empty(&nf_hooks[pf][hook]) ||					       \
-    (__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)  \
+if ((__ret=nf_hook_thresh(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)\
 	__ret = (okfn)(skb);						       \
 __ret;})
-#endif
 
-int nf_hook_slow(int pf, unsigned int hook, struct sk_buff **pskb,
-		 struct net_device *indev, struct net_device *outdev,
-		 int (*okfn)(struct sk_buff *), int thresh);
+#define NF_HOOK(pf, hook, skb, indev, outdev, okfn) \
+	NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, INT_MIN)
 
 /* Call setsockopt() */
 int nf_setsockopt(struct sock *sk, int pf, int optval, char __user *opt, 
include/net/dst.h +1 -10
@@ -224,16 +224,7 @@ static inline void dst_set_expires(struc
 /* Output packet to network from transport.  */
 static inline int dst_output(struct sk_buff *skb)
 {
-	int err;
-
-	for (;;) {
-		err = skb->dst->output(skb);
-
-		if (likely(err == 0))
-			return err;
-		if (unlikely(err != NET_XMIT_BYPASS))
-			return err;
-	}
+	return skb->dst->output(skb);
 }
 
 /* Input packet from network to transport.  */
net/ipv4/xfrm4_output.c +35 -2
@@ -8,8 +8,10 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/compiler.h>
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
+#include <linux/netfilter_ipv4.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -95,7 +97,7 @@ out:
 	return ret;
 }
 
-int xfrm4_output(struct sk_buff *skb)
+static int xfrm4_output_one(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -138,7 +140,7 @@ int xfrm4_output(struct sk_buff *skb)
 		x = dst->xfrm;
 	} while (x && !x->props.mode);
 
-	err = NET_XMIT_BYPASS;
+	err = 0;
 
 out_exit:
 	return err;
@@ -148,3 +150,34 @@ error_nolock:
 	kfree_skb(skb);
 	goto out_exit;
 }
+
+static int xfrm4_output_finish(struct sk_buff *skb)
+{
+	int err;
+
+	while (likely((err = xfrm4_output_one(skb)) == 0)) {
+		nf_reset(skb);
+
+		err = nf_hook(PF_INET, NF_IP_LOCAL_OUT, &skb, NULL,
+			      skb->dst->dev, dst_output);
+		if (unlikely(err != 1))
+			break;
+
+		err = 0;
+		if (!skb->dst->xfrm)
+			break;
+
+		err = nf_hook(PF_INET, NF_IP_POST_ROUTING, &skb, NULL,
+			      skb->dst->dev, xfrm4_output_finish);
+		if (unlikely(err != 1))
+			break;
+	}
+
+	return err;
+}
+
+int xfrm4_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm4_output_finish);
+}
net/ipv6/xfrm6_output.c +35 -2
@@ -9,9 +9,11 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/compiler.h>
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
 #include <linux/icmpv6.h>
+#include <linux/netfilter_ipv6.h>
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/ipv6.h>
@@ -92,7 +94,7 @@ static int xfrm6_tunnel_check_size(struc
 	return ret;
 }
 
-int xfrm6_output(struct sk_buff *skb)
+static int xfrm6_output_one(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -137,7 +139,7 @@ int xfrm6_output(struct sk_buff *skb)
 		x = dst->xfrm;
 	} while (x && !x->props.mode);
 
-	err = NET_XMIT_BYPASS;
+	err = 0;
 
 out_exit:
 	return err;
@@ -147,3 +149,34 @@ error_nolock:
 	kfree_skb(skb);
 	goto out_exit;
 }
+
+static int xfrm6_output_finish(struct sk_buff *skb)
+{
+	int err;
+
+	while (likely((err = xfrm6_output_one(skb)) == 0)) {
+		nf_reset(skb);
+	
+		err = nf_hook(PF_INET6, NF_IP6_LOCAL_OUT, &skb, NULL,
+			      skb->dst->dev, dst_output);
+		if (unlikely(err != 1))
+			break;
+
+		err = 0;
+		if (!skb->dst->xfrm)
+			break;
+
+		err = nf_hook(PF_INET6, NF_IP6_POST_ROUTING, &skb, NULL,
+			      skb->dst->dev, xfrm6_output_finish);
+		if (unlikely(err != 1))
+			break;
+	}
+
+	return err;
+}
+
+int xfrm6_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET6, NF_IP6_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm6_output_finish);
+}
#12 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 11 lines · Herbert Xu
Quoted 2 lines · herbert

Unfortunately it looks like gcc 3.3.5 at least is too dumb to optimise it away. I think we'll need a better strategy.

OK, the idea is still the same: Move the loop from dst_output into xfrm4_output/xfrm6_output since they're the only ones who need to it.

In order to avoid the tail call issue, I've added the inline function nf_hook which is nf_hook_slow plus the empty list check.

Thanks, this looks great. I've changed it to only call the hooks before tunnel mode transforms and added a missing dst_output call for the final packet.

[XFRM4/6]: Netfilter IPsec output hooks

Call netfilter hooks before IPsec transforms. Packets visit the FORWARD/LOCAL_OUT and POST_ROUTING hook before the first encapsulation and the LOCAL_OUT and POST_ROUTING hook before each following tunnel mode transform.

Based in large parts on patch by Herbert Xu <[email protected]>, that hides everything within xfrm{4,6}_output.c. Original description:
-
Move the loop from dst_output into xfrm4_output/xfrm6_output since they're the only ones who need to it.

In order to avoid the tail call issue, I've added the inline function nf_hook which is nf_hook_slow plus the empty list check.
-

---
commit 7abb84c6c3916fc365051a090c752db682b022ab
tree 31b5c4089aaf23cd2c44516f95fddf2158d0fd70
parent b47e6dc58fa6342f2403a32dd1060bc8b1cef56b
author Patrick McHardy <[email protected]> Mon, 28 Nov 2005 01:56:11 +0100 committer Patrick McHardy <[email protected]> Mon, 28 Nov 2005 01:56:11 +0100

 include/linux/netfilter.h |   61 +++++++++++++++++++++++++++------------------
 include/net/dst.h         |   11 +-------
 net/ipv4/xfrm4_output.c   |   39 +++++++++++++++++++++++++++--
 net/ipv6/xfrm6_output.c   |   39 +++++++++++++++++++++++++++--
 4 files changed, 112 insertions(+), 38 deletions(-)
include/linux/netfilter.h +37 -24
index be365e7..79bb977 100644
@@ -168,6 +168,37 @@ void nf_log_packet(int pf,
 		   const struct net_device *out,
 		   struct nf_loginfo *li,
 		   const char *fmt, ...);
+
+int nf_hook_slow(int pf, unsigned int hook, struct sk_buff **pskb,
+		 struct net_device *indev, struct net_device *outdev,
+		 int (*okfn)(struct sk_buff *), int thresh);
+
+/**
+ *	nf_hook_thresh - call a netfilter hook
+ *	
+ *	Returns 1 if the hook has allowed the packet to pass.  The function
+ *	okfn must be invoked by the caller in this case.  Any other return
+ *	value indicates the packet has been consumed by the hook.
+ */
+static inline int nf_hook_thresh(int pf, unsigned int hook,
+				 struct sk_buff **pskb,
+				 struct net_device *indev,
+				 struct net_device *outdev,
+				 int (*okfn)(struct sk_buff *), int thresh)
+{
+#ifndef CONFIG_NETFILTER_DEBUG
+	if (list_empty(&nf_hooks[pf][hook]))
+		return 1;
+#endif
+	return nf_hook_slow(pf, hook, pskb, indev, outdev, okfn, thresh);
+}
+
+static inline int nf_hook(int pf, unsigned int hook, struct sk_buff **pskb,
+			  struct net_device *indev, struct net_device *outdev,
+			  int (*okfn)(struct sk_buff *))
+{
+	return nf_hook_thresh(pf, hook, pskb, indev, outdev, okfn, INT_MIN);
+}
                    
 /* Activate hook; either okfn or kfree_skb called, unless a hook
    returns NF_STOLEN (in which case, it's up to the hook to deal with
@@ -188,35 +219,17 @@ void nf_log_packet(int pf,
 
 /* This is gross, but inline doesn't cut it for avoiding the function
    call in fast path: gcc doesn't inline (needs value tracking?). --RR */
-#ifdef CONFIG_NETFILTER_DEBUG
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			       \
-({int __ret;								       \
-if ((__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, INT_MIN)) == 1) \
-	__ret = (okfn)(skb);						       \
-__ret;})
-#define NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, thresh)	       \
-({int __ret;								       \
-if ((__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)  \
-	__ret = (okfn)(skb);						       \
-__ret;})
-#else
-#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)			       \
-({int __ret;								       \
-if (list_empty(&nf_hooks[pf][hook]) ||					       \
-    (__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, INT_MIN)) == 1) \
-	__ret = (okfn)(skb);						       \
-__ret;})
+
+/* HX: It's slightly less gross now. */
+
 #define NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, thresh)	       \
 ({int __ret;								       \
-if (list_empty(&nf_hooks[pf][hook]) ||					       \
-    (__ret=nf_hook_slow(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)  \
+if ((__ret=nf_hook_thresh(pf, hook, &(skb), indev, outdev, okfn, thresh)) == 1)\
 	__ret = (okfn)(skb);						       \
 __ret;})
-#endif
 
-int nf_hook_slow(int pf, unsigned int hook, struct sk_buff **pskb,
-		 struct net_device *indev, struct net_device *outdev,
-		 int (*okfn)(struct sk_buff *), int thresh);
+#define NF_HOOK(pf, hook, skb, indev, outdev, okfn) \
+	NF_HOOK_THRESH(pf, hook, skb, indev, outdev, okfn, INT_MIN)
 
 /* Call setsockopt() */
 int nf_setsockopt(struct sock *sk, int pf, int optval, char __user *opt, 
include/net/dst.h +1 -10
index 6c196a5..e641dd2 100644
@@ -224,16 +224,7 @@ static inline void dst_set_expires(struc
 /* Output packet to network from transport.  */
 static inline int dst_output(struct sk_buff *skb)
 {
-	int err;
-
-	for (;;) {
-		err = skb->dst->output(skb);
-
-		if (likely(err == 0))
-			return err;
-		if (unlikely(err != NET_XMIT_BYPASS))
-			return err;
-	}
+	return skb->dst->output(skb);
 }
 
 /* Input packet from network to transport.  */
net/ipv4/xfrm4_output.c +37 -2
index 66620a9..67b9483 100644
@@ -8,8 +8,10 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/compiler.h>
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
+#include <linux/netfilter_ipv4.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -95,7 +97,7 @@ out:
 	return ret;
 }
 
-int xfrm4_output(struct sk_buff *skb)
+static int xfrm4_output_one(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -133,7 +135,7 @@ int xfrm4_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
-	err = NET_XMIT_BYPASS;
+	err = 0;
 
 out_exit:
 	return err;
@@ -143,3 +145,36 @@ error_nolock:
 	kfree_skb(skb);
 	goto out_exit;
 }
+
+static int xfrm4_output_finish(struct sk_buff *skb)
+{
+	int err;
+
+	while (likely((err = xfrm4_output_one(skb)) == 0)) {
+		if (!skb->dst->xfrm || skb->dst->xfrm->props.mode) {
+			nf_reset(skb);
+			err = nf_hook(PF_INET, NF_IP_LOCAL_OUT, &skb, NULL,
+				      skb->dst->dev, dst_output);
+			if (unlikely(err != 1))
+				break;
+
+			if (!skb->dst->xfrm) {
+				err = dst_output(skb);
+				break;
+			}
+
+			err = nf_hook(PF_INET, NF_IP_POST_ROUTING, &skb, NULL,
+				      skb->dst->dev, xfrm4_output_finish);
+			if (unlikely(err != 1))
+				break;
+		}
+	}
+
+	return err;
+}
+
+int xfrm4_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET, NF_IP_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm4_output_finish);
+}
net/ipv6/xfrm6_output.c +37 -2
index 6b98677..69ee2bf 100644
@@ -9,9 +9,11 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#include <linux/compiler.h>
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
 #include <linux/icmpv6.h>
+#include <linux/netfilter_ipv6.h>
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/ipv6.h>
@@ -92,7 +94,7 @@ static int xfrm6_tunnel_check_size(struc
 	return ret;
 }
 
-int xfrm6_output(struct sk_buff *skb)
+static int xfrm6_output_one(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb->dst;
 	struct xfrm_state *x = dst->xfrm;
@@ -132,7 +134,7 @@ int xfrm6_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
-	err = NET_XMIT_BYPASS;
+	err = 0;
 
 out_exit:
 	return err;
@@ -142,3 +144,36 @@ error_nolock:
 	kfree_skb(skb);
 	goto out_exit;
 }
+
+static int xfrm6_output_finish(struct sk_buff *skb)
+{
+	int err;
+
+	while (likely((err = xfrm6_output_one(skb)) == 0)) {
+		if (!skb->dst->xfrm || skb->dst->xfrm->props.mode) {
+			nf_reset(skb);
+			err = nf_hook(PF_INET6, NF_IP6_LOCAL_OUT, &skb, NULL,
+				      skb->dst->dev, dst_output);
+			if (unlikely(err != 1))
+				break;
+
+			if (!skb->dst->xfrm) {
+				err = dst_output(skb);
+				break;
+			}
+
+			err = nf_hook(PF_INET6, NF_IP6_POST_ROUTING, &skb, NULL,
+				      skb->dst->dev, xfrm6_output_finish);
+			if (unlikely(err != 1))
+				break;
+		}
+	}
+
+	return err;
+}
+
+int xfrm6_output(struct sk_buff *skb)
+{
+	return NF_HOOK(PF_INET6, NF_IP6_POST_ROUTING, skb, NULL, skb->dst->dev,
+		       xfrm6_output_finish);
+}
maintainer
#13 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 2 lines · Patrick McHardy

Thanks, this looks great. I've changed it to only call the hooks

Glad you liked it :)

Quoted 2 lines

before tunnel mode transforms and added a missing dst_output call for the final packet.

This shouldn't be necessary if you apply it on top of my previous patch which made xfrm[46]_output process the first SA and all subsequent transport mode SAs. I've included that patch here again.

I think it still makes sense to do that because this corresponds with the usual representation of an IPsec connection and it simplifies the handling of netfilter hooks.

Cheers,

Signature · 5 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
include/net/ip.h +0 -0
net/ipv4/xfrm4_output.c +22 -17
@@ -113,26 +113,31 @@ int xfrm4_output(struct sk_buff *skb)
 			goto error_nolock;
 	}
 
-	spin_lock_bh(&x->lock);
-	err = xfrm_state_check(x, skb);
-	if (err)
-		goto error;
-
-	xfrm4_encap(skb);
-
-	err = x->type->output(x, skb);
-	if (err)
-		goto error;
+	do {
+		spin_lock_bh(&x->lock);
+		err = xfrm_state_check(x, skb);
+		if (err)
+			goto error;
+
+		xfrm4_encap(skb);
+
+		err = x->type->output(x, skb);
+		if (err)
+			goto error;
 
-	x->curlft.bytes += skb->len;
-	x->curlft.packets++;
+		x->curlft.bytes += skb->len;
+		x->curlft.packets++;
 
-	spin_unlock_bh(&x->lock);
+		spin_unlock_bh(&x->lock);
 	
-	if (!(skb->dst = dst_pop(dst))) {
-		err = -EHOSTUNREACH;
-		goto error_nolock;
-	}
+		if (!(skb->dst = dst_pop(dst))) {
+			err = -EHOSTUNREACH;
+			goto error_nolock;
+		}
+		dst = skb->dst;
+		x = dst->xfrm;
+	} while (x && !x->props.mode);
+
 	err = NET_XMIT_BYPASS;
 
 out_exit:
net/ipv6/xfrm6_output.c +26 -21

54 lines not on this page · raw message

#14 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 12 lines · Herbert Xu
Quoted 1 line · Patrick McHardy

Thanks, this looks great. I've changed it to only call the hooks

Glad you liked it :)

Quoted 2 lines

before tunnel mode transforms and added a missing dst_output call for the final packet.

Quoted 7 lines

This shouldn't be necessary if you apply it on top of my previous patch which made xfrm[46]_output process the first SA and all subsequent transport mode SAs. I've included that patch here again.

I think it still makes sense to do that because this corresponds with the usual representation of an IPsec connection and it simplifies the handling of netfilter hooks.

I agree, I missed that your patch based on that one. Let me have another look :)

#15 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

In reply to Herbert Xu
Quoted 6 lines · Herbert Xu
Quoted 2 lines

before tunnel mode transforms and added a missing dst_output call for the final packet.

This shouldn't be necessary if you apply it on top of my previous patch which made xfrm[46]_output process the first SA and all subsequent transport mode SAs. I've included that patch here again.

Thanks, I've added the correct patch now :) Unless I missed something, it was still missing a call to dst_output after the last transform in xfrm4_output_finish, unless we keep the loop in dst_output.

maintainer
#16 lore raw

Re: [PATCH 05/13]: [IPV4/6]: Netfilter IPsec output hooks

Quoted 4 lines · Patrick McHardy

Thanks, I've added the correct patch now :) Unless I missed something, it was still missing a call to dst_output after the last transform in xfrm4_output_finish, unless we keep the loop in dst_output.

Good catch. The lines

		err = 0;
		if (!skb->dst->xfrm)
			break;

should read

		if (!skb->dst->xfrm)
			return dst_output(skb);

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#17 lore raw

PATCH 6/13 : [IPV4/6]: Netfilter IPsec input hooks

In reply to Patrick McHardy

[IPV4/6]: Netfilter IPsec input hooks

When the innermost transform uses transport mode the decapsulated packet is not visible to netfilter. Pass the packet through the PRE_ROUTING and LOCAL_IN hooks again before handing it to upper layer protocols to make netfilter-visibility symetrical to the output path.

---
commit 08cf39d5d7d8b942431a6529daa3ab69ecfb34b5
tree 6f2a1a85f915b1b6f89ad50cf3d8855f21a561b6
parent b847425c693f43a63137d18e36e5c8cf0187c175
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:50:22 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 21:50:22 +0100

 include/linux/netfilter_ipv4.h |    2 +-
 include/net/ipv6.h             |    2 ++
 net/ipv4/netfilter.c           |   20 ++++++++++++++++++++
 net/ipv4/xfrm4_input.c         |   14 ++++++++++++++
 net/ipv6/ip6_input.c           |    2 +-
 net/ipv6/xfrm6_input.c         |   13 +++++++++++++
 6 files changed, 51 insertions(+), 2 deletions(-)
include/linux/netfilter_ipv4.h +1 -1
index fdc4a95..e9103fe 100644
@@ -79,7 +79,7 @@ enum nf_ip_hook_priorities {
 
 #ifdef __KERNEL__
 extern int ip_route_me_harder(struct sk_buff **pskb);
-
+extern int ip_xfrm_transport_hook(struct sk_buff *skb);
 #endif /*__KERNEL__*/
 
 #endif /*__LINUX_IP_NETFILTER_H*/
include/net/ipv6.h +2 -0
index 6addb4d..4fbfe43 100644
@@ -414,6 +414,8 @@ extern int			ipv6_rcv(struct sk_buff *sk
 					 struct packet_type *pt,
 					 struct net_device *orig_dev);
 
+extern int			ip6_rcv_finish(struct sk_buff *skb);
+
 /*
  *	upper-layer output functions
  */
net/ipv4/netfilter.c +20 -0
index b93e7cd..3c39296 100644
@@ -105,6 +105,26 @@ int ip_dst_output(struct sk_buff *skb)
 	return dst_output(skb);
 }
 EXPORT_SYMBOL(ip_dst_output);
+
+/*
+ * okfn for transport mode xfrm_input.c hook. Basically a copy of
+ * ip_rcv_finish without statistics and option parsing.
+ */
+int ip_xfrm_transport_hook(struct sk_buff *skb)
+{
+	struct iphdr *iph = skb->nh.iph;
+
+	if (likely(skb->dst == NULL)) {
+		int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos,
+		                         skb->dev);
+		if (unlikely(err))
+			goto drop;
+	}
+	return dst_input(skb);
+drop:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
 #endif /* CONFIG_XFRM */
 
 /*
net/ipv4/xfrm4_input.c +14 -0
index 2d3849c..d90cd93 100644
@@ -11,6 +11,8 @@
 
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv4.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
 #include <net/xfrm.h>
@@ -137,6 +139,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb,
 	memcpy(skb->sp->x+skb->sp->len, xfrm_vec, xfrm_nr*sizeof(struct sec_decap_state));
 	skb->sp->len += xfrm_nr;
 
+	nf_reset(skb);
+
 	if (decaps) {
 		if (!(skb->dev->flags&IFF_LOOPBACK)) {
 			dst_release(skb->dst);
@@ -145,7 +149,17 @@ int xfrm4_rcv_encap(struct sk_buff *skb,
 		netif_rx(skb);
 		return 0;
 	} else {
+#ifdef CONFIG_NETFILTER
+		__skb_push(skb, skb->data - skb->nh.raw);
+		skb->nh.iph->tot_len = htons(skb->len);
+		ip_send_check(skb->nh.iph);
+
+		NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
+		        ip_xfrm_transport_hook);
+		return 0;
+#else
 		return -skb->nh.iph->protocol;
+#endif
 	}
 
 drop_unlock:
net/ipv6/ip6_input.c +1 -1
index 33d3b0e..e84b3cd 100644
@@ -48,7 +48,7 @@
 
 
 
-static inline int ip6_rcv_finish( struct sk_buff *skb) 
+inline int ip6_rcv_finish(struct sk_buff *skb) 
 {
 	if (skb->dst == NULL)
 		ip6_route_input(skb);
net/ipv6/xfrm6_input.c +13 -0
index 28c29d7..9987416 100644
@@ -11,6 +11,8 @@
 
 #include <linux/module.h>
 #include <linux/string.h>
+#include <linux/netfilter.h>
+#include <linux/netfilter_ipv6.h>
 #include <net/dsfield.h>
 #include <net/inet_ecn.h>
 #include <net/ip.h>
@@ -121,6 +123,8 @@ int xfrm6_rcv_spi(struct sk_buff **pskb,
 	skb->sp->len += xfrm_nr;
 	skb->ip_summed = CHECKSUM_NONE;
 
+	nf_reset(skb);
+
 	if (decaps) {
 		if (!(skb->dev->flags&IFF_LOOPBACK)) {
 			dst_release(skb->dst);
@@ -129,7 +133,16 @@ int xfrm6_rcv_spi(struct sk_buff **pskb,
 		netif_rx(skb);
 		return -1;
 	} else {
+#ifdef CONFIG_NETFILTER
+		skb->nh.ipv6h->payload_len = htons(skb->len);
+		__skb_push(skb, skb->data - skb->nh.raw);
+
+		NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
+		        ip6_rcv_finish);
+		return -1;
+#else
 		return 1;
+#endif
 	}
 
 drop_unlock:
#18 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Hi, Patrick,

From: Patrick McHardy <[email protected]>
Date: Sun, 20 Nov 2005 17:31:36 +0100

Quoted 8 lines

[IPV4/6]: Netfilter IPsec input hooks

When the innermost transform uses transport mode the decapsulated packet is not visible to netfilter. Pass the packet through the PRE_ROUTING and LOCAL_IN hooks again before handing it to upper layer protocols to make netfilter-visibility symetrical to the output path.

At first, now I could agree to use same name for hooks before/after xfrm processing, if it's important to keep compatibility than to avoid difficulty to use. Even now I think it's confusing to pass packets before/after xfrm to same hook, and believe it's ideal to use different name for them. But people can configure correctly according to you, then my concern might be needless fear.

Next is about re-visiting stack, I'm beginning to understand your patch. It looks natural to re-route after DNAT on input path of IPv4. That's really needed if packets have been mangled.

But is there any reason that we have to allow packets to re-visit ip_local_deliver_finish() in the case that they have not been mangled at PRE_ROUTING ? I think no. This situation is like ip_nat_out().

And this can be said about IPv6 input path. If packets have not been mangled (this is ordinary case because ip6tables doesn't have neither NAT nor target module to mangle addresses directly), they don't have to re-route and don't have to re-visit ip6_input_finish().

In the other way, if their addresses have been mangled, it's necessary to re-route. I agree re-visiting ip6_input_finish() in this case.

Then, why don't we make xfrm{4,6}_rcv_spi() return 0 (1 if IPv6) so that ip_local_deliver_finish()/ip6_input_finish() can continue to process headers if packets have not been mangled ? Is this difficult or impossible to implement ?

This can solve the issue about twice processing of statistics and IPv6 extension headers. Because ip_local_deliver_finish()/ip6_input_finish() can continue to process extension headers after ESP/AH in ordinary case.

In special case, if some codes mangle IPv6 addresses, that's the codes to take care of the possibility of re-visiting ip6_input_finish().

What do you think ?

# Please note that these are just my opinions and other USAGI guys might have

# other opinions.

Regards,

-- Yasuyuki Kozakai

A message that is not in the archive (sent off-list or not yet received): [email protected]

#20 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 6 lines · Yasuyuki KOZAKAI

At first, now I could agree to use same name for hooks before/after xfrm processing, if it's important to keep compatibility than to avoid difficulty to use. Even now I think it's confusing to pass packets before/after xfrm to same hook, and believe it's ideal to use different name for them. But people can configure correctly according to you, then my concern might be needless fear.

I don't see why it is confusing. Plain text packets are visible before encapsulation (and they have to be because we don't necessarily know if packets will be encapsulated at the time the hooks are called in case the policy lookup after NAT returns a policy), plain text packets are visible after decapsulation. With different hooks we can't have symetrical behaviour because of the case I mentioned above, and that would be confusing IMO.

Quoted 7 lines

Next is about re-visiting stack, I'm beginning to understand your patch. It looks natural to re-route after DNAT on input path of IPv4. That's really needed if packets have been mangled.

But is there any reason that we have to allow packets to re-visit ip_local_deliver_finish() in the case that they have not been mangled at PRE_ROUTING ? I think no. This situation is like ip_nat_out().

My patches don't change when and if packets will reach ip_local_deliver_finish(), they just add a possibility for rerouting. Currently the transforms are called from ip_local_deliver_finish() and in transport mode the decapsulated packet continues its path in ip_local_deliver_finish(), with my patches they will go through two netfilter hooks and continue the exact same codepath, given that they are not NATed to a foreign destination IP on their way.

Quoted 7 lines

And this can be said about IPv6 input path. If packets have not been mangled (this is ordinary case because ip6tables doesn't have neither NAT nor target module to mangle addresses directly), they don't have to re-route and don't have to re-visit ip6_input_finish().

In the other way, if their addresses have been mangled, it's necessary to re-route. I agree re-visiting ip6_input_finish() in this case.

Same goes for ip6_input_finish as for ip_local_deliver_finish(), the packet would continue its path there anyway. Do you actually mean ip6_rcv_finish()?

Quoted 4 lines

Then, why don't we make xfrm{4,6}_rcv_spi() return 0 (1 if IPv6) so that ip_local_deliver_finish()/ip6_input_finish() can continue to process headers if packets have not been mangled ? Is this difficult or impossible to implement ?

I'm not sure I understand. Do you propose to check if the packet was mangled after the PRE_ROUTING hook (if it was not stolen or queued) and if not return directly to ip6_input_finish()? Where would the LOCAL_IN hook be called?

Quoted 3 lines

This can solve the issue about twice processing of statistics and IPv6 extension headers. Because ip_local_deliver_finish()/ip6_input_finish() can continue to process extension headers after ESP/AH in ordinary case.

AFAICT statistics are not affected by my patches, except for the iptables counters. The double parsing of extension headers is indeed a problem I forgot about, it looks like we need to carry nhoff around if it can't be derived from the packet.

Regards
Patrick

maintainer
#21 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

From: Patrick McHardy <[email protected]>
Date: Mon, 21 Nov 2005 07:52:36 +0100

Quoted 7 lines

I don't see why it is confusing. Plain text packets are visible before encapsulation (and they have to be because we don't necessarily know if packets will be encapsulated at the time the hooks are called in case the policy lookup after NAT returns a policy), plain text packets are visible after decapsulation. With different hooks we can't have symetrical behaviour because of the case I mentioned above, and that would be confusing IMO.

I think this is a very important point.

I can see no serious argument against this behavior, especially for output. On input, there is an argument of paranoia about seeing plaintext packets, but administrator could do this anyways with tcpdump or custom kernel module if this system is the decapsulation point.

I've read over Patrick's two most recent postings of these patches and I think they are generally sane and I cannot find any holes in them. Herbert brought up the legitimate concern about defragmentation, but I think that's a detail and does not take away from the structural soundness of Patrick's approach.

maintainer
#22 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 6 lines · David S. Miller

I've read over Patrick's two most recent postings of these patches and I think they are generally sane and I cannot find any holes in them. Herbert brought up the legitimate concern about defragmentation, but I think that's a detail and does not take away from the structural soundness of Patrick's approach.

Yes I agree completely. The new IPsec stack has been around for three years and it's about time that we have proper netfilter support for it.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#23 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

In reply to David S. Miller
Quoted 5 lines · David S. Miller

I've read over Patrick's two most recent postings of these patches and I think they are generally sane and I cannot find any holes in them. Herbert brought up the legitimate concern about defragmentation, but I think that's a detail and does not take away from the structural soundness of Patrick's approach.

I think we implicitly agreed on moving the POST_ROUTING hook before fragmentation and change the user-visible behaviour of the mangle POSTROUTING chain. At least neither Harald not Rusty objected to the patch :)

#24 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

In reply to Patrick McHardy

Hi,

From: Patrick McHardy <[email protected]>
Date: Mon, 21 Nov 2005 07:52:36 +0100

Quoted 7 lines

I don't see why it is confusing. Plain text packets are visible before encapsulation (and they have to be because we don't necessarily know if packets will be encapsulated at the time the hooks are called in case the policy lookup after NAT returns a policy), plain text packets are visible after decapsulation. With different hooks we can't have symetrical behaviour because of the case I mentioned above, and that would be confusing IMO.

Well, what I worried about was just ease to use, not internal processing. I suspected that users can correctly configure IPsec and packet filtering. Just doing "iptables -P INPUT -j DROP; iptables -A INPUT -p esp -j ACCEPT" will drop all input packets and this is different behavior with current kernel, for example. So I just imagined many people would say "Why ?".

But as you said in other mail, probably this is my needles fear, isn't this ? As you know, I'm basically worrier. :)

Quoted 11 lines
Quoted 7 lines

And this can be said about IPv6 input path. If packets have not been mangled (this is ordinary case because ip6tables doesn't have neither NAT nor target module to mangle addresses directly), they don't have to re-route and don't have to re-visit ip6_input_finish().

In the other way, if their addresses have been mangled, it's necessary to re-route. I agree re-visiting ip6_input_finish() in this case.

Same goes for ip6_input_finish as for ip_local_deliver_finish(), the packet would continue its path there anyway. Do you actually mean ip6_rcv_finish()?

No. I mean ip6_input_finish(). calling ip6_input_finish() twice causes problem at processing of IPv6 extension headers. This is different point between IPv4 and IPv6.

Please note that I don't mean IPv4 processing in your patch has problem. I think it will work. What I wanted to do was just avoiding double processing of extension headers and synchronizing IPv4/IPv6 behavior as possible.

Quoted 8 lines
Quoted 4 lines

Then, why don't we make xfrm{4,6}_rcv_spi() return 0 (1 if IPv6) so that ip_local_deliver_finish()/ip6_input_finish() can continue to process headers if packets have not been mangled ? Is this difficult or impossible to implement ?

I'm not sure I understand. Do you propose to check if the packet was mangled after the PRE_ROUTING hook (if it was not stolen or queued) and if not return directly to ip6_input_finish()?

Yes.

Quoted 2 lines

Where would the
LOCAL_IN hook be called?

Ah, indeed. But we can add it just before return directly to ip6_input_finish().

Quoted 8 lines
Quoted 3 lines

This can solve the issue about twice processing of statistics and IPv6 extension headers. Because ip_local_deliver_finish()/ip6_input_finish() can continue to process extension headers after ESP/AH in ordinary case.

AFAICT statistics are not affected by my patches, except for the iptables counters. The double parsing of extension headers is indeed a problem I forgot about, it looks like we need to carry nhoff around if it can't be derived from the packet.

Of cause that is one solution.

-- Yasuyuki Kozakai

A message that is not in the archive (sent off-list or not yet received): [email protected]

#26 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 8 lines · Yasuyuki KOZAKAI
Quoted 7 lines

I don't see why it is confusing. Plain text packets are visible before encapsulation (and they have to be because we don't necessarily know if packets will be encapsulated at the time the hooks are called in case the policy lookup after NAT returns a policy), plain text packets are visible after decapsulation. With different hooks we can't have symetrical behaviour because of the case I mentioned above, and that would be confusing IMO.

Quoted 8 lines

Well, what I worried about was just ease to use, not internal processing. I suspected that users can correctly configure IPsec and packet filtering. Just doing "iptables -P INPUT -j DROP; iptables -A INPUT -p esp -j ACCEPT" will drop all input packets and this is different behavior with current kernel, for example. So I just imagined many people would say "Why ?".

But as you said in other mail, probably this is my needles fear, isn't this ? As you know, I'm basically worrier. :)

:) I think its OK since in tunnel mode the decapsulated packets already need to be allowed by the ruleset, so I think its not too confusing to expect the same in transport mode.

Quoted 9 lines
Quoted 8 lines
Quoted 4 lines

Then, why don't we make xfrm{4,6}_rcv_spi() return 0 (1 if IPv6) so that ip_local_deliver_finish()/ip6_input_finish() can continue to process headers if packets have not been mangled ? Is this difficult or impossible to implement ?

I'm not sure I understand. Do you propose to check if the packet was mangled after the PRE_ROUTING hook (if it was not stolen or queued) and if not return directly to ip6_input_finish()?

Quoted 6 lines

Yes.

Quoted 1 line

Where would the LOCAL_IN hook be called?

Ah, indeed. But we can add it just before return directly to ip6_input_finish().

Please see my mail to Kazunori. The hooks take ownership of the skb, changing this would become pretty ugly because of queued or stolen packets. And it would still leave one path where packets are not directly returned, so I think the double-parsing should be handled otherwise.

Quoted 6 lines
Quoted 4 lines

AFAICT statistics are not affected by my patches, except for the iptables counters. The double parsing of extension headers is indeed a problem I forgot about, it looks like we need to carry nhoff around if it can't be derived from the packet.

Of cause that is one solution.

I'm going to try that if I can't come up with something better.

A message that is not in the archive (sent off-list or not yet received): [email protected]

#28 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Hello.

In article <[email protected]> (at Mon, 21 Nov 2005 17:31:41 +0900), Kazunori Miyazawa <[email protected]> says:

Quoted 5 lines

Your ip_xfrm_transport_hook is a good idea, I think.

We could call ip6_rcv_finish if the netfilter changed the addresses or otherwise we can continue the loop to avoid the cost in a similar way because we can know the change with checking skb->dst.

Well, I agree.

In article <[email protected]> (at Sun, 20 Nov 2005 17:31:36 +0100), Patrick McHardy <[email protected]> says:

Quoted 20 lines
net/ipv4/netfilter.c +12 -0
index b93e7cd..3c39296 100644
@@ -105,6 +105,26 @@ int ip_dst_output(struct sk_buff *skb)
 	return dst_output(skb);
 }
 EXPORT_SYMBOL(ip_dst_output);
+
+/*
+ * okfn for transport mode xfrm_input.c hook. Basically a copy of
+ * ip_rcv_finish without statistics and option parsing.
+ */
+int ip_xfrm_transport_hook(struct sk_buff *skb)
+{
+	struct iphdr *iph = skb->nh.iph;
+
+	if (likely(skb->dst == NULL)) {
+		int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos,
+		                         skb->dev);

net/ipv4/netfilter.c int ip_dst_output(struct sk_buff *skb) · line 129

Quoted 10 lines
net/ipv4/netfilter.c +8 -0
+		if (unlikely(err))
+			goto drop;
+	}
+	return dst_input(skb);
+drop:
+	kfree_skb(skb);
+	return NET_RX_DROP;
+}
 #endif /* CONFIG_XFRM */
 

:

Quoted 6 lines
net/ipv4/netfilter.c +2 -0
@@ -129,7 +133,16 @@ int xfrm6_rcv_spi(struct sk_buff **pskb,
 		netif_rx(skb);
 		return -1;
 	} else {
+#ifdef CONFIG_NETFILTER
+		skb->nh.ipv6h->payload_len = htons(skb->len);

net/ipv4/netfilter.c int xfrm6_rcv_spi(struct sk_buff **pskb, · line 147

Quoted 10 lines
net/ipv4/netfilter.c +7 -0
+		__skb_push(skb, skb->data - skb->nh.raw);
+
+		NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
+		        ip6_rcv_finish);
+		return -1;
+#else
 		return 1;
+#endif
 	}
 

Probably, we can do similarly for ipv6; e.g.:

int ip6_xfrm_transport_hook(struct sk_buff *skb)
{

#if 0 /* We NEVER support NAT. :-) */
     if (likely(skb->dst == NULL)) {
            int err = ip6_route_input()
            if (unlikely(err))
                     goto drop;
     }
#endif
     __skb_pull(skb, skb->h.raw - skb->nh.raw);
     return NET_RX_SUCCESS;
drop:
     kfree_skb(skb);
     return NET_RX_DROP;
}

:

      } else {
#ifdef CONFIG_NETFILTER
             skb->nh.ipv6h->payload_len = htons(skb->len);
	     skb->h.raw = skb->data;
             __skb_push(skb, skb->data - skb->nh.raw);

             if (NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
                         ip6_xfrm_transport_hook) == NET_RX_DROP)
                 return -1;
#endif
             return 1;
      }

Then, we can continue parsing extension headers, I think.

Signature · 2 lines
YOSHIFUJI Hideaki @ USAGI Project  <[email protected]>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA
#29 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 47 lines · YOSHIFUJI Hideaki / $B5HF#1QL@ wrote:

Hello.

In article <[email protected]> (at Mon, 21 Nov 2005 17:31:41 +0900), Kazunori Miyazawa <[email protected]> says:

Quoted 5 lines

Your ip_xfrm_transport_hook is a good idea, I think.

We could call ip6_rcv_finish if the netfilter changed the addresses or otherwise we can continue the loop to avoid the cost in a similar way because we can know the change with checking skb->dst.

Well, I agree.

Probably, we can do similarly for ipv6; e.g.:

int ip6_xfrm_transport_hook(struct sk_buff *skb)
{

#if 0 /* We NEVER support NAT. :-) */
     if (likely(skb->dst == NULL)) {
            int err = ip6_route_input()
            if (unlikely(err))
                     goto drop;
     }
#endif
     __skb_pull(skb, skb->h.raw - skb->nh.raw);
     return NET_RX_SUCCESS;
drop:
     kfree_skb(skb);
     return NET_RX_DROP;
}

:

      } else {
#ifdef CONFIG_NETFILTER
             skb->nh.ipv6h->payload_len = htons(skb->len);
	     skb->h.raw = skb->data;
             __skb_push(skb, skb->data - skb->nh.raw);

             if (NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
                         ip6_xfrm_transport_hook) == NET_RX_DROP)
                 return -1;
#endif
             return 1;
      }
Quoted 1 line

Then, we can continue parsing extension headers, I think.

Is it the rerouting you're concerned about? It will usually not happen because skb->dst is not NULL. Its needed for NFQUEUE, packets can be changes in userspace and need rerouting afterwards. In any case there would still be one path on which extension headers would be parsed twice, so I'm going to look into different ways to fix that.

maintainer
#30 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

In reply to Patrick McHardy
Quoted 6 lines · Patrick McHardy
net/ipv4/xfrm4_input.c +1 -0
@@ -145,7 +149,17 @@ int xfrm4_rcv_encap(struct sk_buff *skb,
 		netif_rx(skb);
 		return 0;
 	} else {
+#ifdef CONFIG_NETFILTER

net/ipv4/xfrm4_input.c int xfrm4_rcv_encap(struct sk_buff *skb, · line 162

Quoted 10 lines
net/ipv4/xfrm4_input.c +9 -0
+		__skb_push(skb, skb->data - skb->nh.raw);
+		skb->nh.iph->tot_len = htons(skb->len);
+		ip_send_check(skb->nh.iph);
+
+		NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
+		        ip_xfrm_transport_hook);
+		return 0;
+#else
 		return -skb->nh.iph->protocol;
+#endif

I'm worried about this bit. This looks like it'll go back to the top of the IP stack with the existing call chain. So could grow as the number of transforms increase.

Perhaps we need to play a dst_input/netif_rx trick here.

Actually, was there a problem with your original netif_rx approach apart from the issue with double counting?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#31 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 14 lines · Herbert Xu
Quoted 12 lines · Patrick McHardy
net/ipv4/xfrm4_input.c +8 -0
@@ -145,7 +149,17 @@ int xfrm4_rcv_encap(struct sk_buff *skb,
 		netif_rx(skb);
 		return 0;
 	} else {
+#ifdef CONFIG_NETFILTER
+		__skb_push(skb, skb->data - skb->nh.raw);
+		skb->nh.iph->tot_len = htons(skb->len);
+		ip_send_check(skb->nh.iph);
+
+		NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
+		        ip_xfrm_transport_hook);
+		return 0;
Quoted 8 lines

net/ipv4/xfrm4_input.c int xfrm4_rcv_encap(struct sk_buff *skb, · line 162

Quoted 3 lines
net/ipv4/xfrm4_input.c +2 -0
+#else
 		return -skb->nh.iph->protocol;
+#endif

I'm worried about this bit. This looks like it'll go back to the top of the IP stack with the existing call chain. So could grow as the number of transforms increase.

Its not so bad. It adds ip_xfrm_transport_hook and ip_local_deliver_finish to the call stack, but since two subsequent transport mode SAs are always processed at once it can't take this path again without calling netif_rx in between.

Quoted 4 lines

Perhaps we need to play a dst_input/netif_rx trick here.

Actually, was there a problem with your original netif_rx approach apart from the issue with double counting?

Besides the double counting, packets also appear on the packet sockets after transport mode decapsulation with the original approach. For IPv6 there's also the double-parsing of extension header issue.

maintainer
#32 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 9 lines · Patrick McHardy
Quoted 4 lines

I'm worried about this bit. This looks like it'll go back to the top of the IP stack with the existing call chain. So could grow as the number of transforms increase.

Its not so bad. It adds ip_xfrm_transport_hook and ip_local_deliver_finish to the call stack, but since two subsequent transport mode SAs are always processed at once it can't take this path again without calling netif_rx in between.

If there is a DNAT in the way, this will jump to the very start of the stack. So if we have a hostile IPsec peer, and the DNAT rules are such that this can occur, then we could be in trouble (especially because policy/selector verification does not occur until all IPsec has been done so we can't check inner address validitiy at this point).

Quoted 3 lines

Besides the double counting, packets also appear on the packet sockets after transport mode decapsulation with the original approach. For IPv6 there's also the double-parsing of extension header issue.

Having the packets appear twice on AF_PACKET is probably desirable :)

I'll need to think about the double-parsing though.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#33 lore raw

Re: [PATCH 06/13]: [IPV4/6]: Netfilter IPsec input hooks

Quoted 7 lines · Herbert Xu

On Sun, Dec 04, 2005 at 11:06:02PM +0100, Patrick McHardy wrote:

If there is a DNAT in the way, this will jump to the very start of the stack. So if we have a hostile IPsec peer, and the DNAT rules are such that this can occur, then we could be in trouble (especially because policy/selector verification does not occur until all IPsec has been done so we can't check inner address validitiy at this point).

We could return NET_XMIT_BYPASS from ip_xfrm_transport_hook(), although it looks a bit ugly to use NET_XMIT* on the input path.

#34 lore raw

PATCH 7/13 : [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

In reply to Patrick McHardy

[NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

ip_route_me_harder doesn't use the port numbers of the xfrm lookup and uses ip_route_input for non-local addresses which doesn't do a xfrm lookup, ip6_route_me_harder doesn't do a xfrm lookup at all.

Use xfrm_decode_session and do the lookup manually, make sure both only do the lookup if the packet hasn't been transformed already.

---
commit ffa4445cd4284d3d9b688c80f5a3b9f8b26d59e6
tree 3edbdce75cc680c51e38697d45479dbfd4404452
parent 08cf39d5d7d8b942431a6529daa3ab69ecfb34b5
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:05:08 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:05:08 +0100

 include/linux/ipv6.h    |    2 ++
 include/net/ip.h        |    1 +
 include/net/xfrm.h      |    2 +-
 net/ipv4/netfilter.c    |    9 ++++++++-
 net/ipv4/xfrm4_output.c |    1 +
 net/ipv6/netfilter.c    |    8 +++++++-
 net/ipv6/xfrm6_output.c |    1 +
 net/xfrm/xfrm_policy.c  |    9 +++++----
 8 files changed, 26 insertions(+), 7 deletions(-)
include/linux/ipv6.h +2 -0
index e0b9227..d7b3fac 100644
@@ -190,6 +190,8 @@ struct inet6_skb_parm {
 	__u16			srcrt;
 	__u16			dst1;
 	__u16			lastopt;
+	__u16			flags;
+#define IP6SKB_XFRM_TRANSFORMED	1
 };
 
 #define IP6CB(skb)	((struct inet6_skb_parm*)((skb)->cb))
include/net/ip.h +1 -0
index 9f09882..377036b 100644
@@ -45,6 +45,7 @@ struct inet_skb_parm
 #define IPSKB_TRANSLATED	2
 #define IPSKB_FORWARDED		4
 #define IPSKB_XFRM_TUNNEL_SIZE	8
+#define IPSKB_XFRM_TRANSFORMED	16
 };
 
 struct ipcm_cookie
include/net/xfrm.h +1 -1
index 5beae1c..19d6aa0 100644
@@ -644,7 +644,7 @@ static inline int xfrm6_policy_check(str
 	return xfrm_policy_check(sk, dir, skb, AF_INET6);
 }
 
-
+extern int xfrm_decode_session(struct sk_buff *skb, struct flowi *fl, unsigned short family);
 extern int __xfrm_route_forward(struct sk_buff *skb, unsigned short family);
 
 static inline int xfrm_route_forward(struct sk_buff *skb, unsigned short family)
net/ipv4/netfilter.c +8 -1
index 3c39296..db330b6 100644
@@ -13,6 +13,7 @@
 #include <linux/ip.h>
 #include <net/route.h>
 #include <net/xfrm.h>
+#include <net/ip.h>
 
 /* route_me_harder function, used by iptable_nat, iptable_mangle + ip_queue */
 int ip_route_me_harder(struct sk_buff **pskb)
@@ -34,7 +35,6 @@ int ip_route_me_harder(struct sk_buff **
 #ifdef CONFIG_IP_ROUTE_FWMARK
 		fl.nl_u.ip4_u.fwmark = (*pskb)->nfmark;
 #endif
-		fl.proto = iph->protocol;
 		if (ip_route_output_key(&rt, &fl) != 0)
 			return -1;
 
@@ -61,6 +61,13 @@ int ip_route_me_harder(struct sk_buff **
 	if ((*pskb)->dst->error)
 		return -1;
 
+#ifdef CONFIG_XFRM
+	if (!(IPCB(*pskb)->flags & IPSKB_XFRM_TRANSFORMED) &&
+	    xfrm_decode_session(*pskb, &fl, AF_INET) == 0)
+		if (xfrm_lookup(&(*pskb)->dst, &fl, (*pskb)->sk, 0))
+			return -1;
+#endif
+
 	/* Change in oif may mean change in hh_len. */
 	hh_len = (*pskb)->dst->dev->hard_header_len;
 	if (skb_headroom(*pskb) < hh_len) {
net/ipv4/xfrm4_output.c +1 -0
index c135746..9e49eeb 100644
@@ -133,6 +133,7 @@ int xfrm4_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IPCB(skb)->flags |= IPSKB_XFRM_TRANSFORMED;
 	nf_reset(skb);
 	err = NET_XMIT_BYPASS;
 
net/ipv6/netfilter.c +7 -1
index 06b275e..8bc6305 100644
@@ -22,7 +22,6 @@ int ip6_route_me_harder(struct sk_buff *
 		{ .ip6_u =
 		  { .daddr = iph->daddr,
 		    .saddr = iph->saddr, } },
-		.proto = iph->nexthdr,
 	};
 
 	dst = ip6_route_output(skb->sk, &fl);
@@ -34,6 +33,13 @@ int ip6_route_me_harder(struct sk_buff *
 		return -EINVAL;
 	}
 
+#ifdef CONFIG_XFRM
+	if (!(IP6CB(skb)->flags & IP6SKB_XFRM_TRANSFORMED) &&
+	    xfrm_decode_session(skb, &fl, AF_INET6) == 0)
+		if (xfrm_lookup(&skb->dst, &fl, skb->sk, 0))
+			return -1;
+#endif
+
 	/* Drop old route. */
 	dst_release(skb->dst);
 
net/ipv6/xfrm6_output.c +1 -0
index a566d25..929e4eb 100644
@@ -132,6 +132,7 @@ int xfrm6_output(struct sk_buff *skb)
 		err = -EHOSTUNREACH;
 		goto error_nolock;
 	}
+	IP6CB(skb)->flags |= IP6SKB_XFRM_TRANSFORMED;
 	nf_reset(skb);
 	err = NET_XMIT_BYPASS;
 
net/xfrm/xfrm_policy.c +5 -4
index 0db9e57..e441f35 100644
@@ -906,8 +906,8 @@ xfrm_policy_ok(struct xfrm_tmpl *tmpl, s
 	return start;
 }
 
-static int
-_decode_session(struct sk_buff *skb, struct flowi *fl, unsigned short family)
+int xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
+                        unsigned short family)
 {
 	struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family);
 
@@ -918,6 +918,7 @@ _decode_session(struct sk_buff *skb, str
 	xfrm_policy_put_afinfo(afinfo);
 	return 0;
 }
+EXPORT_SYMBOL(xfrm_decode_session);
 
 static inline int secpath_has_tunnel(struct sec_path *sp, int k)
 {
@@ -935,7 +936,7 @@ int __xfrm_policy_check(struct sock *sk,
 	struct xfrm_policy *pol;
 	struct flowi fl;
 
-	if (_decode_session(skb, &fl, family) < 0)
+	if (xfrm_decode_session(skb, &fl, family) < 0)
 		return 0;
 
 	/* First, check used SA against their selectors. */
@@ -1007,7 +1008,7 @@ int __xfrm_route_forward(struct sk_buff 
 {
 	struct flowi fl;
 
-	if (_decode_session(skb, &fl, family) < 0)
+	if (xfrm_decode_session(skb, &fl, family) < 0)
 		return 0;
 
 	return xfrm_lookup(&skb->dst, &fl, NULL, 0) == 0;
maintainer
#35 lore raw

Re: [PATCH 07/13]: [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

include/net/ip.h struct inet_skb_parm · line 49

Quoted 11 lines · Patrick McHardy
include/net/ip.h +1 -0
index 9f09882..377036b 100644
@@ -45,6 +45,7 @@ struct inet_skb_parm
 #define IPSKB_TRANSLATED	2
 #define IPSKB_FORWARDED		4
 #define IPSKB_XFRM_TUNNEL_SIZE	8
+#define IPSKB_XFRM_TRANSFORMED	16
 };

My only question about this patch is where should we clear these flags? For instance, when ipip/gre transmits a packet, should this flag (and perhaps other flags here) be cleared?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#36 lore raw

Re: [PATCH 07/13]: [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

Quoted 17 lines · Herbert Xu

include/net/ip.h struct inet_skb_parm · line 49

Quoted 10 lines · Patrick McHardy
include/net/ip.h +1 -0
index 9f09882..377036b 100644
@@ -45,6 +45,7 @@ struct inet_skb_parm
 #define IPSKB_TRANSLATED	2
 #define IPSKB_FORWARDED		4
 #define IPSKB_XFRM_TUNNEL_SIZE	8
+#define IPSKB_XFRM_TRANSFORMED	16
 };

My only question about this patch is where should we clear these flags? For instance, when ipip/gre transmits a packet, should this flag (and perhaps other flags here) be cleared?

Good point. This specific flags should be cleared when a packet (re-)enters the IP stack, I guess by definition of the cb, this holds for the other flags as well. Looking at the other flags:

- IPSKB_MASQUERADED is unused
- IPSKB_TRANSLATED is unused
- IPSKB_FORWARDED is used by ipmr in a way that looks broken, it expects the flags on the input path to be the same it set on the output path.
- IPSKB_XFRM_TUNNEL_SIZE should be cleared when a packet enters the IP stack

It seems in most places where only IPCB(skb)->opt is cleared the entire CB should be cleared. A couple of spots also look completely unnecessary, for example all places clearing the CB before passing the packet to netif_rx. I would expect the next user beeing responsible for clearing the space he needs if neccessary.

maintainer
#37 lore raw

Re: [PATCH 07/13]: [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

Quoted 8 lines · Patrick McHardy

- IPSKB_MASQUERADED is unused
- IPSKB_TRANSLATED is unused
- IPSKB_FORWARDED is used by ipmr in a way that looks broken, it expects the flags on the input path to be the same it set on the output path.
- IPSKB_XFRM_TUNNEL_SIZE should be cleared when a packet enters the IP stack

Yes that looks correct.

Quoted 6 lines

It seems in most places where only IPCB(skb)->opt is cleared the entire CB should be cleared. A couple of spots also look completely unnecessary, for example all places clearing the CB before passing the packet to netif_rx. I would expect the next user beeing responsible for clearing the space he needs if neccessary.

Agreed. However, it seems that ip_rcv() only clears the CB options if ihl is greater than 5. So until that's changed the people feeding netif_rx will have to clear the CB.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#38 lore raw

Re: [PATCH 07/13]: [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

From: Herbert Xu <[email protected]>
Date: Tue, 29 Nov 2005 18:34:41 +1100

Quoted 11 lines · Patrick McHardy
Quoted 6 lines

It seems in most places where only IPCB(skb)->opt is cleared the entire CB should be cleared. A couple of spots also look completely unnecessary, for example all places clearing the CB before passing the packet to netif_rx. I would expect the next user beeing responsible for clearing the space he needs if neccessary.

Agreed. However, it seems that ip_rcv() only clears the CB options if ihl is greater than 5. So until that's changed the people feeding netif_rx will have to clear the CB.

I wonder if that stuff can be simplified somehow.

We only use those options in two ways:

1) To process early in input via ip_options_compile() and the source route check in ip_rcv_finish()

2) To do forwarding processing on options in ip_forward_finish() and the multicast equivalent in ipmr.c

3) For locally destined packets, when the options are to be passed to the user via a recvmsg() CMSG.

Well, there is a 4th, which is what we're talking about here, which is all of the zero'ing out of the thing during encapsulation which is mostly a waste.

I think #1 and #2 can be handled by an on-stack copy of "struct ip_options" high enough in the call chain, but #3 is a bit less trivial to cope with like that.

It would be nice to kill the IPCB() copy, and give us 12 bytes back in skb->cb[] :-)

maintainer
#39 lore raw

Re: [PATCH 07/13]: [NETFILTER]: Fix xfrm lookup in ip_route_me_harder/ip6_route_me_harder

Quoted 7 lines · David S. Miller

I think #1 and #2 can be handled by an on-stack copy of "struct ip_options" high enough in the call chain, but #3 is a bit less trivial to cope with like that.

It would be nice to kill the IPCB() copy, and give us 12 bytes back in skb->cb[] :-)

That would be great. I'm glad that Patrick's patch set is bringing out potential ways of shrinking sk_buff.

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#40 lore raw

PATCH 8/13 : [NETFILTER]: Use conntrack information to determine if packet was NATed

In reply to Patrick McHardy

[NETFILTER]: Use conntrack information to determine if packet was NATed

Preparation for full IPsec support for NAT:

Use conntrack information instead of saving the saving and comparing the addresses to determine if a packet was NATed and needs to be rerouted to make it easier to extend the key.

---
commit 864cef1bc7d011f4a07b01043786107ad570c820
tree dbf671924ab6eb7bd87baaae9118bb57d7b3ab8e
parent ffa4445cd4284d3d9b688c80f5a3b9f8b26d59e6
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:05:26 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:05:26 +0100

 net/ipv4/netfilter/ip_nat_standalone.c |   34 ++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 15 deletions(-)
net/ipv4/netfilter/ip_nat_standalone.c +19 -15
index f04111f..1bb5089 100644
@@ -162,18 +162,20 @@ ip_nat_in(unsigned int hooknum,
           const struct net_device *out,
           int (*okfn)(struct sk_buff *))
 {
-	u_int32_t saddr, daddr;
+	struct ip_conntrack *ct;
+	enum ip_conntrack_info ctinfo;
 	unsigned int ret;
 
-	saddr = (*pskb)->nh.iph->saddr;
-	daddr = (*pskb)->nh.iph->daddr;
-
 	ret = ip_nat_fn(hooknum, pskb, in, out, okfn);
 	if (ret != NF_DROP && ret != NF_STOLEN
-	    && ((*pskb)->nh.iph->saddr != saddr
-	        || (*pskb)->nh.iph->daddr != daddr)) {
-		dst_release((*pskb)->dst);
-		(*pskb)->dst = NULL;
+	    && (ct = ip_conntrack_get(*pskb, &ctinfo)) != NULL) {
+		enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+
+		if (ct->tuplehash[dir].tuple.src.ip !=
+		    ct->tuplehash[!dir].tuple.dst.ip) {
+			dst_release((*pskb)->dst);
+			(*pskb)->dst = NULL;
+		}
 	}
 	return ret;
 }
@@ -200,7 +202,8 @@ ip_nat_local_fn(unsigned int hooknum,
 		const struct net_device *out,
 		int (*okfn)(struct sk_buff *))
 {
-	u_int32_t saddr, daddr;
+	struct ip_conntrack *ct;
+	enum ip_conntrack_info ctinfo;
 	unsigned int ret;
 
 	/* root is playing with raw sockets. */
@@ -208,14 +211,15 @@ ip_nat_local_fn(unsigned int hooknum,
 	    || (*pskb)->nh.iph->ihl * 4 < sizeof(struct iphdr))
 		return NF_ACCEPT;
 
-	saddr = (*pskb)->nh.iph->saddr;
-	daddr = (*pskb)->nh.iph->daddr;
-
 	ret = ip_nat_fn(hooknum, pskb, in, out, okfn);
 	if (ret != NF_DROP && ret != NF_STOLEN
-	    && ((*pskb)->nh.iph->saddr != saddr
-		|| (*pskb)->nh.iph->daddr != daddr))
-		return ip_route_me_harder(pskb) == 0 ? ret : NF_DROP;
+	    && (ct = ip_conntrack_get(*pskb, &ctinfo)) != NULL) {
+		enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+
+		if (ct->tuplehash[dir].tuple.dst.ip !=
+		    ct->tuplehash[!dir].tuple.src.ip)
+			return ip_route_me_harder(pskb) == 0 ? ret : NF_DROP;
+	}
 	return ret;
 }
 
#41 lore raw

PATCH 9/13 : [NETFILTER]: Redo policy lookups after NAT when neccessary

In reply to Patrick McHardy

[NETFILTER]: Redo policy lookups after NAT when neccessary

When NAT changes the key used for the xfrm lookup it needs to be done again.

---
commit 8cb6cfa80dd5dc4da1de280a0278746c262a2d8d
tree efffac2335bd21d0a0c2aa848df544002e4316f4
parent 864cef1bc7d011f4a07b01043786107ad570c820
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:07:34 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:07:34 +0100

 include/net/dst.h                      |    1 +
 net/ipv4/ip_output.c                   |    7 ++++++-
 net/ipv4/netfilter.c                   |    2 +-
 net/ipv4/netfilter/ip_nat_standalone.c |   27 +++++++++++++++++++++++++--
 4 files changed, 33 insertions(+), 4 deletions(-)
include/net/dst.h +1 -0
index 7eadd0c..4630e17 100644
@@ -237,6 +237,7 @@ static inline int dst_output(struct sk_b
 }
 
 #if defined(CONFIG_XFRM) && defined(CONFIG_NETFILTER)
+extern int __ip_dst_output(struct sk_buff *skb);
 extern int ip_dst_output(struct sk_buff *skb);
 extern int ip6_dst_output(struct sk_buff *skb);
 #else
net/ipv4/ip_output.c +6 -1
index 2c91f03..6836389 100644
@@ -195,13 +195,18 @@ static inline int ip_finish_output2(stru
 		return dst->neighbour->output(skb);
 
 	if (net_ratelimit())
-		printk(KERN_DEBUG "ip_finish_output2: No header cache and no neighbour!\n");
+		printk(KERN_DEBUG "ip_finish_output3: No header cache and no neighbour!\n");
 	kfree_skb(skb);
 	return -EINVAL;
 }
 
 static inline int ip_finish_output(struct sk_buff *skb)
 {
+#if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
+	/* Policy lookup after SNAT yielded a new policy */
+	if (skb->dst->xfrm != NULL)
+		return __ip_dst_output(skb);
+#endif
 	if (skb->len > dst_mtu(skb->dst) &&
 	    !(skb_shinfo(skb)->ufo_size || skb_shinfo(skb)->tso_size))
 		return ip_fragment(skb, ip_finish_output2);
net/ipv4/netfilter.c +1 -1
index db330b6..8fda96a 100644
@@ -87,7 +87,7 @@ int ip_route_me_harder(struct sk_buff **
 EXPORT_SYMBOL(ip_route_me_harder);
 
 #ifdef CONFIG_XFRM
-static inline int __ip_dst_output(struct sk_buff *skb)
+inline int __ip_dst_output(struct sk_buff *skb)
 {
 	int err;
 
net/ipv4/netfilter/ip_nat_standalone.c +25 -2
index 1bb5089..b518697 100644
@@ -187,12 +187,30 @@ ip_nat_out(unsigned int hooknum,
 	   const struct net_device *out,
 	   int (*okfn)(struct sk_buff *))
 {
+	struct ip_conntrack *ct;
+	enum ip_conntrack_info ctinfo;
+	unsigned int ret;
+
 	/* root is playing with raw sockets. */
 	if ((*pskb)->len < sizeof(struct iphdr)
 	    || (*pskb)->nh.iph->ihl * 4 < sizeof(struct iphdr))
 		return NF_ACCEPT;
 
-	return ip_nat_fn(hooknum, pskb, in, out, okfn);
+	ret = ip_nat_fn(hooknum, pskb, in, out, okfn);
+	if (ret != NF_DROP && ret != NF_STOLEN
+	    && (ct = ip_conntrack_get(*pskb, &ctinfo)) != NULL) {
+		enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
+
+		if (ct->tuplehash[dir].tuple.src.ip !=
+		    ct->tuplehash[!dir].tuple.dst.ip
+#ifdef CONFIG_XFRM
+		    || ct->tuplehash[dir].tuple.src.u.all !=
+		       ct->tuplehash[!dir].tuple.dst.u.all
+#endif
+		    )
+			return ip_route_me_harder(pskb) == 0 ? ret : NF_DROP;
+	}
+	return ret;
 }
 
 static unsigned int
@@ -217,7 +235,12 @@ ip_nat_local_fn(unsigned int hooknum,
 		enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
 
 		if (ct->tuplehash[dir].tuple.dst.ip !=
-		    ct->tuplehash[!dir].tuple.src.ip)
+		    ct->tuplehash[!dir].tuple.src.ip
+#ifdef CONFIG_XFRM
+		    || ct->tuplehash[dir].tuple.dst.u.all !=
+		       ct->tuplehash[dir].tuple.src.u.all
+#endif
+		    )
 			return ip_route_me_harder(pskb) == 0 ? ret : NF_DROP;
 	}
 	return ret;
#42 lore raw

Re: [PATCH 09/13]: [NETFILTER]: Redo policy lookups after NAT when neccessary

net/ipv4/ip_output.c static inline int ip_finish_output2(stru · line 201

Quoted 13 lines · Patrick McHardy

[NETFILTER]: Redo policy lookups after NAT when neccessary

net/ipv4/ip_output.c +1 -1
@@ -195,13 +195,18 @@ static inline int ip_finish_output2(stru
 		return dst->neighbour->output(skb);
 
 	if (net_ratelimit())
-		printk(KERN_DEBUG "ip_finish_output2: No header cache and no neighbour!\n");
+		printk(KERN_DEBUG "ip_finish_output3: No header cache and no neighbour!\n");
 	kfree_skb(skb);
 	return -EINVAL;
 }

Damnit .. if you apply the patches please edit out this chunk, its a remnant from an earlier series.

Thanks.

#43 lore raw

PATCH 10/13 : [NETFILTER]: Keep the conntrack reference until after policy checks

In reply to Patrick McHardy

[NETFILTER]: Keep the conntrack reference until after policy checks

Keep the conntrack reference until policy checks have been performed for IPsec NAT support. The reference needs to be dropped before a packet is queued to avoid having the conntrack module unloadable and before icmp_send is called to avoid having the reference attached manually to the outgoing ICMP packet and the conntrack entry confirmed when ICMP packets leaves the machine.

---
commit 3f615f37e68903f0eea66f5b242bfbb1875ee204
tree 4c3f9966a01f44784b9cd42e8f7091dbbc09fe5a
parent 8cb6cfa80dd5dc4da1de280a0278746c262a2d8d
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:08:45 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:08:45 +0100

 net/dccp/ipv4.c     |    1 +
 net/ipv4/ip_input.c |   15 +++++++--------
 net/ipv4/raw.c      |    1 +
 net/ipv4/tcp_ipv4.c |    1 +
 net/ipv4/udp.c      |    2 ++
 net/sctp/input.c    |    1 +
 6 files changed, 13 insertions(+), 8 deletions(-)
net/dccp/ipv4.c +1 -0
index ca03521..0030923 100644
@@ -1147,6 +1147,7 @@ int dccp_v4_rcv(struct sk_buff *skb)
 		dccp_pr_debug("xfrm4_policy_check failed\n");
 		goto discard_and_relse;
 	}
+	nf_reset(skb);
 
         if (sk_filter(sk, skb, 0)) {
 		dccp_pr_debug("sk_filter failed\n");
net/ipv4/ip_input.c +7 -8
index 473d0f2..1757778 100644
@@ -203,10 +203,6 @@ static inline int ip_local_deliver_finis
 
 	__skb_pull(skb, ihl);
 
-	/* Free reference early: we don't need it any more, and it may
-           hold ip_conntrack module loaded indefinitely. */
-	nf_reset(skb);
-
         /* Point into the IP datagram, just past the header. */
         skb->h.raw = skb->data;
 
@@ -231,10 +227,12 @@ static inline int ip_local_deliver_finis
 		if ((ipprot = rcu_dereference(inet_protos[hash])) != NULL) {
 			int ret;
 
-			if (!ipprot->no_policy &&
-			    !xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-				kfree_skb(skb);
-				goto out;
+			if (!ipprot->no_policy) {
+				if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+					kfree_skb(skb);
+					goto out;
+				}
+				nf_reset(skb);
 			}
 			ret = ipprot->handler(skb);
 			if (ret < 0) {
@@ -246,6 +244,7 @@ static inline int ip_local_deliver_finis
 			if (!raw_sk) {
 				if (xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 					IP_INC_STATS_BH(IPSTATS_MIB_INUNKNOWNPROTOS);
+					nf_reset(skb);
 					icmp_send(skb, ICMP_DEST_UNREACH,
 						  ICMP_PROT_UNREACH, 0);
 				}
net/ipv4/raw.c +1 -0
index 421538a..8251a28 100644
@@ -255,6 +255,7 @@ int raw_rcv(struct sock *sk, struct sk_b
 		kfree_skb(skb);
 		return NET_RX_DROP;
 	}
+	nf_reset(skb);
 
 	skb_push(skb, skb->data - skb->nh.raw);
 
net/ipv4/tcp_ipv4.c +1 -0
index 4d5021e..66300f9 100644
@@ -1238,6 +1238,7 @@ process:
 
 	if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_and_relse;
+	nf_reset(skb);
 
 	if (sk_filter(sk, skb, 0))
 		goto discard_and_relse;
net/ipv4/udp.c +2 -0
index 2422a5f..483ca69 100644
@@ -1001,6 +1001,7 @@ static int udp_queue_rcv_skb(struct sock
 		kfree_skb(skb);
 		return -1;
 	}
+	nf_reset(skb);
 
 	if (up->encap_type) {
 		/*
@@ -1163,6 +1164,7 @@ int udp_rcv(struct sk_buff *skb)
 
 	if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
 		goto drop;
+	nf_reset(skb);
 
 	/* No socket. Drop packet silently, if checksum is wrong */
 	if (udp_checksum_complete(skb))
net/sctp/input.c +1 -0
index b24ff2c..100f577 100644
@@ -225,6 +225,7 @@ int sctp_rcv(struct sk_buff *skb)
 
 	if (!xfrm_policy_check(sk, XFRM_POLICY_IN, skb, family))
 		goto discard_release;
+	nf_reset(skb);
 
 	ret = sk_filter(sk, skb, 1);
 	if (ret)
#44 lore raw

PATCH 11/13 : [NETFILTER]: Handle NAT in IPsec policy checks

In reply to Patrick McHardy

[NETFILTER]: Handle NAT in IPsec policy checks

Handle NAT of decapsulated IPsec packets by reconstructing the struct flowi of the original packet from the conntrack information for IPsec policy checks.

---
commit 8b46eb2d8365ab18cc965f37681033162a834fe5
tree 5e8b46d5acd6fa4b1445f181fcf8bce94cade5b0
parent 3f615f37e68903f0eea66f5b242bfbb1875ee204
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:09:15 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:09:15 +0100

 include/linux/netfilter.h              |   16 ++++++++++
 net/ipv4/netfilter.c                   |    3 ++
 net/ipv4/netfilter/ip_nat_standalone.c |   50 +++++++++++++++++++++++++++++++-
 net/xfrm/xfrm_policy.c                 |    2 +
 4 files changed, 69 insertions(+), 2 deletions(-)
include/linux/netfilter.h +16 -0
index be365e7..cc9ee97 100644
@@ -261,6 +261,20 @@ struct nf_queue_rerouter {
 extern int nf_register_queue_rerouter(int pf, struct nf_queue_rerouter *rer);
 extern int nf_unregister_queue_rerouter(int pf);
 
+#include <net/flow.h>
+extern void (*ip_nat_decode_session)(struct sk_buff *, struct flowi *);
+
+static inline void
+nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, int family)
+{
+#ifdef CONFIG_IP_NF_NAT_NEEDED
+	void (*decodefn)(struct sk_buff *, struct flowi *);
+
+	if (family == AF_INET && (decodefn = ip_nat_decode_session) != NULL)
+		decodefn(skb, fl);
+#endif
+}
+
 #ifdef CONFIG_PROC_FS
 #include <linux/proc_fs.h>
 extern struct proc_dir_entry *proc_net_netfilter;
@@ -269,6 +283,8 @@ extern struct proc_dir_entry *proc_net_n
 #else /* !CONFIG_NETFILTER */
 #define NF_HOOK(pf, hook, skb, indev, outdev, okfn) (okfn)(skb)
 static inline void nf_ct_attach(struct sk_buff *new, struct sk_buff *skb) {}
+static inline void
+nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, int family) {}
 #endif /*CONFIG_NETFILTER*/
 
 #endif /*__KERNEL__*/
net/ipv4/netfilter.c +3 -0
index 8fda96a..b66856d 100644
@@ -134,6 +134,9 @@ drop:
 }
 #endif /* CONFIG_XFRM */
 
+void (*ip_nat_decode_session)(struct sk_buff *, struct flowi *);
+EXPORT_SYMBOL(ip_nat_decode_session);
+
 /*
  * Extra routing may needed on local out, as the QUEUE target never
  * returns control to the table.
net/ipv4/netfilter/ip_nat_standalone.c +48 -2

71 lines not on this page · raw message

net/xfrm/xfrm_policy.c +2 -0
index e441f35..8351bb0 100644
@@ -22,6 +22,7 @@
 #include <linux/workqueue.h>
 #include <linux/notifier.h>
 #include <linux/netdevice.h>
+#include <linux/netfilter.h>
 #include <linux/module.h>
 #include <net/xfrm.h>
 #include <net/ip.h>
@@ -938,6 +939,7 @@ int __xfrm_policy_check(struct sock *sk,
 
 	if (xfrm_decode_session(skb, &fl, family) < 0)
 		return 0;
+	nf_nat_decode_session(skb, &fl, family);
 
 	/* First, check used SA against their selectors. */
 	if (skb->sp) {
#45 lore raw

PATCH 12/13 : [NETFILTER]: Export ip6_masked_addrcmp, don't pass IPv6 addresses on stack

In reply to Patrick McHardy

[NETFILTER]: Export ip6_masked_addrcmp, don't pass IPv6 addresses on stack

---
commit 055c50b770e63ced784808ae22ef339724b1a44c
tree b8dc07727bb80b83c5b236f4157ed588927f46da
parent 8b46eb2d8365ab18cc965f37681033162a834fe5
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:09:28 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:09:28 +0100

 include/linux/netfilter_ipv6/ip6_tables.h |    4 ++++
 net/ipv6/netfilter/ip6_tables.c           |   18 ++++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)
include/linux/netfilter_ipv6/ip6_tables.h +4 -0
index 2efc046..1e11010 100644
@@ -476,6 +476,10 @@ extern int ip6t_ext_hdr(u8 nexthdr);
 extern int ipv6_find_hdr(const struct sk_buff *skb, unsigned int *offset,
 			 u8 target);
 
+extern int ip6_masked_addrcmp(const struct in6_addr *addr1,
+			      const struct in6_addr *mask,
+			      const struct in6_addr *addr2);
+
 #define IP6T_ALIGN(s) (((s) + (__alignof__(struct ip6t_entry)-1)) & ~(__alignof__(struct ip6t_entry)-1))
 
 #endif /*__KERNEL__*/
net/ipv6/netfilter/ip6_tables.c +10 -8
index 7d49222..71a80e0 100644
@@ -128,13 +128,14 @@ static LIST_HEAD(ip6t_tables);
 #define up(x) do { printk("UP:%u:" #x "\n", __LINE__); up(x); } while(0)
 #endif
 
-static int ip6_masked_addrcmp(struct in6_addr addr1, struct in6_addr mask,
-			      struct in6_addr addr2)
+int
+ip6_masked_addrcmp(const struct in6_addr *addr1, const struct in6_addr *mask,
+                   const struct in6_addr *addr2)
 {
 	int i;
 	for( i = 0; i < 16; i++){
-		if((addr1.s6_addr[i] & mask.s6_addr[i]) != 
-		   (addr2.s6_addr[i] & mask.s6_addr[i]))
+		if((addr1->s6_addr[i] & mask->s6_addr[i]) != 
+		   (addr2->s6_addr[i] & mask->s6_addr[i]))
 			return 1;
 	}
 	return 0;
@@ -168,10 +169,10 @@ ip6_packet_match(const struct sk_buff *s
 
 #define FWINV(bool,invflg) ((bool) ^ !!(ip6info->invflags & invflg))
 
-	if (FWINV(ip6_masked_addrcmp(ipv6->saddr,ip6info->smsk,ip6info->src),
-		  IP6T_INV_SRCIP)
-	    || FWINV(ip6_masked_addrcmp(ipv6->daddr,ip6info->dmsk,ip6info->dst),
-		     IP6T_INV_DSTIP)) {
+	if (FWINV(ip6_masked_addrcmp(&ipv6->saddr, &ip6info->smsk,
+	                             &ip6info->src), IP6T_INV_SRCIP)
+	    || FWINV(ip6_masked_addrcmp(&ipv6->daddr, &ip6info->dmsk,
+	                                &ip6info->dst), IP6T_INV_DSTIP)) {
 		dprintf("Source or dest mismatch.\n");
 /*
 		dprintf("SRC: %u. Mask: %u. Target: %u.%s\n", ip->saddr,
@@ -2094,6 +2095,7 @@ EXPORT_SYMBOL(ip6t_register_target);
 EXPORT_SYMBOL(ip6t_unregister_target);
 EXPORT_SYMBOL(ip6t_ext_hdr);
 EXPORT_SYMBOL(ipv6_find_hdr);
+EXPORT_SYMBOL(ip6_masked_addrcmp);
 
 module_init(init);
 module_exit(fini);
#46 lore raw

PATCH 13/13 : [NETFILTER]: Add ipt_policy/ip6t_policy matches

In reply to Patrick McHardy

[NETFILTER]: Add ipt_policy/ip6t_policy matches

---
commit ff88b88efc987d1267eccf01e16880458d189a25
tree 53c34259c195cf64903940f151becd967bcce74d
parent 055c50b770e63ced784808ae22ef339724b1a44c
author Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:10:02 +0100 committer Patrick McHardy <[email protected]> Sat, 19 Nov 2005 22:10:02 +0100

 include/linux/netfilter_ipv4/ipt_policy.h  |   52 ++++++++
 include/linux/netfilter_ipv6/ip6t_policy.h |   52 ++++++++
 net/ipv4/netfilter/Kconfig                 |   10 ++
 net/ipv4/netfilter/Makefile                |    1 
 net/ipv4/netfilter/ipt_policy.c            |  170 +++++++++++++++++++++++++++
 net/ipv6/netfilter/Kconfig                 |   10 ++
 net/ipv6/netfilter/Makefile                |    1 
 net/ipv6/netfilter/ip6t_policy.c           |  175 ++++++++++++++++++++++++++++
 8 files changed, 471 insertions(+), 0 deletions(-)
include/linux/netfilter_ipv4/ipt_policy.h new file +52 -0
new file mode 100644
index 0000000..7fd1bec
@@ -0,0 +1,52 @@
+#ifndef _IPT_POLICY_H
+#define _IPT_POLICY_H
+
+#define IPT_POLICY_MAX_ELEM	4
+
+enum ipt_policy_flags
+{
+	IPT_POLICY_MATCH_IN	= 0x1,
+	IPT_POLICY_MATCH_OUT	= 0x2,
+	IPT_POLICY_MATCH_NONE	= 0x4,
+	IPT_POLICY_MATCH_STRICT	= 0x8,
+};
+
+enum ipt_policy_modes
+{
+	IPT_POLICY_MODE_TRANSPORT,
+	IPT_POLICY_MODE_TUNNEL
+};
+
+struct ipt_policy_spec
+{
+	u_int8_t	saddr:1,
+			daddr:1,
+			proto:1,
+			mode:1,
+			spi:1,
+			reqid:1;
+};
+
+struct ipt_policy_elem
+{
+	u_int32_t	saddr;
+	u_int32_t	smask;
+	u_int32_t	daddr;
+	u_int32_t	dmask;
+	u_int32_t	spi;
+	u_int32_t	reqid;
+	u_int8_t	proto;
+	u_int8_t	mode;
+
+	struct ipt_policy_spec	match;
+	struct ipt_policy_spec	invert;
+};
+
+struct ipt_policy_info
+{
+	struct ipt_policy_elem pol[IPT_POLICY_MAX_ELEM];
+	u_int16_t flags;
+	u_int16_t len;
+};
+
+#endif /* _IPT_POLICY_H */
include/linux/netfilter_ipv6/ip6t_policy.h new file +52 -0

52 lines not on this page · raw message

net/ipv4/netfilter/Kconfig +10 -0
index 9d3c8b5..5e8189f 100644
@@ -487,6 +487,16 @@ config IP_NF_MATCH_STRING
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_MATCH_POLICY
+       tristate "IPsec policy match support"
+       depends on IP_NF_IPTABLES && XFRM
+       help
+         Policy matching allows you to match packets based on the
+         IPsec policy that was used during decapsulation/will
+         be used during encapsulation.
+
+         To compile it as a module, choose M here.  If unsure, say N.
+
 # `filter', generic and specific targets
 config IP_NF_FILTER
 	tristate "Packet filtering"
net/ipv4/netfilter/Makefile +1 -0
index 058c48e..2202517 100644
@@ -71,6 +71,7 @@ obj-$(CONFIG_IP_NF_MATCH_TCPMSS) += ipt_
 obj-$(CONFIG_IP_NF_MATCH_REALM) += ipt_realm.o
 obj-$(CONFIG_IP_NF_MATCH_ADDRTYPE) += ipt_addrtype.o
 obj-$(CONFIG_IP_NF_MATCH_PHYSDEV) += ipt_physdev.o
+obj-$(CONFIG_IP_NF_MATCH_POLICY) += ipt_policy.o
 obj-$(CONFIG_IP_NF_MATCH_COMMENT) += ipt_comment.o
 obj-$(CONFIG_IP_NF_MATCH_STRING) += ipt_string.o
 
net/ipv4/netfilter/ipt_policy.c new file +170 -0

170 lines not on this page · raw message

net/ipv6/netfilter/Kconfig +10 -0
index 060d612..96eae96 100644
@@ -179,6 +179,16 @@ config IP6_NF_MATCH_PHYSDEV
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP6_NF_MATCH_POLICY
+	tristate "IPsec policy match support"
+	depends on IP6_NF_IPTABLES && XFRM
+	help
+	  Policy matching allows you to match packets based on the
+	  IPsec policy that was used during decapsulation/will
+	  be used during encapsulation.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 # The targets
 config IP6_NF_FILTER
 	tristate "Packet filtering"
net/ipv6/netfilter/Makefile +1 -0
index 9ab5b2c..c0c809b 100644
@@ -13,6 +13,7 @@ obj-$(CONFIG_IP6_NF_MATCH_OPTS) += ip6t_
 obj-$(CONFIG_IP6_NF_MATCH_IPV6HEADER) += ip6t_ipv6header.o
 obj-$(CONFIG_IP6_NF_MATCH_FRAG) += ip6t_frag.o
 obj-$(CONFIG_IP6_NF_MATCH_AHESP) += ip6t_esp.o ip6t_ah.o
+obj-$(CONFIG_IP6_NF_MATCH_POLICY) += ip6t_policy.o
 obj-$(CONFIG_IP6_NF_MATCH_EUI64) += ip6t_eui64.o
 obj-$(CONFIG_IP6_NF_MATCH_MULTIPORT) += ip6t_multiport.o
 obj-$(CONFIG_IP6_NF_MATCH_OWNER) += ip6t_owner.o
net/ipv6/netfilter/ip6t_policy.c new file +175 -0

175 lines not on this page · raw message

A message that is not in the archive (sent off-list or not yet received): [email protected]

#48 lore raw
Quoted 14 lines · Joerg Platte

Am Sonntag, 20. November 2005 17:31 schrieb Patrick McHardy:
Hi!

Quoted 10 lines

- policy lookups after NAT:

When NAT changes a packet it already calls ip_route_me_harder, which reroutes the packet and does a new policy lookup. It only looks at the IP addresses however, changing the port numbers require a new policy lookup as well. It also doesn't reroute in POST_ROUTING, since the packet has already been routed. To behave more like a regular tunnel device a policy lookup is now also done after SNAT and the packet is passed to dst_output again if the lookup yielded a new policy.

Quoted 1 line

I suppose, this is the reason, why masqueraded packages leave a recent kernel unencrypted, even if they would match the policy. It's still not implemented in mainline. Am I right? If yes, I hope your patches will be merged as soon as possible :-)

You're right, that's the reason. Since the patches touch quite a lot of code they won't make it in 2.6.15, though.

maintainer
#49 lore raw
In reply to Patrick McHardy

From: Patrick McHardy <[email protected]>
Date: Sun, 20 Nov 2005 17:31:28 +0100

Quoted 3 lines

This is the latest netfilter/IPsec patchset. Its purpose is to make IPsec look as much as a normal tunnel device to netfilter as possible and to enable NAT support.

I think there are some of these patches that we can merge in right now into net-2.6.16...

I want to do this so that Patrick doesn't have to repost 13 or so patches every time one of the parts still under discussion gets changed.

Actually, it seems the only part under discussion is how to avoid extension header reparsing and routing re-lookups on the ipv6 side. That could be fixed by a follow-on patch and is not %100 necessary for initial integration in my opinion.

Can I get agreement on that? Patrick sends me a dump of the current state of his patch set right now, we put that into net-2.6.16, and fix problems with followon patches.

Ok?

#50 lore raw

In article <[email protected]> (at Tue, 22 Nov 2005 14:34:38 -0800 (PST)), "David S. Miller" <[email protected]> says:

Quoted 9 lines

I want to do this so that Patrick doesn't have to repost 13 or so patches every time one of the parts still under discussion gets changed.

Actually, it seems the only part under discussion is how to avoid extension header reparsing and routing re-lookups on the ipv6 side. That could be fixed by a follow-on patch and is not %100 necessary for initial integration in my opinion.

Quoted 5 lines

Can I get agreement on that? Patrick sends me a dump of the current state of his patch set right now, we put that into net-2.6.16, and fix problems with followon patches.

Ok?

I believe he can manage these patches in his tree, but anyway...

Well, it is very important to fix the packet processing path issues (including the extension header issue) before we merge it to the mainline. Definitely.
What I want to ensure is that they will not reach the mainline tree without resolving those issues.

Thank you.

--yoshfuji

#51 lore raw
Quoted 2 lines · YOSHIFUJI Hideaki / $B5HF#1QL@ wrote:

I believe he can manage these patches in his tree, but anyway...

It is kind of annoying redoing 13 patches for every small change.

Quoted 5 lines

Well, it is very important to fix the packet processing path issues (including the extension header issue) before we merge it to the mainline. Definitely.
What I want to ensure is that they will not reach the mainline tree without resolving those issues.

How about we put the IPv4 patches in now and I continue working on the IPv6 side? The main approach is not going to change, and I think users would appreciate having them available in 2.6.15.

#52 lore raw
In reply to David S. Miller
Quoted 21 lines · David S. Miller

From: Patrick McHardy <[email protected]>
Date: Sun, 20 Nov 2005 17:31:28 +0100

Quoted 3 lines

This is the latest netfilter/IPsec patchset. Its purpose is to make IPsec look as much as a normal tunnel device to netfilter as possible and to enable NAT support.

I think there are some of these patches that we can merge in right now into net-2.6.16...

I want to do this so that Patrick doesn't have to repost 13 or so patches every time one of the parts still under discussion gets changed.

Actually, it seems the only part under discussion is how to avoid extension header reparsing and routing re-lookups on the ipv6 side. That could be fixed by a follow-on patch and is not %100 necessary for initial integration in my opinion.

Quoted 5 lines

Can I get agreement on that? Patrick sends me a dump of the current state of his patch set right now, we put that into net-2.6.16, and fix problems with followon patches.

Ok?

I would appreciate that, but I want to have a look closer look at Herbert's patches first. Unfortunately its late and I have to get up early, so its going to take me a day.

maintainer
#53 lore raw
Quoted 4 lines · Patrick McHardy

I would appreciate that, but I want to have a look closer look at Herbert's patches first. Unfortunately its late and I have to get up early, so its going to take me a day.

How about merging the patches that everybody has agreed on first?

So far, I haven't see any objections to patches 1 and 2 so they can go in straight away. They don't even touch IPv6.

Patches 3-6 could become redundant if you go with my suggestion.

The rest of them I haven't read yet so can't comment :)

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#54 lore raw

From: Herbert Xu <[email protected]>
Date: Wed, 23 Nov 2005 12:35:53 +1100

Quoted 8 lines

How about merging the patches that everybody has agreed on first?

So far, I haven't see any objections to patches 1 and 2 so they can go in straight away. They don't even touch IPv6.

Patches 3-6 could become redundant if you go with my suggestion.

The rest of them I haven't read yet so can't comment :)

View the net-2.6.16 GIT tree as a sort of playpen, much like -mm, until we get close to the real 2.6.16 upstream development openning up.

I rebase all the time, and I can pluck out and change patches at will.

maintainer
#55 lore raw
Quoted 7 lines · David S. Miller

View the net-2.6.16 GIT tree as a sort of playpen, much like -mm, until we get close to the real 2.6.16 upstream development openning up.

I rebase all the time, and I can pluck out and change patches at will.

Sure, I have no objections against merging the patch now and then add clean-ups later.

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#56 lore raw
In reply to David S. Miller

From: "David S. Miller" <[email protected]>
Date: Tue, 22 Nov 2005 19:36:31 -0800 (PST)

Quoted 10 lines

From: Herbert Xu <[email protected]>
Date: Wed, 23 Nov 2005 12:35:53 +1100

Quoted 7 lines

How about merging the patches that everybody has agreed on first?

So far, I haven't see any objections to patches 1 and 2 so they can go in straight away. They don't even touch IPv6.

Patches 3-6 could become redundant if you go with my suggestion.

Quoted 8 lines
Quoted 1 line

The rest of them I haven't read yet so can't comment :)

View the net-2.6.16 GIT tree as a sort of playpen, much like -mm, until we get close to the real 2.6.16 upstream development openning up.

I rebase all the time, and I can pluck out and change patches at will.

Then I agree. It can increase the number of tester, I think.

-- Yasuyuki Kozakai

maintainer
#57 lore raw
In reply to Patrick McHardy

From: Patrick McHardy <[email protected]>
Date: Wed, 23 Nov 2005 02:17:14 +0100

Quoted 3 lines

I would appreciate that, but I want to have a look closer look at Herbert's patches first. Unfortunately its late and I have to get up early, so its going to take me a day.

Take your time :)