[PATCH net v2 0/2] octeontx2-af: Bug fixes for KPU profile and VF RX mode

[PATCH net v2 0/2] octeontx2-af: Bug fixes for KPU profile and VF RX mode

[nshettyj]

From: Harman Kalra <hkalra@marvell.com>

Hello,

This is version 2 of the patch series targeting the net branch. The second 
patch has been rebased against the current HEAD of the net branch to resolve 
a merge conflict. No logical changes were made to either patch.

The first patch addresses a spurious firmware loading warning by
switching to the non-warning variant of the firmware request API when
falling back to alternative loading methods.

The second patch resolves an issue where a VF changing its interface
state could inadvertently delete the RX promiscuous and all-multicast
MCAM rules belonging to the host PF.

Changes in v2:
- Rebased patch 2/2 to resolve a merge conflict on the net branch.
- Patch 1/2 remains unchanged.

Harman Kalra (2):
  octeontx2-af: fix VF bringup affecting PF promiscuous state
  octeontx2-af: suppress kpu profile loading warning

 drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 4 ++--
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.48.1

[PATCH net v2 1/2] octeontx2-af: fix VF bringup affecting PF promiscuous state

[nshettyj]

From: Harman Kalra <hkalra@marvell.com>

Mbox handling of nix_set_rx_mode for a VF with promiscuous and
all_multi flags set to false causes deletion of the PF's promiscuous
and allmulti MCAM rules. This occurs because the APIs that
enable/disable these rules operate only on the PF, even when the
mbox request is made via a VF interface.

Guard both rvu_npc_enable_allmulti_entry() and
rvu_npc_enable_promisc_entry() disable paths with an is_vf() check so
that a VF bringing up or tearing down its interface cannot inadvertently
clear the PF's MCAM rules.

Fixes: 967db3529eca ("octeontx2-af: add support for multicast/promisc packet replication feature")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Nitin Shetty J <nshettyj@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index d8989395e875..a7e0e0e05ad2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -4575,7 +4575,7 @@ int rvu_mbox_handler_nix_set_rx_mode(struct rvu *rvu, struct nix_rx_mode *req,
 		rvu_npc_install_allmulti_entry(rvu, pcifunc, nixlf,
 					       pfvf->rx_chan_base);
 	} else {
-		if (!nix_rx_multicast)
+		if (!nix_rx_multicast && !is_vf(pcifunc))
 			rvu_npc_enable_allmulti_entry(rvu, pcifunc, nixlf, false);
 	}
 
@@ -4585,7 +4585,7 @@ int rvu_mbox_handler_nix_set_rx_mode(struct rvu *rvu, struct nix_rx_mode *req,
 					      pfvf->rx_chan_base,
 					      pfvf->rx_chan_cnt);
 	else
-		if (!nix_rx_multicast)
+		if (!nix_rx_multicast && !is_vf(pcifunc))
 			rvu_npc_enable_promisc_entry(rvu, pcifunc, nixlf, false);
 
 	return 0;
-- 
2.48.1

[PATCH net v2 2/2] octeontx2-af: suppress kpu profile loading warning

[nshettyj]

From: Harman Kalra <hkalra@marvell.com>

There are three ways in which a KPU profile can be loaded
(in high to low priority order):
1. profile image integrated in kernel image
2. firmware database method
3. default profile

In most cases the profile is loaded using the 2nd method, which
causes a spurious warning from the Linux firmware subsystem (method 1)
due to the absence of firmware in the kernel image.

Replace request_firmware_direct() with firmware_request_nowarn() to
suppress such warnings when no image is integrated into the kernel image.

Fixes: c0c9ac88156a ("octeontx2-af: npc: Support for custom KPU profile from filesystem")
Signed-off-by: Harman Kalra <hkalra@marvell.com>
Signed-off-by: Nitin Shetty J <nshettyj@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
index c7bc0b3a29b9..007d3f22b0c9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
@@ -2246,7 +2246,7 @@ static int npc_load_kpu_profile_from_fs(struct rvu *rvu)
 
 	strcat(path, kpu_profile);
 
-	if (request_firmware_direct(&fw, path, rvu->dev))
+	if (firmware_request_nowarn(&fw, path, rvu->dev))
 		return -ENOENT;
 
 	dev_info(rvu->dev, "Loading KPU profile from filesystem: %s\n",
-- 
2.48.1

Re: [PATCH net v2 2/2] octeontx2-af: suppress kpu profile loading warning

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
octeontx2-af: suppress kpu profile loading warning

This commit replaces request_firmware_direct() with
firmware_request_nowarn() in npc_load_kpu_profile_from_fs() to suppress
a warning observed when the KPU profile is loaded via the firmware
database method rather than being integrated into the kernel image.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> index c7bc0b3a29b9..007d3f22b0c9 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc.c
> @@ -2246,7 +2246,7 @@ static int npc_load_kpu_profile_from_fs(struct rvu *rvu)
>  
>  	strcat(path, kpu_profile);
>  
> -	if (request_firmware_direct(&fw, path, rvu->dev))
> +	if (firmware_request_nowarn(&fw, path, rvu->dev))

[Medium]
Does this actually change the warning behavior the commit message
describes?

Looking at the two helpers in drivers/base/firmware_loader/main.c, both
pass FW_OPT_NO_WARN into _request_firmware():

  int firmware_request_nowarn(...) {
        ...
        ret = _request_firmware(firmware, name, device, NULL, 0, 0,
                                FW_OPT_UEVENT | FW_OPT_NO_WARN);
        ...
  }

  int request_firmware_direct(...) {
        ...
        ret = _request_firmware(firmware_p, name, device, NULL, 0, 0,
                                FW_OPT_UEVENT | FW_OPT_NO_WARN |
                                FW_OPT_NOFALLBACK_SYSFS);
        ...
  }

The dev_warn() calls in the firmware loader hot path (for example the
"Direct firmware load for %s failed" message, the "loading %s failed"
message, and the sysfs fallback notice) are all gated by
!(opt_flags & FW_OPT_NO_WARN), and have been since commit 1fe6e4f0b0c4
("firmware_loader: Suppress warning on FW_OPT_NO_WARN flag").

Given that request_firmware_direct() already sets FW_OPT_NO_WARN, where
does the spurious warning mentioned in the commit message
("causes a spurious warning from the Linux firmware subsystem ... due
to the absence of firmware in the kernel image") originate?

[Medium]
This change also silently drops FW_OPT_NOFALLBACK_SYSFS, which is not
mentioned in the commit message.

The kerneldoc for request_firmware_direct() says it "doesn't fall back
to usermode helper even if the firmware couldn't be loaded directly
from fs.  Hence it's useful for loading optional firmwares, which
aren't always present, without extra long timeouts of udev."

firmware_request_nowarn()'s kerneldoc states the opposite: "The sysfs
fallback mechanism is enabled if direct filesystem lookup fails."

On kernels built with CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y, or where
the force_sysfs_fallback sysctl is enabled, fw_run_sysfs_fallback() no
longer short-circuits on FW_OPT_NOFALLBACK_SYSFS, so the absent-firmware
case will now invoke firmware_fallback_sysfs() and wait on udev for the
loading timeout (default 60s) before returning -ENOENT.

The commit message itself states that "in most cases the profile is
loaded using the 2nd method", so this is precisely the optional-firmware
scenario that request_firmware_direct() was designed for. Was the
fallback-sysfs behavior change intentional, and is there a reason not
to keep the no-fallback semantics on configurations that enable the
usermode helper?

>  		return -ENOENT;
>  
>  	dev_info(rvu->dev, "Loading KPU profile from filesystem: %s\n",