[ovs-dev] [PATCH 2/2] netdev-offload-dpdk: Fix vxlan vni cast-align warnings

Eli Britstein elibr at nvidia.com
Thu Jul 22 13:00:15 UTC 2021


On 7/22/2021 3:28 PM, Ilya Maximets wrote:
> External email: Use caution opening links or attachments
>
>
> 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 <harry.van.haaren at intel.com>
>> 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 <elibr at nvidia.com>
>> ---
>>   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.

The code before this patch just uses (for example) put_unaligned_be32, 
which its 1st argument is (ovs_be32 *).

vni in struct rte_flow_item_vxlan  in dpdk is uint8_t vni[3]; /**< VXLAN 
identifier. */

I use ALIGNED_CAST to mute the warning, and the assert to make sure the 
alignment is correct.

I don't understand your suggestion here, unless you suggest to use 
memcpy as suggested in patch#1.

>
> Best regards, Ilya Maximets.


More information about the dev mailing list