[ovs-dev] [ovn-ipv6 16/26] packets: Cleanup ND compose functions.

Ben Pfaff blp at ovn.org
Wed Jul 13 19:36:57 UTC 2016


On Tue, Jul 12, 2016 at 07:26:29PM +0800, Zong Kai Li wrote:
> On Tue, Jul 12, 2016 at 2:56 PM, Justin Pettit <jpettit at ovn.org> wrote:
> > Rename "compose_nd" and "compose_na" to "compose_nd_sol" and
> > "compose_nd_adv", respecively, to be clearer about their functionality.
> > Also change the source and destination IPv6 addresses to take
> > "struct in6_addr" arguments, which are more common in the code base.
> >
> > Signed-off-by: Justin Pettit <jpettit at ovn.org>
> > ---
> 
> >      eth_compose(b, eth_dst, eth_src, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
> > -    na = compose_ipv6(b, IPPROTO_ICMPV6, ipv6_src, ipv6_dst, 0, 0, 255,
> > -                      ND_MSG_LEN + ND_OPT_LEN);
> > +    na = compose_ipv6(b, IPPROTO_ICMPV6,
> > +                      ALIGNED_CAST(ovs_be32 *, ipv6_src->s6_addr),
> > +                      ALIGNED_CAST(ovs_be32 *, ipv6_dst->s6_addr),
> > +                      0, 0, 255, ND_MSG_LEN + ND_OPT_LEN);
> >
> 
> Hi, Justin. It's quite a long time to wait support for IPv6, but I
> think it worth. Thanks. :)
> 
> About using ALIGNED_CAST, would you mind checking
> http://patchwork.ozlabs.org/patch/632026/ ?
> Ben has a comment said "Using ALIGNED_CAST does not solve a problem,
> it only suppresses a warning.".
> So I'm not sure will this be a good way to cleanup.

Yes, I agree.

Here is a more detailed explanation.  I guess that most people reading
this email know that RISC machines fault for misaligned accesses.  In
some cases, 32-bit objects inside packets will be aligned only to a
16-bit boundary.  Thus, we typically use types like
ovs_16aligned_in6_addr to make this obvious and harder to get wrong.
GCC will warn for a cast from a type that requires less alignment
(e.g. 16-bit for ovs_16aligned_in6_addr) to a type that requires more
alignment (e.g. 32-bit for ovs_be32).  We can use ALIGNED_CAST to
suppress this if we know that the object is actually correctly aligned
for the more strictly aligned type, but if this is not the case then it
does not solve any problem, it only suppresses a warning and there is
still a time bomb hidden in the code for RISC architectures.



More information about the dev mailing list