On 7/11/21 7:15 AM, Eli Britstein wrote: > Compiling with -Werror and -Wcast-align has errors like: > > lib/netdev-offload-dpdk.c: In function 'dump_flow_pattern': > lib/netdev-offload-dpdk.c:385:38: error: cast increases required alignment > of target type [-Werror=cast-align] > 385 | ntohl(*(ovs_be32 *) vxlan_spec->vni) >> 8, > | ^ > > Fix them. > > Reported-by: Harry Van Haaren <[email protected]> > Fixes: 4e432d6f8128 ("netdev-offload-dpdk: Support tnl/push using vxlan encap > attribute.") > Fixes: e098c2f966cb ("netdev-dpdk-offload: Add vxlan pattern matching > function.") > Signed-off-by: Eli Britstein <[email protected]> > --- > lib/netdev-offload-dpdk.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index a24f92782..e4b19ae40 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -375,6 +375,8 @@ dump_flow_pattern(struct ds *s, > } else if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN) { > const struct rte_flow_item_vxlan *vxlan_spec = item->spec; > const struct rte_flow_item_vxlan *vxlan_mask = item->mask; > + BUILD_ASSERT_DECL(offsetof(struct rte_flow_item_vxlan, vni) % > + sizeof(ovs_be32) == 0); > > ds_put_cstr(s, "vxlan "); > if (vxlan_spec) { > @@ -382,8 +384,10 @@ dump_flow_pattern(struct ds *s, > vxlan_mask = &rte_flow_item_vxlan_mask; > } > DUMP_PATTERN_ITEM(vxlan_mask->vni, "vni", "%"PRIu32, > - ntohl(*(ovs_be32 *) vxlan_spec->vni) >> 8, > - ntohl(*(ovs_be32 *) vxlan_mask->vni) >> 8); > + ntohl(*ALIGNED_CAST(ovs_be32 *, > + vxlan_spec->vni)) >> 8, > + ntohl(*ALIGNED_CAST(ovs_be32 *, > + vxlan_mask->vni)) >> 8); > } > ds_put_cstr(s, "/ "); > } else { > @@ -417,8 +421,10 @@ dump_vxlan_encap(struct ds *s, const struct > rte_flow_item *items) > ds_put_format(s, "set vxlan ip-version %s ", > ipv4 ? "ipv4" : ipv6 ? "ipv6" : "ERR"); > if (vxlan) { > + BUILD_ASSERT_DECL(offsetof(struct rte_flow_item_vxlan, vni) % > + sizeof(ovs_be32) == 0); > ds_put_format(s, "vni %"PRIu32" ", > - ntohl(*(ovs_be32 *) vxlan->vni) >> 8); > + ntohl(*ALIGNED_CAST(ovs_be32 *, vxlan->vni)) >> 8); > } > if (udp) { > ds_put_format(s, "udp-src %"PRIu16" udp-dst %"PRIu16" ", > @@ -1003,9 +1009,11 @@ parse_vxlan_match(struct flow_patterns *patterns, > vx_spec = xzalloc(sizeof *vx_spec); > vx_mask = xzalloc(sizeof *vx_mask); > > - put_unaligned_be32((ovs_be32 *) vx_spec->vni, > + BUILD_ASSERT_DECL(offsetof(struct rte_flow_item_vxlan, vni) % > + sizeof(ovs_be32) == 0); > + put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_spec->vni), > htonl(ntohll(match->flow.tunnel.tun_id) << 8)); > - put_unaligned_be32((ovs_be32 *) vx_mask->vni, > + put_unaligned_be32(ALIGNED_CAST(ovs_be32 *, vx_mask->vni), > htonl(ntohll(match->wc.masks.tunnel.tun_id) << 8)); > > consumed_masks->tunnel.tun_id = 0; >
Same concerns here about the build time assertion as in the patch #1. It also seems redundant to use put_unaligned_* functions and have a build assertion at the same time. Suggesting to just use put/get_unaligned_* in all cases and remove build time assertions. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
