From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5666BA0487 for ; Tue, 2 Jul 2019 09:56:44 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6E8ADCFA6; Tue, 2 Jul 2019 09:56:43 +0200 (CEST) Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id B98F05B3A for ; Tue, 2 Jul 2019 09:56:42 +0200 (CEST) Received: from lfbn-lil-1-176-160.w90-45.abo.wanadoo.fr ([90.45.26.160] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hiDha-0001sl-Ek; Tue, 02 Jul 2019 09:59:48 +0200 Received: by droids-corp.org (sSMTP sendmail emulation); Tue, 02 Jul 2019 09:56:40 +0200 Date: Tue, 2 Jul 2019 09:56:40 +0200 From: Olivier Matz To: Stephen Hemminger Cc: dev@dpdk.org, Rami Rosen , Andrew Rybchenko Message-ID: <20190702075640.2vnyiyy2gjoh5buf@platinum> References: <20190516180427.17270-1-stephen@networkplumber.org> <20190624204435.29452-1-stephen@networkplumber.org> <20190624204435.29452-2-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190624204435.29452-2-stephen@networkplumber.org> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v5 1/8] net/rte_ether: deinline non-critical functions 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Stephen, On Mon, Jun 24, 2019 at 01:44:28PM -0700, Stephen Hemminger wrote: > Formatting Ethernet address and getting a random value are > not in critical path so they should not be inlined. > > Signed-off-by: Stephen Hemminger > Acked-by: Rami Rosen > Reviewed-by: Andrew Rybchenko > --- > lib/librte_net/Makefile | 1 + > lib/librte_net/meson.build | 2 +- > lib/librte_net/rte_ether.c | 29 +++++++++++++++++++++++++++++ > lib/librte_net/rte_ether.h | 25 ++++--------------------- > lib/librte_net/rte_net_version.map | 7 +++++++ > 5 files changed, 42 insertions(+), 22 deletions(-) > create mode 100644 lib/librte_net/rte_ether.c > > diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile > index c3082069ab50..1244c9fd54c9 100644 > --- a/lib/librte_net/Makefile > +++ b/lib/librte_net/Makefile > @@ -14,6 +14,7 @@ LIBABIVER := 1 > > SRCS-$(CONFIG_RTE_LIBRTE_NET) := rte_net.c > SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_net_crc.c > +SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_ether.c > SRCS-$(CONFIG_RTE_LIBRTE_NET) += rte_arp.c > > # install includes > diff --git a/lib/librte_net/meson.build b/lib/librte_net/meson.build > index 7d66f693cbf3..868a93fd6b6b 100644 > --- a/lib/librte_net/meson.build > +++ b/lib/librte_net/meson.build > @@ -16,5 +16,5 @@ headers = files('rte_ip.h', > 'rte_net_crc.h', > 'rte_mpls.h') > > -sources = files('rte_arp.c', 'rte_net.c', 'rte_net_crc.c') > +sources = files('rte_arp.c', 'rte_ether.c', 'rte_net.c', 'rte_net_crc.c') > deps += ['mbuf'] > diff --git a/lib/librte_net/rte_ether.c b/lib/librte_net/rte_ether.c > new file mode 100644 > index 000000000000..974fe815b335 > --- /dev/null > +++ b/lib/librte_net/rte_ether.c > @@ -0,0 +1,29 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2010-2014 Intel Corporation > + */ > + > +#include > + > +void > +rte_eth_random_addr(uint8_t *addr) > +{ > + uint64_t rand = rte_rand(); > + uint8_t *p = (uint8_t *)&rand; > + > + rte_memcpy(addr, p, RTE_ETHER_ADDR_LEN); > + addr[0] &= (uint8_t)~RTE_ETHER_GROUP_ADDR; /* clear multicast bit */ > + addr[0] |= RTE_ETHER_LOCAL_ADMIN_ADDR; /* set local assignment bit */ > +} > + > +void > +rte_ether_format_addr(char *buf, uint16_t size, > + const struct rte_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]); > +} > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h > index 7be9b4890af7..3caae0d98f6d 100644 > --- a/lib/librte_net/rte_ether.h > +++ b/lib/librte_net/rte_ether.h > @@ -207,15 +207,8 @@ static inline int rte_is_valid_assigned_ether_addr(const struct rte_ether_addr * > * @param addr > * A pointer to Ethernet address. > */ > -static inline void rte_eth_random_addr(uint8_t *addr) > -{ > - uint64_t rand = rte_rand(); > - uint8_t *p = (uint8_t *)&rand; > - > - rte_memcpy(addr, p, RTE_ETHER_ADDR_LEN); > - addr[0] &= (uint8_t)~RTE_ETHER_GROUP_ADDR; /* clear multicast bit */ > - addr[0] |= RTE_ETHER_LOCAL_ADMIN_ADDR; /* set local assignment bit */ > -} > +void > +rte_eth_random_addr(uint8_t *addr); > > /** > * Fast copy an Ethernet address. > @@ -254,19 +247,9 @@ static inline void rte_ether_addr_copy(const struct rte_ether_addr *ea_from, > * @param eth_addr > * A pointer to a ether_addr structure. > */ > -static inline void > +void > rte_ether_format_addr(char *buf, uint16_t size, > - const struct rte_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]); > -} > - > + const struct rte_ether_addr *eth_addr); > /** > * Ethernet header: Contains the destination address, source address > * and frame type. > diff --git a/lib/librte_net/rte_net_version.map b/lib/librte_net/rte_net_version.map > index 26c06e7c7ae7..f1e1b84ab491 100644 > --- a/lib/librte_net/rte_net_version.map > +++ b/lib/librte_net/rte_net_version.map > @@ -13,6 +13,13 @@ DPDK_17.05 { > > } DPDK_16.11; > > +DPDK_19.08 { > + global: > + > + rte_eth_random_addr; > + rte_ether_format_addr; > +} DPDK_17.05; > + > EXPERIMENTAL { > global: > > -- > 2.20.1 > In drivers/net/memif and drivers/net/axgbe, rte_ether_format_addr() is used, but -lrte_net is not passed to LDLIBS in the Makefile. This was probably ok before because it was an inline function, but I wonder if it shouldn't be added in this patch.