From owner-freebsd-current@FreeBSD.ORG Mon Apr 25 22:46:47 2005 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 83F4316A4CE for ; Mon, 25 Apr 2005 22:46:47 +0000 (GMT) Received: from odin.ac.hmc.edu (Odin.AC.HMC.Edu [134.173.32.75]) by mx1.FreeBSD.org (Postfix) with ESMTP id 33E8C43D6E for ; Mon, 25 Apr 2005 22:46:47 +0000 (GMT) (envelope-from brdavis@odin.ac.hmc.edu) Received: from odin.ac.hmc.edu (localhost.localdomain [127.0.0.1]) by odin.ac.hmc.edu (8.13.0/8.13.0) with ESMTP id j3PMkkje006994; Mon, 25 Apr 2005 15:46:46 -0700 Received: (from brdavis@localhost) by odin.ac.hmc.edu (8.13.0/8.13.0/Submit) id j3PMkkko006993; Mon, 25 Apr 2005 15:46:46 -0700 Date: Mon, 25 Apr 2005 15:46:46 -0700 From: Brooks Davis To: Brooks Davis Message-ID: <20050425224646.GA6848@odin.ac.hmc.edu> References: <17001.9557.627987.505930@roam.psg.com> <17001.16520.774703.612151@roam.psg.com> <20050422112246.A70611@xorpc.icir.org> <17001.16764.512962.411616@roam.psg.com> <20050422115518.C70611@xorpc.icir.org> <20050422214418.GB11806@odin.ac.hmc.edu> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="T4sUOijqQbZv57TR" Content-Disposition: inline In-Reply-To: <20050422214418.GB11806@odin.ac.hmc.edu> User-Agent: Mutt/1.4.1i X-Virus-Scanned: by amavisd-new X-Spam-Status: No, hits=0.0 required=8.0 tests=none autolearn=no version=2.63 X-Spam-Checker-Version: SpamAssassin 2.63 (2004-01-11) on odin.ac.hmc.edu cc: Randy Bush cc: Luigi Rizzo cc: FreeBSD Current Subject: Re: significant increase in ipfw pullup failed X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Apr 2005 22:46:47 -0000 --T4sUOijqQbZv57TR Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Apr 22, 2005 at 02:44:18PM -0700, Brooks Davis wrote: > On Fri, Apr 22, 2005 at 11:55:18AM -0700, Luigi Rizzo wrote: > > On Fri, Apr 22, 2005 at 08:25:00AM -1000, Randy Bush wrote: > > > > wonder if it is related to the recent import of ipfw v6 support... > > >=20 > > > could be, no idea really. but fwiw, ipv6 is not enabled here. > >=20 > > yes but there is some new code in the common path. > > anyways i have cc-ed Brooks who committed the code >=20 > I suspect this is due to over agressive pullups of icmp packets (at > least that's the most obvious place where the length changed) which are > caused by poor design of the icmp struct. We're pulling up the full > length and should instead be pulling up 4 bytes. I'm not sure what the > best fix it. Long term, creating a struct icmphdr is probably the right > answer. For now, the thing to do may be to add it and use it in ipfw, > but not modify struct icmp just yet. Here's a diff implementing that. Could someone who is experinceing thse extra warnings, please test it? -- Brooks Index: netinet/ip_fw2.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: /usr/cvs/src/sys/netinet/ip_fw2.c,v retrieving revision 1.95 diff -u -p -r1.95 ip_fw2.c --- netinet/ip_fw2.c 19 Apr 2005 10:04:38 -0000 1.95 +++ netinet/ip_fw2.c 25 Apr 2005 21:57:30 -0000 @@ -328,11 +328,11 @@ SYSCTL_INT(_net_inet_ip_fw, OID_AUTO, dy #define L3HDR(T, ip) ((T *)((u_int32_t *)(ip) + (ip)->ip_hl)) #define TCP(p) ((struct tcphdr *)(p)) #define UDP(p) ((struct udphdr *)(p)) -#define ICMP(p) ((struct icmp *)(p)) +#define ICMP(p) ((struct icmphdr *)(p)) #define ICMP6(p) ((struct icmp6_hdr *)(p)) =20 static __inline int -icmptype_match(struct icmp *icmp, ipfw_insn_u32 *cmd) +icmptype_match(struct icmphdr *icmp, ipfw_insn_u32 *cmd) { int type =3D icmp->icmp_type; =20 @@ -343,7 +343,7 @@ icmptype_match(struct icmp *icmp, ipfw_i (1 << ICMP_TSTAMP) | (1 << ICMP_IREQ) | (1 << ICMP_MASKREQ) ) =20 static int -is_icmp_query(struct icmp *icmp) +is_icmp_query(struct icmphdr *icmp) { int type =3D icmp->icmp_type; =20 @@ -760,7 +760,7 @@ ipfw_log(struct ip_fw *f, u_int hlen, st } else { struct ip *ip =3D mtod(m, struct ip *); /* these three are all aliases to the same thing */ - struct icmp *const icmp =3D L3HDR(struct icmp, ip); + struct icmphdr *const icmp =3D L3HDR(struct icmphdr, ip); struct tcphdr *const tcp =3D (struct tcphdr *)icmp; struct udphdr *const udp =3D (struct udphdr *)icmp; =20 @@ -2108,11 +2108,7 @@ do { \ break; =20 case IPPROTO_ICMP: - /* - * we only care for 4 bytes: type, code, - * checksum - */ - PULLUP_TO(hlen, ulp, struct icmp); + PULLUP_TO(hlen, ulp, struct icmphdr); args->f_id.flags =3D ICMP(ulp)->icmp_type; break; =20 Index: netinet/ip_icmp.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: /usr/cvs/src/sys/netinet/ip_icmp.h,v retrieving revision 1.24 diff -u -p -r1.24 ip_icmp.h --- netinet/ip_icmp.h 21 Apr 2005 14:29:34 -0000 1.24 +++ netinet/ip_icmp.h 25 Apr 2005 22:44:43 -0000 @@ -49,6 +49,17 @@ struct icmp_ra_addr { /* * Structure of an icmp header. */ +struct icmphdr { + u_char icmp_type; /* type of message, see below */ + u_char icmp_code; /* type sub code */ + u_short icmp_cksum; /* ones complement cksum of struct */ +}; + +/* + * Structure of an icmp packet. + * + * XXX: should start with a struct icmphdr. + */ struct icmp { u_char icmp_type; /* type of message, see below */ u_char icmp_code; /* type sub code */ --=20 Any statement of the form "X is the one, true Y" is FALSE. PGP fingerprint 655D 519C 26A7 82E7 2529 9BF0 5D8E 8BE9 F238 1AD4 --T4sUOijqQbZv57TR Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.1 (GNU/Linux) iD8DBQFCbXNVXY6L6fI4GtQRAghcAJ9FzZfOTx/++DpFe+CX4vwKOhC+hgCfaUJQ ddABVf5rR5j8IIIgFE05aGs= =Tpxu -----END PGP SIGNATURE----- --T4sUOijqQbZv57TR--