From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2CFC74565E; Fri, 26 Jul 2024 14:36:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id A8A90427A1; Fri, 26 Jul 2024 14:36:41 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 85A3A40DFD for ; Fri, 26 Jul 2024 14:36:40 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 666D62091E; Fri, 26 Jul 2024 14:36:39 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: Portable alternative to inet_ntop? Date: Fri, 26 Jul 2024 14:36:36 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9F5B5@smartserver.smartshare.dk> X-MimeOLE: Produced By Microsoft Exchange V6.5 In-Reply-To: <20240724092113.4977b047@hermes.local> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: Portable alternative to inet_ntop? Thread-Index: Adrd5YWBiXM1O0W5T+yT0uznJ+hZqgBaawWQ References: <20240724092113.4977b047@hermes.local> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Stephen Hemminger" Cc: , "Tyler Retzlaff" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Wednesday, 24 July 2024 18.21 >=20 > The function inet_ntop is useful to make printable addresses for > debugging. > It is available on Linux and FreeBSD but not on Windows. >=20 > There are some alternatives: > - add yet another OS shim in lib/eal/windows/include. > Win32 has similar InetNtoP but it uses wide characters. >=20 > - copy/paste code from FreeBSD into some new functions. >=20 > Hate duplicating code, but portability is a problem here. +1 for "duplicating code" in this case. >=20 > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h > index 0d103d4127..a9404b4b41 100644 > --- a/lib/net/rte_ip.h > +++ b/lib/net/rte_ip.h > @@ -839,6 +839,27 @@ rte_ipv6_get_next_ext(const uint8_t *p, int = proto, > size_t *ext_len) > return next_proto; > } >=20 > + > +#define RTE_IPV4_ADDR_FMT_SIZE 16 > +#define RTE_IPV6_ADDR_FMT_SIZE 46 > + > +__rte_experimental > +int > +rte_ipv4_format_addr(char *buf, uint16_t size, const void *addr); This resembles rte_ether_format_addr() [1], so I agree to passing the = address by reference instead of by value. [1]: = https://elixir.bootlin.com/dpdk/v24.07-rc3/source/lib/net/rte_ether.h#L26= 4 I consider rte_be32_t the "official" type for an IPv4 address in network = byte order. This is the type used in the IPv4 header [2]. [2]: = https://elixir.bootlin.com/dpdk/v24.07-rc3/source/lib/net/rte_ip.h#L41 With this in mind, please change the address parameter's type from = "const void *" to "const rte_be32_t *". I speculate that you used "void *" to be compatible with both the types = unsigned char[4] and rte_be32_t, and avoid alignment issues with the = latter. I fear this could set a very bad precedent; using "void *" instead of = the proper type would make APIs difficult to read, because the actual = types are omitted. We don't want to start using "void *" for API = parameters to avoid type casting. (C++ would allow providing the same function with a variety of parameter = types, but we are stuck with C.) > + > +__rte_experimental > +void > +rte_ipv4_unformat_addr(const char *str, void *addr); Same as above; please change output type from void* to rte_be32_t*. > + > +__rte_experimental > +void > +rte_ipv6_format_addr(char *buf, uint16_t size, const void *addr); I suppose this outputs the IPv6 address string in packed format (using = "::" if possible), as I suppose the IPv4 address string is output = without leading zeroes (%u, not %03u). Alternatively, consider adding a formatting flags parameter to specify = the output format (leading zeroes or not, and "::" or not). Same as above; addr parameter should be "const uint8_t addr[16]", = reflecting the "official" IPv6 address type. This will be updated to = "const struct rte_ipv6_addr *addr" with Robin's coming 24.11 patch = series. > + > +__rte_experimental > +void > +rte_ipv4_unformat_addr(const char *str, void *addr); Same as above; addr parameter type should reflect the "official" IPv6 = address type. And a typo in the function name: ipv4 -> ipv6