> From: David Marchand [mailto:david.march...@redhat.com]
> Sent: Tuesday, 5 November 2024 09.59
>
> The new function breaks compilation with -Wcast-align.
>
> In file included from /home/runner/work/ovs/ovs/dpdk-
> dir/include/rte_ip.h:9:
> /home/runner/work/ovs/ovs/dpdk-dir/include/rte_ip4.h:191:10:
> error: cast from 'const uint8_t *' (aka 'const unsigned char *')
> to 'const unaligned_uint16_t *' (aka 'const unsigned short *')
> increases required alignment from 1 to 2 [-Werror,-Wcast-align]
> v16_h = (const unaligned_uint16_t *)&ipv4_hdr->version_ihl;
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Fix this by aligning rte_ipv4_hdr to two bytes, and point at the start
> of the structure rather than the first field (which happens to be 1
> byte
> large).
>
> Fixes: f9e1d67f237a ("net: add IPv4 cksum function for simple cases")
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> doc/guides/rel_notes/release_24_11.rst | 2 ++
> lib/net/rte_ip4.h | 4 ++--
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/rel_notes/release_24_11.rst
> b/doc/guides/rel_notes/release_24_11.rst
> index 53a5ffebe5..d0152c558c 100644
> --- a/doc/guides/rel_notes/release_24_11.rst
> +++ b/doc/guides/rel_notes/release_24_11.rst
> @@ -291,6 +291,8 @@ API Changes
> releases: it handles key=value and only-key cases.
> * Both ``rte_kvargs_process`` and ``rte_kvargs_process_opt`` reject
> a NULL ``kvlist`` parameter.
>
> +* net: The IPv4 header structure ``rte_ipv4_hdr`` has been marked as
> two bytes aligned.
> +
> * net: The ICMP message types ``RTE_IP_ICMP_ECHO_REPLY`` and
> ``RTE_IP_ICMP_ECHO_REQUEST``
> are marked as deprecated, and are replaced
> by ``RTE_ICMP_TYPE_ECHO_REPLY`` and ``RTE_ICMP_TYPE_ECHO_REQUEST``.
> diff --git a/lib/net/rte_ip4.h b/lib/net/rte_ip4.h
> index 4dd0058cc5..f9b8333332 100644
> --- a/lib/net/rte_ip4.h
> +++ b/lib/net/rte_ip4.h
> @@ -39,7 +39,7 @@ extern "C" {
> /**
> * IPv4 Header
> */
> -struct rte_ipv4_hdr {
> +struct __rte_aligned(2) rte_ipv4_hdr {
<unrelated>
I wonder why we have a convention of putting __rte_packed after the struct, and
not between the "struct" tag and the name of the struct.
It would make the code much more readable having it here, like __rte_aligned().
</unrelated>
> __extension__
> union {
> uint8_t version_ihl; /**< version and header length */
> @@ -188,7 +188,7 @@ rte_ipv4_cksum_simple(const struct rte_ipv4_hdr
> *ipv4_hdr)
> * Compute the sum of successive 16-bit words of the IPv4 header,
> * skipping the checksum field of the header.
> */
> - v16_h = (const unaligned_uint16_t *)&ipv4_hdr->version_ihl;
> + v16_h = (const uint16_t *)ipv4_hdr;
> ip_cksum = v16_h[0] + v16_h[1] + v16_h[2] + v16_h[3] +
> v16_h[4] + v16_h[6] + v16_h[7] + v16_h[8] + v16_h[9];
>
> --
> 2.46.2
Reviewed-by: Morten Brørup <m...@smartsharesystems.com>
For consistency, could one of you - David or Robin - please also 2-byte align
the IPv6 header structure?
