[v2,2/7] ether: do not mark ethernet address and header as packed
Checks
Commit Message
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(-)
Comments
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?
> /* VXLAN-GPE next protocol types */
> #define VXLAN_GPE_TYPE_IPV4 1 /**< IPv4 Protocol. */
> --
> 2.20.1
>
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
@@ -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__));
+};
/* VXLAN-GPE next protocol types */
#define VXLAN_GPE_TYPE_IPV4 1 /**< IPv4 Protocol. */