[dpdk-dev] [PATCH v2] net/failsafe: fix tx sub device deactivating

Matan Azrad matan at mellanox.com
Wed Aug 16 16:53:43 CEST 2017


Hi

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> Sent: Wednesday, August 16, 2017 5:39 PM
> To: Matan Azrad <matan at mellanox.com>
> Cc: dev at dpdk.org; stable at dpdk.org
> Subject: Re: [PATCH v2] net/failsafe: fix tx sub device deactivating
> 
> On Wed, Aug 16, 2017 at 05:19:28PM +0300, Matan Azrad wrote:
> > The corrupted code couldn't recognize that all sub devices were not
> > ready for tx traffic when failsafe PMD was trying to switch device
> > because of an unreachable condition using.
> >
> > Hence, the current tx sub device variable was not updated correctly.
> >
> > The fix removed the unreachable branch and added new one in the right
> > place respecting the original intent.
> >
> > Fixes: ebea83f899d8 ("net/failsafe: add plug-in support")
> > Fixes: 598fb8aec6f6 ("net/failsafe: support device removal")
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: Matan Azrad <matan at mellanox.com>
> > Acked-by: Gaetan Rivet <gaetan.rivet at 6wind.com>
> > ---
> >  drivers/net/failsafe/failsafe_private.h | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/failsafe/failsafe_private.h
> > b/drivers/net/failsafe/failsafe_private.h
> > index 0361cf4..ef646db 100644
> > --- a/drivers/net/failsafe/failsafe_private.h
> > +++ b/drivers/net/failsafe/failsafe_private.h
> > @@ -334,7 +334,7 @@ fs_switch_dev(struct rte_eth_dev *dev,
> >  	} else if ((txd && txd->state < req_state) ||
> >  		   txd == NULL ||
> >  		   txd == banned) {
> > -		struct sub_device *sdev;
> > +		struct sub_device *sdev = NULL;
> 
> Good catch, actually this makes me think that the FOREACH_SUBDEV_STATE
> macro is not following the usual tailq API (which sets the iterator to NULL
> upon terminating). This can throw-off the unsuspecting writer.

I think you right.
Else, you should go all over this macro using and initiate sdev to NULL.

> 
> I will fix this soon™. In the meantime, sdev should be initialized to NULL.

Great 

> 
> >  		uint8_t i;
> >
> >  		/* Using acceptable device */
> > @@ -346,9 +346,10 @@ fs_switch_dev(struct rte_eth_dev *dev,
> >  			PRIV(dev)->subs_tx = i;
> >  			break;
> >  		}
> > -	} else if (txd && txd->state < req_state) {
> > -		DEBUG("No device ready, deactivating tx_dev");
> > -		PRIV(dev)->subs_tx = PRIV(dev)->subs_tail;
> > +		if (i >= PRIV(dev)->subs_tail || sdev == NULL) {
> > +			DEBUG("No device ready, deactivating tx_dev");
> > +			PRIV(dev)->subs_tx = PRIV(dev)->subs_tail;
> > +		}
> >  	} else {
> >  		return;
> >  	}
> > --
> > 2.7.4
> >
> 
> --
> Gaëtan Rivet
> 6WIND


More information about the dev mailing list