DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: check for invalid device name
@ 2019-03-11 18:15 Stephen Hemminger
  2019-03-13 12:52 ` Zhang, Qi Z
  2019-03-14  7:23 ` Ali Alnubani
  0 siblings, 2 replies; 7+ messages in thread
From: Stephen Hemminger @ 2019-03-11 18:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Do not allow creating a ethernet device with a name over the
allowed maximum (or zero length). This is safer than silently truncating
which is what happens now.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
v1 - previously sent as RFC

 lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 85c1794968dd..0b81980ff71c 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
 	uint16_t port_id;
 	struct rte_eth_dev *eth_dev = NULL;
 
+	if (*name) {
+		RTE_ETHDEV_LOG(ERR, "Zero length Ethernet device name\n");
+		return NULL;
+	}
+
+	if (strnlen(name, RTE_ETH_NAME_MAX_LEN) >= RTE_ETH_NAME_MAX_LEN) {
+		RTE_ETHDEV_LOG(ERR, "Ethernet device name is too long\n");
+		return NULL;
+	}
+
 	rte_eth_dev_shared_data_prepare();
 
 	/* Synchronize port creation between primary and secondary threads. */
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-11 18:15 [dpdk-dev] [PATCH] ethdev: check for invalid device name Stephen Hemminger
@ 2019-03-13 12:52 ` Zhang, Qi Z
  2019-03-13 17:32   ` Stephen Hemminger
  2019-03-14  7:23 ` Ali Alnubani
  1 sibling, 1 reply; 7+ messages in thread
From: Zhang, Qi Z @ 2019-03-13 12:52 UTC (permalink / raw)
  To: Stephen Hemminger, dev

HI

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Tuesday, March 12, 2019 2:16 AM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> 
> Do not allow creating a ethernet device with a name over the allowed maximum
> (or zero length). This is safer than silently truncating which is what happens now.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> v1 - previously sent as RFC
> 
>  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 85c1794968dd..0b81980ff71c 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
>  	uint16_t port_id;
>  	struct rte_eth_dev *eth_dev = NULL;
> 
> +	if (*name) {

Is above check same as "strlen(name) == 0"?

> +		RTE_ETHDEV_LOG(ERR, "Zero length Ethernet device name\n");
> +		return NULL;
> +	}
> +
> +	if (strnlen(name, RTE_ETH_NAME_MAX_LEN) >=
> RTE_ETH_NAME_MAX_LEN) {
> +		RTE_ETHDEV_LOG(ERR, "Ethernet device name is too long\n");
> +		return NULL;
> +	}
> +
>  	rte_eth_dev_shared_data_prepare();
> 
>  	/* Synchronize port creation between primary and secondary threads. */
> --
> 2.17.1

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-13 12:52 ` Zhang, Qi Z
@ 2019-03-13 17:32   ` Stephen Hemminger
  2019-03-14  1:01     ` Zhang, Qi Z
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-03-13 17:32 UTC (permalink / raw)
  To: Zhang, Qi Z; +Cc: dev

On Wed, 13 Mar 2019 12:52:50 +0000
"Zhang, Qi Z" <qi.z.zhang@intel.com> wrote:

> HI
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> > Sent: Tuesday, March 12, 2019 2:16 AM
> > To: dev@dpdk.org
> > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> > 
> > Do not allow creating a ethernet device with a name over the allowed maximum
> > (or zero length). This is safer than silently truncating which is what happens now.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > ---
> > v1 - previously sent as RFC
> > 
> >  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index 85c1794968dd..0b81980ff71c 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
> >  	uint16_t port_id;
> >  	struct rte_eth_dev *eth_dev = NULL;
> > 
> > +	if (*name) {  
> 
> Is above check same as "strlen(name) == 0"?

Yes, but checking for first null byte is slightly quicker. Alternative would
be to call strnlen() once.

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-13 17:32   ` Stephen Hemminger
@ 2019-03-14  1:01     ` Zhang, Qi Z
  2019-03-14  1:01       ` Zhang, Qi Z
  0 siblings, 1 reply; 7+ messages in thread
From: Zhang, Qi Z @ 2019-03-14  1:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, March 14, 2019 1:33 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> 
> On Wed, 13 Mar 2019 12:52:50 +0000
> "Zhang, Qi Z" <qi.z.zhang@intel.com> wrote:
> 
> > HI
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> > > Hemminger
> > > Sent: Tuesday, March 12, 2019 2:16 AM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> > >
> > > Do not allow creating a ethernet device with a name over the allowed
> > > maximum (or zero length). This is safer than silently truncating which is what
> happens now.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > ---
> > > v1 - previously sent as RFC
> > >
> > >  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > > b/lib/librte_ethdev/rte_ethdev.c index 85c1794968dd..0b81980ff71c
> > > 100644
> > > --- a/lib/librte_ethdev/rte_ethdev.c
> > > +++ b/lib/librte_ethdev/rte_ethdev.c
> > > @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
> > >  	uint16_t port_id;
> > >  	struct rte_eth_dev *eth_dev = NULL;
> > >
> > > +	if (*name) {
> >
> > Is above check same as "strlen(name) == 0"?
> 
> Yes, but checking for first null byte is slightly quicker. Alternative would be to call
> strnlen() once.

But you are checking the not null byte here right? should it be if (!(*name))?
	

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-14  1:01     ` Zhang, Qi Z
@ 2019-03-14  1:01       ` Zhang, Qi Z
  0 siblings, 0 replies; 7+ messages in thread
From: Zhang, Qi Z @ 2019-03-14  1:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev



> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Thursday, March 14, 2019 1:33 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> 
> On Wed, 13 Mar 2019 12:52:50 +0000
> "Zhang, Qi Z" <qi.z.zhang@intel.com> wrote:
> 
> > HI
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen
> > > Hemminger
> > > Sent: Tuesday, March 12, 2019 2:16 AM
> > > To: dev@dpdk.org
> > > Cc: Stephen Hemminger <stephen@networkplumber.org>
> > > Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> > >
> > > Do not allow creating a ethernet device with a name over the allowed
> > > maximum (or zero length). This is safer than silently truncating which is what
> happens now.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> > > ---
> > > v1 - previously sent as RFC
> > >
> > >  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/librte_ethdev/rte_ethdev.c
> > > b/lib/librte_ethdev/rte_ethdev.c index 85c1794968dd..0b81980ff71c
> > > 100644
> > > --- a/lib/librte_ethdev/rte_ethdev.c
> > > +++ b/lib/librte_ethdev/rte_ethdev.c
> > > @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
> > >  	uint16_t port_id;
> > >  	struct rte_eth_dev *eth_dev = NULL;
> > >
> > > +	if (*name) {
> >
> > Is above check same as "strlen(name) == 0"?
> 
> Yes, but checking for first null byte is slightly quicker. Alternative would be to call
> strnlen() once.

But you are checking the not null byte here right? should it be if (!(*name))?
	

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-11 18:15 [dpdk-dev] [PATCH] ethdev: check for invalid device name Stephen Hemminger
  2019-03-13 12:52 ` Zhang, Qi Z
@ 2019-03-14  7:23 ` Ali Alnubani
  2019-03-14  7:23   ` Ali Alnubani
  1 sibling, 1 reply; 7+ messages in thread
From: Ali Alnubani @ 2019-03-14  7:23 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Thomas Monjalon, Shahaf Shuler

Hi Stephen,

I see failures in performance tests caused by this patch:
http://mails.dpdk.org/archives/test-report/2019-March/076705.html
http://mails.dpdk.org/archives/test-report/2019-March/076704.html

After applying your patch, the devices will not be probed:

'''
Zero length Ethernet device name
net_mlx5: can not allocate rte ethdev
net_mlx5: probe of PCI device 0000:12:00.1 aborted after encountering an error: Cannot allocate memory
EAL: Requested device 0000:12:00.1 cannot be used
testpmd: No probed ethernet devices
'''

Testpmd command:
./x86_64-native-linuxapp-gcc/build/app/test-pmd/testpmd -c 0x3 -n 4 -w 12:00.0 -w 12:00.1 -- --txd=256 --rxd=256 -i

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Monday, March 11, 2019 8:16 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> 
> Do not allow creating a ethernet device with a name over the allowed
> maximum (or zero length). This is safer than silently truncating which is what
> happens now.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> v1 - previously sent as RFC
> 
>  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 85c1794968dd..0b81980ff71c 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
>  	uint16_t port_id;
>  	struct rte_eth_dev *eth_dev = NULL;
> 
> +	if (*name) {
> +		RTE_ETHDEV_LOG(ERR, "Zero length Ethernet device
> name\n");
> +		return NULL;
> +	}
> +
> +	if (strnlen(name, RTE_ETH_NAME_MAX_LEN) >=
> RTE_ETH_NAME_MAX_LEN) {
> +		RTE_ETHDEV_LOG(ERR, "Ethernet device name is too
> long\n");
> +		return NULL;
> +	}
> +
>  	rte_eth_dev_shared_data_prepare();
> 
>  	/* Synchronize port creation between primary and secondary
> threads. */
> --
> 2.17.1

Regards,
Ali

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

* Re: [dpdk-dev] [PATCH] ethdev: check for invalid device name
  2019-03-14  7:23 ` Ali Alnubani
@ 2019-03-14  7:23   ` Ali Alnubani
  0 siblings, 0 replies; 7+ messages in thread
From: Ali Alnubani @ 2019-03-14  7:23 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Thomas Monjalon, Shahaf Shuler

Hi Stephen,

I see failures in performance tests caused by this patch:
http://mails.dpdk.org/archives/test-report/2019-March/076705.html
http://mails.dpdk.org/archives/test-report/2019-March/076704.html

After applying your patch, the devices will not be probed:

'''
Zero length Ethernet device name
net_mlx5: can not allocate rte ethdev
net_mlx5: probe of PCI device 0000:12:00.1 aborted after encountering an error: Cannot allocate memory
EAL: Requested device 0000:12:00.1 cannot be used
testpmd: No probed ethernet devices
'''

Testpmd command:
./x86_64-native-linuxapp-gcc/build/app/test-pmd/testpmd -c 0x3 -n 4 -w 12:00.0 -w 12:00.1 -- --txd=256 --rxd=256 -i

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Monday, March 11, 2019 8:16 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [PATCH] ethdev: check for invalid device name
> 
> Do not allow creating a ethernet device with a name over the allowed
> maximum (or zero length). This is safer than silently truncating which is what
> happens now.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> v1 - previously sent as RFC
> 
>  lib/librte_ethdev/rte_ethdev.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 85c1794968dd..0b81980ff71c 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -439,6 +439,16 @@ rte_eth_dev_allocate(const char *name)
>  	uint16_t port_id;
>  	struct rte_eth_dev *eth_dev = NULL;
> 
> +	if (*name) {
> +		RTE_ETHDEV_LOG(ERR, "Zero length Ethernet device
> name\n");
> +		return NULL;
> +	}
> +
> +	if (strnlen(name, RTE_ETH_NAME_MAX_LEN) >=
> RTE_ETH_NAME_MAX_LEN) {
> +		RTE_ETHDEV_LOG(ERR, "Ethernet device name is too
> long\n");
> +		return NULL;
> +	}
> +
>  	rte_eth_dev_shared_data_prepare();
> 
>  	/* Synchronize port creation between primary and secondary
> threads. */
> --
> 2.17.1

Regards,
Ali

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

end of thread, other threads:[~2019-03-14  7:23 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 18:15 [dpdk-dev] [PATCH] ethdev: check for invalid device name Stephen Hemminger
2019-03-13 12:52 ` Zhang, Qi Z
2019-03-13 17:32   ` Stephen Hemminger
2019-03-14  1:01     ` Zhang, Qi Z
2019-03-14  1:01       ` Zhang, Qi Z
2019-03-14  7:23 ` Ali Alnubani
2019-03-14  7:23   ` Ali Alnubani

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