diff mbox series

[v3,3/6] ethdev: use rte_eth_unformat_addr

Message ID 20190605010852.28395-4-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/ether: improvements and optimizations |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Stephen Hemminger June 5, 2019, 1:08 a.m. UTC 
  Use rte_eth_unformat_addr, so that ethdev can be built and work
without the cmdline library. The dependency on cmdline was
an arrangement of convenience anyway.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/Makefile                      | 1 -
 lib/librte_ethdev/Makefile        | 2 +-
 lib/librte_ethdev/meson.build     | 2 +-
 lib/librte_ethdev/rte_class_eth.c | 9 +--------
 4 files changed, 3 insertions(+), 11 deletions(-)

Comments

Andrew Rybchenko June 5, 2019, 8:59 a.m. UTC | #1 
On 6/5/19 4:08 AM, Stephen Hemminger wrote:
> Use rte_eth_unformat_addr, so that ethdev can be built and work
> without the cmdline library. The dependency on cmdline was
> an arrangement of convenience anyway.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

[...]

> diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
> index 873a653532ad..6338355e2557 100644
> --- a/lib/librte_ethdev/rte_class_eth.c
> +++ b/lib/librte_ethdev/rte_class_eth.c
> @@ -4,7 +4,6 @@
>   
>   #include <string.h>
>   
> -#include <cmdline_parse_etheraddr.h>
>   #include <rte_class.h>
>   #include <rte_compat.h>
>   #include <rte_errno.h>
> @@ -43,19 +42,13 @@ static int
>   eth_mac_cmp(const char *key __rte_unused,
>   		const char *value, void *opaque)
>   {
> -	int ret;
>   	struct rte_ether_addr mac;
>   	const struct rte_eth_dev_data *data = opaque;
>   	struct rte_eth_dev_info dev_info;
>   	uint32_t index;
>   
>   	/* Parse devargs MAC address. */
> -	/*
> -	 * cannot use ether_aton_r(value, &mac)
> -	 * because of include conflict with rte_ether.h

Why not ether_aton_r()? Isn't conflict resolved now after patch series 
from Olivier?
I think it would be nice to explain it in the changeset description.

> -	 */
> -	ret = cmdline_parse_etheraddr(NULL, value, &mac, sizeof(mac));
> -	if (ret < 0)
> +	if (rte_ether_unformat_addr(value, &mac) < 0)
>   		return -1; /* invalid devargs value */
>   
>   	/* Return 0 if devargs MAC is matching one of the device MACs. */
Stephen Hemminger June 5, 2019, 3:02 p.m. UTC | #2 
On Wed, 5 Jun 2019 11:59:52 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 6/5/19 4:08 AM, Stephen Hemminger wrote:
> > Use rte_eth_unformat_addr, so that ethdev can be built and work
> > without the cmdline library. The dependency on cmdline was
> > an arrangement of convenience anyway.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> [...]
> 
> > diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
> > index 873a653532ad..6338355e2557 100644
> > --- a/lib/librte_ethdev/rte_class_eth.c
> > +++ b/lib/librte_ethdev/rte_class_eth.c
> > @@ -4,7 +4,6 @@
> >   
> >   #include <string.h>
> >   
> > -#include <cmdline_parse_etheraddr.h>
> >   #include <rte_class.h>
> >   #include <rte_compat.h>
> >   #include <rte_errno.h>
> > @@ -43,19 +42,13 @@ static int
> >   eth_mac_cmp(const char *key __rte_unused,
> >   		const char *value, void *opaque)
> >   {
> > -	int ret;
> >   	struct rte_ether_addr mac;
> >   	const struct rte_eth_dev_data *data = opaque;
> >   	struct rte_eth_dev_info dev_info;
> >   	uint32_t index;
> >   
> >   	/* Parse devargs MAC address. */
> > -	/*
> > -	 * cannot use ether_aton_r(value, &mac)
> > -	 * because of include conflict with rte_ether.h  
> 
> Why not ether_aton_r()? Isn't conflict resolved now after patch series 
> from Olivier?
> I think it would be nice to explain it in the changeset description.

ether_aton_r won;t work for 3 three ushorts model.
Stephen Hemminger June 5, 2019, 4:33 p.m. UTC | #3 
On Wed, 5 Jun 2019 11:59:52 +0300
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> On 6/5/19 4:08 AM, Stephen Hemminger wrote:
> > Use rte_eth_unformat_addr, so that ethdev can be built and work
> > without the cmdline library. The dependency on cmdline was
> > an arrangement of convenience anyway.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>  
> 
> [...]
> 
> > diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
> > index 873a653532ad..6338355e2557 100644
> > --- a/lib/librte_ethdev/rte_class_eth.c
> > +++ b/lib/librte_ethdev/rte_class_eth.c
> > @@ -4,7 +4,6 @@
> >   
> >   #include <string.h>
> >   
> > -#include <cmdline_parse_etheraddr.h>
> >   #include <rte_class.h>
> >   #include <rte_compat.h>
> >   #include <rte_errno.h>
> > @@ -43,19 +42,13 @@ static int
> >   eth_mac_cmp(const char *key __rte_unused,
> >   		const char *value, void *opaque)
> >   {
> > -	int ret;
> >   	struct rte_ether_addr mac;
> >   	const struct rte_eth_dev_data *data = opaque;
> >   	struct rte_eth_dev_info dev_info;
> >   	uint32_t index;
> >   
> >   	/* Parse devargs MAC address. */
> > -	/*
> > -	 * cannot use ether_aton_r(value, &mac)
> > -	 * because of include conflict with rte_ether.h  
> 
> Why not ether_aton_r()? Isn't conflict resolved now after patch series 
> from Olivier?
> I think it would be nice to explain it in the changeset description.

The problem with using ether_aton_r is that it does not accept the same
input formats as old code.
diff mbox series

Patch

diff --git a/lib/Makefile b/lib/Makefile
index 791e0d9911d6..82b2c4bfa8ea 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -25,7 +25,6 @@  DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ethdev
 DEPDIRS-librte_ethdev := librte_net librte_eal librte_mempool librte_ring
 DEPDIRS-librte_ethdev += librte_mbuf
 DEPDIRS-librte_ethdev += librte_kvargs
-DEPDIRS-librte_ethdev += librte_cmdline
 DEPDIRS-librte_ethdev += librte_meter
 DIRS-$(CONFIG_RTE_LIBRTE_BBDEV) += librte_bbdev
 DEPDIRS-librte_bbdev := librte_eal librte_mempool librte_mbuf
diff --git a/lib/librte_ethdev/Makefile b/lib/librte_ethdev/Makefile
index 8d4a02630c4f..60bcc2227878 100644
--- a/lib/librte_ethdev/Makefile
+++ b/lib/librte_ethdev/Makefile
@@ -12,7 +12,7 @@  CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring
-LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_cmdline -lrte_meter
+LDLIBS += -lrte_mbuf -lrte_kvargs -lrte_meter
 
 EXPORT_MAP := rte_ethdev_version.map
 
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index 8d6165b2a7dd..f75d428c96d0 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -26,4 +26,4 @@  headers = files('rte_ethdev.h',
 	'rte_tm.h',
 	'rte_tm_driver.h')
 
-deps += ['net', 'kvargs', 'cmdline', 'meter']
+deps += ['net', 'kvargs', 'meter']
diff --git a/lib/librte_ethdev/rte_class_eth.c b/lib/librte_ethdev/rte_class_eth.c
index 873a653532ad..6338355e2557 100644
--- a/lib/librte_ethdev/rte_class_eth.c
+++ b/lib/librte_ethdev/rte_class_eth.c
@@ -4,7 +4,6 @@ 
 
 #include <string.h>
 
-#include <cmdline_parse_etheraddr.h>
 #include <rte_class.h>
 #include <rte_compat.h>
 #include <rte_errno.h>
@@ -43,19 +42,13 @@  static int
 eth_mac_cmp(const char *key __rte_unused,
 		const char *value, void *opaque)
 {
-	int ret;
 	struct rte_ether_addr mac;
 	const struct rte_eth_dev_data *data = opaque;
 	struct rte_eth_dev_info dev_info;
 	uint32_t index;
 
 	/* Parse devargs MAC address. */
-	/*
-	 * cannot use ether_aton_r(value, &mac)
-	 * because of include conflict with rte_ether.h
-	 */
-	ret = cmdline_parse_etheraddr(NULL, value, &mac, sizeof(mac));
-	if (ret < 0)
+	if (rte_ether_unformat_addr(value, &mac) < 0)
 		return -1; /* invalid devargs value */
 
 	/* Return 0 if devargs MAC is matching one of the device MACs. */