DPDK patches and discussions
 help / color / mirror / Atom feed
* Portable alternative to inet_ntop?
@ 2024-07-24 16:21 Stephen Hemminger
  2024-07-26 12:36 ` Morten Brørup
  2024-08-06  5:35 ` Tyler Retzlaff
  0 siblings, 2 replies; 4+ messages in thread
From: Stephen Hemminger @ 2024-07-24 16:21 UTC (permalink / raw)
  To: dev, Tyler Retzlaff

The function inet_ntop is useful to make printable addresses for debugging.
It is available on Linux and FreeBSD but not on Windows.

There are some alternatives:
  - add yet another OS shim in lib/eal/windows/include.
    Win32 has similar InetNtoP but it uses wide characters.

  - copy/paste code from FreeBSD into some new functions.

Hate duplicating code, but portability is a problem here.

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;
 }
 
+
+#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);
+
+__rte_experimental
+void
+rte_ipv4_unformat_addr(const char *str, void *addr);
+
+__rte_experimental
+void
+rte_ipv6_format_addr(char *buf, uint16_t size, const void *addr);
+
+__rte_experimental
+void
+rte_ipv4_unformat_addr(const char *str, void *addr);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: Portable alternative to inet_ntop?
  2024-07-24 16:21 Portable alternative to inet_ntop? Stephen Hemminger
@ 2024-07-26 12:36 ` Morten Brørup
  2024-08-06  5:35 ` Tyler Retzlaff
  1 sibling, 0 replies; 4+ messages in thread
From: Morten Brørup @ 2024-07-26 12:36 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Tyler Retzlaff

> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, 24 July 2024 18.21
> 
> The function inet_ntop is useful to make printable addresses for
> debugging.
> It is available on Linux and FreeBSD but not on Windows.
> 
> There are some alternatives:
>   - add yet another OS shim in lib/eal/windows/include.
>     Win32 has similar InetNtoP but it uses wide characters.
> 
>   - copy/paste code from FreeBSD into some new functions.
> 
> Hate duplicating code, but portability is a problem here.

+1 for "duplicating code" in this case.

> 
> 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;
>  }
> 
> +
> +#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#L264

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Portable alternative to inet_ntop?
  2024-07-24 16:21 Portable alternative to inet_ntop? Stephen Hemminger
  2024-07-26 12:36 ` Morten Brørup
@ 2024-08-06  5:35 ` Tyler Retzlaff
  2024-08-07 22:23   ` Stephen Hemminger
  1 sibling, 1 reply; 4+ messages in thread
From: Tyler Retzlaff @ 2024-08-06  5:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Wed, Jul 24, 2024 at 09:21:13AM -0700, Stephen Hemminger wrote:
> The function inet_ntop is useful to make printable addresses for debugging.
> It is available on Linux and FreeBSD but not on Windows.

i know my reply is late. but the following wasn't usable?
https://learn.microsoft.com/en-us/windows/win32/api/ws2tcpip/nf-ws2tcpip-inet_ntop
it isn't wide (PSTR).

i know trying to use winsock headers is a mess though so if it is hard
duplicating seems fine. i would not be unhappy for more generalized apis
to be built up within dpdk to avoid the platform coupling.

ty

> 
> There are some alternatives:
>   - add yet another OS shim in lib/eal/windows/include.
>     Win32 has similar InetNtoP but it uses wide characters.
> 
>   - copy/paste code from FreeBSD into some new functions.
> 
> Hate duplicating code, but portability is a problem here.
> 
> 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;
>  }
>  
> +
> +#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);
> +
> +__rte_experimental
> +void
> +rte_ipv4_unformat_addr(const char *str, void *addr);
> +
> +__rte_experimental
> +void
> +rte_ipv6_format_addr(char *buf, uint16_t size, const void *addr);
> +
> +__rte_experimental
> +void
> +rte_ipv4_unformat_addr(const char *str, void *addr);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Portable alternative to inet_ntop?
  2024-08-06  5:35 ` Tyler Retzlaff
@ 2024-08-07 22:23   ` Stephen Hemminger
  0 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2024-08-07 22:23 UTC (permalink / raw)
  To: Tyler Retzlaff; +Cc: dev

On Mon, 5 Aug 2024 22:35:27 -0700
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:

> On Wed, Jul 24, 2024 at 09:21:13AM -0700, Stephen Hemminger wrote:
> > The function inet_ntop is useful to make printable addresses for debugging.
> > It is available on Linux and FreeBSD but not on Windows.  
> 
> i know my reply is late. but the following wasn't usable?
> https://learn.microsoft.com/en-us/windows/win32/api/ws2tcpip/nf-ws2tcpip-inet_ntop
> it isn't wide (PSTR).
> 
> i know trying to use winsock headers is a mess though so if it is hard
> duplicating seems fine. i would not be unhappy for more generalized apis
> to be built up within dpdk to avoid the platform coupling.
> 
> ty

I got inet_ntop to work on windows (as long as the right includes are used).

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-08-07 22:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-24 16:21 Portable alternative to inet_ntop? Stephen Hemminger
2024-07-26 12:36 ` Morten Brørup
2024-08-06  5:35 ` Tyler Retzlaff
2024-08-07 22:23   ` Stephen Hemminger

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).