[PATCH net 0/3] bnxt_en: Bug fixes

STALE1723d

Revision v1 of 20 in this series.

8 messages, 4 authors, 2021-11-16 · open the first message on its own page

[PATCH net 0/3] bnxt_en: Bug fixes

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-11-15 07:38:29

This series includes 3 fixes.  The first one fixes a race condition
between devlink reload and SR-IOV configuration.  The second one
fixes a type mismatch warning in devlink fw live patching.  The
last one fixes unwanted OVS TC dmesg error logs when tc-hw-offload is
disabled on bnxt_en.

Edwin Peer (2):
  bnxt_en: extend RTNL to VF check in devlink driver_reinit
  bnxt_en: fix format specifier in live patch error message

Sriharsha Basavapatna (1):
  bnxt_en: reject indirect blk offload when hw-tc-offload is off

 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 9 +++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c      | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.18.1

[PATCH net 1/3] bnxt_en: extend RTNL to VF check in devlink driver_reinit

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-11-15 07:38:34

From: Edwin Peer <redacted>

The fixes the race condition between configuring SR-IOV and devlink
reload.  The SR-IOV configure logic already takes the RTNL lock,
setting sriov_cfg under the lock while changes are underway. Extend
the lock scope in devlink driver_reinit to cover the VF check and
don't run concurrently with SR-IOV configure.

Reported-by: Leon Romanovsky <leon@kernel.org>
Fixes: 228ea8c187d8 ("bnxt_en: implement devlink dev reload driver_reinit")
Cc: Leon Romanovsky <leon@kernel.org>
Reviewed-by: Somnath Kotur <redacted>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Reviewed-by: Andy Gospodarek <redacted>
Signed-off-by: Edwin Peer <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 5c464ea73576..a0a9af402642 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -441,12 +441,13 @@ static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
 
 	switch (action) {
 	case DEVLINK_RELOAD_ACTION_DRIVER_REINIT: {
-		if (BNXT_PF(bp) && bp->pf.active_vfs) {
+		rtnl_lock();
+		if (BNXT_PF(bp) && (bp->pf.active_vfs || bp->sriov_cfg)) {
 			NL_SET_ERR_MSG_MOD(extack,
-					   "reload is unsupported when VFs are allocated");
+					   "reload is unsupported while VFs are allocated or being configured");
+			rtnl_unlock();
 			return -EOPNOTSUPP;
 		}
-		rtnl_lock();
 		if (bp->dev->reg_state == NETREG_UNREGISTERED) {
 			rtnl_unlock();
 			return -ENODEV;
-- 
2.18.1

[PATCH net 2/3] bnxt_en: fix format specifier in live patch error message

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-11-15 07:38:38

From: Edwin Peer <redacted>

This fixes type mismatch warning.

Reported-by: kernel test robot <redacted>
Fixes: 3c4153394e2c ("bnxt_en: implement firmware live patching")
Signed-off-by: Edwin Peer <redacted>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index a0a9af402642..6fe9e9b59f83 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -360,7 +360,7 @@ bnxt_dl_livepatch_report_err(struct bnxt *bp, struct netlink_ext_ack *extack,
 		NL_SET_ERR_MSG_MOD(extack, "Live patch already applied");
 		break;
 	default:
-		netdev_err(bp->dev, "Unexpected live patch error: %hhd\n", err);
+		netdev_err(bp->dev, "Unexpected live patch error: %d\n", err);
 		NL_SET_ERR_MSG_MOD(extack, "Failed to activate live patch");
 		break;
 	}
-- 
2.18.1

[PATCH net 3/3] bnxt_en: reject indirect blk offload when hw-tc-offload is off

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-11-15 07:38:51

From: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>

The driver does not check if hw-tc-offload is enabled for the device
before offloading a flow in the context of indirect block callback.
Fix this by checking NETIF_F_HW_TC in the features flag and rejecting
the offload request.  This will avoid unnecessary dmesg error logs when
hw-tc-offload is disabled, such as these:

bnxt_en 0000:19:00.1 eno2np1: dev(ifindex=294) not on same switch
bnxt_en 0000:19:00.1 eno2np1: Error: bnxt_tc_add_flow: cookie=0xffff8dace1c88000 error=-22
bnxt_en 0000:19:00.0 eno1np0: dev(ifindex=294) not on same switch
bnxt_en 0000:19:00.0 eno1np0: Error: bnxt_tc_add_flow: cookie=0xffff8dace1c88000 error=-22

Reported-by: Marcelo Ricardo Leitner <redacted>
Fixes: 627c89d00fb9 ("bnxt_en: flow_offload: offload tunnel decap rules via indirect callbacks")
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index e6a4a768b10b..1471b6130a2b 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -1868,7 +1868,7 @@ static int bnxt_tc_setup_indr_block_cb(enum tc_setup_type type,
 	struct flow_cls_offload *flower = type_data;
 	struct bnxt *bp = priv->bp;
 
-	if (flower->common.chain_index)
+	if (!tc_cls_can_offload_and_chain0(bp->dev, type_data))
 		return -EOPNOTSUPP;
 
 	switch (type) {
-- 
2.18.1

Re: [PATCH net 0/3] bnxt_en: Bug fixes

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-15 14:21:57

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller [off-list ref]:

On Mon, 15 Nov 2021 02:37:58 -0500 you wrote:
This series includes 3 fixes.  The first one fixes a race condition
between devlink reload and SR-IOV configuration.  The second one
fixes a type mismatch warning in devlink fw live patching.  The
last one fixes unwanted OVS TC dmesg error logs when tc-hw-offload is
disabled on bnxt_en.

Edwin Peer (2):
  bnxt_en: extend RTNL to VF check in devlink driver_reinit
  bnxt_en: fix format specifier in live patch error message

[...]
Here is the summary with links:
  - [net,1/3] bnxt_en: extend RTNL to VF check in devlink driver_reinit
    https://git.kernel.org/netdev/net/c/46d08f55d24e
  - [net,2/3] bnxt_en: fix format specifier in live patch error message
    https://git.kernel.org/netdev/net/c/b68a1a933fe4
  - [net,3/3] bnxt_en: reject indirect blk offload when hw-tc-offload is off
    https://git.kernel.org/netdev/net/c/b0757491a118

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net 1/3] bnxt_en: extend RTNL to VF check in devlink driver_reinit

From: kernel test robot <hidden>
Date: 2021-11-15 16:32:00

Hi Michael,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/0day-ci/linux/commits/Michael-Chan/bnxt_en-Bug-fixes/20211115-154005
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 1aa3b2207e889a948049c9a8016cedb0218c2389
config: mips-randconfig-r031-20211115 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project fbe72e41b99dc7994daac300d208a955be3e4a0a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/b17bc036418692ffb8055b7cbcdff979ec378e37
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Michael-Chan/bnxt_en-Bug-fixes/20211115-154005
        git checkout b17bc036418692ffb8055b7cbcdff979ec378e37
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All errors (new ones prefixed by >>):

   drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:363:62: warning: format specifies type 'char' but the argument has type 'int' [-Wformat]
                   netdev_err(bp->dev, "Unexpected live patch error: %hhd\n", err);
                                                                     ~~~~     ^~~
                                                                     %d
quoted
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:445:48: error: no member named 'sriov_cfg' in 'struct bnxt'
                   if (BNXT_PF(bp) && (bp->pf.active_vfs || bp->sriov_cfg)) {
                                                            ~~  ^
   1 warning and 1 error generated.


vim +445 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c

   433	
   434	static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
   435				       enum devlink_reload_action action,
   436				       enum devlink_reload_limit limit,
   437				       struct netlink_ext_ack *extack)
   438	{
   439		struct bnxt *bp = bnxt_get_bp_from_dl(dl);
   440		int rc = 0;
   441	
   442		switch (action) {
   443		case DEVLINK_RELOAD_ACTION_DRIVER_REINIT: {
   444			rtnl_lock();
 > 445			if (BNXT_PF(bp) && (bp->pf.active_vfs || bp->sriov_cfg)) {
   446				NL_SET_ERR_MSG_MOD(extack,
   447						   "reload is unsupported while VFs are allocated or being configured");
   448				rtnl_unlock();
   449				return -EOPNOTSUPP;
   450			}
   451			if (bp->dev->reg_state == NETREG_UNREGISTERED) {
   452				rtnl_unlock();
   453				return -ENODEV;
   454			}
   455			bnxt_ulp_stop(bp);
   456			if (netif_running(bp->dev)) {
   457				rc = bnxt_close_nic(bp, true, true);
   458				if (rc) {
   459					NL_SET_ERR_MSG_MOD(extack, "Failed to close");
   460					dev_close(bp->dev);
   461					rtnl_unlock();
   462					break;
   463				}
   464			}
   465			bnxt_vf_reps_free(bp);
   466			rc = bnxt_hwrm_func_drv_unrgtr(bp);
   467			if (rc) {
   468				NL_SET_ERR_MSG_MOD(extack, "Failed to deregister");
   469				if (netif_running(bp->dev))
   470					dev_close(bp->dev);
   471				rtnl_unlock();
   472				break;
   473			}
   474			bnxt_cancel_reservations(bp, false);
   475			bnxt_free_ctx_mem(bp);
   476			kfree(bp->ctx);
   477			bp->ctx = NULL;
   478			break;
   479		}
   480		case DEVLINK_RELOAD_ACTION_FW_ACTIVATE: {
   481			if (limit == DEVLINK_RELOAD_LIMIT_NO_RESET)
   482				return bnxt_dl_livepatch_activate(bp, extack);
   483			if (~bp->fw_cap & BNXT_FW_CAP_HOT_RESET) {
   484				NL_SET_ERR_MSG_MOD(extack, "Device not capable, requires reboot");
   485				return -EOPNOTSUPP;
   486			}
   487			if (!bnxt_hwrm_reset_permitted(bp)) {
   488				NL_SET_ERR_MSG_MOD(extack,
   489						   "Reset denied by firmware, it may be inhibited by remote driver");
   490				return -EPERM;
   491			}
   492			rtnl_lock();
   493			if (bp->dev->reg_state == NETREG_UNREGISTERED) {
   494				rtnl_unlock();
   495				return -ENODEV;
   496			}
   497			if (netif_running(bp->dev))
   498				set_bit(BNXT_STATE_FW_ACTIVATE, &bp->state);
   499			rc = bnxt_hwrm_firmware_reset(bp->dev,
   500						      FW_RESET_REQ_EMBEDDED_PROC_TYPE_CHIP,
   501						      FW_RESET_REQ_SELFRST_STATUS_SELFRSTASAP,
   502						      FW_RESET_REQ_FLAGS_RESET_GRACEFUL |
   503						      FW_RESET_REQ_FLAGS_FW_ACTIVATION);
   504			if (rc) {
   505				NL_SET_ERR_MSG_MOD(extack, "Failed to activate firmware");
   506				clear_bit(BNXT_STATE_FW_ACTIVATE, &bp->state);
   507				rtnl_unlock();
   508			}
   509			break;
   510		}
   511		default:
   512			rc = -EOPNOTSUPP;
   513		}
   514	
   515		return rc;
   516	}
   517	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH net 1/3] bnxt_en: extend RTNL to VF check in devlink driver_reinit

From: Jakub Kicinski <kuba@kernel.org>
Date: 2021-11-16 14:40:45

On Tue, 16 Nov 2021 00:30:34 +0800 kernel test robot wrote:
quoted
quoted
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:445:48: error: no member named 'sriov_cfg' in 'struct bnxt'  
                   if (BNXT_PF(bp) && (bp->pf.active_vfs || bp->sriov_cfg)) {
                                                            ~~  ^
Hi Michael, is this a false positive? Is the fix coming?

Re: [PATCH net 1/3] bnxt_en: extend RTNL to VF check in devlink driver_reinit

From: Michael Chan <michael.chan@broadcom.com>
Date: 2021-11-16 16:20:41

On Tue, Nov 16, 2021 at 6:39 AM Jakub Kicinski [off-list ref] wrote:
On Tue, 16 Nov 2021 00:30:34 +0800 kernel test robot wrote:
quoted
quoted
quoted
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c:445:48: error: no member named 'sriov_cfg' in 'struct bnxt'
                   if (BNXT_PF(bp) && (bp->pf.active_vfs || bp->sriov_cfg)) {
                                                            ~~  ^
Hi Michael, is this a false positive? Is the fix coming?
It's a real error when CONFIG_BNXT_SRIOV is not enabled.  I'll send
the fix later today.  Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help