I believe what you are proposing is userland/kernel ABI break,
utilizing exposure of a __packed data structure. __packed equiv
is not supported by some languages, so they would be unable to
parse these headers.
Furthermore, it means objects after the packing are now misaligned
if their alignment requirements are greater.
I'll go out on a limb and guess you aren't testing this on a strict
aligment architecture, and are only hunting an XXX you don't understand.
So, uhm, on way.
Artturi Alm <artturi....@gmail.com> wrote:
> 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
> >
> > /*
