[v4,16/22] net/bnxt: fix table reference count for shadow tcam

Message ID 20200728063439.23114-17-ajit.khaparde@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series bnxt patches |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ajit Khaparde July 28, 2020, 6:34 a.m. UTC
  From: Mike Baucom <michael.baucom@broadcom.com>

Moved setting the refcnt for shadow tcam and table entries to the
allocation path only.  The insert can be called multiple times for
updates and was resetting the refcnt to 1 each time.  Now multiple
insertion/modifications will not change the reference count.

Fixes: 3cf8fb975df9 ("net/bnxt: add shadow and search capability to tcam")

Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
Reviewed-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
---
 drivers/net/bnxt/tf_core/tf_shadow_tbl.c  | 2 --
 drivers/net/bnxt/tf_core/tf_shadow_tcam.c | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)
  

Comments

Ferruh Yigit July 28, 2020, 5 p.m. UTC | #1
On 7/28/2020 7:34 AM, Ajit Khaparde wrote:
> From: Mike Baucom <michael.baucom@broadcom.com>
> 
> Moved setting the refcnt for shadow tcam and table entries to the
> allocation path only.  The insert can be called multiple times for
> updates and was resetting the refcnt to 1 each time.  Now multiple
> insertion/modifications will not change the reference count.
> 
> Fixes: 3cf8fb975df9 ("net/bnxt: add shadow and search capability to tcam")
> 
> Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
> Reviewed-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

The fix patch already in this set, why not fix the issue at first place where it
is introduced?

This patch distributed to 1/22 & 12/22 while merging, please confirm.
  
Ajit Khaparde July 28, 2020, 5:33 p.m. UTC | #2
On Tue, Jul 28, 2020 at 10:00 AM Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 7/28/2020 7:34 AM, Ajit Khaparde wrote:
> > From: Mike Baucom <michael.baucom@broadcom.com>
> >
> > Moved setting the refcnt for shadow tcam and table entries to the
> > allocation path only.  The insert can be called multiple times for
> > updates and was resetting the refcnt to 1 each time.  Now multiple
> > insertion/modifications will not change the reference count.
> >
> > Fixes: 3cf8fb975df9 ("net/bnxt: add shadow and search capability to
> tcam")
> >
> > Signed-off-by: Mike Baucom <michael.baucom@broadcom.com>
> > Reviewed-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
>
> The fix patch already in this set, why not fix the issue at first place
> where it
> is introduced?
>
> This patch distributed to 1/22 & 12/22 while merging, please confirm.
>
Ferruh,
I took a quick look at the patches and I think we may be able to merge them.
But I have not squashed and compiled the patches yet. So I can't tell if
merging them
will break the build.

Thanks
Ajit
  
Ferruh Yigit July 28, 2020, 5:38 p.m. UTC | #3
On 7/28/2020 6:33 PM, Ajit Khaparde wrote:
> 
> 
> On Tue, Jul 28, 2020 at 10:00 AM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
> 
>     On 7/28/2020 7:34 AM, Ajit Khaparde wrote:
>     > From: Mike Baucom <michael.baucom@broadcom.com
>     <mailto:michael.baucom@broadcom.com>>
>     >
>     > Moved setting the refcnt for shadow tcam and table entries to the
>     > allocation path only.  The insert can be called multiple times for
>     > updates and was resetting the refcnt to 1 each time.  Now multiple
>     > insertion/modifications will not change the reference count.
>     >
>     > Fixes: 3cf8fb975df9 ("net/bnxt: add shadow and search capability to tcam")
>     >
>     > Signed-off-by: Mike Baucom <michael.baucom@broadcom.com
>     <mailto:michael.baucom@broadcom.com>>
>     > Reviewed-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com
>     <mailto:kishore.padmanabha@broadcom.com>>
> 
>     The fix patch already in this set, why not fix the issue at first place where it
>     is introduced?
> 
>     This patch distributed to 1/22 & 12/22 while merging, please confirm.
> 
> Ferruh,
> I took a quick look at the patches and I think we may be able to merge them.
> But I have not squashed and compiled the patches yet. So I can't tell if merging
> them
> will break the build.

I have verified the patch by patch build, which looks good and pushed to next-net.
Also verified that final code is same, but better to have another eye to double
check.
  
Ajit Khaparde July 28, 2020, 6:06 p.m. UTC | #4
On Tue, Jul 28, 2020 at 10:39 AM Ferruh Yigit <ferruh.yigit@intel.com>
wrote:

> On 7/28/2020 6:33 PM, Ajit Khaparde wrote:
> >
> >
> > On Tue, Jul 28, 2020 at 10:00 AM Ferruh Yigit <ferruh.yigit@intel.com
> > <mailto:ferruh.yigit@intel.com>> wrote:
> >
> >     On 7/28/2020 7:34 AM, Ajit Khaparde wrote:
> >     > From: Mike Baucom <michael.baucom@broadcom.com
> >     <mailto:michael.baucom@broadcom.com>>
> >     >
> >     > Moved setting the refcnt for shadow tcam and table entries to the
> >     > allocation path only.  The insert can be called multiple times for
> >     > updates and was resetting the refcnt to 1 each time.  Now multiple
> >     > insertion/modifications will not change the reference count.
> >     >
> >     > Fixes: 3cf8fb975df9 ("net/bnxt: add shadow and search capability
> to tcam")
> >     >
> >     > Signed-off-by: Mike Baucom <michael.baucom@broadcom.com
> >     <mailto:michael.baucom@broadcom.com>>
> >     > Reviewed-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com
> >     <mailto:kishore.padmanabha@broadcom.com>>
> >
> >     The fix patch already in this set, why not fix the issue at first
> place where it
> >     is introduced?
> >
> >     This patch distributed to 1/22 & 12/22 while merging, please confirm.
> >
> > Ferruh,
> > I took a quick look at the patches and I think we may be able to merge
> them.
> > But I have not squashed and compiled the patches yet. So I can't tell if
> merging
> > them
> > will break the build.
>
> I have verified the patch by patch build, which looks good and pushed to
> next-net.
> Also verified that final code is same, but better to have another eye to
> double
> check.
>
Thanks Ferruh. I am checking the code now.
  

Patch

diff --git a/drivers/net/bnxt/tf_core/tf_shadow_tbl.c b/drivers/net/bnxt/tf_core/tf_shadow_tbl.c
index 019a26eba..a4207eb3a 100644
--- a/drivers/net/bnxt/tf_core/tf_shadow_tbl.c
+++ b/drivers/net/bnxt/tf_core/tf_shadow_tbl.c
@@ -687,8 +687,6 @@  tf_shadow_tbl_insert(struct tf_shadow_tbl_insert_parms *parms)
 	if (!TF_SHADOW_HB_HANDLE_IS_VALID(sr_entry->hb_handle))
 		return 0;
 
-	sr_entry->refcnt = 1;
-
 	return 0;
 }
 
diff --git a/drivers/net/bnxt/tf_core/tf_shadow_tcam.c b/drivers/net/bnxt/tf_core/tf_shadow_tcam.c
index a0130d6a8..e2c347a1e 100644
--- a/drivers/net/bnxt/tf_core/tf_shadow_tcam.c
+++ b/drivers/net/bnxt/tf_core/tf_shadow_tcam.c
@@ -472,6 +472,7 @@  tf_shadow_tcam_bind_index(struct tf_shadow_tcam_bind_index_parms *parms)
 	/* Write the result table */
 	sr_entry->key_size = parms->key_size;
 	sr_entry->hb_handle = parms->hb_handle;
+	sr_entry->refcnt = 1;
 
 	return 0;
 }
@@ -738,7 +739,6 @@  tf_shadow_tcam_insert(struct tf_shadow_tcam_insert_parms *parms)
 
 	memcpy(sr_entry->result, sparms->result, sparms->result_size);
 	sr_entry->result_size = sparms->result_size;
-	sr_entry->refcnt = 1;
 
 	return 0;
 }