DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ether: remove packing and set two-byte alignment
@ 2019-10-10 10:46 Bruce Richardson
  2019-10-27 17:13 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Bruce Richardson @ 2019-10-10 10:46 UTC (permalink / raw)
  To: dev; +Cc: olivier.matz, thomas, Bruce Richardson, Stephen Hemminger

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-dev] [PATCH] ether: remove packing and set two-byte alignment
  2019-10-10 10:46 [dpdk-dev] [PATCH] ether: remove packing and set two-byte alignment Bruce Richardson
@ 2019-10-27 17:13 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2019-10-27 17:13 UTC (permalink / raw)
  To: Bruce Richardson, Stephen Hemminger; +Cc: dev, olivier.matz

10/10/2019 12:46, Bruce Richardson:
> 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>

Applied, thanks




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-10-27 17:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 10:46 [dpdk-dev] [PATCH] ether: remove packing and set two-byte alignment Bruce Richardson
2019-10-27 17:13 ` Thomas Monjalon

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).