Re: [PATCH net-next v2 7/9] nfp: add metadata to each flow offload
flat view
From: Jakub Kicinski <hidden>
Date: 2017-06-29 02:33:08
On Wed, 28 Jun 2017 22:30:00 +0200, Simon Horman wrote:
From: Pieter Jansen van Vuuren <redacted> 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 <redacted> Signed-off-by: Simon Horman <redacted>
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
+ kfree(app->priv); + return err; + } + return 0; }
quoted hunk ↗ jump to hunk
+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?
quoted hunk ↗ jump to hunk
+ return -ENOENT; + } + + return 0; +} +
quoted hunk ↗ jump to hunk
+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?
quoted hunk ↗ jump to hunk
+ 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; +}