On Wed, 28 Jun 2017 22:30:00 +0200, Simon Horman wrote:
> From: Pieter Jansen van Vuuren <pieter.jansenvanvuu...@netronome.com>
>
> Adds metadata describing the mask id of each flow and keeps track of
> flows installed in hardware. Previously a flow could not be removed
> from hardware as there was no way of knowing if that a specific flow
> was installed. This is solved by storing the offloaded flows in a
> hash table.
>
> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuu...@netronome.com>
> Signed-off-by: Simon Horman <simon.hor...@netronome.com>
> diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c
> b/drivers/net/ethernet/netronome/nfp/flower/main.c
> index 19f20f819e2f..1103d23a8ec7 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/main.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/main.c
> @@ -50,14 +50,6 @@
>
> #define NFP_FLOWER_ALLOWED_VER 0x0001000000010000UL
>
> -/**
> - * struct nfp_flower_priv - Flower APP per-vNIC priv data
> - * @nn: Pointer to vNIC
> - */
> -struct nfp_flower_priv {
> - struct nfp_net *nn;
> -};
> -
> static const char *nfp_flower_extra_cap(struct nfp_app *app, struct nfp_net
> *nn)
> {
> return "FLOWER";
> @@ -351,6 +343,12 @@ static int nfp_flower_init(struct nfp_app *app)
> if (!app->priv)
> return -ENOMEM;
>
> + err = nfp_flower_metadata_init(app);
> + if (nfp_flower_metadata_init(app)) {
You're calling init twice here. Also please do the error path with
goto, as explained in review of patch 8 having a proper unwind makes
later patches easier to review.
> + kfree(app->priv);
> + return err;
> + }
> +
> return 0;
> }
>
> +static int nfp_mask_alloc(struct nfp_app *app, u8 *mask_id)
> +{
> + struct nfp_flower_priv *priv = app->priv;
> + struct timespec64 delta, now;
> + struct circ_buf *ring;
> + u8 temp_id, freed_id;
> +
> + ring = &priv->mask_ids.mask_id_free_list;
> + freed_id = NFP_FLOWER_MASK_ENTRY_RS - 1;
> + /* Checking for unallocated entries first. */
> + if (priv->mask_ids.init_unallocated > 0) {
> + *mask_id = priv->mask_ids.init_unallocated;
> + priv->mask_ids.init_unallocated--;
> + return 0;
> + }
> +
> + /* Checking if buffer is empty. */
> + if (ring->head == ring->tail) {
> + *mask_id = freed_id;
> + return -ENOENT;
> + }
> +
> + memcpy(&temp_id, &ring->buf[ring->tail], NFP_FLOWER_MASK_ELEMENT_RS);
> + *mask_id = temp_id;
> + memcpy(&ring->buf[ring->tail], &freed_id, NFP_FLOWER_MASK_ELEMENT_RS);
> + ring->tail = (ring->tail + NFP_FLOWER_MASK_ELEMENT_RS) %
> + (NFP_FLOWER_MASK_ENTRY_RS * NFP_FLOWER_MASK_ELEMENT_RS);
> +
> + getnstimeofday64(&now);
> + delta = timespec64_sub(now, priv->mask_ids.last_used[*mask_id]);
> +
> + if (timespec64_to_ns(&delta) < NFP_FL_MASK_REUSE_TIME_NS) {
> + nfp_release_mask_id(app, *mask_id);
nfp_release_mask_id() will reset the time stamp and put the mask at the
end of the queue. Is that OK?
> + return -ENOENT;
> + }
> +
> + return 0;
> +}
> +
> +int nfp_flower_metadata_init(struct nfp_app *app)
> +{
> + struct nfp_flower_priv *priv = app->priv;
> +
> + hash_init(priv->mask_table);
> + hash_init(priv->flow_table);
> +
> + /* Init ring buffer and unallocated mask_ids. */
> + priv->mask_ids.mask_id_free_list.buf =
> + kmalloc(NFP_FLOWER_MASK_ENTRY_RS * NFP_FLOWER_MASK_ELEMENT_RS,
> + GFP_KERNEL);
kmalloc_array, perhaps?
> + if (!priv->mask_ids.mask_id_free_list.buf)
> + return -ENOMEM;
> +
> + priv->mask_ids.init_unallocated = NFP_FLOWER_MASK_ENTRY_RS - 1;
> +
> + /* Init timestamps for mask id*/
> + priv->mask_ids.last_used =
> + kmalloc_array(NFP_FLOWER_MASK_ENTRY_RS,
> + sizeof(*priv->mask_ids.last_used), GFP_KERNEL);
> + if (!priv->mask_ids.last_used) {
> + kfree(priv->mask_ids.mask_id_free_list.buf);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
