RE: [PATCH 02/12] net: add function to pretty print IPv4

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, January 19, 2022 2:24 PM
> To: Morten Brørup <mb@smartsharesystems.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Randles, Ronan <ronan.randles@intel.com>;
> Van Haaren, Harry <harry.van.haaren@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH 02/12] net: add function to pretty print IPv4
> 
> 15/12/2021 14:06, Ananyev, Konstantin:
> >
> > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > > > Sent: Wednesday, 15 December 2021 04.21
> > > >
> > > > On Wed, 15 Dec 2021 01:06:14 +0000
> > > > "Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:
> > > > > > > --- a/lib/net/rte_ip.h
> > > > > > > +++ b/lib/net/rte_ip.h
> > > > > > > @@ -444,6 +444,26 @@ __rte_experimental
> > > > > > >  int32_t
> > > > > > >  rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
> > > > > > >
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * Print IP address from 32 bit int into char * buffer.
> > > > > > > + *
> > > > > > > + * @param ip_addr
> > > > > > > + *   ip address to be printed.
> > > > > > > + * @param buffer
> > > > > > > + *   The buffer the string will be saved into.
> > > > > > > + * @param buffer_size
> > > > > > > + *   size of buffer to be used.
> > > > > > > + *
> > > > > > > + * @retval 0
> > > > > > > + *   Success.
> > > > > > > + * @retval -1
> > > > > > > + *   Failure due to invalid input arguments.
> > > > > > > + */
> > > > > > > +__rte_experimental
> > > > > > > +int32_t
> > > > > > > +rte_ip_print_addr(uint32_t ip_addr, char *buffer, uint32_t
> > > > > > > buffer_size);
> > > > > > > +
> > > > > >
> > > > > > In continuation of my email reply about the IPv4 parse function...
> > > > > >
> > > > > > I have a few suggestions to the IPv4 print function too:
> > > > > >
> > > > > > The return value should be the number of characters written to the
> > > > output string, and still -1 on error. With this modification, you could
> > > > > > use the return type ssize_t instead of int32_t.
> > > > > >
> > > > > > Furthermore, I would prefer having the parameters in the same order
> > > > as snprintf(): char *str, size_t size, const uint32_t ip_addr. Please
> > > > > > also notice the suggested changed type for the size, and the const
> > > > added to the ip_addr.
> > > > > >
> > > > > Honestly, I don't understand why we need to introduce such functions
> > > > > inside DPDK at all.
> > > > > What's wrong with existing standard ones: inet_ntop() and
> > > > inet_pton()?
> > > >
> > > > Agreed, I see no added value in reinventing here
> > >
> > > I think that DPDK functions for converting all sorts of types to/from strings
> would be useful; not only IP addresses, but also MAC addresses,
> > > TCP/UDP port numbers and VLAN IDs.
> >
> > For MACs we already have:
> > rte_ether_format_addr()/rte_ether_unformat_addr()
> >
> > >
> > > If you don't like IP address string conversion functions in the net library, DPDK
> could have a string conversions library. That library could
> > > expose a multitude of APIs for the same purpose, so the application can use
> the API that best fits each application use.
> >
> > I don’t mind to add new functions into net lib, if they are useful ones.
> > But for that particular case, I just don't see what is the reason to
> > develop and maintain our own functions while existing analogues:
> > - are well known, widely adopted and field proven
> > - do provide the same or even more comprehensive functionality
> 
> +1
> Waiting for an answer from the authors. One month silence so far.

Hi All,

Ronan and I are working on a V2 patchset, and hope to share it in the next days.

Personally I think there is value in DPDK having various functionality "built in", and
if that functionality is available for e.g. Ether, I see no reason why IPv4 or other
protocols shouldn't have that functionality available. If I was a newcomer to DPDK,
I would find it difficult to understand why I can format/unformat Ether with DPDK,
but had to use Linux/Windows functions to format/unformat IP/UDP.

I liked Morten's feedback around real-world usage, however we have not got around
to implementing that functionality in the V2 patchset. We intended to propose that
change in signature/extra-functionality being added to V3.

I believe the gen library was discussed briefly at last tech-board call (unfortunately I was not
available to participate). Perhaps a quick chat/discussion at the next tech-board would be helpful
to identify community stance on the library? At that point the V2 should be available too.

Regards, -Harry