On Thu, May 25, 2017 at 01:52:16PM -0700, Joe Stringer wrote:
> On 25 May 2017 at 13:35, Ben Pfaff <b...@ovn.org> wrote:
> > On Thu, May 25, 2017 at 01:05:11PM -0700, Joe Stringer wrote:
> >> On 24 May 2017 at 20:02, Ben Pfaff <b...@ovn.org> wrote:
> >> > On Tue, May 23, 2017 at 04:02:12PM -0700, Joe Stringer wrote:
> >> >> Clang 4.0 complains:
> >> >>
> >> >> ../lib/odp-execute.c:61:37: error: taking address of packed member
> >> >> 'eth_dst' of
> >> >> class or structure 'eth_header' may result in an unaligned pointer value
> >> >> [-Werror,-Waddress-of-packed-member]
> >> >> ether_addr_copy_masked(&eh->eth_src, key->eth_src,
> >> >> mask->eth_src);
> >> >> ^~~~~~~~~~~
> >> >> ../lib/odp-execute.c:62:37: error: taking address of packed member
> >> >> 'eth_dst' of
> >> >> class or structure 'eth_header' may result in an unaligned pointer value
> >> >> [-Werror,-Waddress-of-packed-member]
> >> >> ether_addr_copy_masked(&eh->eth_dst, key->eth_dst,
> >> >> mask->eth_dst);
> >> >>
> >> >> Ethernet source addresses are 48 bits offset into the Ethernet header,
> >> >> so taking a pointer for this is not guaranteed to be valid on all
> >> >> architectures. Fix this by referencing the memory direct from the
> >> >> Ethernet header pointer.
> >> >>
> >> >> Signed-off-by: Joe Stringer <j...@ovn.org>
> >> >
> >> > I don't understand--why does Clang think that there's something packed
> >> > here? I don't see any packed annotation on struct eth_header (and I
> >> > don't think it needs one).
> >>
> >> https://github.com/openvswitch/ovs/blob/master/lib/packets.h#L398
> >>
> >> I believe that this is the wire-formatted version that we use for
> >> assembling PDUs for protocols such as STP, so I think it needs to be
> >> properly packed.
> >
> > Oh, oops, somehow I was blind to the difference between eth_header and
> > eth_addr.
> >
> > But I don't actually see a need to pack this structure. There's nothing
> > in it that would cause a compile to insert padding. It has all 16-bit
> > members (including nested members), and you never get padding (on normal
> > architectures) when all the members of a struct have the same size;
> > otherwise arrays wouldn't work reasonably.
> >
> > I'll send some patches.
>
> Ok thanks, I'll abandon this series and look for your patches.
I sent my patch (just one, I guess):
https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332864.html
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
