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 <[email protected]> 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;
>
