RE: [PATCH v2 02/24] net/nfp: add the structures and functions for flow offload

[Chaoyong He <chaoyong.he@corigine.com>]

> On 10/10/2022 7:08 AM, Chaoyong He wrote:
> > Add the structures and functions to process mask table, flow table,
> > and flow stats id, which are used in the rte_flow offload logics.
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
> > +__rte_unused static int
> > +nfp_flow_table_delete(struct nfp_flow_priv *priv,
> > +             struct rte_flow *nfp_flow) {
> > +     int ret;
> > +     char *hash_data;
> > +     uint32_t hash_key;
> > +
> > +     hash_data = (char *)(nfp_flow->payload.unmasked_data);
> > +     hash_key = rte_jhash(hash_data, nfp_flow->length,
> > + priv->hash_seed);
> 
> It can be an option to store hash information in 'struct rte_flow', instead of
> keep calculating it.
> 

Okay, I will revise like this in the next version patch, thanks.

> <...>
> 
> > +     /* flow stats */
> > +     rte_spinlock_init(&priv->stats_lock);
> > +     stats_size = (ctx_count & NFP_FL_STAT_ID_STAT) |
> > +                     ((ctx_split - 1) & NFP_FL_STAT_ID_MU_NUM);
> > +     PMD_INIT_LOG(INFO, "ctx_count:%0lx, ctx_split:%0lx, stats_size:%0lx
> ",
> > +                     ctx_count, ctx_split, stats_size);
> > +     priv->stats = rte_zmalloc("nfp_flow_stats",
> > +                     stats_size * sizeof(struct nfp_fl_stats), 0);
> 
> Is this allocates 'stats' for max possible flow rule count? Isn't this a too big
> number to allocate in advance?

For now, the 'stats_size' for our card is 500,000, and sizeof(structure nfp_fl_stats) is 16,
so the size of memory need alloc here is about 8MB. Maybe it's not a very big memory?

And we also calculate an index use these two macros(in function nfp_stats_id_alloc()),
and then send it to firmware, and the firmware will send the stats with this index to PMD.
If we alloc a small memory at start and then enlarge it at some point, we can't make sure 
the index we calculated is unique anymore, and the stats will confusion, which is not good.