Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-08-06 Thread Joakim Koskela
On Tuesday 17 July 2007 17:30:21 Joakim Koskela wrote:
  Joakim Koskela wrote:
   diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
   index fa1902d..7a39f4c 100644
   --- a/net/ipv4/xfrm4_input.c
   +++ b/net/ipv4/xfrm4_input.c
   @@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16
   encap_type) if (x-mode-input(x, skb))
 goto drop;
  
   - if (x-props.mode == XFRM_MODE_TUNNEL) {
   + if (x-props.mode == XFRM_MODE_TUNNEL ||
   + x-props.mode == XFRM_MODE_BEET) {
 decaps = 1;
 break;
 }
 
  I was under the impression that one of the main points of BEET is that
  it offers tunnel semantics but does only transport mode processing.
  Its necessary for inter-family tunnels, but shouldn't this be avoided
  for normal use?

 Yes, this is actually quite a nice improvement to the interfamily
 processing I (at least) haven't thought of before. Tested it  works fine
 (ipv4-ipv4).

It's been a while, but as a fyi in case there are comments / suggestions 
before submitting the whole patch again - it seems that this had some 
problems after all. Works ok for normal cases, but fails when using ip 
options for the inner packet as they don't get processed after being 
extracted from the pseudoheader. Calling something like ip_options_compile 
from beet_mode's input when handling ipv4 would do the trick, but seems a bit 
ugly  perhaps unsafe, I'd rather just put the whole packet through the loop 
again.

br, j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-08-06 Thread Patrick McHardy
Joakim Koskela wrote:
 On Tuesday 17 July 2007 17:30:21 Joakim Koskela wrote:
 
@@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16
encap_type) if (x-mode-input(x, skb))
goto drop;

-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL ||
+   x-props.mode == XFRM_MODE_BEET) {
decaps = 1;
break;
}

 It's been a while, but as a fyi in case there are comments / suggestions 
 before submitting the whole patch again - it seems that this had some 
 problems after all. Works ok for normal cases, but fails when using ip 
 options for the inner packet as they don't get processed after being 
 extracted from the pseudoheader. Calling something like ip_options_compile 
 from beet_mode's input when handling ipv4 would do the trick, but seems a bit 
 ugly  perhaps unsafe, I'd rather just put the whole packet through the loop 
 again.


Won't the options get parsed by ip_rcv() on the second reception?
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-08-06 Thread Joakim Koskela
On Monday 06 August 2007 15:08:12 Patrick McHardy wrote:
 
  It's been a while, but as a fyi in case there are comments / suggestions
  before submitting the whole patch again - it seems that this had some
  problems after all. Works ok for normal cases, but fails when using ip
  options for the inner packet as they don't get processed after being
  extracted from the pseudoheader. Calling something like
  ip_options_compile from beet_mode's input when handling ipv4 would do the
  trick, but seems a bit ugly  perhaps unsafe, I'd rather just put the
  whole packet through the loop again.

 Won't the options get parsed by ip_rcv() on the second reception?

Yes. The thing was that it seemed like we could get by with only a 
transport-mode- amount of processing of same-family beet packets. But unless 
we do some special processing during beet reception (which doesn't seem that 
elegant), it won't work. So I'm changing it back to tunnel-like processing.

br, j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-08-06 Thread Patrick McHardy
Joakim Koskela wrote:
 On Monday 06 August 2007 15:08:12 Patrick McHardy wrote:
 
It's been a while, but as a fyi in case there are comments / suggestions
before submitting the whole patch again - it seems that this had some
problems after all. Works ok for normal cases, but fails when using ip
options for the inner packet as they don't get processed after being
extracted from the pseudoheader. Calling something like
ip_options_compile from beet_mode's input when handling ipv4 would do the
trick, but seems a bit ugly  perhaps unsafe, I'd rather just put the
whole packet through the loop again.

Won't the options get parsed by ip_rcv() on the second reception?
 
 
 Yes. The thing was that it seemed like we could get by with only a 
 transport-mode- amount of processing of same-family beet packets. But unless 
 we do some special processing during beet reception (which doesn't seem that 
 elegant), it won't work. So I'm changing it back to tunnel-like processing.


I think you could parse the options directly after decapsulation
in xfrm4_mode_beet.c, that doesn't seem very ugly to me.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-31 Thread Joakim Koskela
On Thursday 19 July 2007 17:46:42 Patrick McHardy wrote:
 Joakim Koskela wrote:
  +   skb_push(skb, hdrlen);
  +   iphv6 = ipv6_hdr(skb);
  +
  +   skb_reset_network_header(skb);
  +   top_iphv6 = ipv6_hdr(skb);
  +
  +   protocol = iphv6-nexthdr;
  +   skb_pull(skb, delta);
  +   skb_reset_network_header(skb);
  +   top_iphv4 = ip_hdr(skb);
  +   skb_set_transport_header(skb, hdrlen);
  +   top_iphv4-ihl = (sizeof(struct iphdr)  2);
  +   top_iphv4-version = 4;
  +   top_iphv4-id = 0;
  +   top_iphv4-frag_off = htons(IP_DF);
  +   top_iphv4-ttl = dst_metric(dst-child, RTAX_HOPLIMIT);
  +   top_iphv4-saddr = x-props.saddr.a4;
  +   top_iphv4-daddr = x-id.daddr.a4;
  +   skb-transport_header += top_iphv4-ihl*4;
  +   top_iphv4-protocol = protocol;
  +
  +   skb-protocol = htons(ETH_P_IP);
  +#endif

 The output function in the IPv6/IPv4 case is called from
 xfrm6_output_one, which loops until after a tunnel mode
 encapsulation is done and then returns to the outer loop
 in xfrm6_output_finish2, which passes the packet through
 the netfilter hooks and continues with the next transform.


I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer, right? 
Isn't that called from xfrm4_output_one and subsequently passed through the 
right filters as well (as it has a ipv4 header by then)?

 There are multiple problems resulting from the inter-family
 encapsulation. First of all, the inner loop continues after
 beet mode encapsulation, skipping the netfilter hooks in
 case there are more transforms. It should (as with decaps = 1
 on input) at least call netfilter hooks after an inter-family
 transform. If the beet transform is the last one, the IPv4
 skb will be passed through the IPv6 netfilter hooks, which is
 clearly wrong. To fix these problems some restructuring in
 xfrm[46]_output.c seems to be necessary.

Couldn't this be solved just by ending the inner loop in case of beet mode (as 
it is done for tunnel)?

br, j

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-31 Thread Patrick McHardy
Joakim Koskela wrote:
 On Thursday 19 July 2007 17:46:42 Patrick McHardy wrote:
 
Joakim Koskela wrote:

+skb_push(skb, hdrlen);
+iphv6 = ipv6_hdr(skb);
+
+skb_reset_network_header(skb);
+top_iphv6 = ipv6_hdr(skb);
+
+protocol = iphv6-nexthdr;
+skb_pull(skb, delta);
+skb_reset_network_header(skb);
+top_iphv4 = ip_hdr(skb);
+skb_set_transport_header(skb, hdrlen);
+top_iphv4-ihl = (sizeof(struct iphdr)  2);
+top_iphv4-version = 4;
+top_iphv4-id = 0;
+top_iphv4-frag_off = htons(IP_DF);
+top_iphv4-ttl = dst_metric(dst-child, RTAX_HOPLIMIT);
+top_iphv4-saddr = x-props.saddr.a4;
+top_iphv4-daddr = x-id.daddr.a4;
+skb-transport_header += top_iphv4-ihl*4;
+top_iphv4-protocol = protocol;
+
+skb-protocol = htons(ETH_P_IP);
+#endif

The output function in the IPv6/IPv4 case is called from
xfrm6_output_one, which loops until after a tunnel mode
encapsulation is done and then returns to the outer loop
in xfrm6_output_finish2, which passes the packet through
the netfilter hooks and continues with the next transform.

 
 
 I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer, 
 right? 
 Isn't that called from xfrm4_output_one and subsequently passed through the 
 right filters as well (as it has a ipv4 header by then)?


I think you're right, it uses xfrm4_output. But there's a mismatch
in either case, in both cases (IPv4 and IPv6) we first call the
POSTROUTING hook for this family, than do the transform (changing
the family), then call the OUTPUT hook for the same family. So
either the POSTROUTING or the OUTPUT hook is called for the wrong
family.

There are multiple problems resulting from the inter-family
encapsulation. First of all, the inner loop continues after
beet mode encapsulation, skipping the netfilter hooks in
case there are more transforms. It should (as with decaps = 1
on input) at least call netfilter hooks after an inter-family
transform. If the beet transform is the last one, the IPv4
skb will be passed through the IPv6 netfilter hooks, which is
clearly wrong. To fix these problems some restructuring in
xfrm[46]_output.c seems to be necessary.
 
 
 Couldn't this be solved just by ending the inner loop in case of beet mode 
 (as 
 it is done for tunnel)?


If the assumption that xfrm4_output is used for IPv6 in IPv4
encapsulation is correct and the problem mentioned above is
fixed, that should work fine.
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-31 Thread Joakim Koskela
On Tuesday 31 July 2007 13:51:42 Patrick McHardy wrote:
 Joakim Koskela wrote:
  I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer,
  right? Isn't that called from xfrm4_output_one and subsequently passed
  through the right filters as well (as it has a ipv4 header by then)?

 I think you're right, it uses xfrm4_output. But there's a mismatch
 in either case, in both cases (IPv4 and IPv6) we first call the
 POSTROUTING hook for this family, than do the transform (changing
 the family), then call the OUTPUT hook for the same family. So
 either the POSTROUTING or the OUTPUT hook is called for the wrong
 family.

Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF  
hook based on the skb's [current] family should put things through the right 
hoops, right?

br, j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-31 Thread Patrick McHardy
Joakim Koskela wrote:
 On Tuesday 31 July 2007 13:51:42 Patrick McHardy wrote:
 
Joakim Koskela wrote:

I'm not sure I really got this. IPv6/IPv4 means IPv6 inner, IPv4 outer,
right? Isn't that called from xfrm4_output_one and subsequently passed
through the right filters as well (as it has a ipv4 header by then)?

I think you're right, it uses xfrm4_output. But there's a mismatch
in either case, in both cases (IPv4 and IPv6) we first call the
POSTROUTING hook for this family, than do the transform (changing
the family), then call the OUTPUT hook for the same family. So
either the POSTROUTING or the OUTPUT hook is called for the wrong
family.
 
 
 Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF  
 hook based on the skb's [current] family should put things through the right 
 hoops, right?


Almost, in xfrm4_output the conditional calling of the hook should
only be done for IPv4 and the IPCB is not valid for IPv6 of course.
Speaking of which, shouldn't the entire cb be zeroed for interfamily
transforms? xfrm4_tunnel_output only clears out the options, and I
think your patch didn't touch it at all ..

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-31 Thread Joakim Koskela
On Tuesday 31 July 2007 14:14:30 Patrick McHardy wrote:
 Joakim Koskela wrote:
  Ok, so changing int xfrm[46]_output(struct sk_buff*) to use the right PF
   hook based on the skb's [current] family should put things through the
  right hoops, right?

 Almost, in xfrm4_output the conditional calling of the hook should
 only be done for IPv4 and the IPCB is not valid for IPv6 of course.
 Speaking of which, shouldn't the entire cb be zeroed for interfamily
 transforms? xfrm4_tunnel_output only clears out the options, and I
 think your patch didn't touch it at all ..

Right, thanks for pointing out (to my understanding both flags and opts should 
be reset for 6-4, on 4-4 only the opts)!

br ,j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-19 Thread Joakim Koskela
Hi all,

Here's once again a corrected version of the patch adding support for
ipv4/ipv6 interfamily addressing for the ipsec BEET (Bound End-to-End
Tunnel) mode, as specified by the ietf draft found at:

http://www.ietf.org/internet-drafts/draft-nikander-esp-beet-mode-07.txt

The previous implementation required that both address pairs in the SA
were of the same family. This patch enables mixing ipv4 and ipv6
addresses. All combinations (4-4, 4-6, 6-4, 6-6) have been tested
using manual key setups.

Signed-off-by: Joakim Koskela [EMAIL PROTECTED]
Signed-off-by: Herbert Xu [EMAIL PROTECTED]
Signed-off-by: Diego Beltrami [EMAIL PROTECTED]
Signed-off-by: Miika Komu [EMAIL PROTECTED]
---

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index fa1902d..43c0d80 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -108,7 +108,9 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16 encap_type)
if (x-mode-input(x, skb))
goto drop;
 
-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL ||
+   (x-props.mode == XFRM_MODE_BEET 
+x-sel.family != AF_INET)) {
decaps = 1;
break;
}
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
index a73e710..20e0610 100644
--- a/net/ipv4/xfrm4_mode_beet.c
+++ b/net/ipv4/xfrm4_mode_beet.c
@@ -6,6 +6,7 @@
  *Herbert Xu [EMAIL PROTECTED]
  *Abhinav Pathak [EMAIL PROTECTED]
  *Jeff Ahrenholz [EMAIL PROTECTED]
+ *Joakim Koskela [EMAIL PROTECTED]
  */
 
 #include linux/init.h
@@ -24,92 +25,179 @@
  *  tot_len
  *  check
  *
- * On exit, skb-h will be set to the start of the payload to be processed
- * by x-type-output and skb-nh will be set to the top IP header.
+ * On exit, skb-transport_header will be set to the start of the
+ * payload to be processed by x-type-output and skb-network_header
+ * will be set to the top IP header.
  */
 static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
 {
-   struct iphdr *iph, *top_iph;
-   int hdrlen, optlen;
-
-   iph = ip_hdr(skb);
-   skb-transport_header = skb-network_header;
-
-   hdrlen = 0;
-   optlen = iph-ihl * 4 - sizeof(*iph);
-   if (unlikely(optlen))
-   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
-
-   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + hdrlen);
-   skb_reset_network_header(skb);
-   top_iph = ip_hdr(skb);
-   skb-transport_header += sizeof(*iph) - hdrlen;
-
-   memmove(top_iph, iph, sizeof(*iph));
-   if (unlikely(optlen)) {
-   struct ip_beet_phdr *ph;
-
-   BUG_ON(optlen  0);
-
-   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
-   ph-padlen = 4 - (optlen  4);
-   ph-hdrlen = optlen / 8;
-   ph-nexthdr = top_iph-protocol;
-   if (ph-padlen)
-   memset(ph + 1, IPOPT_NOP, ph-padlen);
-
-   top_iph-protocol = IPPROTO_BEETPH;
-   top_iph-ihl = sizeof(struct iphdr) / 4;
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+   struct ipv6hdr *iphv6, *top_iphv6;
+#endif
+   struct dst_entry *dst = skb-dst;
+   struct iphdr *iphv4, *top_iphv4;
+   int hdrlen;
+
+   if (ip_hdr(skb)-version == 4) {
+   int optlen;
+
+   /* 4-4 */
+   iphv4 = ip_hdr(skb);
+   skb-transport_header = skb-network_header;
+
+   hdrlen = 0;
+   optlen = iphv4-ihl * 4 - sizeof(*iphv4);
+   if (unlikely(optlen))
+   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
+
+   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + 
hdrlen);
+   skb_reset_network_header(skb);
+   top_iphv4 = ip_hdr(skb);
+   skb-transport_header += sizeof(*iphv4) - hdrlen;
+
+   memmove(top_iphv4, iphv4, sizeof(*iphv4));
+   if (unlikely(optlen)) {
+   struct ip_beet_phdr *ph;
+
+   BUG_ON(optlen  0);
+
+   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
+   ph-padlen = 4 - (optlen  4);
+   ph-hdrlen = optlen / 8;
+   ph-nexthdr = iphv4-protocol;
+   if (ph-padlen)
+   memset(ph + 1, IPOPT_NOP, ph-padlen);
+   top_iphv4-protocol = IPPROTO_BEETPH;
+   top_iphv4-ihl = sizeof(struct iphdr) / 4;
+   }
+
+   top_iphv4-saddr = x-props.saddr.a4;
+   top_iphv4-daddr = x-id.daddr.a4;
+
+   skb-protocol = htons(ETH_P_IP);
+   } else if (ip_hdr(skb)-version == 6) {
+#if 

Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-19 Thread Patrick McHardy
Joakim Koskela wrote:
  static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
  {

[... ipv4 handling, looks fine ...]

 +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
 + int delta = sizeof(struct ipv6hdr) - sizeof(struct iphdr);
 + u8 protocol;
 +
 + /* Inner = 6, Outer = 4 : changing the external IP hdr
 +  * to the outer addresses
 +  */
 + hdrlen = x-props.header_len - IPV4_BEET_PHMAXLEN;


This still looks wrong. You removed the IPV4_BEET_PHMAXLEN addition
in esp6_init_state, which was correct since you don't do option
encapsulation for IPv6, but you still substract it here. What
also seems to be missing is accounting for the size difference
between IPv4 and IPv6 headers. In the inner IPv6, outer IPv4 case
its not too important, the other way around we need 20 bytes of
additional space plus room for option encapsulation. By not including
it in the header_len you're breaking MTU calculation and potentially
causing unnecessary reallocations.

 + skb_push(skb, hdrlen);
 + iphv6 = ipv6_hdr(skb);
 +
 + skb_reset_network_header(skb);
 + top_iphv6 = ipv6_hdr(skb);
 +
 + protocol = iphv6-nexthdr;
 + skb_pull(skb, delta);
 + skb_reset_network_header(skb);
 + top_iphv4 = ip_hdr(skb);
 + skb_set_transport_header(skb, hdrlen);
 + top_iphv4-ihl = (sizeof(struct iphdr)  2);
 + top_iphv4-version = 4;
 + top_iphv4-id = 0;
 + top_iphv4-frag_off = htons(IP_DF);
 + top_iphv4-ttl = dst_metric(dst-child, RTAX_HOPLIMIT);
 + top_iphv4-saddr = x-props.saddr.a4;
 + top_iphv4-daddr = x-id.daddr.a4;
 + skb-transport_header += top_iphv4-ihl*4;
 + top_iphv4-protocol = protocol;
 +
 + skb-protocol = htons(ETH_P_IP);
 +#endif


The output function in the IPv6/IPv4 case is called from
xfrm6_output_one, which loops until after a tunnel mode
encapsulation is done and then returns to the outer loop
in xfrm6_output_finish2, which passes the packet through
the netfilter hooks and continues with the next transform.

There are multiple problems resulting from the inter-family
encapsulation. First of all, the inner loop continues after
beet mode encapsulation, skipping the netfilter hooks in
case there are more transforms. It should (as with decaps = 1
on input) at least call netfilter hooks after an inter-family
transform. If the beet transform is the last one, the IPv4
skb will be passed through the IPv6 netfilter hooks, which is
clearly wrong. To fix these problems some restructuring in
xfrm[46]_output.c seems to be necessary.

  static int xfrm4_beet_input(struct xfrm_state *x, struct sk_buff *skb)
  {
 [...]
 +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
 + } else if (x-sel.family == AF_INET6) {
 + /* Here, the inner family is 6, therefore I have to
 +  * substitute the IPhdr by enlarging it.
 +  */
 + if (skb_tailroom(skb)  delta) {
 + if (pskb_expand_head(skb, 0, delta, GFP_ATOMIC))


You want skb_headroom I suppose.

 diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
 index 4ff8ed3..ff3d638 100644
 --- a/net/ipv4/xfrm4_policy.c
 +++ b/net/ipv4/xfrm4_policy.c
 @@ -15,6 +15,7 @@
  
  static struct dst_ops xfrm4_dst_ops;
  static struct xfrm_policy_afinfo xfrm4_policy_afinfo;
 +static void xfrm4_update_pmtu(struct dst_entry *dst, u32 mtu);
  
  static int xfrm4_dst_lookup(struct xfrm_dst **dst, struct flowi *fl)
  {
 @@ -81,10 +82,17 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, struct 
 xfrm_state **xfrm, int
   }
   }
   };
 + union {
 +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
 + struct in6_addr *in6;
 +#endif
 + struct in_addr *in;
 + } remote, local;
   int i;
   int err;
   int header_len = 0;
   int trailer_len = 0;
 + unsigned short encap_family = 0;
  
   dst = dst_prev = NULL;
   dst_hold(rt-u.dst);
 @@ -113,12 +121,26 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, 
 struct xfrm_state **xfrm, int
  
   dst1-next = dst_prev;
   dst_prev = dst1;
 -
 + if (xfrm[i]-props.mode != XFRM_MODE_TRANSPORT) {
 + encap_family = xfrm[i]-props.family;
 + if (encap_family == AF_INET) {
 + remote.in = (struct in_addr *)
 + xfrm[i]-id.daddr.a4;
 + local.in  = (struct in_addr *)
 + xfrm[i]-props.saddr.a4;
 +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
 + } else if (encap_family == AF_INET6) {
 + remote.in6 = (struct in6_addr *)
 +

Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-19 Thread Joakim Koskela
On Thursday 19 July 2007 17:46:42 Patrick McHardy wrote:
  -
  +   if (xfrm[i]-props.mode != XFRM_MODE_TRANSPORT) {
  +   encap_family = xfrm[i]-props.family;
  +   if (encap_family == AF_INET) {
  +   remote.in = (struct in_addr *)
  +   xfrm[i]-id.daddr.a4;
  +   local.in  = (struct in_addr *)
  +   xfrm[i]-props.saddr.a4;
  +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
  +   } else if (encap_family == AF_INET6) {
  +   remote.in6 = (struct in6_addr *)
  +   xfrm[i]-id.daddr.a6;
  +   local.in6 = (struct in6_addr *)
  +   xfrm[i]-props.saddr.a6;
  +#endif

 You set the addresses above ..

..

 and don't seem to use them for anything.


Right. Thought I removed that [redundant code], but apparently only on the 
ipv6 side, thanks.


  diff --git a/net/ipv6/xfrm6_state.c b/net/ipv6/xfrm6_state.c
  +   /* Rule 5: select IPsec BEET */
  +   for (i = 0; i  n; i++) {
  +   if (src[i] 
  +   src[i]-props.mode == XFRM_MODE_BEET) {
  +   dst[j++] = src[i];
  +   src[i] = NULL;
  +   }
  +   }

 Just out of interest, is there any particular logic behind the
 ordering of the rules?


Got me there. Not that familiar with the details of the other modes to make 
even any educated guesses..

  if (likely(j == n))
  goto end;
 
  diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
  index 157bfbd..75fdb7d 100644
  --- a/net/xfrm/xfrm_policy.c
  +++ b/net/xfrm/xfrm_policy.c
  @@ -1299,7 +1299,8 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy,
  struct flowi *fl, xfrm_address_t *local  = saddr;
  struct xfrm_tmpl *tmpl = policy-xfrm_vec[i];
 
  -   if (tmpl-mode == XFRM_MODE_TUNNEL) {
  +   if (tmpl-mode == XFRM_MODE_TUNNEL ||
  +   tmpl-mode == XFRM_MODE_BEET) {

 Is this a bugfix?

  remote = tmpl-id.daddr;
  local = tmpl-saddr;
  family = tmpl-encap_family;
  diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
  index dfacb9c..0a2ff8e 100644
  --- a/net/xfrm/xfrm_state.c
  +++ b/net/xfrm/xfrm_state.c
  @@ -611,7 +611,7 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t
  *saddr, selector.
   */
  if (x-km.state == XFRM_STATE_VALID) {
  -   if (!xfrm_selector_match(x-sel, fl, family) ||
  +   if (!xfrm_selector_match(x-sel, fl, 
  x-sel.family) ||
  !security_xfrm_state_pol_flow_match(x, pol, 
  fl))
  continue;
  if (!best ||
  @@ -623,7 +623,7 @@ xfrm_state_find(xfrm_address_t *daddr, xfrm_address_t
  *saddr, acquire_in_progress = 1;
  } else if (x-km.state == XFRM_STATE_ERROR ||
 x-km.state == XFRM_STATE_EXPIRED) {
  -   if (xfrm_selector_match(x-sel, fl, family) 
  +   if (xfrm_selector_match(x-sel, fl, 
  x-sel.family) 
  security_xfrm_state_pol_flow_match(x, pol, 
  fl))
  error = -ESRCH;
  }

 And these two? Also look like bugfixes ..
 -

Well yes if we're using interfamily anywhere. D'you think they deserve a patch 
for themselves?

Thanks again for reviewing, I'll address the other issues asap. Sort of eager 
to get this out as its been dangling for such a long time, but seems I'm 
taking a lot of things for granted (..as its been sitting around 'ok' for so 
long). 

br, j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-19 Thread Patrick McHardy

Joakim Koskela wrote:

On Thursday 19 July 2007 17:46:42 Patrick McHardy wrote:
  

And these two? Also look like bugfixes ..




Well yes if we're using interfamily anywhere. D'you think they deserve a patch 
for themselves?
  



Yes, that looks worth it. And it will help reduce the size of your
patch and make it easier reviewable.


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-18 Thread David Miller
From: Arnaldo Carvalho de Melo [EMAIL PROTECTED]
Date: Mon, 16 Jul 2007 09:56:59 -0300

 Sorry for not commented that the code you were using (and David said
 it was invalid) is in fact valid:
 
 skb-transport_header = skb-network_header;
 
 This works for both offsets and pointers, i.e. both transport_header
 and network_header are in the same address space.
 
 
  skb_set_transport_header(skb, skb_network_offset(skb));
 
 Also works, but its too convoluted IMHO, for pointers it would reduce to:
 
   skb-transport_header = skb-data + skb-network_header
 - skb-data;
 
 for offsets:
 
   skb-transport_header = skb-data - skb-head;
   skb-transport_header += skb-head + skb-network_header
 - skb-data;
 
 I.e. both reduce to:
 
skb-transport_header = skb-network_header;
 
 Some more comments below, but I think this time, sans the above
 possible cleanup, your patch is OK wrt offsets/pointers.

Thanks for the correction, indeed you are correct :)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-17 Thread Joakim Koskela
On Monday 16 July 2007 21:47:40 Patrick McHardy wrote:

 I lost interest here, but the reintroduced bugs make me think that
 some old version was simply rediffed without even checking the
 output and the state initialization also seems to need a bit more work.


Thanks for reviewing the code, really appreciate it (whoa, would have been a 
lot of problems [re-]introduced)! And yes, you're right - it seemed at the 
time easier to just convert the old code to run in the new kernel as it's 
been working fine for us. Quickly scanned the existing (non-interfamily) beet 
implementation, but I guess not thoroughly enough. Anyway, merged back the 
latest non-interfamily versions and rolling with those now. Should have a 
fixed version ready soon..

Some other comments:

 Joakim Koskela wrote:
  diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
  index fa1902d..7a39f4c 100644
  --- a/net/ipv4/xfrm4_input.c
  +++ b/net/ipv4/xfrm4_input.c
  @@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16
  encap_type) if (x-mode-input(x, skb))
  goto drop;
 
  -   if (x-props.mode == XFRM_MODE_TUNNEL) {
  +   if (x-props.mode == XFRM_MODE_TUNNEL ||
  +   x-props.mode == XFRM_MODE_BEET) {
  decaps = 1;
  break;
  }

 I was under the impression that one of the main points of BEET is that
 it offers tunnel semantics but does only transport mode processing.
 Its necessary for inter-family tunnels, but shouldn't this be avoided
 for normal use?


Yes, this is actually quite a nice improvement to the interfamily processing I 
(at least) haven't thought of before. Tested it  works fine (ipv4-ipv4).


  diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
  index 44ef208..8db7910 100644
  --- a/net/ipv4/xfrm4_output.c
  +++ b/net/ipv4/xfrm4_output.c
  @@ -53,7 +53,8 @@ static int xfrm4_output_one(struct sk_buff *skb)
  goto error_nolock;
  }
 
  -   if (x-props.mode == XFRM_MODE_TUNNEL) {
  +   if (x-props.mode == XFRM_MODE_TUNNEL ||
  +   x-props.mode == XFRM_MODE_BEET) {
  err = xfrm4_tunnel_check_size(skb);

 Its not a real tunnel and all packets are generated locally, why
 does it need to send ICMPs?

Guess not. I'll have to still trace through, but can probably be removed.

  +   if (xfrm[i]-props.mode != XFRM_MODE_TRANSPORT) {
  +   encap_family = xfrm[i]-props.family;
  +   if (encap_family == AF_INET) {
  +   remote.in = (struct in_addr *)
  +   xfrm[i]-id.daddr.a4;
  +   local.in  = (struct in_addr *)
  +   xfrm[i]-props.saddr.a4;
  +   } else if (encap_family == AF_INET6) {
  +   remote.in6 = (struct in6_addr *)
  +   xfrm[i]-id.daddr.a6;
  +   local.in6 = (struct in6_addr *)
  +   xfrm[i]-props.saddr.a6;
  +   }

 No ifdefs here?

Thanks for noticing!

   static int ipip_init_state(struct xfrm_state *x)
   {
  -   if (x-props.mode != XFRM_MODE_TUNNEL)
  +   if (x-props.mode != XFRM_MODE_TUNNEL ||
  +   x-props.mode != XFRM_MODE_BEET)
  return -EINVAL;

 Looks like a bug fix that should be seperated.


Probably. This has been there for a while, don't know what's the story behind 
it, have to check..

br, j
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-17 Thread Patrick McHardy
Joakim Koskela wrote:
 On Monday 16 July 2007 21:47:40 Patrick McHardy wrote:
 
diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
index 44ef208..8db7910 100644
--- a/net/ipv4/xfrm4_output.c
+++ b/net/ipv4/xfrm4_output.c
@@ -53,7 +53,8 @@ static int xfrm4_output_one(struct sk_buff *skb)
 goto error_nolock;
 }

-if (x-props.mode == XFRM_MODE_TUNNEL) {
+if (x-props.mode == XFRM_MODE_TUNNEL ||
+x-props.mode == XFRM_MODE_BEET) {
 err = xfrm4_tunnel_check_size(skb);

Its not a real tunnel and all packets are generated locally, why
does it need to send ICMPs?
 
 
 Guess not. I'll have to still trace through, but can probably be removed.


Just FYI: it does make a difference with netfilter since packets
may be NATed to match a policy, but thats a more general problem
that also affects transport mode and should be dealt with within
netfilter, possibly by propagating PMTU values amonst dst_entries.

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-16 Thread Joakim Koskela
Hi all,

Here's again a cleaned-up and corrected version of the patch adding
support for ipv4/ipv6 interfamily addressing for the ipsec BEET (Bound
End-to-End Tunnel) mode, as specified by the ietf draft found at:

http://www.ietf.org/internet-drafts/draft-nikander-esp-beet-mode-07.txt

The previous implementation required that both address pairs in the SA
were of the same family. This patch enables mixing ipv4 and ipv6
addresses. All combinations (4-4, 4-6, 6-4, 6-6) have been tested
using manual key setups.

Signed-off-by: Joakim Koskela [EMAIL PROTECTED]
Signed-off-by: Herbert Xu [EMAIL PROTECTED]
Signed-off-by: Diego Beltrami [EMAIL PROTECTED]
Signed-off-by: Miika Komu [EMAIL PROTECTED]
---

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index fa1902d..7a39f4c 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16 encap_type)
if (x-mode-input(x, skb))
goto drop;
 
-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL ||
+   x-props.mode == XFRM_MODE_BEET) {
decaps = 1;
break;
}
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
index a73e710..2994dc5 100644
--- a/net/ipv4/xfrm4_mode_beet.c
+++ b/net/ipv4/xfrm4_mode_beet.c
@@ -6,6 +6,7 @@
  *Herbert Xu [EMAIL PROTECTED]
  *Abhinav Pathak [EMAIL PROTECTED]
  *Jeff Ahrenholz [EMAIL PROTECTED]
+ *Joakim Koskela [EMAIL PROTECTED]
  */
 
 #include linux/init.h
@@ -29,86 +30,176 @@
  */
 static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
 {
-   struct iphdr *iph, *top_iph;
-   int hdrlen, optlen;
-
-   iph = ip_hdr(skb);
-   skb-transport_header = skb-network_header;
-
-   hdrlen = 0;
-   optlen = iph-ihl * 4 - sizeof(*iph);
-   if (unlikely(optlen))
-   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
-
-   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + hdrlen);
-   skb_reset_network_header(skb);
-   top_iph = ip_hdr(skb);
-   skb-transport_header += sizeof(*iph) - hdrlen;
-
-   memmove(top_iph, iph, sizeof(*iph));
-   if (unlikely(optlen)) {
-   struct ip_beet_phdr *ph;
-
-   BUG_ON(optlen  0);
-
-   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
-   ph-padlen = 4 - (optlen  4);
-   ph-hdrlen = optlen / 8;
-   ph-nexthdr = top_iph-protocol;
-   if (ph-padlen)
-   memset(ph + 1, IPOPT_NOP, ph-padlen);
-
-   top_iph-protocol = IPPROTO_BEETPH;
-   top_iph-ihl = sizeof(struct iphdr) / 4;
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+   struct ipv6hdr *iphv6, *top_iphv6;
+#endif
+   struct dst_entry *dst = skb-dst;
+   struct iphdr *iphv4, *top_iphv4;
+   int hdrlen;
+
+   if (ip_hdr(skb)-version == 4) {
+   int optlen;
+
+   /* 4-4 */
+   iphv4 = ip_hdr(skb);
+   skb_set_transport_header(skb, skb_network_offset(skb));
+
+   hdrlen = x-props.header_len;
+   optlen = iphv4-ihl * 4 - sizeof(*iphv4);
+   if (!optlen) {
+   hdrlen -= IPV4_BEET_PHMAXLEN;
+   } else {
+   skb-transport_header -=
+   (IPV4_BEET_PHMAXLEN - (optlen  4));
+   hdrlen -= optlen  4;
+   }
+
+   skb_push(skb, hdrlen);
+   skb_reset_network_header(skb);
+
+   top_iphv4 = ip_hdr(skb);
+   hdrlen = iphv4-ihl * 4 - optlen;
+   skb-transport_header += hdrlen;
+   memmove(top_iphv4, iphv4, hdrlen);
+
+   if (unlikely(optlen)) {
+   struct ip_beet_phdr *ph;
+
+   BUG_ON(optlen  0);
+
+   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
+   ph-padlen = 4 - (optlen  4);
+   ph-hdrlen = (optlen + ph-padlen + sizeof(*ph)) / 8;
+   ph-nexthdr = iphv4-protocol;
+   top_iphv4-protocol = IPPROTO_BEETPH;
+   top_iphv4-ihl = sizeof(struct iphdr) / 4;
+   }
+
+   top_iphv4-saddr = x-props.saddr.a4;
+   top_iphv4-daddr = x-id.daddr.a4;
+
+   skb-protocol = htons(ETH_P_IP);
+   } else if (ip_hdr(skb)-version == 6) {
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+   int delta = sizeof(struct ipv6hdr) - sizeof(struct iphdr);
+   u8 protocol;
+
+   /* Inner = 6, Outer = 4 : changing the external IP hdr
+* to the outer addresses
+ 

Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-16 Thread Arnaldo Carvalho de Melo

On 7/16/07, Joakim Koskela [EMAIL PROTECTED] wrote:

Hi all,

Here's again a cleaned-up and corrected version of the patch adding
support for ipv4/ipv6 interfamily addressing for the ipsec BEET (Bound
End-to-End Tunnel) mode, as specified by the ietf draft found at:

http://www.ietf.org/internet-drafts/draft-nikander-esp-beet-mode-07.txt

The previous implementation required that both address pairs in the SA
were of the same family. This patch enables mixing ipv4 and ipv6
addresses. All combinations (4-4, 4-6, 6-4, 6-6) have been tested
using manual key setups.

Signed-off-by: Joakim Koskela [EMAIL PROTECTED]
Signed-off-by: Herbert Xu [EMAIL PROTECTED]
Signed-off-by: Diego Beltrami [EMAIL PROTECTED]
Signed-off-by: Miika Komu [EMAIL PROTECTED]
---

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index fa1902d..7a39f4c 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16 encap_type)
if (x-mode-input(x, skb))
goto drop;

-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL ||
+   x-props.mode == XFRM_MODE_BEET) {
decaps = 1;
break;
}
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
index a73e710..2994dc5 100644
--- a/net/ipv4/xfrm4_mode_beet.c
+++ b/net/ipv4/xfrm4_mode_beet.c
@@ -6,6 +6,7 @@
  *Herbert Xu [EMAIL PROTECTED]
  *Abhinav Pathak [EMAIL PROTECTED]
  *Jeff Ahrenholz [EMAIL PROTECTED]
+ *Joakim Koskela [EMAIL PROTECTED]
  */

 #include linux/init.h
@@ -29,86 +30,176 @@
  */
 static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
 {
-   struct iphdr *iph, *top_iph;
-   int hdrlen, optlen;
-
-   iph = ip_hdr(skb);
-   skb-transport_header = skb-network_header;
-
-   hdrlen = 0;
-   optlen = iph-ihl * 4 - sizeof(*iph);
-   if (unlikely(optlen))
-   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
-
-   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + hdrlen);
-   skb_reset_network_header(skb);
-   top_iph = ip_hdr(skb);
-   skb-transport_header += sizeof(*iph) - hdrlen;
-
-   memmove(top_iph, iph, sizeof(*iph));
-   if (unlikely(optlen)) {
-   struct ip_beet_phdr *ph;
-
-   BUG_ON(optlen  0);
-
-   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
-   ph-padlen = 4 - (optlen  4);
-   ph-hdrlen = optlen / 8;
-   ph-nexthdr = top_iph-protocol;
-   if (ph-padlen)
-   memset(ph + 1, IPOPT_NOP, ph-padlen);
-
-   top_iph-protocol = IPPROTO_BEETPH;
-   top_iph-ihl = sizeof(struct iphdr) / 4;
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+   struct ipv6hdr *iphv6, *top_iphv6;
+#endif
+   struct dst_entry *dst = skb-dst;
+   struct iphdr *iphv4, *top_iphv4;
+   int hdrlen;
+
+   if (ip_hdr(skb)-version == 4) {
+   int optlen;
+
+   /* 4-4 */
+   iphv4 = ip_hdr(skb);
+   skb_set_transport_header(skb, skb_network_offset(skb));


Sorry for not commented that the code you were using (and David said
it was invalid) is in fact valid:

   skb-transport_header = skb-network_header;

This works for both offsets and pointers, i.e. both transport_header
and network_header are in the same address space.


skb_set_transport_header(skb, skb_network_offset(skb));

Also works, but its too convoluted IMHO, for pointers it would reduce to:

 skb-transport_header = skb-data + skb-network_header
- skb-data;

for offsets:

 skb-transport_header = skb-data - skb-head;
 skb-transport_header += skb-head + skb-network_header
- skb-data;

I.e. both reduce to:

  skb-transport_header = skb-network_header;

Some more comments below, but I think this time, sans the above
possible cleanup, your patch is OK wrt offsets/pointers.

- Arnaldo


+
+   hdrlen = x-props.header_len;
+   optlen = iphv4-ihl * 4 - sizeof(*iphv4);
+   if (!optlen) {
+   hdrlen -= IPV4_BEET_PHMAXLEN;
+   } else {
+   skb-transport_header -=
+   (IPV4_BEET_PHMAXLEN - (optlen  4));
+   hdrlen -= optlen  4;
+   }
+
+   skb_push(skb, hdrlen);
+   skb_reset_network_header(skb);
+
+   top_iphv4 = ip_hdr(skb);
+   hdrlen = iphv4-ihl * 4 - optlen;
+   skb-transport_header += hdrlen;
+   memmove(top_iphv4, iphv4, hdrlen);
+
+   if (unlikely(optlen)) {
+   struct ip_beet_phdr *ph;
+
+   

Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-16 Thread Patrick McHardy
Joakim Koskela wrote:
 diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
 index fa1902d..7a39f4c 100644
 --- a/net/ipv4/xfrm4_input.c
 +++ b/net/ipv4/xfrm4_input.c
 @@ -108,7 +108,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16 encap_type)
   if (x-mode-input(x, skb))
   goto drop;
  
 - if (x-props.mode == XFRM_MODE_TUNNEL) {
 + if (x-props.mode == XFRM_MODE_TUNNEL ||
 + x-props.mode == XFRM_MODE_BEET) {
   decaps = 1;
   break;
   }


I was under the impression that one of the main points of BEET is that
it offers tunnel semantics but does only transport mode processing.
Its necessary for inter-family tunnels, but shouldn't this be avoided
for normal use?

 - ph-padlen = 4 - (optlen  4);
 - ph-hdrlen = optlen / 8;
 - ph-nexthdr = top_iph-protocol;
 - if (ph-padlen)
 - memset(ph + 1, IPOPT_NOP, ph-padlen);
 -

 + ph = (struct ip_beet_phdr *)skb_transport_header(skb);
 + ph-padlen = 4 - (optlen  4);
 + ph-hdrlen = (optlen + ph-padlen + sizeof(*ph)) / 8;


Reintroduces pseudo-header length bug.

 + ph-nexthdr = iphv4-protocol;
 + top_iphv4-protocol = IPPROTO_BEETPH;
 + top_iphv4-ihl = sizeof(struct iphdr) / 4;


Where did padding go?

  static int xfrm4_beet_input(struct xfrm_state *x, struct sk_buff *skb)
  {

 - if (unlikely(iph-protocol == IPPROTO_BEETPH)) {
 - struct ip_beet_phdr *ph;
 -
 - if (!pskb_may_pull(skb, sizeof(*ph)))
 - goto out;
 - ph = (struct ip_beet_phdr *)(ipip_hdr(skb) + 1);
 -
 - phlen = sizeof(*ph) + ph-padlen;
 - optlen = ph-hdrlen * 8 + (IPV4_BEET_PHMAXLEN - phlen);
 - if (optlen  0 || optlen  3 || optlen  250)
 - goto out;

 + if (unlikely(iph-protocol == IPPROTO_BEETPH)) {
 + struct ip_beet_phdr *ph =
 + (struct ip_beet_phdr*)(iph + 1);
 +
 + if (!pskb_may_pull(skb, sizeof(*ph)))
 + goto out;
 +
 + phlen = ph-hdrlen * 8;
 + optlen = phlen - ph-padlen - sizeof(*ph);

Reintroduces header length bug.

 diff --git a/net/ipv4/xfrm4_output.c b/net/ipv4/xfrm4_output.c
 index 44ef208..8db7910 100644
 --- a/net/ipv4/xfrm4_output.c
 +++ b/net/ipv4/xfrm4_output.c
 @@ -53,7 +53,8 @@ static int xfrm4_output_one(struct sk_buff *skb)
   goto error_nolock;
   }
  
 - if (x-props.mode == XFRM_MODE_TUNNEL) {
 + if (x-props.mode == XFRM_MODE_TUNNEL ||
 + x-props.mode == XFRM_MODE_BEET) {
   err = xfrm4_tunnel_check_size(skb);


Its not a real tunnel and all packets are generated locally, why
does it need to send ICMPs?

   if (err)
 @@ -81,10 +82,15 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, struct 
 xfrm_state **xfrm, int
   }
   }
   };
 + union {
 + struct in6_addr *in6;
 + struct in_addr *in;
 + } remote, local;
   int i;
   int err;
   int header_len = 0;
   int trailer_len = 0;
 + unsigned short encap_family = 0;
  
   dst = dst_prev = NULL;
   dst_hold(rt-u.dst);
 @@ -113,12 +119,24 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, 
 struct xfrm_state **xfrm, int
  
   dst1-next = dst_prev;
   dst_prev = dst1;
 -
 + if (xfrm[i]-props.mode != XFRM_MODE_TRANSPORT) {
 + encap_family = xfrm[i]-props.family;
 + if (encap_family == AF_INET) {
 + remote.in = (struct in_addr *)
 + xfrm[i]-id.daddr.a4;
 + local.in  = (struct in_addr *)
 + xfrm[i]-props.saddr.a4;
 + } else if (encap_family == AF_INET6) {
 + remote.in6 = (struct in6_addr *)
 + xfrm[i]-id.daddr.a6;
 + local.in6 = (struct in6_addr *)
 + xfrm[i]-props.saddr.a6;
 + }


No ifdefs here?

 + }
   header_len += xfrm[i]-props.header_len;
   trailer_len += xfrm[i]-props.trailer_len;
  
 - if (xfrm[i]-props.mode == XFRM_MODE_TUNNEL) {
 - unsigned short encap_family = xfrm[i]-props.family;
 + if (encap_family) {
   switch (encap_family) {
   case AF_INET:
   fl_tunnel.fl4_dst = xfrm[i]-id.daddr.a4;
 @@ -198,6 +216,12 @@ __xfrm4_bundle_create(struct xfrm_policy *policy, struct 
 xfrm_state 

Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-14 Thread David Miller
From: Joakim Koskela [EMAIL PROTECTED]
Date: Thu, 12 Jul 2007 12:25:23 +0300

 This (resubmitted) patch adds support for ipv4/ipv6 interfamily
 addressing for the ipsec BEET (Bound End-to-End Tunnel) mode, as
 specified by the ietf draft found at:
 
 http://www.ietf.org/internet-drafts/draft-nikander-esp-beet-mode-07.txt
 
 The previous implementation required that both address pairs in the SA
 were of the same family. This patch enables mixing ipv4 and ipv6
 addresses. All combinations (4-4, 4-6, 6-4, 6-6) have been tested
 using manual key setups.
 
 Signed-off-by: Joakim Koskela [EMAIL PROTECTED]
 Signed-off-by: Herbert Xu [EMAIL PROTECTED]
 Signed-off-by: Diego Beltrami [EMAIL PROTECTED]
 Signed-off-by: Miika Komu [EMAIL PROTECTED]

This patch has tons of problems, there is no way I can apply this.

Please validate your patches with a whitespace validator such as:

git apply --check --whitespace=error-all diff

in GIT as one example.

You patch adds tons of empty trailing whitespace, at least
24 lines had this problem.  In fact, nearly every single
empty line you added had this problem.

Your patch also uses spaces instead of proper tab characters.

Also, these lines are waaay too long, 80 columns please:

-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL || x-props.mode == 
XFRM_MODE_BEET) {

This may be a side effect of how you edit code or some setting in your
editor, please check it out.

And this patch has also serious bugs:

ph = (struct ip_beet_phdr *) skb-transport_header;

and later

struct ip_beet_phdr *ph = (struct ip_beet_phdr*)(skb-transport_header);

and again.

memmove(skb-data, skb-network_header, size);

skb-transport_header and skb-network_header ARE NOT pointers.  They
are either a pointer or a 32-bit offset in the SKB header buffer,
depending upon the definition of skb_buff_data_t.  Thus, the above
code will crash on 64-bit systems.

You also can't do this:

if (unlikely(!optlen))
skb-transport_header = skb-network_header;

Use the proper SKB header offset manipulation macros instead.

__u8 proto = ((struct ipv6hdr*)ip_hdr(skb))-nexthdr;
__u8 hops = ((struct ipv6hdr*)ip_hdr(skb))-hop_limit;

This kind of casting of an ipv4 header into an ipv6 one is incredibly
ugly and completely unnecessary, use the proper ipv6 header accessor
macro instead.

All of these trailing BUG_ON(1) statements don't really add any
value, if we get in here with the encapsulation type not AF_INET
or AF_INET6 we will crash in other more serious ways before we
get to these code blocks.  So please remove these dangling
BUG_ON(1) statements.

Please fix these bugs up and look for other instances of the same
problems.  There are standard interfaces to get the pointer of the
SKB transport header.

Do so starting with the following copy of your patch which has all
of the whitespace and coding-style issues fixed up.

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index 2fa1082..772c507 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -101,7 +101,8 @@ static int xfrm4_rcv_encap(struct sk_buff *skb, __u16 
encap_type)
if (x-mode-input(x, skb))
goto drop;
 
-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL ||
+   x-props.mode == XFRM_MODE_BEET) {
decaps = 1;
break;
}
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
index a73e710..d00c8e5 100644
--- a/net/ipv4/xfrm4_mode_beet.c
+++ b/net/ipv4/xfrm4_mode_beet.c
@@ -6,6 +6,7 @@
  *Herbert Xu [EMAIL PROTECTED]
  *Abhinav Pathak [EMAIL PROTECTED]
  *Jeff Ahrenholz [EMAIL PROTECTED]
+ *Joakim Koskela [EMAIL PROTECTED]
  */
 
 #include linux/init.h
@@ -29,41 +30,87 @@
  */
 static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
 {
-   struct iphdr *iph, *top_iph;
-   int hdrlen, optlen;
-
-   iph = ip_hdr(skb);
-   skb-transport_header = skb-network_header;
-
-   hdrlen = 0;
-   optlen = iph-ihl * 4 - sizeof(*iph);
-   if (unlikely(optlen))
-   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
-
-   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + hdrlen);
-   skb_reset_network_header(skb);
-   top_iph = ip_hdr(skb);
-   skb-transport_header += sizeof(*iph) - hdrlen;
-
-   memmove(top_iph, iph, sizeof(*iph));
-   if (unlikely(optlen)) {
-   struct ip_beet_phdr *ph;
-
-   BUG_ON(optlen  0);
-
-   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
-   ph-padlen = 4 - (optlen  4);
-   ph-hdrlen = optlen / 8;
-   ph-nexthdr = top_iph-protocol;
-   

[PATCH net-2.6.22-rc7] xfrm beet interfamily support

2007-07-12 Thread Joakim Koskela
Hi,

This (resubmitted) patch adds support for ipv4/ipv6 interfamily
addressing for the ipsec BEET (Bound End-to-End Tunnel) mode, as
specified by the ietf draft found at:

http://www.ietf.org/internet-drafts/draft-nikander-esp-beet-mode-07.txt

The previous implementation required that both address pairs in the SA
were of the same family. This patch enables mixing ipv4 and ipv6
addresses. All combinations (4-4, 4-6, 6-4, 6-6) have been tested
using manual key setups.

Signed-off-by: Joakim Koskela [EMAIL PROTECTED]
Signed-off-by: Herbert Xu [EMAIL PROTECTED]
Signed-off-by: Diego Beltrami [EMAIL PROTECTED]
Signed-off-by: Miika Komu [EMAIL PROTECTED]
---

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index fa1902d..105b36a 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -108,7 +108,7 @@ int xfrm4_rcv_encap(struct sk_buff *skb, __u16 encap_type)
if (x-mode-input(x, skb))
goto drop;
 
-   if (x-props.mode == XFRM_MODE_TUNNEL) {
+   if (x-props.mode == XFRM_MODE_TUNNEL || x-props.mode == 
XFRM_MODE_BEET) {
decaps = 1;
break;
}
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
index a73e710..004dc6b 100644
--- a/net/ipv4/xfrm4_mode_beet.c
+++ b/net/ipv4/xfrm4_mode_beet.c
@@ -6,6 +6,7 @@
  *Herbert Xu [EMAIL PROTECTED]
  *Abhinav Pathak [EMAIL PROTECTED]
  *Jeff Ahrenholz [EMAIL PROTECTED]
+ *Joakim Koskela [EMAIL PROTECTED]
  */
 
 #include linux/init.h
@@ -29,86 +30,175 @@
  */
 static int xfrm4_beet_output(struct xfrm_state *x, struct sk_buff *skb)
 {
-   struct iphdr *iph, *top_iph;
-   int hdrlen, optlen;
-
-   iph = ip_hdr(skb);
-   skb-transport_header = skb-network_header;
-
-   hdrlen = 0;
-   optlen = iph-ihl * 4 - sizeof(*iph);
-   if (unlikely(optlen))
-   hdrlen += IPV4_BEET_PHMAXLEN - (optlen  4);
-
-   skb_push(skb, x-props.header_len - IPV4_BEET_PHMAXLEN + hdrlen);
-   skb_reset_network_header(skb);
-   top_iph = ip_hdr(skb);
-   skb-transport_header += sizeof(*iph) - hdrlen;
-
-   memmove(top_iph, iph, sizeof(*iph));
-   if (unlikely(optlen)) {
-   struct ip_beet_phdr *ph;
-
-   BUG_ON(optlen  0);
-
-   ph = (struct ip_beet_phdr *)skb_transport_header(skb);
-   ph-padlen = 4 - (optlen  4);
-   ph-hdrlen = optlen / 8;
-   ph-nexthdr = top_iph-protocol;
-   if (ph-padlen)
-   memset(ph + 1, IPOPT_NOP, ph-padlen);
-
-   top_iph-protocol = IPPROTO_BEETPH;
-   top_iph-ihl = sizeof(struct iphdr) / 4;
-   }
-
-   top_iph-saddr = x-props.saddr.a4;
-   top_iph-daddr = x-id.daddr.a4;
-
-   return 0;
+struct dst_entry *dst = skb-dst;
+int hdrlen;
+struct iphdr *iphv4, *top_iphv4;
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+struct ipv6hdr *iphv6, *top_iphv6;
+#endif
+if (ip_hdr(skb)-version == 4) {
+int optlen;
+
+/* 4-4 */
+
+iphv4 = ip_hdr(skb);
+skb-transport_header = skb-network_header;
+
+hdrlen = x-props.header_len;
+optlen = iphv4-ihl * 4 - sizeof(*iphv4);
+
+if (!optlen) {
+hdrlen -= IPV4_BEET_PHMAXLEN;
+} else {
+skb-transport_header -= (IPV4_BEET_PHMAXLEN - (optlen 
 4));
+hdrlen -= optlen  4;
+}
+
+skb-network_header = skb_push(skb, hdrlen);
+
+top_iphv4 = ip_hdr(skb);
+hdrlen = iphv4-ihl * 4 - optlen;
+skb-transport_header += hdrlen;
+memmove(top_iphv4, iphv4, hdrlen);
+
+if (unlikely(optlen)) {
+struct ip_beet_phdr *ph;
+
+BUG_ON(optlen  0);
+
+ph = (struct ip_beet_phdr *)skb-transport_header;
+ph-padlen = 4 - (optlen  4);
+ph-hdrlen = (optlen + ph-padlen + sizeof(*ph)) / 8;
+ph-nexthdr = iphv4-protocol;
+top_iphv4-protocol = IPPROTO_BEETPH;
+top_iphv4-ihl = sizeof(struct iphdr) / 4;
+}
+
+top_iphv4-saddr = x-props.saddr.a4;
+top_iphv4-daddr = x-id.daddr.a4;
+
+skb-protocol = htons(ETH_P_IP);
+
+   } else if (ip_hdr(skb)-version == 6) {
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+
+u8 protocol;
+