DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev <dev@dpdk.org>
Subject: Re: [dpdk-dev] [RFC 1/4] net/ether: deinline non-critical functions
Date: Thu, 16 May 2019 09:10:21 +0200	[thread overview]
Message-ID: <CAJFAV8zmYZmVMQE8CfNx0xiYFQCL3BG_jEwUqY9i5ojfMeiYcA@mail.gmail.com> (raw)
Message-ID: <20190516071021._e-5bV9fC63Mbou1Sok0r1wBsESLSTAWPYZDGcZfPwY@z> (raw)
In-Reply-To: <20190515221952.21959-2-stephen@networkplumber.org>

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 <stephen@networkplumber.org>
> ---
>  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 <rte_ether.h>
> +
> +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

  parent reply	other threads:[~2019-05-16  7:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-15 22:19 [dpdk-dev] [RFC 0/4] net/ether: improvements Stephen Hemminger
2019-05-15 22:19 ` Stephen Hemminger
2019-05-15 22:19 ` [dpdk-dev] [RFC 1/4] net/ether: deinline non-critical functions Stephen Hemminger
2019-05-15 22:19   ` Stephen Hemminger
2019-05-16  7:10   ` David Marchand [this message]
2019-05-16  7:10     ` David Marchand
2019-05-15 22:19 ` [dpdk-dev] [RFC 2/4] net/ether: add eth_unformat_addr Stephen Hemminger
2019-05-15 22:19   ` Stephen Hemminger
2019-05-16  7:28   ` David Marchand
2019-05-16  7:28     ` David Marchand
2019-05-15 22:19 ` [dpdk-dev] [RFC 3/4] ethdev: use eth_unformat_addr Stephen Hemminger
2019-05-15 22:19   ` Stephen Hemminger
2019-05-16  7:32   ` David Marchand
2019-05-16  7:32     ` David Marchand
2019-05-16 10:19   ` Bruce Richardson
2019-05-16 10:19     ` Bruce Richardson
2019-05-15 22:19 ` [dpdk-dev] [RFC 4/4] net/ether: use bitops to speedup comparison Stephen Hemminger
2019-05-15 22:19   ` Stephen Hemminger
2019-05-16  9:03   ` Mattias Rönnblom
2019-05-16  9:03     ` Mattias Rönnblom
2019-05-16 15:32     ` Stephen Hemminger
2019-05-16 16:03   ` Bruce Richardson
2019-05-16 16:06     ` Stephen Hemminger
2019-05-16 16:07       ` Bruce Richardson
2019-05-16 16:36         ` Bruce Richardson
2019-05-16 17:04           ` Stephen Hemminger
2019-05-16 20:37             ` Bruce Richardson
2019-05-16 20:41               ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJFAV8zmYZmVMQE8CfNx0xiYFQCL3BG_jEwUqY9i5ojfMeiYcA@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).