DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: dev@dpdk.org
Cc: olivier.matz@6wind.com, thomas@monjalon.net,
	Bruce Richardson <bruce.richardson@intel.com>,
	Stephen Hemminger <stephen@networkplumber.org>
Subject: [dpdk-dev] [PATCH] ether: remove packing and set two-byte alignment
Date: Thu, 10 Oct 2019 11:46:23 +0100	[thread overview]
Message-ID: <20191010104623.10090-1-bruce.richardson@intel.com> (raw)

The ether header does not need to be packed since that makes no sense for
structures with only bytes in them, but it should be aligned to a two-byte
boundary to simplify access to it from code. Other packed structures that
use this also need to be updated to take account of the change, either by
removing packing - where it is clearly unneeded - or by explicitly giving
those structures 2-byte alignment also.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/rel_notes/deprecation.rst      | 10 ----------
 doc/guides/rel_notes/release_19_11.rst    |  6 ++++++
 drivers/net/bonding/rte_eth_bond_8023ad.h | 14 +++++++-------
 lib/librte_net/rte_arp.h                  |  4 ++--
 lib/librte_net/rte_ether.h                | 12 ++++++------
 lib/librte_pipeline/rte_table_action.c    | 22 +++++++++++-----------
 6 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 0ee8533b1..86ec962d0 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -61,16 +61,6 @@ Deprecation Notices
   structure would be made internal (or removed if all dependencies are cleared)
   in future releases.
 
-* net: The Ethernet address and header definitions will change
-  attributes. The Ethernet address struct will no longer be marked as
-  packed since the packed attribute is meaningless on a byte
-  array. The Ethernet header will be marked as aligned on a 2-byte
-  boundary and will no longer have the packed attribute. This allows
-  for efficient access on CPU architectures where unaligned access is
-  expensive. These changes should not impact normal usage because drivers
-  naturally align the Ethernet header on receive and all known
-  encapsulations preserve the alignment of the header.
-
 * ethdev: The function ``rte_eth_dev_count`` will be removed in DPDK 20.02.
   It is replaced by the function ``rte_eth_dev_count_avail``.
   If the intent is to iterate over ports, ``RTE_ETH_FOREACH_*`` macros
diff --git a/doc/guides/rel_notes/release_19_11.rst b/doc/guides/rel_notes/release_19_11.rst
index 27cfbd9e3..efaba7872 100644
--- a/doc/guides/rel_notes/release_19_11.rst
+++ b/doc/guides/rel_notes/release_19_11.rst
@@ -110,6 +110,12 @@ ABI Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* net: The Ethernet address and other header definitions have changed
+  attributes. They have been modified to be aligned on 2-byte boundaries.
+  These changes should not impact normal usage because drivers naturally
+  align the Ethernet header on receive and all known encapsulations
+  preserve the alignment of the header.
+
 
 Shared Library Versions
 -----------------------
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.h b/drivers/net/bonding/rte_eth_bond_8023ad.h
index cbad59aa7..62265f449 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.h
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.h
@@ -60,7 +60,7 @@ struct slow_protocol {
 struct slow_protocol_frame {
 	struct rte_ether_hdr eth_hdr;
 	struct slow_protocol slow_protocol;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct port_params {
 	uint16_t system_priority;
@@ -73,7 +73,7 @@ struct port_params {
 	/**< Priority of this (unused in current implementation) */
 	uint16_t port_number;
 	/**< Port number. It corresponds to slave port id. */
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct lacpdu_actor_partner_params {
 	uint8_t tlv_type_info;
@@ -81,7 +81,7 @@ struct lacpdu_actor_partner_params {
 	struct port_params port_params;
 	uint8_t state;
 	uint8_t reserved_3[3];
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 /** LACPDU structure (5.4.2 in 802.1AX documentation). */
 struct lacpdu {
@@ -99,13 +99,13 @@ struct lacpdu {
 	uint8_t tlv_type_terminator;
 	uint8_t terminator_length;
 	uint8_t reserved_50[50];
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 /** LACPDU frame: Contains ethernet header and LACPDU. */
 struct lacpdu_header {
 	struct rte_ether_hdr eth_hdr;
 	struct lacpdu lacpdu;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct marker {
 	uint8_t subtype;
@@ -121,12 +121,12 @@ struct marker {
 	uint8_t tlv_type_terminator;
 	uint8_t terminator_length;
 	uint8_t reserved_90[90];
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct marker_header {
 	struct rte_ether_hdr eth_hdr;
 	struct marker marker;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct rte_eth_bond_8023ad_conf {
 	uint32_t fast_periodic_ms;
diff --git a/lib/librte_net/rte_arp.h b/lib/librte_net/rte_arp.h
index ccb6875cf..fabd68624 100644
--- a/lib/librte_net/rte_arp.h
+++ b/lib/librte_net/rte_arp.h
@@ -26,7 +26,7 @@ struct rte_arp_ipv4 {
 	uint32_t          arp_sip;  /**< sender IP address */
 	struct rte_ether_addr arp_tha;  /**< target hardware address */
 	uint32_t          arp_tip;  /**< target IP address */
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 /**
  * ARP header.
@@ -47,7 +47,7 @@ struct rte_arp_hdr {
 #define	RTE_ARP_OP_INVREPLY   9 /* response identifying peer */
 
 	struct rte_arp_ipv4 arp_data;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 /**
  * @warning
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index aa6eff037..5b81a7a96 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -59,7 +59,7 @@ extern "C" {
  */
 struct rte_ether_addr {
 	uint8_t addr_bytes[RTE_ETHER_ADDR_LEN]; /**< Addr bytes in tx order */
-} __attribute__((__packed__));
+} __attribute__((aligned(2)));
 
 #define RTE_ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */
 #define RTE_ETHER_GROUP_ADDR  0x01 /**< Multicast or broadcast Eth. address. */
@@ -81,8 +81,8 @@ struct rte_ether_addr {
 static inline int rte_is_same_ether_addr(const struct rte_ether_addr *ea1,
 				     const struct rte_ether_addr *ea2)
 {
-	const unaligned_uint16_t *w1 = (const uint16_t *)ea1;
-	const unaligned_uint16_t *w2 = (const uint16_t *)ea2;
+	const uint16_t *w1 = (const uint16_t *)ea1;
+	const uint16_t *w2 = (const uint16_t *)ea2;
 
 	return ((w1[0] ^ w2[0]) | (w1[1] ^ w2[1]) | (w1[2] ^ w2[2])) == 0;
 }
@@ -99,7 +99,7 @@ static inline int rte_is_same_ether_addr(const struct rte_ether_addr *ea1,
  */
 static inline int rte_is_zero_ether_addr(const struct rte_ether_addr *ea)
 {
-	const unaligned_uint16_t *w = (const uint16_t *)ea;
+	const uint16_t *w = (const uint16_t *)ea;
 
 	return (w[0] | w[1] | w[2]) == 0;
 }
@@ -146,7 +146,7 @@ static inline int rte_is_multicast_ether_addr(const struct rte_ether_addr *ea)
  */
 static inline int rte_is_broadcast_ether_addr(const struct rte_ether_addr *ea)
 {
-	const unaligned_uint16_t *ea_words = (const unaligned_uint16_t *)ea;
+	const uint16_t *ea_words = (const uint16_t *)ea;
 
 	return (ea_words[0] == 0xFFFF && ea_words[1] == 0xFFFF &&
 		ea_words[2] == 0xFFFF);
@@ -273,7 +273,7 @@ struct rte_ether_hdr {
 	struct rte_ether_addr d_addr; /**< Destination address. */
 	struct rte_ether_addr s_addr; /**< Source address. */
 	uint16_t ether_type;      /**< Frame type. */
-} __attribute__((__packed__));
+} __attribute__((aligned(2)));
 
 /**
  * Ethernet VLAN Header.
diff --git a/lib/librte_pipeline/rte_table_action.c b/lib/librte_pipeline/rte_table_action.c
index 47d7efbc1..962404b7a 100644
--- a/lib/librte_pipeline/rte_table_action.c
+++ b/lib/librte_pipeline/rte_table_action.c
@@ -438,7 +438,7 @@ encap_cfg_check(struct rte_table_action_encap_config *encap)
 
 struct encap_ether_data {
 	struct rte_ether_hdr ether;
-} __attribute__((__packed__));
+};
 
 #define VLAN(pcp, dei, vid)                                \
 	((uint16_t)((((uint64_t)(pcp)) & 0x7LLU) << 13) |  \
@@ -448,13 +448,13 @@ struct encap_ether_data {
 struct encap_vlan_data {
 	struct rte_ether_hdr ether;
 	struct rte_vlan_hdr vlan;
-} __attribute__((__packed__));
+};
 
 struct encap_qinq_data {
 	struct rte_ether_hdr ether;
 	struct rte_vlan_hdr svlan;
 	struct rte_vlan_hdr cvlan;
-} __attribute__((__packed__));
+};
 
 #define ETHER_TYPE_MPLS_UNICAST                            0x8847
 
@@ -470,7 +470,7 @@ struct encap_mpls_data {
 	struct rte_ether_hdr ether;
 	uint32_t mpls[RTE_TABLE_ACTION_MPLS_LABELS_MAX];
 	uint32_t mpls_count;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 #define PPP_PROTOCOL_IP                                    0x0021
 
@@ -479,12 +479,12 @@ struct pppoe_ppp_hdr {
 	uint16_t session_id;
 	uint16_t length;
 	uint16_t protocol;
-} __attribute__((__packed__));
+};
 
 struct encap_pppoe_data {
 	struct rte_ether_hdr ether;
 	struct pppoe_ppp_hdr pppoe_ppp;
-} __attribute__((__packed__));
+};
 
 #define IP_PROTO_UDP                                       17
 
@@ -493,7 +493,7 @@ struct encap_vxlan_ipv4_data {
 	struct rte_ipv4_hdr ipv4;
 	struct rte_udp_hdr udp;
 	struct rte_vxlan_hdr vxlan;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct encap_vxlan_ipv4_vlan_data {
 	struct rte_ether_hdr ether;
@@ -501,14 +501,14 @@ struct encap_vxlan_ipv4_vlan_data {
 	struct rte_ipv4_hdr ipv4;
 	struct rte_udp_hdr udp;
 	struct rte_vxlan_hdr vxlan;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct encap_vxlan_ipv6_data {
 	struct rte_ether_hdr ether;
 	struct rte_ipv6_hdr ipv6;
 	struct rte_udp_hdr udp;
 	struct rte_vxlan_hdr vxlan;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct encap_vxlan_ipv6_vlan_data {
 	struct rte_ether_hdr ether;
@@ -516,14 +516,14 @@ struct encap_vxlan_ipv6_vlan_data {
 	struct rte_ipv6_hdr ipv6;
 	struct rte_udp_hdr udp;
 	struct rte_vxlan_hdr vxlan;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 struct encap_qinq_pppoe_data {
 	struct rte_ether_hdr ether;
 	struct rte_vlan_hdr svlan;
 	struct rte_vlan_hdr cvlan;
 	struct pppoe_ppp_hdr pppoe_ppp;
-} __attribute__((__packed__));
+} __attribute__((__packed__)) __attribute__((aligned(2)));
 
 static size_t
 encap_data_size(struct rte_table_action_encap_config *encap)
-- 
2.21.0


             reply	other threads:[~2019-10-10 10:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-10 10:46 Bruce Richardson [this message]
2019-10-27 17:13 ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191010104623.10090-1-bruce.richardson@intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).