From owner-freebsd-net Fri Jun 7 13:40:22 2002 Delivered-To: freebsd-net@freebsd.org Received: from rwcrmhc51.attbi.com (rwcrmhc51.attbi.com [204.127.198.38]) by hub.freebsd.org (Postfix) with ESMTP id 60FE737B405 for ; Fri, 7 Jun 2002 13:40:10 -0700 (PDT) Received: from InterJet.elischer.org ([12.232.206.8]) by rwcrmhc51.attbi.com (InterMail vM.4.01.03.27 201-229-121-127-20010626) with ESMTP id <20020607204010.ZXFN11426.rwcrmhc51.attbi.com@InterJet.elischer.org>; Fri, 7 Jun 2002 20:40:10 +0000 Received: from localhost (localhost.elischer.org [127.0.0.1]) by InterJet.elischer.org (8.9.1a/8.9.1) with ESMTP id NAA65580; Fri, 7 Jun 2002 13:29:37 -0700 (PDT) Date: Fri, 7 Jun 2002 13:29:36 -0700 (PDT) From: Julian Elischer To: Archie Cobbs Cc: freebsd-net@freebsd.org Subject: Re: m->m_pkthdr.header In-Reply-To: <200206071955.g57JtrJ65814@arch20m.dellroad.org> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Fri, 7 Jun 2002, Archie Cobbs wrote: > Hi, > > I'd like to get rid of this mbuf field "m->m_pkthdr.header". > Here are my reasons: > > - It's broken. Consider this code: > > copy = m_copypacket(m); > m_freem(m); > > Typically m->m_pkthdr.header points into the data area of 'm'. > So now copy->m_pkthdr.header points at the same data, which has > already been freed and possibly reused. > > - It is an obscure thing most people don't know about and is > ripe for causing hard to find bugs. the comment says: 82 /* variables for ip and tcp reassembly */ 83 void *header; /* pointer to packet header */ which seems untrue.. in any case it's WAY out of scope. that is a protocol specific thing.. for that matter the use of the header for 84 /* variables for hardware checksum */ 85 int csum_flags; /* flags regarding checksum*/ 86 int csum_data; /* data field used by csum routines */ seem also to be way out of scope. there should be some other protocol-specific way to associate this information with a packet. > > - It's only used in two places, (a) for secret communication between > the oltr driver and if_ether.c, and (b) as a temporary variable > in ip_input.c: are tehy the only things that break if you compile LINT with it commented out? > > $ find . -type f -print | xargs grep -w m_pkthdr.header > ./contrib/dev/oltr/if_oltr.c: m0->m_pkthdr.header = (void *)th; > ./netinet/if_ether.c: th = (struct iso88025_header *)m->m_pkthdr.header; > ./netinet/ip_input.c: m->m_pkthdr.header = ip; > ./netinet/ip_input.c:#define GETIP(m) ((struct ip*)((m)->m_pkthdr.header)) > > My proposal: > > - Replace the obfuscating GETIP() macro in ip_input.c with a variable. well I BELIEVE that this wouldn't work after the packet has been queued for reassembly... but I guess you could manage it.. > - Rejigger the oltr driver to pass its "secret" information using > an auxillary mbuf instead of m->m_pkthdr.header. The whole issue of auxiliary dataa (metadata) and protocol specific data storage needs to be solved once and for all.. (we have our own answer in Netgraph but it's a stopgap) > > Any comments/objections? > > Thanks, > -Archie > > __________________________________________________________________________ > Archie Cobbs * Packet Design * http://www.packetdesign.com > > To Unsubscribe: send mail to majordomo@FreeBSD.org > with "unsubscribe freebsd-net" in the body of the message > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message