On Sat, Apr 16, 2016 at 11:10:14AM +0100, Stuart Henderson wrote: > On 2016/04/15 23:07, Philip Guenther wrote: > > On Thu, Apr 14, 2016 at 3:15 PM, Martin Natano <nat...@natano.net> wrote: > > > In rbootd a struct bpf_timeval (with 32bit tv_sec) is copied to a struct > > > timeval (with 64 tv_sec) via bcopy(). This most likely causes > > > connections to not time out correctly in rbootd. I don't have an HP > > > machine to test this with. Who owns such a machine and is willing to > > > test this? > > > > The current code is simply wrong: sizeof(struct timeval) > > > sizeof(struct bpf_timeval). I think we should make this change (ok > > guenther@) and then work to update struct bpf_hdr or a replacement > > thereof to not be limited to 32bit time_t values. Maybe it should be > > using a timespec instead too? Have any other free OSes added timespec > > (or bintime) APIs? > > > > (BPF has been around since 1990; the time from then to now is less > > than the time from now to 2038 when 32bit time_t fails...) > > I'd like this for ports too; it is not sanely possible to patch some > programs to support bpf_timeval (in some programs it's common to > mix timevals from bpf with timevals on the system and want to > calculate time differences etc - patches to fix these cases are > often very intrusive). > > I have a WIP diff to switch bpf to timevals, and convert in libpcap when > writing dump files - these need to be 32-bit unsigned in the current > format; there is a newer pcapng file format (draft-tuexen-opsawg-pcapng) > that has 64-bit timestamps but also has many other changes, libpcap > upstream only had partial support last time I checked. > > It works fine on amd64 but last time I tried it on spar64 I was seeing > unaligned access with tcpdump, and dhclient didn't work, I haven't > managed to track that down yet (I have some ideas but my x1 died, my > other sparc64 is noisy and has no LOM so progress is slow!).
I have bumping bpf_timeval to 64bit on my todo list too. However I'm not sure what the best course of action is there. Replacing bpf_timeval with timeval might be problematic, because that means bpf wouldn't be binary stable across platforms. (tv_usec in struct timeval is a long int.) natano > > Index: lib/libpcap/pcap-int.h > =================================================================== > RCS file: /cvs/src/lib/libpcap/pcap-int.h,v > retrieving revision 1.13 > diff -u -p -r1.13 pcap-int.h > --- lib/libpcap/pcap-int.h 11 Apr 2014 04:08:58 -0000 1.13 > +++ lib/libpcap/pcap-int.h 16 Apr 2016 10:05:57 -0000 > @@ -120,11 +120,20 @@ struct pcap { > }; > > /* > + * MI timeval structure as used in the dumpfile. > + */ > + > +struct pcap_timeval { > + u_int32_t tv_sec; > + u_int32_t tv_usec; > +}; > + > +/* > * How a `pcap_pkthdr' is actually stored in the dumpfile. > */ > > struct pcap_sf_pkthdr { > - struct bpf_timeval ts; /* time stamp */ > + struct pcap_timeval ts; /* time stamp */ > bpf_u_int32 caplen; /* length of portion present */ > bpf_u_int32 len; /* length this packet (off wire) */ > }; > Index: lib/libpcap/pcap.h > =================================================================== > RCS file: /cvs/src/lib/libpcap/pcap.h,v > retrieving revision 1.18 > diff -u -p -r1.18 pcap.h > --- lib/libpcap/pcap.h 6 Apr 2016 08:02:56 -0000 1.18 > +++ lib/libpcap/pcap.h 16 Apr 2016 10:05:57 -0000 > @@ -90,7 +90,7 @@ typedef enum { > * packet interfaces. > */ > struct pcap_pkthdr { > - struct bpf_timeval ts; /* time stamp */ > + struct timeval ts; /* time stamp */ > bpf_u_int32 caplen; /* length of portion present */ > bpf_u_int32 len; /* length this packet (off wire) */ > }; > Index: lib/libpcap/savefile.c > =================================================================== > RCS file: /cvs/src/lib/libpcap/savefile.c,v > retrieving revision 1.16 > diff -u -p -r1.16 savefile.c > --- lib/libpcap/savefile.c 22 Dec 2015 19:51:04 -0000 1.16 > +++ lib/libpcap/savefile.c 16 Apr 2016 10:05:57 -0000 > @@ -211,20 +211,26 @@ pcap_fopen_offline(FILE *fp, char *errbu > static int > sf_next_packet(pcap_t *p, struct pcap_pkthdr *hdr, u_char *buf, int buflen) > { > + struct pcap_sf_pkthdr sf_hdr; > FILE *fp = p->sf.rfile; > > /* read the stamp */ > - if (fread((char *)hdr, sizeof(struct pcap_pkthdr), 1, fp) != 1) { > + if (fread(&sf_hdr, sizeof(struct pcap_sf_pkthdr), 1, fp) != 1) { > /* probably an EOF, though could be a truncated packet */ > return (1); > } > > if (p->sf.swapped) { > /* these were written in opposite byte order */ > - hdr->caplen = SWAPLONG(hdr->caplen); > - hdr->len = SWAPLONG(hdr->len); > - hdr->ts.tv_sec = SWAPLONG(hdr->ts.tv_sec); > - hdr->ts.tv_usec = SWAPLONG(hdr->ts.tv_usec); > + hdr->caplen = SWAPLONG(sf_hdr.caplen); > + hdr->len = SWAPLONG(sf_hdr.len); > + hdr->ts.tv_sec = SWAPLONG(sf_hdr.ts.tv_sec); > + hdr->ts.tv_usec = SWAPLONG(sf_hdr.ts.tv_usec); > + } else { > + hdr->caplen = sf_hdr.caplen; > + hdr->len = sf_hdr.len; > + hdr->ts.tv_sec = sf_hdr.ts.tv_sec; > + hdr->ts.tv_usec = sf_hdr.ts.tv_usec; > } > /* > * We interchanged the caplen and len fields at version 2.3, > @@ -332,10 +338,16 @@ void > pcap_dump(u_char *user, const struct pcap_pkthdr *h, const u_char *sp) > { > FILE *f; > + struct pcap_sf_pkthdr sf_hdr; > > f = (FILE *)user; > + /* XXX does this need memcpy instead for strict alignment arch? */ > + sf_hdr.ts.tv_sec = h->ts.tv_sec; > + sf_hdr.ts.tv_usec = h->ts.tv_usec; > + sf_hdr.caplen = h->caplen; > + sf_hdr.len = h->len; > /* XXX we should check the return status */ > - (void)fwrite((char *)h, sizeof(*h), 1, f); > + (void)fwrite(&sf_hdr, sizeof(sf_hdr), 1, f); > (void)fwrite((char *)sp, h->caplen, 1, f); > } > > Index: lib/libpcap/shlib_version > =================================================================== > RCS file: /cvs/src/lib/libpcap/shlib_version,v > retrieving revision 1.15 > diff -u -p -r1.15 shlib_version > --- lib/libpcap/shlib_version 6 Apr 2016 08:02:56 -0000 1.15 > +++ lib/libpcap/shlib_version 16 Apr 2016 10:05:57 -0000 > @@ -1,2 +1,2 @@ > -major=8 > -minor=1 > +major=9 > +minor=0 > Index: share/man/man4/bpf.4 > =================================================================== > RCS file: /cvs/src/share/man/man4/bpf.4,v > retrieving revision 1.37 > diff -u -p -r1.37 bpf.4 > --- share/man/man4/bpf.4 10 Mar 2016 04:48:27 -0000 1.37 > +++ share/man/man4/bpf.4 16 Apr 2016 10:05:58 -0000 > @@ -475,7 +475,7 @@ The following structure is prepended to > .Xr read 2 : > .Bd -literal -offset indent > struct bpf_hdr { > - struct bpf_timeval bh_tstamp; > + struct timeval bh_tstamp; > u_int32_t bh_caplen; > u_int32_t bh_datalen; > u_int16_t bh_hdrlen; > Index: sys/net/bpf.h > =================================================================== > RCS file: /cvs/src/sys/net/bpf.h,v > retrieving revision 1.55 > diff -u -p -r1.55 bpf.h > --- sys/net/bpf.h 3 Apr 2016 01:37:26 -0000 1.55 > +++ sys/net/bpf.h 16 Apr 2016 10:05:58 -0000 > @@ -126,16 +126,11 @@ struct bpf_version { > #define BPF_DIRECTION_IN 1 > #define BPF_DIRECTION_OUT (1<<1) > > -struct bpf_timeval { > - u_int32_t tv_sec; > - u_int32_t tv_usec; > -}; > - > /* > * Structure prepended to each packet. > */ > struct bpf_hdr { > - struct bpf_timeval bh_tstamp; /* time stamp */ > + struct timeval bh_tstamp; /* time stamp */ > u_int32_t bh_caplen; /* length of captured portion */ > u_int32_t bh_datalen; /* original length of packet */ > u_int16_t bh_hdrlen; /* length of bpf header (this struct > @@ -151,9 +146,10 @@ struct bpf_hdr { > */ > #ifdef _KERNEL > #if defined(__arm__) || defined(__i386__) || defined(__m68k__) || \ > - defined(__mips__) || defined(__ns32k__) || defined(__sparc__) || \ > - defined(__sparc64__) > -#define SIZEOF_BPF_HDR 18 > + defined(__mips__) || defined(__ns32k__) || defined(__sparc__) > +#define SIZEOF_BPF_HDR 22 > +#elif defined(__sparc64__) > +#define SIZEOF_BPF_HDR 26 > #else > #define SIZEOF_BPF_HDR sizeof(struct bpf_hdr) > #endif > Index: usr.sbin/tcpdump/interface.h > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/interface.h,v > retrieving revision 1.66 > diff -u -p -r1.66 interface.h > --- usr.sbin/tcpdump/interface.h 15 Nov 2015 20:35:36 -0000 1.66 > +++ usr.sbin/tcpdump/interface.h 16 Apr 2016 10:05:58 -0000 > @@ -146,9 +146,8 @@ extern const u_char *snapend; > #define TCHECK(var) TCHECK2(var, sizeof(var)) > > struct timeval; > -struct bpf_timeval; > > -extern void ts_print(const struct bpf_timeval *); > +extern void ts_print(const struct timeval *); > > extern int fn_print(const u_char *, const u_char *); > extern int fn_printn(const u_char *, u_int, const u_char *); > Index: usr.sbin/tcpdump/util.c > =================================================================== > RCS file: /cvs/src/usr.sbin/tcpdump/util.c,v > retrieving revision 1.27 > diff -u -p -r1.27 util.c > --- usr.sbin/tcpdump/util.c 16 Nov 2015 00:16:39 -0000 1.27 > +++ usr.sbin/tcpdump/util.c 16 Apr 2016 10:05:58 -0000 > @@ -113,12 +113,12 @@ fn_printn(const u_char *s, u_int n, cons > * Print the timestamp > */ > void > -ts_print(const struct bpf_timeval *tvp) > +ts_print(const struct timeval *tvp) > { > int s; > #define TSBUFLEN 32 > static char buf[TSBUFLEN]; > - static struct bpf_timeval last; > + static struct timeval last; > struct timeval diff; > time_t t; >