Date: Sat, 11 Feb 2006 00:14:15 +0200 From: Ruslan Ermilov <ru@FreeBSD.org> To: Andre Oppermann <andre@FreeBSD.org> Cc: net@FreeBSD.org Subject: [FIX] dummynet breaks IP reassembly Message-ID: <20060210221415.GC33588@ip.net.ua>
next in thread | raw e-mail | index | archive | help
--St7VIuEGZ6dlpu13 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Andre, If I'm not mistaken, *you* converted ipfw to use pfil(9). During the conversion, the following bug was introduced. When forwarding fragmented packets through a dummynet pipe (ip_input -> ip_forward -> ip_output -> pipe -> ip_output) the last ip_output() in the chain that does the actual IP delivery sets ip_id of all fragments to different values, making it impossible to reassemble the packet at receive side. Example of forwarding a fragmented IP datagram from em0 to xl0: # tcpdump -nvi em0 net 192.168.2 tcpdump: listening on em0, link-type EN10MB (Ethernet), capture size 96 byt= es 00:04:35.170994 IP (tos 0x0, ttl 64, id 2186, offset 0, flags [+], proto: = ICMP (1), length: 1500) 192.168.1.3 > 192.168.2.1: ICMP echo request, id 28= 19, seq 0, length 1480 00:04:35.171242 IP (tos 0x0, ttl 64, id 2186, offset 1480, flags [none], p= roto: ICMP (1), length: 548) 192.168.1.3 > 192.168.2.1: icmp # tcpdump -nvi xl0 net 192.168.2 tcpdump: listening on xl0, link-type EN10MB (Ethernet), capture size 96 byt= es 00:04:35.171028 IP (tos 0x0, ttl 63, id 10560, offset 0, flags [+], proto:= ICMP (1), length: 1500) 192.168.1.3 > 192.168.2.1: ICMP echo request, id 2= 819, seq 0, length 1480 00:04:35.171266 IP (tos 0x0, ttl 63, id 10561, offset 1480, flags [none], = proto: ICMP (1), length: 548) 192.168.1.3 > 192.168.2.1: icmp Note that IDs were rewritten from the original 2186 and worse, they are different. In 4,x, the "flags" field (see the patch below) was used to keep the IP_FORWARDING bit passed to ip_output() by ip_forward(). This bit was kept in the dummynet packet tag (in the "flags" field) and later passed to the second ip_output() call, causing the latter to NOT touch the IP header again. This feature was lost, resulting in a bug. Since dummynet never calls ip_output() with unfilled header, it's safe to always call ip_output() from dummynet with the IP_FORWARDING bit set, to indicate it's forwarded =66rom another ip_output() and so it shouldn't attempt to modify the header. Surprisingly, it "seemed" to work for packets exceeding MTU and originating from the dummynetting host, mainly because the packet wasn't fragmented while in dummynet. Yet, the ip_id field was always incremented in my tests (pipe with no bandwidth limitation), comparing it before and after the dummynet processing. Now, the ip_id is always preserved. # tcpdump -nvi em0 net 192.168.2 tcpdump: listening on em0, link-type EN10MB (Ethernet), capture size 96 byt= es 00:12:04.654669 IP (tos 0x0, ttl 64, id 2192, offset 0, flags [+], proto: = ICMP (1), length: 1500) 192.168.1.3 > 192.168.2.1: ICMP echo request, id 79= 39, seq 0, length 1480 00:12:04.654917 IP (tos 0x0, ttl 64, id 2192, offset 1480, flags [none], p= roto: ICMP (1), length: 548) 192.168.1.3 > 192.168.2.1: icmp # tcpdump -nvi xl0 net 192.168.2 tcpdump: listening on xl0, link-type EN10MB (Ethernet), capture size 96 byt= es 00:12:04.654703 IP (tos 0x0, ttl 63, id 2192, offset 0, flags [+], proto: = ICMP (1), length: 1500) 192.168.1.3 > 192.168.2.1: ICMP echo request, id 79= 39, seq 0, length 1480 00:12:04.654939 IP (tos 0x0, ttl 63, id 2192, offset 1480, flags [none], p= roto: ICMP (1), length: 548) 192.168.1.3 > 192.168.2.1: icmp (Note the ip_id is preserved when forwarding.) I'm not sure about the IPv6 portion but it's consistent with the normal forwarding path so I believe it's correct. Comments? %%% Index: ip_dummynet.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/netinet/ip_dummynet.c,v retrieving revision 1.98 diff -u -p -r1.98 ip_dummynet.c --- ip_dummynet.c 3 Feb 2006 11:38:19 -0000 1.98 +++ ip_dummynet.c 10 Feb 2006 21:20:59 -0000 @@ -769,7 +769,7 @@ dummynet_send(struct mbuf *m) pkt =3D dn_tag_get(m); switch (pkt->dn_dir) { case DN_TO_IP_OUT: - ip_output(m, NULL, NULL, pkt->flags, NULL, NULL); + ip_output(m, NULL, NULL, IP_FORWARDING, NULL, NULL); break ; case DN_TO_IP_IN : ip =3D mtod(m, struct ip *); @@ -783,7 +783,7 @@ dummynet_send(struct mbuf *m) break; =20 case DN_TO_IP6_OUT: - ip6_output(m, NULL, NULL, pkt->flags, NULL, NULL, NULL); + ip6_output(m, NULL, NULL, IPV6_FORWARDING, NULL, NULL, NULL); break; #endif case DN_TO_IFB_FWD: @@ -1129,7 +1129,6 @@ locate_pipe(int pipe_nr) * ifp the 'ifp' parameter from the caller. * NULL in ip_input, destination interface in ip_output, * rule matching rule, in case of multiple passes - * flags flags from the caller, only used in ip_output * */ static int @@ -1213,8 +1212,6 @@ dummynet_io(struct mbuf *m, int dir, str pkt->dn_dir =3D dir ; =20 pkt->ifp =3D fwa->oif; - if (dir =3D=3D DN_TO_IP_OUT || dir =3D=3D DN_TO_IP6_OUT) - pkt->flags =3D fwa->flags; =20 if (q->head =3D=3D NULL) q->head =3D m; Index: ip_dummynet.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/netinet/ip_dummynet.h,v retrieving revision 1.38 diff -u -p -r1.38 ip_dummynet.h --- ip_dummynet.h 29 Nov 2005 00:11:01 -0000 1.38 +++ ip_dummynet.h 10 Feb 2006 21:13:45 -0000 @@ -130,7 +130,6 @@ struct dn_pkt_tag { =20 dn_key output_time; /* when the pkt is due for delivery */ struct ifnet *ifp; /* interface, for ip_output */ - int flags ; /* flags, for ip_output (IPv6 ?) */ struct _ip6dn_args ip6opt; /* XXX ipv6 options */ }; #endif /* _KERNEL */ Index: ip_fw.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/netinet/ip_fw.h,v retrieving revision 1.103 diff -u -p -r1.103 ip_fw.h --- ip_fw.h 13 Dec 2005 12:16:02 -0000 1.103 +++ ip_fw.h 10 Feb 2006 21:15:41 -0000 @@ -510,8 +510,6 @@ struct ip_fw_args { struct ip_fw *rule; /* matching rule */ struct ether_header *eh; /* for bridged packets */ =20 - int flags; /* for dummynet */ - struct ipfw_flow_id f_id; /* grabbed from IP header */ u_int32_t cookie; /* a cookie depending on rule action */ struct inpcb *inp; %%% Cheers, --=20 Ruslan Ermilov ru@FreeBSD.org FreeBSD committer --St7VIuEGZ6dlpu13 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (FreeBSD) iD8DBQFD7RA3qRfpzJluFF4RAixfAJ9hPuJK5tiSZsXGuRzCIi/WsH9jrQCcCkAe cgPUE8BgRuyhmE8MvRF3FXc= =+aWq -----END PGP SIGNATURE----- --St7VIuEGZ6dlpu13--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060210221415.GC33588>