Herbert Xu wrote:

On Sun, Nov 20, 2005 at 04:31:34PM +0000, Patrick McHardy wrote:

quoted

diff --git a/net/ipv4/netfilter.c b/net/ipv4/netfilter.c
index ae0779d..b93e7cd 100644
--- a/net/ipv4/netfilter.c
+++ b/net/ipv4/netfilter.c

@@ -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

+	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.

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

diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 66620a9..c135746 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c

@@ -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?

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.