Skip to contentSkip to conversation

resolve conflict between net/ethernet.h and rte_ethdev.h

RFC In review Stephen Hemminger 3 participants 6 messages Dec 27, 2014 – Mar 10, 2015

lore
Share & tools
lore
#0 lore raw

RFC 1/1 resolve conflict between net/ethernet.h and rte_ethdev.h

This is a patch to address the conflict between <net/ethernet.h> and the definitions in <rte_ethdev.h>. It has two side effects worth discussion:
1. It forces inclusion of net/ethernet.h
2. It has definition to deal with the differing structure elements in the two versions of struct ether_addr.

By doing this ether_ntoa and related functions can be used without messing with prototypes.

Alternative is more complex #ifdef magic like linux/libc-compat.h

lib/librte_ether/rte_ether.h +18 -37
@@ -46,17 +46,11 @@
 
 #include <stdint.h>
 #include <stdio.h>
+#include <net/ethernet.h>
 
 #include <rte_memcpy.h>
 #include <rte_random.h>
 
-#define ETHER_ADDR_LEN  6 /**< Length of Ethernet address. */
-#define ETHER_TYPE_LEN  2 /**< Length of Ethernet type field. */
-#define ETHER_CRC_LEN   4 /**< Length of Ethernet CRC. */
-#define ETHER_HDR_LEN   \
-	(ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN) /**< Length of Ethernet header. */
-#define ETHER_MIN_LEN   64    /**< Minimum frame len, including CRC. */
-#define ETHER_MAX_LEN   1518  /**< Maximum frame len, including CRC. */
 #define ETHER_MTU       \
 	(ETHER_MAX_LEN - ETHER_HDR_LEN - ETHER_CRC_LEN) /**< Ethernet MTU. */
 
@@ -69,25 +63,12 @@
 #define ETHER_MAX_VLAN_ID  4095 /**< Maximum VLAN ID. */
 
 #define ETHER_MIN_MTU 68 /**< Minimum MTU for IPv4 packets, see RFC 791. */
-
-/**
- * Ethernet address:
- * A universally administered address is uniquely assigned to a device by its
- * manufacturer. The first three octets (in transmission order) contain the
- * Organizationally Unique Identifier (OUI). The following three (MAC-48 and
- * EUI-48) octets are assigned by that organization with the only constraint
- * of uniqueness.
- * A locally administered address is assigned to a device by a network
- * administrator and does not contain OUIs.
- * See http://standards.ieee.org/regauth/groupmac/tutorial.html
- */
-struct ether_addr {
-	uint8_t addr_bytes[ETHER_ADDR_LEN]; /**< Address bytes in transmission order */
-} __attribute__((__packed__));
-
 #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */
 #define ETHER_GROUP_ADDR       0x01 /**< Multicast or broadcast Eth. address. */
 
+/* Deprecated definition to allow for compatiablity with net/ethernet.h */
+#define addr_bytes	ether_addr_octet
+
 /**
  * Check if two Ethernet addresses are the same.
  *
@@ -107,7 +88,7 @@
 {
 	int i;
 	for (i = 0; i < ETHER_ADDR_LEN; i++)
-		if (ea1->addr_bytes[i] != ea2->addr_bytes[i])
+		if (ea1->ether_addr_octet[i] != ea2->ether_addr_octet[i])
 			return 0;
 	return 1;
 }
@@ -126,7 +107,7 @@
 {
 	int i;
 	for (i = 0; i < ETHER_ADDR_LEN; i++)
-		if (ea->addr_bytes[i] != 0x00)
+		if (ea->ether_addr_octet[i] != 0x00)
 			return 0;
 	return 1;
 }
@@ -143,7 +124,7 @@
  */
 static inline int is_unicast_ether_addr(const struct ether_addr *ea)
 {
-	return ((ea->addr_bytes[0] & ETHER_GROUP_ADDR) == 0);
+	return ((ea->ether_addr_octet[0] & ETHER_GROUP_ADDR) == 0);
 }
 
 /**
@@ -158,7 +139,7 @@
  */
 static inline int is_multicast_ether_addr(const struct ether_addr *ea)
 {
-	return (ea->addr_bytes[0] & ETHER_GROUP_ADDR);
+	return (ea->ether_addr_octet[0] & ETHER_GROUP_ADDR);
 }
 
 /**
@@ -191,7 +172,7 @@
  */
 static inline int is_universal_ether_addr(const struct ether_addr *ea)
 {
-	return ((ea->addr_bytes[0] & ETHER_LOCAL_ADMIN_ADDR) == 0);
+	return ((ea->ether_addr_octet[0] & ETHER_LOCAL_ADMIN_ADDR) == 0);
 }
 
 /**
@@ -206,7 +187,7 @@
  */
 static inline int is_local_admin_ether_addr(const struct ether_addr *ea)
 {
-	return ((ea->addr_bytes[0] & ETHER_LOCAL_ADMIN_ADDR) != 0);
+	return ((ea->ether_addr_octet[0] & ETHER_LOCAL_ADMIN_ADDR) != 0);
 }
 
 /**
@@ -253,8 +234,8 @@
 				   struct ether_addr *ea_to)
 {
 #ifdef __INTEL_COMPILER
-	uint16_t *from_words = (uint16_t *)(ea_from->addr_bytes);
-	uint16_t *to_words   = (uint16_t *)(ea_to->addr_bytes);
+	uint16_t *from_words = (uint16_t *)(ea_from->ether_addr_octet);
+	uint16_t *to_words   = (uint16_t *)(ea_to->ether_addr_octet);
 
 	to_words[0] = from_words[0];
 	to_words[1] = from_words[1];
@@ -283,12 +264,12 @@
 		  const struct ether_addr *eth_addr)
 {
 	snprintf(buf, size, "%02X:%02X:%02X:%02X:%02X:%02X",
-		 eth_addr->addr_bytes[0],
-		 eth_addr->addr_bytes[1],
-		 eth_addr->addr_bytes[2],
-		 eth_addr->addr_bytes[3],
-		 eth_addr->addr_bytes[4],
-		 eth_addr->addr_bytes[5]);
+		 eth_addr->ether_addr_octet[0],
+		 eth_addr->ether_addr_octet[1],
+		 eth_addr->ether_addr_octet[2],
+		 eth_addr->ether_addr_octet[3],
+		 eth_addr->ether_addr_octet[4],
+		 eth_addr->ether_addr_octet[5]);
 }
 
 /**
Acked-by on v1 1/1
#1 lore raw
Quoted 7 lines · Stephen Hemminger

This is a patch to address the conflict between <net/ethernet.h> and the definitions in <rte_ethdev.h>. It has two side effects worth discussion:
1. It forces inclusion of net/ethernet.h
2. It has definition to deal with the differing structure elements in the two versions of struct ether_addr.

Quoted 6 lines

By doing this ether_ntoa and related functions can be used without messing with prototypes.

Alternative is more complex #ifdef magic like linux/libc-compat.h

This seems like a reasonable fix for the problem, and nasty compat-work

#2 lore raw
In reply to Stephen Hemminger

2014-12-27 15:13, Stephen Hemminger:

Quoted 11 lines

This is a patch to address the conflict between <net/ethernet.h> and the definitions in <rte_ethdev.h>. It has two side effects worth discussion:
1. It forces inclusion of net/ethernet.h
2. It has definition to deal with the differing structure elements in the two versions of struct ether_addr.

By doing this ether_ntoa and related functions can be used without messing with prototypes.

Alternative is more complex #ifdef magic like linux/libc-compat.h

[...]

Quoted 1 line

+#include <net/ethernet.h>

[...]

lib/librte_ether/rte_ether.h line 70

Quoted 2 lines
lib/librte_ether/rte_ether.h +2 -0
+/* Deprecated definition to allow for compatiablity with net/ethernet.h */
+#define addr_bytes	ether_addr_octet

This is defining a common identifier without prefix.
So it will be forbidden to use addr_bytes as variable name.
I understand you are trying to keep compatibility with both structures, but the drawback is really nasty.
Is there another solution? Or at least, we could mark it as deprecated and remove it in release 2.1.

Signature · 1 line
Thomas
#3 lore raw

2015-01-06 11:44, Thomas Monjalon:

Quoted 19 lines

2014-12-27 15:13, Stephen Hemminger:

Quoted 11 lines

This is a patch to address the conflict between <net/ethernet.h> and the definitions in <rte_ethdev.h>. It has two side effects worth discussion:
1. It forces inclusion of net/ethernet.h
2. It has definition to deal with the differing structure elements in the two versions of struct ether_addr.

By doing this ether_ntoa and related functions can be used without messing with prototypes.

Alternative is more complex #ifdef magic like linux/libc-compat.h

[...]

Quoted 1 line

+#include <net/ethernet.h>

[...]

Quoted 9 lines

lib/librte_ether/rte_ether.h line 70

Quoted 2 lines
lib/librte_ether/rte_ether.h +2 -0
+/* Deprecated definition to allow for compatiablity with net/ethernet.h */
+#define addr_bytes	ether_addr_octet

This is defining a common identifier without prefix.
So it will be forbidden to use addr_bytes as variable name.
I understand you are trying to keep compatibility with both structures, but the drawback is really nasty.
Is there another solution? Or at least, we could mark it as deprecated and remove it in release 2.1.

ping
Any opinion?

#4 lore raw

2015-03-05 00:16, Thomas Monjalon:

Quoted 30 lines

2015-01-06 11:44, Thomas Monjalon:

Quoted 19 lines

2014-12-27 15:13, Stephen Hemminger:

Quoted 11 lines

This is a patch to address the conflict between <net/ethernet.h> and the definitions in <rte_ethdev.h>. It has two side effects worth discussion:
1. It forces inclusion of net/ethernet.h
2. It has definition to deal with the differing structure elements in the two versions of struct ether_addr.

By doing this ether_ntoa and related functions can be used without messing with prototypes.

Alternative is more complex #ifdef magic like linux/libc-compat.h

[...]

Quoted 1 line

+#include <net/ethernet.h>

[...]

Quoted 9 lines

lib/librte_ether/rte_ether.h line 70

Quoted 2 lines
lib/librte_ether/rte_ether.h +2 -0
+/* Deprecated definition to allow for compatiablity with net/ethernet.h */
+#define addr_bytes	ether_addr_octet

This is defining a common identifier without prefix.
So it will be forbidden to use addr_bytes as variable name.
I understand you are trying to keep compatibility with both structures, but the drawback is really nasty.
Is there another solution? Or at least, we could mark it as deprecated and remove it in release 2.1.

Quoted 2 lines

ping
Any opinion?

Hi Stephen,
If, by any chance, you are willing to reply to this thread, maybe you would like to send a non-rfc patch with these 2 additions:
- rename addr_bytes to ether_addr_octet everywhere
- mark addr_bytes as deprecated in doc/guides/rel_notes/abi.rst

#5 lore raw
Quoted 5 lines · On Tue, 10 Mar 2015 14:29:11 +0100 Thomas Monjalon…

Hi Stephen,
If, by any chance, you are willing to reply to this thread, maybe you would like to send a non-rfc patch with these 2 additions:
- rename addr_bytes to ether_addr_octet everywhere
- mark addr_bytes as deprecated in doc/guides/rel_notes/abi.rst

I can respin with the global changes, just not a big fan of deprecation since it only pisses off users.