Re: [PATCH v3 2/4] hash: add dynamic polynomial calculation

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 16 Oct 2024 17:28:17 +0100
"Medvedkin, Vladimir" <vladimir.medvedkin@intel.com> wrote:

> Hi Stephen,
> 
> On 15/10/2024 23:29, Stephen Hemminger wrote:
> > On Fri, 11 Oct 2024 18:17:00 +0000
> > Vladimir Medvedkin<vladimir.medvedkin@intel.com>  wrote:
> >  
> >> +
> >> +uint32_t
> >> +rte_thash_get_rand_poly(uint32_t poly_degree)
> >> +{
> >> +	uint32_t ret_poly;
> >> +
> >> +	if (poly_degree > 32)

Error log here?

> >> +		return 0;
> >> +
> >> +	do
> >> +		ret_poly = __thash_get_rand_poly(poly_degree);
> >> +	while (thash_test_poly_order(ret_poly, poly_degree));  
> > Unbounded loop adds some risk, should there be an upper limit on retries.  
> 
> Thisis the probabilisticpartof the algorithm.
> 
> __thash_get_rand_poly() returns a random polynomial that either 
> satisfies the order criteria (element <x> of the field must generate 
> multiplicative subgroup of order not less than some number), or not. The 
> probability that it does not meet this criteria is strictly less than 1. 
> Thus, with each attempt, the probability of not finding suitable 
> polynomial exponentially tends to zero.

Never trust probabilities to converge. Just add an upper bound of something big
like 32 tries and log error and give up.

> 
> >  
> >> +
> >> +	return ret_poly;
> >> +}
> >> diff --git a/lib/hash/version.map b/lib/hash/version.map
> >> index 4f13f1d5aa..7ce6ab1121 100644
> >> --- a/lib/hash/version.map
> >> +++ b/lib/hash/version.map
> >> @@ -61,4 +61,5 @@ INTERNAL {
> >>   
> >>   	rte_thash_gfni_stub;
> >>   	rte_thash_gfni_bulk_stub;
> >> +	rte_thash_get_rand_poly;  
> > Why does this function need to be moved to its own file?
> > Only used in one place in rte_thash.c.  
> It was done just for convenience. If you insist, I'll move it to rte_thash.c


It is actually good to put in seperate file, but you can keep the old name.
Best to reserve names with rte_ prefix for user visible prefixes.