[PATCH v2 5/5] baseband/acc100: add protection for some negative scenario

[Chautru, Nicolas]

Hi Tom, 

> -----Original Message-----
> From: Tom Rix <trix at redhat.com>
> Sent: Sunday, May 8, 2022 6:56 AM
> To: Chautru, Nicolas <nicolas.chautru at intel.com>; dev at dpdk.org;
> gakhil at marvell.com
> Cc: thomas at monjalon.net; Kinsella, Ray <ray.kinsella at intel.com>; Richardson,
> Bruce <bruce.richardson at intel.com>; hemant.agrawal at nxp.com; Zhang,
> Mingshan <mingshan.zhang at intel.com>; david.marchand at redhat.com
> Subject: Re: [PATCH v2 5/5] baseband/acc100: add protection for some
> negative scenario
> 
> 
> On 4/27/22 11:17 AM, Nicolas Chautru wrote:
> > Catch exception in PMD in case of invalid input parameter.
> 
> It is not clear if this is 1 fix or 2.
> 
> But it does look like an acc100 fix so it should be split from the
> acc101 patchset.
> 

What is the concern? This is a different commit related to acc100.  

> >
> > Signed-off-by: Nicolas Chautru <nicolas.chautru at intel.com>
> > ---
> >   drivers/baseband/acc100/rte_acc100_pmd.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
> > b/drivers/baseband/acc100/rte_acc100_pmd.c
> > index b588f5f..a13966c 100644
> > --- a/drivers/baseband/acc100/rte_acc100_pmd.c
> > +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
> > @@ -1241,6 +1241,8 @@
> >   			return (bg == 1 ? ACC100_K0_3_1 : ACC100_K0_3_2)
> * z_c;
> >   	}
> >   	/* LBRM case - includes a division by N */
> > +	if (unlikely(z_c == 0))
> > +		return 0;
> 
> This check should be moved to earlier, if 'n' is set to 0 in the statement above,
> there is div by 0 later

N is purely a factor of z_c, I don’t see the concern is order. 

> 
> Tom
> 
> >   	if (rv_index == 1)
> >   		return (((bg == 1 ? ACC100_K0_1_1 : ACC100_K0_1_2) *
> n_cb)
> >   				/ n) * z_c;
> > @@ -1916,6 +1918,10 @@ static inline uint32_t hq_index(uint32_t
> > offset)
> >
> >   	/* Soft output */
> >   	if (check_bit(op->turbo_dec.op_flags,
> RTE_BBDEV_TURBO_SOFT_OUTPUT))
> > {
> > +		if (op->turbo_dec.soft_output.data == 0) {
> > +			rte_bbdev_log(ERR, "Soft output is not defined");
> > +			return -1;
> > +		}
> >   		if (check_bit(op->turbo_dec.op_flags,
> >   				RTE_BBDEV_TURBO_EQUALIZER))
> >   			*s_out_length = e;