Subject: bin/28531: tcpdump doesn't correctly display FDDI packets
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org>
From: None <gadams@avernus.com>
List: netbsd-bugs
Date: 12/04/2004 00:00:00
>Number:         28531
>Category:       bin
>Synopsis:       tcpdump doesn't correctly display FDDI packets
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Dec 04 00:00:00 +0000 2004
>Originator:     Geoff Adams
>Release:        NetBSD 2.0
>Organization:
	
>Environment:
	
	
System: NetBSD fornax.avernus.com 2.0 NetBSD 2.0 (FORNAX-$Revision: 1.4 $) #1: Thu Dec 2 23:26:00 EST 2004 gadams@fornax.avernus.com:/build/netbsd/sys/arch/alpha/compile/FORNAX alpha
Architecture: alpha
Machine: alpha
>Description:
	
The tcpdump program expects a slightly different format for struct
fddi_header than the NetBSD bpf and libpcap give it. tcpdump includes its
own "fddi.h" header file that defines an incorrect (for NetBSD, at least)
format for the struct -- it lacks the three leading pad bytes that the
<net/if_fddi.h> version has.

For tcpdump to work correctly with FDDI networks on NetBSD, we need to
use the OS-defined struct fddi_header from <net/if_fddi.h>, rather than
the "fddi.h" one in tcpdump. There is an attempt to address this problem
in the NetBSD version of tcpdump/print-fddi.c, but it also assumes access
to the system header to calculate the size of the padding. However, this
can't work, because if it were using the system's version, then no
adjustment would be needed.

I have fixed the problem by using the system's header under NetBSD. Since
a new version of tcpdump was imported after the 2.0 branch, I have included
separate patches for dist/tcpdump/print-fddi.c for both -2.0 and -current.

>How-To-Repeat:
	
Attach a FDDI interface, bring it up, and type 'tcpdump' (or
'tcpdump -i intf_name' if it's not the bpf default interface), and notice
that it is unable to decode any of the frames. Indeed, you'll discover
bits of the source and dest MAC addresses in the link-layer section, but
t's clearly all garbled. This is because tcpdump is interpreting the
frame header starting three bytes too early.
>Fix:
	
The following patch applies to NetBSD 2.0:

Index: dist/tcpdump/print-fddi.c
===================================================================
RCS file: /cvsroot/src/dist/tcpdump/print-fddi.c,v
retrieving revision 1.5
diff -u -r1.5 print-fddi.c
--- dist/tcpdump/print-fddi.c	17 Jan 2003 02:44:33 -0000	1.5
+++ dist/tcpdump/print-fddi.c	3 Dec 2004 23:30:37 -0000
@@ -55,7 +55,12 @@
 #include "ethertype.h"
 
 #include "ether.h"
+#ifdef __NetBSD__
+#include <net/if_fddi.h>
+#define FDDI_HDRLEN sizeof(struct fddi_header)
+#else
 #include "fddi.h"
+#endif
 
 /*
  * Some FDDI interfaces use bit-swapped addresses.
@@ -333,25 +338,17 @@
 fddi_if_print(u_char *pcap, const struct pcap_pkthdr *h,
 	      register const u_char *p)
 {
-	u_int caplen = h->caplen;
-	u_int length = h->len;
-
 	++infodelay;
 	ts_print(&h->ts);
 
-#ifdef __NetBSD__
-	p += offsetof(struct fddi_header, fddi_fc);
-	caplen -= offsetof(struct fddi_header, fddi_fc);
-#endif
-
-	fddi_print(p, length, caplen);
+	fddi_print(p, h->len, h->caplen);
 
 	/*
 	 * If "-x" was specified, print stuff past the FDDI header,
 	 * if there's anything to print.
 	 */
-	if (xflag && caplen > FDDI_HDRLEN)
-		default_print(p + FDDI_HDRLEN, caplen - FDDI_HDRLEN);
+	if (xflag && h->caplen > FDDI_HDRLEN)
+		default_print(p + FDDI_HDRLEN, h->caplen - FDDI_HDRLEN);
 
 	putchar('\n');
 

And this patch is for -current:

Index: dist/tcpdump/print-fddi.c
===================================================================
RCS file: /cvsroot/src/dist/tcpdump/print-fddi.c,v
retrieving revision 1.6
diff -u -r1.6 print-fddi.c
--- dist/tcpdump/print-fddi.c	27 Sep 2004 23:04:24 -0000	1.6
+++ dist/tcpdump/print-fddi.c	3 Dec 2004 23:34:13 -0000
@@ -47,7 +47,13 @@
 #include "ethertype.h"
 
 #include "ether.h"
+
+#ifdef __NetBSD__
+#include <net/if_fddi.h>
+#define FDDI_HDRLEN sizeof(struct fddi_header)
+#else
 #include "fddi.h"
+#endif
 
 /*
  * Some FDDI interfaces use bit-swapped addresses.
@@ -312,15 +318,7 @@
 u_int
 fddi_if_print(const struct pcap_pkthdr *h, register const u_char *p)
 {
-	u_int caplen = h->caplen;
-	u_int length = h->len;
-
-#ifdef __NetBSD__
-	p += offsetof(struct fddi_header, fddi_fc);
-	caplen -= offsetof(struct fddi_header, fddi_fc);
-#endif
-
-	fddi_print(p, length, caplen);
+	fddi_print(p, h->len, h->caplen);
 
 	return (FDDI_HDRLEN);
 }

>Unformatted: