Date: Fri, 7 Jun 2002 13:29:36 -0700 (PDT) From: Julian Elischer <julian@elischer.org> To: Archie Cobbs <archie@dellroad.org> Cc: freebsd-net@freebsd.org Subject: Re: m->m_pkthdr.header Message-ID: <Pine.BSF.4.21.0206071303280.53816-100000@InterJet.elischer.org> In-Reply-To: <200206071955.g57JtrJ65814@arch20m.dellroad.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0206071303280.53816-100000>