From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id CC1EA1B36E for ; Fri, 22 Dec 2017 14:34:32 +0100 (CET) Received: from lfbn-lil-1-110-231.w90-45.abo.wanadoo.fr ([90.45.197.231] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1eSNZD-0006ZN-2W; Fri, 22 Dec 2017 14:40:53 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Fri, 22 Dec 2017 14:34:21 +0100 Date: Fri, 22 Dec 2017 14:34:21 +0100 From: Olivier MATZ To: Adrien Mazarguil Cc: dev@dpdk.org, Bruce Richardson Message-ID: <20171222133420.64zut4nbwcuft3fs@platinum> References: <20171221122458.811-1-adrien.mazarguil@6wind.com> <20171221122458.811-7-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171221122458.811-7-adrien.mazarguil@6wind.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v1 6/6] net: fix rte_ether conflicts with libc X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Dec 2017 13:34:32 -0000 Hi Adrien, On Thu, Dec 21, 2017 at 02:00:06PM +0100, Adrien Mazarguil wrote: > Applications can't combine either net/ethernet.h or netinet/ether.h > together with rte_ether.h due to the redefinition of struct ether_addr and > various macros by the latter. > > This patch adapts rte_ether.h to rely on system definitions while > maintaining DPDK additions. > > An unforeseen consequence of involving more system header files compilation > issues with some base drivers (i40e, ixgbe) defining their own conflicting > types (e.g. __le64). This is addressed by explicitly including rte_ether.h > where missing to ensure system definitions always come first. > > Signed-off-by: Adrien Mazarguil > Cc: Olivier Matz > Cc: Bruce Richardson [...] > --- a/drivers/net/qede/base/bcm_osal.h > +++ b/drivers/net/qede/base/bcm_osal.h > @@ -334,7 +334,6 @@ u32 qede_find_first_zero_bit(unsigned long *, u32); > qede_find_first_zero_bit(bitmap, length) > > #define OSAL_BUILD_BUG_ON(cond) nothing > -#define ETH_ALEN ETHER_ADDR_LEN > > #define OSAL_BITMAP_WEIGHT(bitmap, count) 0 > Not sure we can update code in a 'base' driver as easily. It should be checked how it can be done with the qede maintainer. > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h > index 06d7b486c..19e62ea89 100644 > --- a/lib/librte_net/rte_ether.h > +++ b/lib/librte_net/rte_ether.h > @@ -44,6 +44,7 @@ > extern "C" { > #endif > > +#include > #include > #include > > @@ -52,15 +53,7 @@ extern "C" { > #include > #include > > -#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. */ > +#define ETHER_MTU ETHERMTU /**< Deprecated, defined for compatibility. */ > > #define ETHER_MAX_VLAN_FRAME_LEN \ > (ETHER_MAX_LEN + 4) /**< Maximum VLAN frame length, including CRC. */ > @@ -72,8 +65,11 @@ extern "C" { > > #define ETHER_MIN_MTU 68 /**< Minimum MTU for IPv4 packets, see RFC 791. */ > > +#ifdef __DOXYGEN__ > + > /** > - * Ethernet address: > + * 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 > @@ -82,11 +78,25 @@ extern "C" { > * 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 > + * > + * This structure is defined system-wide by "net/ethernet.h", however since > + * the name of its data field is OS-dependent, a macro named "addr_bytes" is > + * defined as an alias for the convenience of DPDK applications. > + * > + * The following definition is only for documentation purposes. > */ > struct ether_addr { > uint8_t addr_bytes[ETHER_ADDR_LEN]; /**< Addr bytes in tx order */ > } __attribute__((__packed__)); > > +#endif /* __DOXYGEN__ */ > + > +#if defined(__FreeBSD__) > +#define addr_bytes octet > +#else > +#define addr_bytes ether_addr_octet > +#endif > + > #define ETHER_LOCAL_ADMIN_ADDR 0x02 /**< Locally assigned Eth. address. */ > #define ETHER_GROUP_ADDR 0x01 /**< Multicast or broadcast Eth. address. */ This kind of #define looks a bit dangerous to me: it can trigger strange bugs because it will replace all occurences of addr_bytes after this header is included. Wouldn't it be a good opportunity to think about adding the rte_ prefix to all variables/functions of rte_ether.h?