On 7/1/21 6:36 PM, Van Haaren, Harry wrote:
>> -----Original Message-----
>> From: dev <[email protected]> On Behalf Of Eli Britstein
>> Sent: Wednesday, June 23, 2021 4:53 PM
>> To: [email protected]; Ilya Maximets <[email protected]>
>> Cc: Eli Britstein <[email protected]>; Ivan Malov <[email protected]>;
>> Majd
>> Dibbiny <[email protected]>
>> Subject: [ovs-dev] [PATCH V7 13/13] netdev-dpdk-offload: Add vxlan pattern
>> matching function.
>>
>> For VXLAN offload, matches should be done on outer header for tunnel
>> properties as well as inner packet matches. Add a function for parsing
>> VXLAN tunnel matches.
>>
>> Signed-off-by: Eli Britstein <[email protected]>
>> Reviewed-by: Gaetan Rivet <[email protected]>
>> Acked-by: Sriharsha Basavapatna <[email protected]>
>> Tested-by: Emma Finn <[email protected]>
>> Tested-by: Marko Kovacevic <[email protected]>
>> Signed-off-by: Ilya Maximets <[email protected]>
>> ---
>> NEWS | 2 +
>> lib/netdev-offload-dpdk.c | 155 +++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 156 insertions(+), 1 deletion(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 6f8e62f7f..10b3ab053 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -19,6 +19,8 @@ Post-v2.15.0
>> * New debug appctl command 'dpdk/get-malloc-stats'.
>> * Add hardware offload support for tunnel pop action (experimental).
>> Available only if DPDK experimantal APIs enabled during the build.
>> + * Add hardware offload support for VXLAN flows (experimental).
>> + Available only if DPDK experimantal APIs enabled during the build.
>> - ovsdb-tool:
>> * New option '--election-timer' to the 'create-cluster' command to set
>> the
>> leader election timer during cluster creation.
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index 6220394de..6bd5b6c9f 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -372,6 +372,20 @@ dump_flow_pattern(struct ds *s,
>> ipv6_mask->hdr.hop_limits);
>> }
>> ds_put_cstr(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;
>> +
>> + ds_put_cstr(s, "vxlan ");
>> + if (vxlan_spec) {
>> + if (!vxlan_mask) {
>> + 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);
>> + }
>> + ds_put_cstr(s, "/ ");
>> } else {
>> ds_put_format(s, "unknown rte flow pattern (%d)\n", item->type);
>> }
>> @@ -865,15 +879,154 @@ out:
>> return ret;
>> }
>
> Hi All,
>
> Is there a build failure in OVS master branch since this has been merged?
I didn't notice any build failures. The main factor here is that you just
can't build with DPDK without -Wno-cast-align anyway with neither modern gcc
nor clang. So, this should not be an issue.
gcc 10.3.1 on fedora generates myriads of cast-align warnings in DPDK headers:
./configure CC=gcc --enable-Werror --with-dpdk=static
Part of the build log:
...
/dpdk/build/include/rte_memcpy.h: In function ‘rte_mov16’:
/dpdk/build/include/rte_memcpy.h:499:42: error: cast increases required
alignment of target type [-Werror=cast-align]
499 | xmm0 = _mm_loadu_si128((const __m128i *)(const __m128i *)src);
| ^
/dpdk/build/include/rte_memcpy.h:500:19: error: cast increases required
alignment of target type [-Werror=cast-align]
500 | _mm_storeu_si128((__m128i *)dst, xmm0);
| ^
/dpdk/build/include/rte_memcpy.h: In function ‘rte_memcpy_generic’:
/dpdk/build/include/rte_memcpy.h:584:32: error: cast increases required
alignment of target type [-Werror=cast-align]
584 | xmm0 = _mm_loadu_si128((const __m128i *)((const uint8_t *)src -
offset + 0 * 16)); \
...
clang always complained about them, as far as I remember.
Also, gcc on ubuntu 18.04 in our GHA doesn't seem to generate warnings that
you're seeing, but it doesn't generate warnings for DPDK headers either,
so it maybe just not that smart.
> It seems the above casts (ovs_be32 *) are causing issues with increasing
> alignment?
>
> From DPDK's struct rte_flow_item_vxlan, the vni is a "uint8_t vni[3];"
>
> The VNI is not a 32-bit value, so is not required to be aligned on 32-bits.
> The cast here
> technically requires the alignment of the casted pointer to be 32-bit/4 byte,
> which is
> not guaranteed to be true. I think the compiler rightly flags a cast
> alignment issue?
I briefly inspected the resulted binary and I didn't notice any actual
changes in alignments, so this, likely, doesn't cause any real problems,
at least on x86. But, I agree that we, probably, should fix that to
have a more correct code.
>
> Casting through a (void *) might solve, or else using some of the BE/LE
> conversion
> and alignment helpers in byte-order.h and unaligned.h could perhaps work?
I think, get_unaligned_be32() is a way to go here. Feel free to submit
a patch.
>
> My testing as follows below. Regards, -Harry
>
>
> Configure:
> ./configure CFLAGS="-O3 -g -msse4.2 -mpopcnt -Wall -Wextra"
> --with-dpdk=static --enable-Werror
>
> Compiler, from Ubuntu 21.04:
> gcc (Ubuntu 10.3.0-1ubuntu1) 10.3.0
>
> Build Failure:
> 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,
> | ^
> lib/netdev-offload-dpdk.c:189:48: note: in definition of macro
> 'DUMP_PATTERN_ITEM'
> 189 | ds_put_format(s, field " is " fmt " ", spec_pri); \
> | ^~~~~~~~
> 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,
> | ^
> lib/netdev-offload-dpdk.c:192:23: note: in definition of macro
> 'DUMP_PATTERN_ITEM'
> 192 | spec_pri, mask_pri); \
> | ^~~~~~~~
> lib/netdev-offload-dpdk.c:386:38: error: cast increases required alignment of
> target type [-Werror=cast-align]
> 386 | ntohl(*(ovs_be32 *) vxlan_mask->vni) >>
> 8);
> | ^
> lib/netdev-offload-dpdk.c:192:33: note: in definition of macro
> 'DUMP_PATTERN_ITEM'
> 192 | spec_pri, mask_pri); \
> | ^~~~~~~~
> In file included from /usr/include/netinet/ip6.h:22,
> from lib/netdev-offload-dpdk.c:20:
> lib/netdev-offload-dpdk.c: In function 'dump_vxlan_encap':
> lib/netdev-offload-dpdk.c:421:30: error: cast increases required alignment of
> target type [-Werror=cast-align]
> 421 | ntohl(*(ovs_be32 *) vxlan->vni) >> 8);
> | ^
> lib/netdev-offload-dpdk.c: In function 'dump_flow_action':
> lib/netdev-offload-dpdk.c:572:30: error: cast increases required alignment of
> target type [-Werror=cast-align]
> 572 | ipv6_format_addr((struct in6_addr *)
> &set_ipv6->ipv6_addr, s);
> | ^
> lib/netdev-offload-dpdk.c: In function 'parse_vxlan_match':
> lib/netdev-offload-dpdk.c:1002:24: error: cast increases required alignment
> of target type [-Werror=cast-align]
> 1002 | put_unaligned_be32((ovs_be32 *) vx_spec->vni,
> | ^
> lib/netdev-offload-dpdk.c:1004:24: error: cast increases required alignment
> of target type [-Werror=cast-align]
> 1004 | put_unaligned_be32((ovs_be32 *) vx_mask->vni,
> | ^
> cc1: all warnings being treated as errors
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
