Re: diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)
On 04/04/2012 07:34 AM, YASUOKA Masahiko wrote: On Tue, 31 Jan 2012 13:59:17 +0100 Sebastian Reitenbachsebas...@l00-bugdead-prods.de wrote: However, I noted with tcpdump, listening on tun0: # tcpdump -n -i tun0 tcpdump: listening on tun0, link-type LOOP 13:51:15.354776 tcpdump: WARNING: compensating for unaligned libpcap packets 13:51:15.354795 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:16.334984 13:51:16.334997 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:17.355463 (snip) The incoming packets look weird, and this is not only true for icmp, with tcp/udp its the same. Attached diff will fix the problem. bpf requires to use different byte order for DLT_LOOP and DLT_NULL on address family header. pipex works with both pppx(DLT_NULL) and tun(DLT_LOOP), so it should switch the byte order. ok? or comment? Index: sys/net/pipex.c === RCS file: /cvs/src/sys/net/pipex.c,v retrieving revision 1.26 diff -u -p -r1.26 pipex.c --- sys/net/pipex.c 31 Jan 2012 12:04:20 - 1.26 +++ sys/net/pipex.c 8 Feb 2012 05:23:13 - @@ -1162,8 +1162,13 @@ pipex_ip_input(struct mbuf *m0, struct p len = m0-m_pkthdr.len; #if NBPFILTER 0 - if (ifp-if_bpf) - bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + if (ifp-if_bpf) { + if (ifp-if_type == IFT_TUNNEL) + bpf_mtap_af(ifp-if_bpf, htonl(AF_INET), m0, + BPF_DIRECTION_IN); + else + bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + } #endif s = splnet(); @@ -1239,8 +1244,14 @@ pipex_ip6_input(struct mbuf *m0, struct len = m0-m_pkthdr.len; #if NBPFILTER 0 - if (ifp-if_bpf) - bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + if (ifp-if_bpf) { + if (ifp-if_type == IFT_TUNNEL) + bpf_mtap_af(ifp-if_bpf, htonl(AF_INET6), m0, + BPF_DIRECTION_IN); + else + bpf_mtap_af(ifp-if_bpf, AF_INET6, m0, + BPF_DIRECTION_IN); + } #endif s = splnet(); Breaks nothing for me so far. //maxim
Re: diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)
On Wed, Apr 04, 2012 at 02:34:46PM +0900, Yasuoka Masahiko wrote: On Tue, 31 Jan 2012 13:59:17 +0100 Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote: However, I noted with tcpdump, listening on tun0: # tcpdump -n -i tun0 tcpdump: listening on tun0, link-type LOOP 13:51:15.354776 tcpdump: WARNING: compensating for unaligned libpcap packets 13:51:15.354795 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:16.334984 13:51:16.334997 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:17.355463 (snip) The incoming packets look weird, and this is not only true for icmp, with tcp/udp its the same. Attached diff will fix the problem. bpf requires to use different byte order for DLT_LOOP and DLT_NULL on address family header. pipex works with both pppx(DLT_NULL) and tun(DLT_LOOP), so it should switch the byte order. ok? or comment? Since pppx(4) is only used by npppd/pipex shouldn't we change the DLT there so that we don't need any magic inside pipex? -- :wq Claudio
diff: fix bpf problem of pipex (was Re: diff: fix LCP keepalive failures on L2TP.)
On Tue, 31 Jan 2012 13:59:17 +0100 Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote: However, I noted with tcpdump, listening on tun0: # tcpdump -n -i tun0 tcpdump: listening on tun0, link-type LOOP 13:51:15.354776 tcpdump: WARNING: compensating for unaligned libpcap packets 13:51:15.354795 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:16.334984 13:51:16.334997 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:17.355463 (snip) The incoming packets look weird, and this is not only true for icmp, with tcp/udp its the same. Attached diff will fix the problem. bpf requires to use different byte order for DLT_LOOP and DLT_NULL on address family header. pipex works with both pppx(DLT_NULL) and tun(DLT_LOOP), so it should switch the byte order. ok? or comment? Index: sys/net/pipex.c === RCS file: /cvs/src/sys/net/pipex.c,v retrieving revision 1.26 diff -u -p -r1.26 pipex.c --- sys/net/pipex.c 31 Jan 2012 12:04:20 - 1.26 +++ sys/net/pipex.c 8 Feb 2012 05:23:13 - @@ -1162,8 +1162,13 @@ pipex_ip_input(struct mbuf *m0, struct p len = m0-m_pkthdr.len; #if NBPFILTER 0 - if (ifp-if_bpf) - bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + if (ifp-if_bpf) { + if (ifp-if_type == IFT_TUNNEL) + bpf_mtap_af(ifp-if_bpf, htonl(AF_INET), m0, + BPF_DIRECTION_IN); + else + bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + } #endif s = splnet(); @@ -1239,8 +1244,14 @@ pipex_ip6_input(struct mbuf *m0, struct len = m0-m_pkthdr.len; #if NBPFILTER 0 - if (ifp-if_bpf) - bpf_mtap_af(ifp-if_bpf, AF_INET, m0, BPF_DIRECTION_IN); + if (ifp-if_bpf) { + if (ifp-if_type == IFT_TUNNEL) + bpf_mtap_af(ifp-if_bpf, htonl(AF_INET6), m0, + BPF_DIRECTION_IN); + else + bpf_mtap_af(ifp-if_bpf, AF_INET6, m0, + BPF_DIRECTION_IN); + } #endif s = splnet();
Re: diff: fix LCP keepalive failures on L2TP.
On Tuesday, January 31, 2012 10:02 CET, Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote: On Monday, January 30, 2012 15:43 CET, YASUOKA Masahiko yasu...@yasuoka.net wrote: On Mon, 30 Jan 2012 22:49:22 +0900 (JST) YASUOKA Masahiko yasu...@yasuoka.net wrote: pipex hook in udp_usrreq() mistakenly assumed that `inp' is connected. The hook could not use the destination address properly, so it failed to find the pipex session. This bug caused LCP keepalive failures on L2TP from client that does LCP keepalive and uses sequence number on the L2TP data channel (xl2tpd + pppd). The diff includes kernel header file changes. ok? Oops. Let me update the diff. The given struct sockaddr object of pipex_l2tp_userland_lookup_session() became passed from the userland, so its address family must be checked. ok? tested IPSec/L2TP with xl2tpd, strongswan client. I did setup the tunnel yesterday evening, and its still alive this morning. Without the patch, the tunnel usually died after a couple of minutes. So this definitely works for me, but I'm probably not the right guy to OK that in this phase of release cycle. I'll now try with Android phone, since I've seen the same problems with dying tunnel too. The tunnel established with Android 2.2, now also seems to be stable, at least for the last two hours and 15 minutes. Prior to the patch, like with the Linux client, the tunnel died within a couple of minutes, maybe half an hour or hour. However, I noted with tcpdump, listening on tun0: # tcpdump -n -i tun0 tcpdump: listening on tun0, link-type LOOP 13:51:15.354776 tcpdump: WARNING: compensating for unaligned libpcap packets 13:51:15.354795 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:16.334984 13:51:16.334997 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:17.355463 13:51:17.355474 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:18.375712 13:51:18.375722 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:19.354994 13:51:19.355004 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:20.375274 13:51:20.375285 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:21.355354 13:51:21.355364 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:22.334948 13:51:22.334959 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:23.394820 13:51:23.394831 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:24.374594 13:51:24.374605 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:25.394998 13:51:25.395009 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:26.394767 13:51:26.394777 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:27.415028 13:51:27.415039 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:28.375882 13:51:28.375892 10.66.66.1 10.66.66.129: icmp: echo reply (DF) Also when I show the contents with -X: # tcpdump -n -i tun0 -s 2000 -X tcpdump: listening on tun0, link-type LOOP 13:55:32.395223 : 4500 0054 4000 4001 a1a3 0a42 4281 E..T..@.@BB. 0010: 0a42 4201 0800 6b54 6063 08bc 43e5 274f .BB...kT`c..C.'O 0020: c554 0800 0809 0a0b 0c0d 0e0f 1011 1213 .T.. 0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223 !# 0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233 $%'()*+,-./0123 0050: 3435 36374567 tcpdump: WARNING: compensating for unaligned libpcap packets 13:55:32.395249 10.66.66.1 10.66.66.129: icmp: echo reply (DF) : 8266 4000 ff01 603c 0a42 4201 0a42 4281 .f@...`.BB..BB. 0010: 7354 6063 08bc 43e5 274f c554 0800 ..sT`c..C.'O.T.. 0020: 0809 0a0b 0c0d 0e0f 1011 1213 1415 1617 0030: 1819 1a1b 1c1d 1e1f 2021 2223 2425 2627 !#$%' 0040: 2829 2a2b 2c2d 2e2f 3031 3233 3435 3637 ()*+,-./01234567 0050: 13:55:34.315241 : 4500 0054 4000 4001 a1a3 0a42 4281 E..T..@.@BB. 0010: 0a42 4201 0800 5e54 6063 08bd 44e5 274f .BB...^T`c..D.'O 0020: d153 0800 0809 0a0b 0c0d 0e0f 1011 1213 .S.. 0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223 !# 0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233 $%'()*+,-./0123 0050: 3435 36374567 13:55:34.315253 10.66.66.1 10.66.66.129: icmp: echo reply (DF) : cd82 4000 ff01 1520 0a42 4201 0a42 4281 ..@ .BB..BB. 0010: 6654 6063 08bd 44e5 274f d153 0800 ..fT`c..D.'O.S.. 0020: 0809 0a0b 0c0d 0e0f 1011 1213 1415 1617 0030: 1819 1a1b 1c1d 1e1f 2021 2223 2425 2627 !#$%' 0040: 2829 2a2b 2c2d 2e2f 3031 3233 3435 3637 ()*+,-./01234567 0050: 13:55:34.994293 : 4500 0054 4000 4001 a1a3 0a42 4281 E..T..@.@BB. 0010: 0a42 4201 0800 8752 6063 08be 45e5 274f .BBR`c..E.'O 0020: a754 0800 0809 0a0b 0c0d 0e0f 1011 1213 .T.. 0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223
Re: diff: fix LCP keepalive failures on L2TP.
Hi, On Tue, 31 Jan 2012 13:59:17 +0100 Sebastian Reitenbach sebas...@l00-bugdead-prods.de wrote: However, I noted with tcpdump, listening on tun0: # tcpdump -n -i tun0 tcpdump: listening on tun0, link-type LOOP 13:51:15.354776 tcpdump: WARNING: compensating for unaligned libpcap packets 13:51:15.354795 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:16.334984 13:51:16.334997 10.66.66.1 10.66.66.129: icmp: echo reply (DF) 13:51:17.355463 (snip) # tcpdump -n -i tun0 -s 2000 -X tcpdump: listening on tun0, link-type LOOP 13:55:32.395223 : 4500 0054 4000 4001 a1a3 0a42 4281 E..T..@.@BB. 0010: 0a42 4201 0800 6b54 6063 08bc 43e5 274f .BB...kT`c..C.'O 0020: c554 0800 0809 0a0b 0c0d 0e0f 1011 1213 .T.. 0030: 1415 1617 1819 1a1b 1c1d 1e1f 2021 2223 !# 0040: 2425 2627 2829 2a2b 2c2d 2e2f 3031 3233 $%'()*+,-./0123 0050: 3435 36374567 This is a separate issue. I'll look into this. Thank you for your tests and reports. --yasuoka
diff: fix LCP keepalive failures on L2TP.
pipex hook in udp_usrreq() mistakenly assumed that `inp' is connected. The hook could not use the destination address properly, so it failed to find the pipex session. This bug caused LCP keepalive failures on L2TP from client that does LCP keepalive and uses sequence number on the L2TP data channel (xl2tpd + pppd). The diff includes kernel header file changes. ok? Index: sys/net/pipex.c === RCS file: /cvs/src/sys/net/pipex.c,v retrieving revision 1.25 diff -u -p -r1.25 pipex.c --- sys/net/pipex.c 23 Jan 2012 03:36:21 - 1.25 +++ sys/net/pipex.c 30 Jan 2012 12:11:16 - @@ -2210,7 +2210,7 @@ pipex_l2tp_userland_lookup_session_ipv6( } #endif -Static struct pipex_session * +struct pipex_session * pipex_l2tp_userland_lookup_session(struct mbuf *m0, struct sockaddr *sa) { struct pipex_l2tp_header l2tp; Index: sys/net/pipex.h === RCS file: /cvs/src/sys/net/pipex.h,v retrieving revision 1.11 diff -u -p -r1.11 pipex.h --- sys/net/pipex.h 23 Jan 2012 03:36:21 - 1.11 +++ sys/net/pipex.h 30 Jan 2012 12:11:16 - @@ -210,6 +210,7 @@ struct pipex_session *pipex_pptp_lookup struct mbuf *pipex_pptp_input (struct mbuf *, struct pipex_session *); struct pipex_session *pipex_pptp_userland_lookup_session_ipv4 (struct mbuf *, struct in_addr); struct pipex_session *pipex_pptp_userland_lookup_session_ipv6 (struct mbuf *, struct in6_addr); +struct pipex_session *pipex_l2tp_userland_lookup_session(struct mbuf *, struct sockaddr *); struct mbuf *pipex_pptp_userland_output (struct mbuf *, struct pipex_session *); struct pipex_session *pipex_l2tp_lookup_session (struct mbuf *, int); struct mbuf *pipex_l2tp_input (struct mbuf *, int off, struct pipex_session *); Index: sys/net/pipex_local.h === RCS file: /cvs/src/sys/net/pipex_local.h,v retrieving revision 1.14 diff -u -p -r1.14 pipex_local.h --- sys/net/pipex_local.h 25 Nov 2011 13:05:06 - 1.14 +++ sys/net/pipex_local.h 30 Jan 2012 12:11:16 - @@ -406,7 +406,6 @@ Static struct pipex_session *pipex_pptp #ifdef PIPEX_L2TP Static void pipex_l2tp_output (struct mbuf *, struct pipex_session *); -Static struct pipex_session *pipex_l2tp_userland_lookup_session(struct mbuf *, struct sockaddr *); #endif #ifdef PIPEX_MPPE Index: sys/netinet/udp_usrreq.c === RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.145 diff -u -p -r1.145 udp_usrreq.c --- sys/netinet/udp_usrreq.c8 Jul 2011 18:30:17 - 1.145 +++ sys/netinet/udp_usrreq.c30 Jan 2012 12:11:17 - @@ -1198,6 +1198,12 @@ udp_usrreq(struct socket *so, int req, s #ifdef PIPEX if (inp-inp_pipex) { struct pipex_session *session; + + if (addr != NULL) + session = + pipex_l2tp_userland_lookup_session(m, + mtod(addr, struct sockaddr *)); + else #ifdef INET6 if (inp-inp_flags INP_IPV6) session =
Re: diff: fix LCP keepalive failures on L2TP.
On Mon, 30 Jan 2012 22:49:22 +0900 (JST) YASUOKA Masahiko yasu...@yasuoka.net wrote: pipex hook in udp_usrreq() mistakenly assumed that `inp' is connected. The hook could not use the destination address properly, so it failed to find the pipex session. This bug caused LCP keepalive failures on L2TP from client that does LCP keepalive and uses sequence number on the L2TP data channel (xl2tpd + pppd). The diff includes kernel header file changes. ok? Oops. Let me update the diff. The given struct sockaddr object of pipex_l2tp_userland_lookup_session() became passed from the userland, so its address family must be checked. ok? Index: sys/net/pipex.c === RCS file: /cvs/src/sys/net/pipex.c,v retrieving revision 1.25 diff -u -p -r1.25 pipex.c --- sys/net/pipex.c 23 Jan 2012 03:36:21 - 1.25 +++ sys/net/pipex.c 30 Jan 2012 14:37:31 - @@ -2210,13 +2210,16 @@ pipex_l2tp_userland_lookup_session_ipv6( } #endif -Static struct pipex_session * +struct pipex_session * pipex_l2tp_userland_lookup_session(struct mbuf *m0, struct sockaddr *sa) { struct pipex_l2tp_header l2tp; struct pipex_hash_head *list; struct pipex_session *session; uint16_t session_id, tunnel_id, flags; + + if (sa-sa_family != AF_INET sa-sa_family != AF_INET6) + return (NULL); /* pullup */ if (m0-m_pkthdr.len sizeof(l2tp)) { Index: sys/net/pipex.h === RCS file: /cvs/src/sys/net/pipex.h,v retrieving revision 1.11 diff -u -p -r1.11 pipex.h --- sys/net/pipex.h 23 Jan 2012 03:36:21 - 1.11 +++ sys/net/pipex.h 30 Jan 2012 14:37:31 - @@ -210,6 +210,7 @@ struct pipex_session *pipex_pptp_lookup struct mbuf *pipex_pptp_input (struct mbuf *, struct pipex_session *); struct pipex_session *pipex_pptp_userland_lookup_session_ipv4 (struct mbuf *, struct in_addr); struct pipex_session *pipex_pptp_userland_lookup_session_ipv6 (struct mbuf *, struct in6_addr); +struct pipex_session *pipex_l2tp_userland_lookup_session(struct mbuf *, struct sockaddr *); struct mbuf *pipex_pptp_userland_output (struct mbuf *, struct pipex_session *); struct pipex_session *pipex_l2tp_lookup_session (struct mbuf *, int); struct mbuf *pipex_l2tp_input (struct mbuf *, int off, struct pipex_session *); Index: sys/net/pipex_local.h === RCS file: /cvs/src/sys/net/pipex_local.h,v retrieving revision 1.14 diff -u -p -r1.14 pipex_local.h --- sys/net/pipex_local.h 25 Nov 2011 13:05:06 - 1.14 +++ sys/net/pipex_local.h 30 Jan 2012 14:37:31 - @@ -406,7 +406,6 @@ Static struct pipex_session *pipex_pptp #ifdef PIPEX_L2TP Static void pipex_l2tp_output (struct mbuf *, struct pipex_session *); -Static struct pipex_session *pipex_l2tp_userland_lookup_session(struct mbuf *, struct sockaddr *); #endif #ifdef PIPEX_MPPE Index: sys/netinet/udp_usrreq.c === RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.145 diff -u -p -r1.145 udp_usrreq.c --- sys/netinet/udp_usrreq.c8 Jul 2011 18:30:17 - 1.145 +++ sys/netinet/udp_usrreq.c30 Jan 2012 14:37:32 - @@ -1198,6 +1198,12 @@ udp_usrreq(struct socket *so, int req, s #ifdef PIPEX if (inp-inp_pipex) { struct pipex_session *session; + + if (addr != NULL) + session = + pipex_l2tp_userland_lookup_session(m, + mtod(addr, struct sockaddr *)); + else #ifdef INET6 if (inp-inp_flags INP_IPV6) session =