Re: [dpdk-dev] [PATCH] ether: mark ethernet addresses as being 2-byte aligned

[Olivier Matz <olivier.matz@6wind.com>]

Hi Bruce,

On Thu, May 16, 2019 at 04:54:57PM +0100, Bruce Richardson wrote:
> When including the rte_ether.h header in applications with warnings
> enabled, a warning was given because of the assumption of 2-byte alignment
> of ethernet addresses when processing them.
> 
> .../include/rte_ether.h:149:2: warning: converting a packed ‘const
>   struct ether_addr’ pointer (alignment 1) to a ‘unaligned_uint16_t’
>   {aka ‘const short unsigned int’} pointer (alignment 2) may result in
>   an unaligned pointer value [-Waddress-of-packed-member]
> 149 |  const unaligned_uint16_t *ea_words = (const unaligned_uint16_t *)ea;
>     |  ^~~~~
> 
> Since ethernet addresses should always be aligned on a two-byte boundary,

I'm a bit reserved about this last assumption. The ethernet address
structure may be used in a private structure, whose alignment is 1. Are
we sure that there is no (funny) protocol that carries unaligned
ethernet addresses?

Shouldn't we change the definition of unaligned_uint16_t instead?
Or change the rte_is_broadcast_ether_addr() function?


> we can just inform the compiler of this assumption to remove the warnings
> and allow us to always access the addresses using 16-bit operations.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> ---
> 
> Although this is an ABI break, the network structures are all being renamed
> in this release, and a deprecation notice was previously posted for it.

Yes, but the network renaming is identified in the release note as an
API break, not an ABI break.


> ---
>  lib/librte_net/rte_ether.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 3a87ff184..8090b7c01 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -55,7 +55,8 @@ extern "C" {
>   * See http://standards.ieee.org/regauth/groupmac/tutorial.html
>   */
>  struct ether_addr {
> -	uint8_t addr_bytes[ETHER_ADDR_LEN]; /**< Addr bytes in tx order */
> +	/** Addr bytes in tx order */
> +	uint8_t addr_bytes[ETHER_ADDR_LEN] __rte_aligned(2);
>  } __attribute__((__packed__));
>  
>  #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */
> -- 
> 2.21.0
>