From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-f68.google.com (mail-ua1-f68.google.com [209.85.222.68]) by dpdk.org (Postfix) with ESMTP id 47C442F7D for ; Thu, 16 May 2019 09:10:33 +0200 (CEST) Received: by mail-ua1-f68.google.com with SMTP id e9so885288uar.9 for ; Thu, 16 May 2019 00:10:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8GOUBs2CplkxqFJb/qNArEiqZLwNNR3zO0vMXEE2K4E=; b=I4w5vDB/kYYV/Bfp1X0CrBfBhyJn7YLwK3fhHf07v4nV+jO0f5hY8q9WCfOVD0m82e DrFlAA593J4bJ+4ziku8+Q+kBhKs8dnBJCOEF4KfkkOI9S00R2SVu/ssUofCt6w90hKA dkiUw6TZHB3Bu6Y9raEVFMhRsaXXlJ7Ospduf4pQ6GFJ7c33hjxJ4n5PEB4mHbnN1f0/ M7NObF1JfzHC7MR76H6/xK44nEsD9gObNjhlJNdqz0FBZBwIwmeh3fumYg1ynBSg0gYb giYCsEi0mSpnX3F7YHNlIZnvDlcumfTyIznObktL5LOUNsGpt/0bo9XDemSkLNCIuR6+ M51Q== X-Gm-Message-State: APjAAAXAspP7ggHlbF87AmGTeROY+cVOrCbopvhH00Wv/4ZsJSScjvY3 uvJsfkYnGzcKc3t4s91HzI9MCScUMnL8qejUalzZnXwx X-Google-Smtp-Source: APXvYqyyruQCiJZNaL6515Ja5fNAIpx6TjNmRs+CcD3tOE8u1UrNBxMiPVEMeACjxtiOVFucVn0oshWYuwxs4c1nbbg= X-Received: by 2002:ab0:5930:: with SMTP id n45mr10779450uad.87.1557990632550; Thu, 16 May 2019 00:10:32 -0700 (PDT) MIME-Version: 1.0 References: <20190515221952.21959-1-stephen@networkplumber.org> <20190515221952.21959-2-stephen@networkplumber.org> In-Reply-To: <20190515221952.21959-2-stephen@networkplumber.org> From: David Marchand Date: Thu, 16 May 2019 09:10:21 +0200 Message-ID: To: Stephen Hemminger Cc: dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC 1/4] net/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: , X-List-Received-Date: Thu, 16 May 2019 07:10:33 -0000 On Thu, May 16, 2019 at 12:20 AM Stephen Hemminger < stephen@networkplumber.org> 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 > --- > lib/librte_net/Makefile | 1 + > lib/librte_net/rte_ether.c | 29 +++++++++++++++++++++++++++++ > lib/librte_net/rte_ether.h | 24 ++++-------------------- > lib/librte_net/rte_net_version.map | 8 ++++++++ > 4 files changed, 42 insertions(+), 20 deletions(-) > create mode 100644 lib/librte_net/rte_ether.c > > diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile > index c3082069ab50..f68b42cd0e8f 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 > Missing the meson counterpart. diff --git a/lib/librte_net/rte_ether.c b/lib/librte_net/rte_ether.c > new file mode 100644 > index 000000000000..d4b41f122a16 > --- /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 > +eth_random_addr(uint8_t *addr) > +{ > + uint64_t rand = rte_rand(); > + uint8_t *p = (uint8_t *)&rand; > + > + rte_memcpy(addr, p, ETHER_ADDR_LEN); > + addr[0] &= (uint8_t)~ETHER_GROUP_ADDR; /* clear multicast bit */ > + addr[0] |= ETHER_LOCAL_ADMIN_ADDR; /* set local assignment > bit */ > +} > + > +void > +ether_format_addr(char *buf, uint16_t size, > + 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]); > +} > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h > index 3a87ff184900..46d40412763c 100644 > --- a/lib/librte_net/rte_ether.h > +++ b/lib/librte_net/rte_ether.h > @@ -204,15 +204,8 @@ static inline int is_valid_assigned_ether_addr(const > struct ether_addr *ea) > * @param addr > * A pointer to Ethernet address. > */ > -static inline void eth_random_addr(uint8_t *addr) > -{ > - uint64_t rand = rte_rand(); > - uint8_t *p = (uint8_t *)&rand; > - > - rte_memcpy(addr, p, ETHER_ADDR_LEN); > - addr[0] &= (uint8_t)~ETHER_GROUP_ADDR; /* clear multicast > bit */ > - addr[0] |= ETHER_LOCAL_ADMIN_ADDR; /* set local assignment bit */ > -} > +void > +eth_random_addr(uint8_t *addr); > > /** > * Fast copy an Ethernet address. > @@ -251,18 +244,9 @@ static inline void ether_addr_copy(const struct > ether_addr *ea_from, > * @param eth_addr > * A pointer to a ether_addr structure. > */ > -static inline void > +void > ether_format_addr(char *buf, uint16_t size, > - 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]); > -} > + const struct ether_addr *eth_addr); > > /** > * Ethernet header: Contains the destination address, source address > diff --git a/lib/librte_net/rte_net_version.map > b/lib/librte_net/rte_net_version.map > index 26c06e7c7ae7..49d34093781c 100644 > --- a/lib/librte_net/rte_net_version.map > +++ b/lib/librte_net/rte_net_version.map > @@ -13,6 +13,14 @@ DPDK_17.05 { > > } DPDK_16.11; > > +DPDK_19.08 { > + global: > + > + eth_random_addr; > + eth_format_addr; > +} DPDK_17.05; > A bit sad to introduce unprefixed symbols in ABI. These were inlined before, so the existing users already have their own copy of this code embedded. Can't we go and add rte_eth_(random/format)_addr from the start ? + > + > (nit: one empty line is enough.) EXPERIMENTAL { > global: > > -- > 2.20.1 > > -- David Marchand From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id D9CE4A00E6 for ; Thu, 16 May 2019 09:10:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B54AD397D; Thu, 16 May 2019 09:10:33 +0200 (CEST) Received: from mail-ua1-f68.google.com (mail-ua1-f68.google.com [209.85.222.68]) by dpdk.org (Postfix) with ESMTP id 47C442F7D for ; Thu, 16 May 2019 09:10:33 +0200 (CEST) Received: by mail-ua1-f68.google.com with SMTP id e9so885288uar.9 for ; Thu, 16 May 2019 00:10:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8GOUBs2CplkxqFJb/qNArEiqZLwNNR3zO0vMXEE2K4E=; b=I4w5vDB/kYYV/Bfp1X0CrBfBhyJn7YLwK3fhHf07v4nV+jO0f5hY8q9WCfOVD0m82e DrFlAA593J4bJ+4ziku8+Q+kBhKs8dnBJCOEF4KfkkOI9S00R2SVu/ssUofCt6w90hKA dkiUw6TZHB3Bu6Y9raEVFMhRsaXXlJ7Ospduf4pQ6GFJ7c33hjxJ4n5PEB4mHbnN1f0/ M7NObF1JfzHC7MR76H6/xK44nEsD9gObNjhlJNdqz0FBZBwIwmeh3fumYg1ynBSg0gYb giYCsEi0mSpnX3F7YHNlIZnvDlcumfTyIznObktL5LOUNsGpt/0bo9XDemSkLNCIuR6+ M51Q== X-Gm-Message-State: APjAAAXAspP7ggHlbF87AmGTeROY+cVOrCbopvhH00Wv/4ZsJSScjvY3 uvJsfkYnGzcKc3t4s91HzI9MCScUMnL8qejUalzZnXwx X-Google-Smtp-Source: APXvYqyyruQCiJZNaL6515Ja5fNAIpx6TjNmRs+CcD3tOE8u1UrNBxMiPVEMeACjxtiOVFucVn0oshWYuwxs4c1nbbg= X-Received: by 2002:ab0:5930:: with SMTP id n45mr10779450uad.87.1557990632550; Thu, 16 May 2019 00:10:32 -0700 (PDT) MIME-Version: 1.0 References: <20190515221952.21959-1-stephen@networkplumber.org> <20190515221952.21959-2-stephen@networkplumber.org> In-Reply-To: <20190515221952.21959-2-stephen@networkplumber.org> From: David Marchand Date: Thu, 16 May 2019 09:10:21 +0200 Message-ID: To: Stephen Hemminger Cc: dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [RFC 1/4] net/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" Message-ID: <20190516071021._e-5bV9fC63Mbou1Sok0r1wBsESLSTAWPYZDGcZfPwY@z> On Thu, May 16, 2019 at 12:20 AM Stephen Hemminger < stephen@networkplumber.org> 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 > --- > lib/librte_net/Makefile | 1 + > lib/librte_net/rte_ether.c | 29 +++++++++++++++++++++++++++++ > lib/librte_net/rte_ether.h | 24 ++++-------------------- > lib/librte_net/rte_net_version.map | 8 ++++++++ > 4 files changed, 42 insertions(+), 20 deletions(-) > create mode 100644 lib/librte_net/rte_ether.c > > diff --git a/lib/librte_net/Makefile b/lib/librte_net/Makefile > index c3082069ab50..f68b42cd0e8f 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 > Missing the meson counterpart. diff --git a/lib/librte_net/rte_ether.c b/lib/librte_net/rte_ether.c > new file mode 100644 > index 000000000000..d4b41f122a16 > --- /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 > +eth_random_addr(uint8_t *addr) > +{ > + uint64_t rand = rte_rand(); > + uint8_t *p = (uint8_t *)&rand; > + > + rte_memcpy(addr, p, ETHER_ADDR_LEN); > + addr[0] &= (uint8_t)~ETHER_GROUP_ADDR; /* clear multicast bit */ > + addr[0] |= ETHER_LOCAL_ADMIN_ADDR; /* set local assignment > bit */ > +} > + > +void > +ether_format_addr(char *buf, uint16_t size, > + 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]); > +} > diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h > index 3a87ff184900..46d40412763c 100644 > --- a/lib/librte_net/rte_ether.h > +++ b/lib/librte_net/rte_ether.h > @@ -204,15 +204,8 @@ static inline int is_valid_assigned_ether_addr(const > struct ether_addr *ea) > * @param addr > * A pointer to Ethernet address. > */ > -static inline void eth_random_addr(uint8_t *addr) > -{ > - uint64_t rand = rte_rand(); > - uint8_t *p = (uint8_t *)&rand; > - > - rte_memcpy(addr, p, ETHER_ADDR_LEN); > - addr[0] &= (uint8_t)~ETHER_GROUP_ADDR; /* clear multicast > bit */ > - addr[0] |= ETHER_LOCAL_ADMIN_ADDR; /* set local assignment bit */ > -} > +void > +eth_random_addr(uint8_t *addr); > > /** > * Fast copy an Ethernet address. > @@ -251,18 +244,9 @@ static inline void ether_addr_copy(const struct > ether_addr *ea_from, > * @param eth_addr > * A pointer to a ether_addr structure. > */ > -static inline void > +void > ether_format_addr(char *buf, uint16_t size, > - 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]); > -} > + const struct ether_addr *eth_addr); > > /** > * Ethernet header: Contains the destination address, source address > diff --git a/lib/librte_net/rte_net_version.map > b/lib/librte_net/rte_net_version.map > index 26c06e7c7ae7..49d34093781c 100644 > --- a/lib/librte_net/rte_net_version.map > +++ b/lib/librte_net/rte_net_version.map > @@ -13,6 +13,14 @@ DPDK_17.05 { > > } DPDK_16.11; > > +DPDK_19.08 { > + global: > + > + eth_random_addr; > + eth_format_addr; > +} DPDK_17.05; > A bit sad to introduce unprefixed symbols in ABI. These were inlined before, so the existing users already have their own copy of this code embedded. Can't we go and add rte_eth_(random/format)_addr from the start ? + > + > (nit: one empty line is enough.) EXPERIMENTAL { > global: > > -- > 2.20.1 > > -- David Marchand