[01/12] net: add string to IPv4 parse function
Checks
Commit Message
Added function that accepts ip string as a parameter and returns an ip
address represented by a uint32_t. Relevant unit test for this function
is also included.
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
Signed-off-by: Ronan Randles <ronan.randles@intel.com>
---
app/test/meson.build | 2 ++
app/test/test_net.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
lib/net/meson.build | 1 +
lib/net/rte_ip.c | 43 +++++++++++++++++++++++++++++++
lib/net/rte_ip.h | 18 +++++++++++++
lib/net/version.map | 8 ++++++
6 files changed, 133 insertions(+)
create mode 100644 app/test/test_net.c
create mode 100644 lib/net/rte_ip.c
Comments
> From: Ronan Randles [mailto:ronan.randles@intel.com]
> Sent: Tuesday, 14 December 2021 15.13
>
> Added function that accepts ip string as a parameter and returns an ip
> address represented by a uint32_t. Relevant unit test for this function
> is also included.
>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> Signed-off-by: Ronan Randles <ronan.randles@intel.com>
> ---
[snip]
> diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> index c575250852..188054fda4 100644
> --- a/lib/net/rte_ip.h
> +++ b/lib/net/rte_ip.h
> @@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct
> rte_ipv4_hdr *ipv4_hdr,
> return 0;
> }
>
> +/**
> + * IP address parser.
> + *
> + * @param src_ip
> + * The IP address to be parsed.
> + * @param output_addr
> + * The array in which the parsed digits will be saved.
> + *
> + * @retval 0
> + * Success.
> + * @retval -1
> + * Failure due to invalid input arguments.
> + */
> +
> +__rte_experimental
> +int32_t
> +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
> +
Good initiative!
This should set a precedent for to/from string functions, so be careful about names and calling conventions.
I have a few suggestions:
The function should take a parameter to tell if the input string must be zero-terminated or not. This is highly useful for parsing subnet strings (e.g. "192.0.2.0/24") and IP range strings (e.g. "192.0.2.2-192.0.2.253").
The return value should be the number of characters read from the input string, and still -1 on error. With this modification, also consider using the return type ssize_t instead of int32_t.
-Morten
On Tue, Dec 14, 2021 at 06:31:06PM +0100, Morten Brørup wrote:
> > From: Ronan Randles [mailto:ronan.randles@intel.com]
> > Sent: Tuesday, 14 December 2021 15.13
> >
> > Added function that accepts ip string as a parameter and returns an ip
> > address represented by a uint32_t. Relevant unit test for this function
> > is also included.
> >
> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > Signed-off-by: Ronan Randles <ronan.randles@intel.com>
> > ---
>
> [snip]
>
> > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> > index c575250852..188054fda4 100644
> > --- a/lib/net/rte_ip.h
> > +++ b/lib/net/rte_ip.h
> > @@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct
> > rte_ipv4_hdr *ipv4_hdr,
> > return 0;
> > }
> >
> > +/**
> > + * IP address parser.
> > + *
> > + * @param src_ip
> > + * The IP address to be parsed.
> > + * @param output_addr
> > + * The array in which the parsed digits will be saved.
> > + *
> > + * @retval 0
> > + * Success.
> > + * @retval -1
> > + * Failure due to invalid input arguments.
> > + */
> > +
> > +__rte_experimental
> > +int32_t
> > +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
> > +
>
> Good initiative!
>
> This should set a precedent for to/from string functions, so be careful about names and calling conventions.
>
> I have a few suggestions:
>
> The function should take a parameter to tell if the input string must be zero-terminated or not. This is highly useful for parsing subnet strings (e.g. "192.0.2.0/24") and IP range strings (e.g. "192.0.2.2-192.0.2.253").
>
> The return value should be the number of characters read from the input string, and still -1 on error. With this modification, also consider using the return type ssize_t instead of int32_t.
>
Interesting point on the "must be zero-terminated" parameter. However, if
the return value is the number of characters read, then the user can check
for null-termination very easily after the call, and not need the
parameter. Therefore, I would suggest having the function always assume
chars after the address and let the user check for null if needed
afterwards, to keep function simpler.
/Bruce
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, 15 December 2021 10.27
>
> On Tue, Dec 14, 2021 at 06:31:06PM +0100, Morten Brørup wrote:
> > > From: Ronan Randles [mailto:ronan.randles@intel.com]
> > > Sent: Tuesday, 14 December 2021 15.13
> > >
> > > Added function that accepts ip string as a parameter and returns an
> ip
> > > address represented by a uint32_t. Relevant unit test for this
> function
> > > is also included.
> > >
> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > Signed-off-by: Ronan Randles <ronan.randles@intel.com>
> > > ---
> >
> > [snip]
> >
> > > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> > > index c575250852..188054fda4 100644
> > > --- a/lib/net/rte_ip.h
> > > +++ b/lib/net/rte_ip.h
> > > @@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct
> > > rte_ipv4_hdr *ipv4_hdr,
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * IP address parser.
> > > + *
> > > + * @param src_ip
> > > + * The IP address to be parsed.
> > > + * @param output_addr
> > > + * The array in which the parsed digits will be saved.
> > > + *
> > > + * @retval 0
> > > + * Success.
> > > + * @retval -1
> > > + * Failure due to invalid input arguments.
> > > + */
> > > +
> > > +__rte_experimental
> > > +int32_t
> > > +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
> > > +
> >
> > Good initiative!
> >
> > This should set a precedent for to/from string functions, so be
> careful about names and calling conventions.
> >
> > I have a few suggestions:
> >
> > The function should take a parameter to tell if the input string must
> be zero-terminated or not. This is highly useful for parsing subnet
> strings (e.g. "192.0.2.0/24") and IP range strings (e.g. "192.0.2.2-
> 192.0.2.253").
> >
> > The return value should be the number of characters read from the
> input string, and still -1 on error. With this modification, also
> consider using the return type ssize_t instead of int32_t.
> >
> Interesting point on the "must be zero-terminated" parameter. However,
> if
> the return value is the number of characters read, then the user can
> check
> for null-termination very easily after the call, and not need the
> parameter. Therefore, I would suggest having the function always assume
> chars after the address and let the user check for null if needed
> afterwards, to keep function simpler.
That would require additional lines of code in the application. I would rather have that additional code inside the utility function. There is no need to keep the function simple.
On Wed, Dec 15, 2021 at 10:35:44AM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Wednesday, 15 December 2021 10.27
> >
> > On Tue, Dec 14, 2021 at 06:31:06PM +0100, Morten Brørup wrote:
> > > > From: Ronan Randles [mailto:ronan.randles@intel.com]
> > > > Sent: Tuesday, 14 December 2021 15.13
> > > >
> > > > Added function that accepts ip string as a parameter and returns an
> > ip
> > > > address represented by a uint32_t. Relevant unit test for this
> > function
> > > > is also included.
> > > >
> > > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > > Signed-off-by: Ronan Randles <ronan.randles@intel.com>
> > > > ---
> > >
> > > [snip]
> > >
> > > > diff --git a/lib/net/rte_ip.h b/lib/net/rte_ip.h
> > > > index c575250852..188054fda4 100644
> > > > --- a/lib/net/rte_ip.h
> > > > +++ b/lib/net/rte_ip.h
> > > > @@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct
> > > > rte_ipv4_hdr *ipv4_hdr,
> > > > return 0;
> > > > }
> > > >
> > > > +/**
> > > > + * IP address parser.
> > > > + *
> > > > + * @param src_ip
> > > > + * The IP address to be parsed.
> > > > + * @param output_addr
> > > > + * The array in which the parsed digits will be saved.
> > > > + *
> > > > + * @retval 0
> > > > + * Success.
> > > > + * @retval -1
> > > > + * Failure due to invalid input arguments.
> > > > + */
> > > > +
> > > > +__rte_experimental
> > > > +int32_t
> > > > +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
> > > > +
> > >
> > > Good initiative!
> > >
> > > This should set a precedent for to/from string functions, so be
> > careful about names and calling conventions.
> > >
> > > I have a few suggestions:
> > >
> > > The function should take a parameter to tell if the input string must
> > be zero-terminated or not. This is highly useful for parsing subnet
> > strings (e.g. "192.0.2.0/24") and IP range strings (e.g. "192.0.2.2-
> > 192.0.2.253").
> > >
> > > The return value should be the number of characters read from the
> > input string, and still -1 on error. With this modification, also
> > consider using the return type ssize_t instead of int32_t.
> > >
> > Interesting point on the "must be zero-terminated" parameter. However,
> > if
> > the return value is the number of characters read, then the user can
> > check
> > for null-termination very easily after the call, and not need the
> > parameter. Therefore, I would suggest having the function always assume
> > chars after the address and let the user check for null if needed
> > afterwards, to keep function simpler.
>
> That would require additional lines of code in the application. I would rather have that additional code inside the utility function. There is no need to keep the function simple.
>
Well, it's only an extra item in the error-check conditional, but point
taken. I have no strong opinion here.
14/12/2021 15:12, Ronan Randles:
> --- a/lib/net/rte_ip.h
> +++ b/lib/net/rte_ip.h
> +/**
> + * IP address parser.
> + *
> + * @param src_ip
> + * The IP address to be parsed.
> + * @param output_addr
> + * The array in which the parsed digits will be saved.
> + *
> + * @retval 0
> + * Success.
> + * @retval -1
> + * Failure due to invalid input arguments.
> + */
> +
> +__rte_experimental
> +int32_t
> +rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
Is it similar to inet_aton() ?
Does it support IPv6? If not, why not adding a number 4 in the name?
@@ -100,6 +100,7 @@ test_sources = files(
'test_meter.c',
'test_mcslock.c',
'test_mp_secondary.c',
+ 'test_net.c',
'test_per_lcore.c',
'test_pflock.c',
'test_pmd_perf.c',
@@ -177,6 +178,7 @@ test_deps = [
'ipsec',
'lpm',
'member',
+ 'net',
'node',
'pipeline',
'port',
new file mode 100644
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+
+#include <rte_ip.h>
+#include <rte_common.h>
+#include "test.h"
+
+static int
+test_rte_ip_parse_addr(void)
+{
+ printf("Running IP parsing tests...\n");
+
+ struct str_ip_t {
+ const char *str;
+ uint32_t exp_output;
+ uint32_t expected_to_fail;
+ } str_ip_tests[] = {
+ { .str = "1.2.3.4", .exp_output = RTE_IPV4(1, 2, 3, 4)},
+ { .str = "192.168.255.255", .exp_output =
+ RTE_IPV4(192, 168, 255, 255)},
+ { .str = "172.16.0.9", .exp_output =
+ RTE_IPV4(172, 16, 0, 9)},
+ { .str = "1.2.3", .expected_to_fail = 1},
+ { .str = "1.2.3.4.5", .expected_to_fail = 1},
+ { .str = "fail.1.2.3", .expected_to_fail = 1},
+ { .str = "", .expected_to_fail = 1},
+ { .str = "1.2.3.fail", .expected_to_fail = 1}
+ };
+
+ uint32_t i;
+ for (i = 0; i < RTE_DIM(str_ip_tests); i++) {
+ uint32_t test_addr;
+ int32_t err = rte_ip_parse_addr(str_ip_tests[i].str,
+ &test_addr);
+ if (!test_addr) {
+ if (str_ip_tests[i].expected_to_fail != 1)
+ return -1;
+ }
+
+ if (err || test_addr != str_ip_tests[i].exp_output) {
+ if (str_ip_tests[i].expected_to_fail != 1)
+ return -1;
+ }
+ }
+
+
+ return 0;
+}
+
+static int
+test_net_tests(void)
+{
+ int ret = test_rte_ip_parse_addr();
+ return ret;
+}
+
+REGISTER_TEST_COMMAND(net_autotest, test_net_tests);
@@ -26,6 +26,7 @@ headers = files(
sources = files(
'rte_arp.c',
'rte_ether.c',
+ 'rte_ip.c',
'rte_net.c',
'rte_net_crc.c',
)
new file mode 100644
@@ -0,0 +1,43 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2021 Intel Corporation
+ */
+
+#include <string.h>
+#include <rte_ip.h>
+
+int32_t
+rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr)
+{
+ int32_t ret = 0;
+ char *current_position;
+
+ if (src_ip == NULL)
+ return -1;
+
+ char *tok = strdup(src_ip);
+ if (tok == NULL)
+ return -1;
+
+ char *current_digit = strtok_r(tok, ".", ¤t_position);
+
+ *output_addr = 0;
+ uint32_t i = 0;
+ while (current_digit) {
+ uint32_t shift = ((3 - i) * 8);
+ unsigned long parsed_value = strtoul(current_digit, NULL, 0)
+ << shift;
+
+ if (parsed_value == 0 && strcmp(current_digit, "0"))
+ break;
+
+ *output_addr |= parsed_value;
+ current_digit = strtok_r(NULL, ".", ¤t_position);
+ i++;
+
+ }
+ if (i != 4)
+ return -1;
+
+ free(tok);
+ return ret;
+}
@@ -426,6 +426,24 @@ rte_ipv4_udptcp_cksum_verify(const struct rte_ipv4_hdr *ipv4_hdr,
return 0;
}
+/**
+ * IP address parser.
+ *
+ * @param src_ip
+ * The IP address to be parsed.
+ * @param output_addr
+ * The array in which the parsed digits will be saved.
+ *
+ * @retval 0
+ * Success.
+ * @retval -1
+ * Failure due to invalid input arguments.
+ */
+
+__rte_experimental
+int32_t
+rte_ip_parse_addr(const char *src_ip, uint32_t *output_addr);
+
/**
* IPv6 Header
*/
@@ -12,3 +12,11 @@ DPDK_22 {
local: *;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_net_skip_ip6_ext;
+ rte_ether_unformat_addr;
+ rte_ip_parse_addr;
+};