significant increase in ipfw pullup failed
Brooks Davis
brooks at one-eyed-alien.net
Mon Apr 25 15:46:47 PDT 2005
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...
> > >
> > > could be, no idea really. but fwiw, ipv6 is not enabled here.
> >
> > yes but there is some new code in the common path.
> > anyways i have cc-ed Brooks who committed the code
>
> 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
===================================================================
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))
static __inline int
-icmptype_match(struct icmp *icmp, ipfw_insn_u32 *cmd)
+icmptype_match(struct icmphdr *icmp, ipfw_insn_u32 *cmd)
{
int type = icmp->icmp_type;
@@ -343,7 +343,7 @@ icmptype_match(struct icmp *icmp, ipfw_i
(1 << ICMP_TSTAMP) | (1 << ICMP_IREQ) | (1 << ICMP_MASKREQ) )
static int
-is_icmp_query(struct icmp *icmp)
+is_icmp_query(struct icmphdr *icmp)
{
int type = icmp->icmp_type;
@@ -760,7 +760,7 @@ ipfw_log(struct ip_fw *f, u_int hlen, st
} else {
struct ip *ip = mtod(m, struct ip *);
/* these three are all aliases to the same thing */
- struct icmp *const icmp = L3HDR(struct icmp, ip);
+ struct icmphdr *const icmp = L3HDR(struct icmphdr, ip);
struct tcphdr *const tcp = (struct tcphdr *)icmp;
struct udphdr *const udp = (struct udphdr *)icmp;
@@ -2108,11 +2108,7 @@ do { \
break;
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 = ICMP(ulp)->icmp_type;
break;
Index: netinet/ip_icmp.h
===================================================================
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 */
--
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20050425/faeb1681/attachment.bin
More information about the freebsd-current
mailing list