[ovs-dev] [PATCH 1/4] Add OF actions for generic encap and decap

[Yang, Yi Y]

Jan, here is ALIGNED_CAST macro

/* Given ATTR, and TYPE, cast the ATTR to TYPE by first casting ATTR to
 * (void *). This is to suppress the alignment warning issued by clang. */
#define ALIGNED_CAST(TYPE, ATTR) ((TYPE) (void *) (ATTR))

It is just for suppressing the error message Ben saw, so we can safely use it, the below patch is perfect solution to the issue :-)

-----Original Message-----
From: Jan Scheurich [mailto:jan.scheurich at ericsson.com] 
Sent: Friday, July 14, 2017 1:34 AM
To: Ben Pfaff <blp at ovn.org>; Zoltán Balogh <zoltan.balogh at ericsson.com>
Cc: 'dev at openvswitch.org' <dev at openvswitch.org>; Georg Schmuecking <Georg.Schmuecking at ericsson.com>; Jiri Benc (jbenc at redhat.com) <jbenc at redhat.com>; Yang, Yi Y <yi.y.yang at intel.com>
Subject: RE: [PATCH 1/4] Add OF actions for generic encap and decap

The simple way out is to use ALIGNED_CAST instead.

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 04dd048..cc19db4 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -4189,7 +4189,8 @@ parse_ENCAP(char *arg,
         return error;
     }
     /* ofbuf out may have been re-allocated. */
-    encap = (struct ofpact_encap *) ((char *)out->data + start_ofs);
+    encap = ALIGNED_CAST(struct ofpact_encap *, (char *)out->data + 
+ start_ofs);
     encap->n_props = n_props;
     ofpact_finish_ENCAP(out, &encap);
     return NULL;
@@ -4224,7 +4225,7 @@ format_ed_props(struct ds *s, uint16_t n_props,
         format_ed_prop_value(s, prop);
         ds_put_cstr(s, ")");
         p += ROUND_UP(prop->len, 8);
-        prop = (const struct ofpact_ed_prop *) p;
+        prop = ALIGNED_CAST(const struct ofpact_ed_prop *, p);
     }
 }

While I'm sure this is correct for the second case, I don't understand enough of the ofpbuf implementation to be certain that the buffer has the same alignment after a re-allocation. If that was guaranteed, the ALIGNED_CAST should be safe as the start_ofs does not change.

/Jan

> -----Original Message-----
> From: Ben Pfaff [mailto:blp at ovn.org]
> Sent: Thursday, 13 July, 2017 19:22
> To: Zoltán Balogh <zoltan.balogh at ericsson.com>
> Cc: 'dev at openvswitch.org' <dev at openvswitch.org>; Jan Scheurich 
> <jan.scheurich at ericsson.com>; Georg Schmuecking 
> <Georg.Schmuecking at ericsson.com>; Jiri Benc (jbenc at redhat.com) 
> <jbenc at redhat.com>; Yang, Yi Y <yi.y.yang at intel.com>
> Subject: Re: [PATCH 1/4] Add OF actions for generic encap and decap
> 
> On Thu, Jul 13, 2017 at 04:49:09PM +0000, Zoltán Balogh wrote:
> > Hello Ben,
> >
> >
> > > GCC says:
> > >
> > >     ../lib/ofp-actions.c:4189:13: error: cast from 'char *' to 
> > > 'struct ofpact_encap *' increases required alignment from 1 to 4 [-Werror,-Wcast-align]
> > >     ../lib/ofp-actions.c:4222:16: error: cast from 'const uint8_t 
> > > *' (aka 'const unsigned char *') to 'const struct ofpact_ed_prop 
> > > *' increases required alignment from 1 to 2 [-Werror,-Wcast-align]
> >
> > I cannot see these errors. I've tested with GCC 4.8.4 on Ubuntu 14.04.5 LTS and with GCC 6.3.0 on Ubuntu 17.04.
> > Which version of GCC and Linux distro did you use? Did you pass any special args to make or to the configure script?
> 
> It might be specific to 32-bit builds, since I build and test on i386.