DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
@ 2019-06-21 22:08 Stephen Hemminger
  2019-06-24  8:15 ` Gaëtan Rivet
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-06-21 22:08 UTC (permalink / raw)
  To: gaetan.rivet; +Cc: dev, Stephen Hemminger

When secondary process is run was noticing that the log always
contained complaints about unable to parse devargs.

It turns out that an empty devargs turns into "" and this
value is not parsable. Change the failsafe secondary to just
skip doing devargs if it empty.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/failsafe/failsafe.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index e91c274d8059..04ca0cab0d78 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
 		 * A sub-device can be plugged later.
 		 */
 		FOREACH_SUBDEV(sdev, i, eth_dev) {
+			/* skip empty devargs */
+			if (sdev->devargs.name[0] == '\0')
+				continue;
+
 			/* rebuild devargs to be able to get the bus name. */
 			ret = rte_devargs_parse(&devargs,
 						sdev->devargs.name);
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-06-21 22:08 [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary Stephen Hemminger
@ 2019-06-24  8:15 ` Gaëtan Rivet
  2019-06-24 15:23   ` Stephen Hemminger
  0 siblings, 1 reply; 7+ messages in thread
From: Gaëtan Rivet @ 2019-06-24  8:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

Hello Stephen,

On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
> When secondary process is run was noticing that the log always
> contained complaints about unable to parse devargs.
> 
> It turns out that an empty devargs turns into "" and this
> value is not parsable. Change the failsafe secondary to just
> skip doing devargs if it empty.
> 

Commit log needs a little rework, a few typos.

> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  drivers/net/failsafe/failsafe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> index e91c274d8059..04ca0cab0d78 100644
> --- a/drivers/net/failsafe/failsafe.c
> +++ b/drivers/net/failsafe/failsafe.c
> @@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
>  		 * A sub-device can be plugged later.
>  		 */
>  		FOREACH_SUBDEV(sdev, i, eth_dev) {
> +			/* skip empty devargs */
> +			if (sdev->devargs.name[0] == '\0')
> +				continue;
> +

An empty devargs being named "" is part of the internals of rte_devargs.
The clean solution would be to add a `bool rte_devargs_empty()` function
and test the devargs with it.

The simple solution is your proposition.

Clean seems a little heavy-handed, but it would be more stable. If you
agree, you can add the helper. I'm ok with keeping it simple otherwise.

>  			/* rebuild devargs to be able to get the bus name. */
>  			ret = rte_devargs_parse(&devargs,
>  						sdev->devargs.name);
> -- 
> 2.20.1
> 

-- 
Gaëtan Rivet
6WIND

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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-06-24  8:15 ` Gaëtan Rivet
@ 2019-06-24 15:23   ` Stephen Hemminger
  2019-06-24 16:27     ` Gaëtan Rivet
  0 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-06-24 15:23 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: dev

On Mon, 24 Jun 2019 10:15:58 +0200
Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:

> Hello Stephen,
> 
> On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
> > When secondary process is run was noticing that the log always
> > contained complaints about unable to parse devargs.
> > 
> > It turns out that an empty devargs turns into "" and this
> > value is not parsable. Change the failsafe secondary to just
> > skip doing devargs if it empty.
> >   
> 
> Commit log needs a little rework, a few typos.
> 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  drivers/net/failsafe/failsafe.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> > index e91c274d8059..04ca0cab0d78 100644
> > --- a/drivers/net/failsafe/failsafe.c
> > +++ b/drivers/net/failsafe/failsafe.c
> > @@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
> >  		 * A sub-device can be plugged later.
> >  		 */
> >  		FOREACH_SUBDEV(sdev, i, eth_dev) {
> > +			/* skip empty devargs */
> > +			if (sdev->devargs.name[0] == '\0')
> > +				continue;
> > +  
> 
> An empty devargs being named "" is part of the internals of rte_devargs.
> The clean solution would be to add a `bool rte_devargs_empty()` function
> and test the devargs with it.
> 
> The simple solution is your proposition.
> 
> Clean seems a little heavy-handed, but it would be more stable. If you
> agree, you can add the helper. I'm ok with keeping it simple otherwise.
> 
> >  			/* rebuild devargs to be able to get the bus name. */
> >  			ret = rte_devargs_parse(&devargs,
> >  						sdev->devargs.name);
> > -- 
> > 2.20.1
> >   
> 

Simpler is better.
Sorry, after working with failsafe my impression is that it is not
built with that in mind.

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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-06-24 15:23   ` Stephen Hemminger
@ 2019-06-24 16:27     ` Gaëtan Rivet
  2019-07-17 18:21       ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Gaëtan Rivet @ 2019-06-24 16:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Mon, Jun 24, 2019 at 08:23:38AM -0700, Stephen Hemminger wrote:
> On Mon, 24 Jun 2019 10:15:58 +0200
> Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> 
> > Hello Stephen,
> > 
> > On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
> > > When secondary process is run was noticing that the log always
> > > contained complaints about unable to parse devargs.
> > > 
> > > It turns out that an empty devargs turns into "" and this
> > > value is not parsable. Change the failsafe secondary to just
> > > skip doing devargs if it empty.
> > >   
> > 
> > Commit log needs a little rework, a few typos.
> > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  drivers/net/failsafe/failsafe.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> > > index e91c274d8059..04ca0cab0d78 100644
> > > --- a/drivers/net/failsafe/failsafe.c
> > > +++ b/drivers/net/failsafe/failsafe.c
> > > @@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
> > >  		 * A sub-device can be plugged later.
> > >  		 */
> > >  		FOREACH_SUBDEV(sdev, i, eth_dev) {
> > > +			/* skip empty devargs */
> > > +			if (sdev->devargs.name[0] == '\0')
> > > +				continue;
> > > +  
> > 
> > An empty devargs being named "" is part of the internals of rte_devargs.
> > The clean solution would be to add a `bool rte_devargs_empty()` function
> > and test the devargs with it.
> > 
> > The simple solution is your proposition.
> > 
> > Clean seems a little heavy-handed, but it would be more stable. If you
> > agree, you can add the helper. I'm ok with keeping it simple otherwise.
> > 
> > >  			/* rebuild devargs to be able to get the bus name. */
> > >  			ret = rte_devargs_parse(&devargs,
> > >  						sdev->devargs.name);
> > > -- 
> > > 2.20.1
> > >   
> > 
> 
> Simpler is better.

Ok

> Sorry, after working with failsafe my impression is that it is not
> built with that in mind.

-- 
Gaëtan Rivet
6WIND

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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-06-24 16:27     ` Gaëtan Rivet
@ 2019-07-17 18:21       ` Ferruh Yigit
  2019-07-18 11:18         ` Gaëtan Rivet
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2019-07-17 18:21 UTC (permalink / raw)
  To: Gaëtan Rivet, Stephen Hemminger; +Cc: dev

On 6/24/2019 5:27 PM, Gaëtan Rivet wrote:
> On Mon, Jun 24, 2019 at 08:23:38AM -0700, Stephen Hemminger wrote:
>> On Mon, 24 Jun 2019 10:15:58 +0200
>> Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
>>
>>> Hello Stephen,
>>>
>>> On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
>>>> When secondary process is run was noticing that the log always
>>>> contained complaints about unable to parse devargs.
>>>>
>>>> It turns out that an empty devargs turns into "" and this
>>>> value is not parsable. Change the failsafe secondary to just
>>>> skip doing devargs if it empty.
>>>>   
>>>
>>> Commit log needs a little rework, a few typos.
>>>
>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>>>> ---
>>>>  drivers/net/failsafe/failsafe.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
>>>> index e91c274d8059..04ca0cab0d78 100644
>>>> --- a/drivers/net/failsafe/failsafe.c
>>>> +++ b/drivers/net/failsafe/failsafe.c
>>>> @@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
>>>>  		 * A sub-device can be plugged later.
>>>>  		 */
>>>>  		FOREACH_SUBDEV(sdev, i, eth_dev) {
>>>> +			/* skip empty devargs */
>>>> +			if (sdev->devargs.name[0] == '\0')
>>>> +				continue;
>>>> +  
>>>
>>> An empty devargs being named "" is part of the internals of rte_devargs.
>>> The clean solution would be to add a `bool rte_devargs_empty()` function
>>> and test the devargs with it.
>>>
>>> The simple solution is your proposition.
>>>
>>> Clean seems a little heavy-handed, but it would be more stable. If you
>>> agree, you can add the helper. I'm ok with keeping it simple otherwise.
>>>
>>>>  			/* rebuild devargs to be able to get the bus name. */
>>>>  			ret = rte_devargs_parse(&devargs,
>>>>  						sdev->devargs.name);
>>>> -- 
>>>> 2.20.1
>>>>   
>>>
>>
>> Simpler is better.
> 
> Ok

is this an ack :)

> 
>> Sorry, after working with failsafe my impression is that it is not
>> built with that in mind.
> 


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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-07-17 18:21       ` Ferruh Yigit
@ 2019-07-18 11:18         ` Gaëtan Rivet
  2019-07-19 12:36           ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Gaëtan Rivet @ 2019-07-18 11:18 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Stephen Hemminger, dev

On Wed, Jul 17, 2019 at 07:21:51PM +0100, Ferruh Yigit wrote:
> On 6/24/2019 5:27 PM, Gaëtan Rivet wrote:
> > On Mon, Jun 24, 2019 at 08:23:38AM -0700, Stephen Hemminger wrote:
> >> On Mon, 24 Jun 2019 10:15:58 +0200
> >> Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> >>
> >>> Hello Stephen,
> >>>
> >>> On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
> >>>> When secondary process is run was noticing that the log always
> >>>> contained complaints about unable to parse devargs.
> >>>>
> >>>> It turns out that an empty devargs turns into "" and this
> >>>> value is not parsable. Change the failsafe secondary to just
> >>>> skip doing devargs if it empty.
> >>>>   
> >>>
> >>> Commit log needs a little rework, a few typos.
> >>>
> >>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >>>> ---
> >>>>  drivers/net/failsafe/failsafe.c | 4 ++++
> >>>>  1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
> >>>> index e91c274d8059..04ca0cab0d78 100644
> >>>> --- a/drivers/net/failsafe/failsafe.c
> >>>> +++ b/drivers/net/failsafe/failsafe.c
> >>>> @@ -364,6 +364,10 @@ rte_pmd_failsafe_probe(struct rte_vdev_device *vdev)
> >>>>  		 * A sub-device can be plugged later.
> >>>>  		 */
> >>>>  		FOREACH_SUBDEV(sdev, i, eth_dev) {
> >>>> +			/* skip empty devargs */
> >>>> +			if (sdev->devargs.name[0] == '\0')
> >>>> +				continue;
> >>>> +  
> >>>
> >>> An empty devargs being named "" is part of the internals of rte_devargs.
> >>> The clean solution would be to add a `bool rte_devargs_empty()` function
> >>> and test the devargs with it.
> >>>
> >>> The simple solution is your proposition.
> >>>
> >>> Clean seems a little heavy-handed, but it would be more stable. If you
> >>> agree, you can add the helper. I'm ok with keeping it simple otherwise.
> >>>
> >>>>  			/* rebuild devargs to be able to get the bus name. */
> >>>>  			ret = rte_devargs_parse(&devargs,
> >>>>  						sdev->devargs.name);
> >>>> -- 
> >>>> 2.20.1
> >>>>   
> >>>
> >>
> >> Simpler is better.
> > 
> > Ok
> 
> is this an ack :)
> 

Yes sorry,

Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

> > 
> >> Sorry, after working with failsafe my impression is that it is not
> >> built with that in mind.
> > 
> 

-- 
Gaëtan Rivet
6WIND

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

* Re: [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary
  2019-07-18 11:18         ` Gaëtan Rivet
@ 2019-07-19 12:36           ` Ferruh Yigit
  0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2019-07-19 12:36 UTC (permalink / raw)
  To: Gaëtan Rivet; +Cc: Stephen Hemminger, dev

On 7/18/2019 12:18 PM, Gaëtan Rivet wrote:
> On Wed, Jul 17, 2019 at 07:21:51PM +0100, Ferruh Yigit wrote:
>> On 6/24/2019 5:27 PM, Gaëtan Rivet wrote:
>>> On Mon, Jun 24, 2019 at 08:23:38AM -0700, Stephen Hemminger wrote:
>>>> On Mon, 24 Jun 2019 10:15:58 +0200
>>>> Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
>>>>
>>>>> Hello Stephen,
>>>>>
>>>>> On Fri, Jun 21, 2019 at 03:08:24PM -0700, Stephen Hemminger wrote:
>>>>>> When secondary process is run was noticing that the log always
>>>>>> contained complaints about unable to parse devargs.
>>>>>>
>>>>>> It turns out that an empty devargs turns into "" and this
>>>>>> value is not parsable. Change the failsafe secondary to just
>>>>>> skip doing devargs if it empty.
>>>>>>   
>>>>>
>>>>> Commit log needs a little rework, a few typos.
>>>>>
>>>>>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

<...>

> 
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Applied to dpdk-next-net/master, thanks.

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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 22:08 [dpdk-dev] [PATCH] failsafe: skip devargs if not present in secondary Stephen Hemminger
2019-06-24  8:15 ` Gaëtan Rivet
2019-06-24 15:23   ` Stephen Hemminger
2019-06-24 16:27     ` Gaëtan Rivet
2019-07-17 18:21       ` Ferruh Yigit
2019-07-18 11:18         ` Gaëtan Rivet
2019-07-19 12:36           ` Ferruh Yigit

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox