Re: [PATCH net-2.6.22-rc7] xfrm beet interfamily support
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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; +