On Wed, Jul 18, 2018 at 07:57:57AM +0300, Artturi Alm wrote:
> Hi,
>
> rather random "i want to clear those XXXs"-moment with morning coffee,
> did seem like this was screaming for __packed from sys/cdefs.h,
> and less MD in sys/net/, if nothing else.
>
> With a bit of googling i also ran into a different solution,
> which is as ugly as what i'm replacing, imo.., that is:
> #define SIZEOF_BPF_HDR (sizeof(struct bpf_hdr) <= 20 ? 18 :
> sizeof(struct bpf_hdr)) /* from some apple bpf.h */
>
> compile-tested diff from was-current-before-hackathon-src-tree.
>
> -Artturi
>
now i suppose this doesn't work on some arch i can't test with, but
would anyone care to tell me which arch(s) and/or why not?
the diff i was thinking, without comment/define changes below.
-Artturi
diff --git sys/net/bpf.h sys/net/bpf.h
index 604bdcbab55..095ee283c03 100644
--- sys/net/bpf.h
+++ sys/net/bpf.h
@@ -140,7 +140,7 @@ struct bpf_hdr {
u_int32_t bh_datalen; /* original length of packet */
u_int16_t bh_hdrlen; /* length of bpf header (this struct
plus alignment padding) */
-};
+} __packed;
/*
* Because the structure above is not a multiple of 4 bytes, some compilers
* will insist on inserting padding; hence, sizeof(struct bpf_hdr) won't work.
@@ -156,6 +156,7 @@ struct bpf_hdr {
#else
#define SIZEOF_BPF_HDR sizeof(struct bpf_hdr)
#endif
+CTASSERT(sizeof(struct bpf_hdr) == 18); /* prove XXXs above !true w/packed */
#endif
/*
>
> diff --git a/sys/net/bpf.c b/sys/net/bpf.c
> index a6bcf31d471..151a34e19f2 100644
> --- a/sys/net/bpf.c
> +++ b/sys/net/bpf.c
> @@ -1600,11 +1600,12 @@ bpfsattach(caddr_t *bpfp, const char *name, u_int
> dlt, u_int hdrlen)
>
> /*
> * Compute the length of the bpf header. This is not necessarily
> - * equal to SIZEOF_BPF_HDR because we want to insert spacing such
> - * that the network layer header begins on a longword boundary (for
> - * performance reasons and to alleviate alignment restrictions).
> + * equal to sizeof(struct bpf_hdr), because we want to insert spacing
> + * such that the network layer header begins on a longword boundary
> + * (for performance reasons and to alleviate alignment restrictions).
> */
> - bp->bif_hdrlen = BPF_WORDALIGN(hdrlen + SIZEOF_BPF_HDR) - hdrlen;
> + bp->bif_hdrlen =
> + BPF_WORDALIGN(hdrlen + sizeof(struct bpf_hdr)) - hdrlen;
>
> return (bp);
> }
> diff --git a/sys/net/bpf.h b/sys/net/bpf.h
> index 604bdcbab55..80444bc17ad 100644
> --- a/sys/net/bpf.h
> +++ b/sys/net/bpf.h
> @@ -140,22 +140,14 @@ struct bpf_hdr {
> u_int32_t bh_datalen; /* original length of packet */
> u_int16_t bh_hdrlen; /* length of bpf header (this struct
> plus alignment padding) */
> -};
> +} __packed;
> +#ifdef _KERNEL
> /*
> * Because the structure above is not a multiple of 4 bytes, some compilers
> * will insist on inserting padding; hence, sizeof(struct bpf_hdr) won't
> work.
> * Only the kernel needs to know about it; applications use bh_hdrlen.
> - * XXX To save a few bytes on 32-bit machines, we avoid end-of-struct
> - * XXX padding by using the size of the header data elements. This is
> - * XXX fail-safe: on new machines, we just use the 'safe' sizeof.
> */
> -#ifdef _KERNEL
> -#if defined(__arm__) || defined(__i386__) || defined(__mips__) || \
> - defined(__sparc64__)
> -#define SIZEOF_BPF_HDR 18
> -#else
> -#define SIZEOF_BPF_HDR sizeof(struct bpf_hdr)
> -#endif
> +CTASSERT(sizeof(struct bpf_hdr) == 18);
> #endif
>
> /*
