* [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 related [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, other threads:[~2019-07-19 12:37 UTC | newest] 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
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).