Re: [dpdk-dev] [PATCH v2 2/7] ether: do not mark ethernet address and header as packed

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 16 May 2019 21:40:05 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Thu, May 16, 2019 at 11:04:22AM -0700, Stephen Hemminger wrote:
> > Since the Ethernet address and header are naturally aligned
> > correctly on all architectures, there is no need to mark them
> > as packed. This results in faster code, and also gets rid of
> > warnings with gcc-9 about getting address of packed structure.
> > 
> > Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/librte_net/rte_ether.h | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> > index ac8897681aca..0954d183cca7 100644
> > --- a/lib/librte_net/rte_ether.h
> > +++ b/lib/librte_net/rte_ether.h
> > @@ -57,7 +57,7 @@ extern "C" {
> >  struct ether_addr {
> >  	/** 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. */
> >  #define ETHER_GROUP_ADDR       0x01 /**< Multicast or broadcast Eth. address. */
> > @@ -273,7 +273,7 @@ struct ether_hdr {
> >  	struct ether_addr d_addr; /**< Destination address. */
> >  	struct ether_addr s_addr; /**< Source address. */
> >  	uint16_t ether_type;      /**< Frame type. */
> > -} __attribute__((__packed__));
> > +};
> >  
> >  /**
> >   * Ethernet VLAN Header.
> > @@ -283,7 +283,7 @@ struct ether_hdr {
> >  struct vlan_hdr {
> >  	uint16_t vlan_tci; /**< Priority (3) + CFI (1) + Identifier Code (12) */
> >  	uint16_t eth_proto;/**< Ethernet type of encapsulated frame. */
> > -} __attribute__((__packed__));
> > +};
> >  
> >  /**
> >   * VXLAN protocol header.
> > @@ -293,7 +293,7 @@ struct vlan_hdr {
> >  struct vxlan_hdr {
> >  	uint32_t vx_flags; /**< flag (8) + Reserved (24). */
> >  	uint32_t vx_vni;   /**< VNI (24) + Reserved (8). */
> > -} __attribute__((__packed__));
> > +};
> >  
> >  /* Ethernet frame types */
> >  #define ETHER_TYPE_IPv4 0x0800 /**< IPv4 Protocol. */
> > @@ -325,7 +325,7 @@ struct vxlan_gpe_hdr {
> >  	uint8_t reserved[2]; /**< Reserved (16). */
> >  	uint8_t proto;       /**< next-protocol (8). */
> >  	uint32_t vx_vni;     /**< VNI (24) + Reserved (8). */
> > -} __attribute__((__packed__));
> > +};
> >    
> 
> Are we sure that these last two structures are always 4-byte aligned in
> packets?

That is a general issue of unaligned IP header problem.
Really we should be doing a combination of packed and 2 byte alignment.

I will drop the Vxlan and NVGre ones