2023-07-05 03:52:37

by Ratheesh Kannoth

[permalink] [raw]
Subject: [PATCH net] octeontx2-af: Promisc enable/disable through mbox

In Legacy silicon, promisc mode is only modified
through CGX mbox messages. In CN10KB silicon, it modified
from CGX mbox and NIX. This breaks legacy application
behaviour. Fix this by removing call from NIX.

Fixes: d6c9784baf59 ("octeontx2-af: Invoke exact match functions if supported")
Signed-off-by: Ratheesh Kannoth <[email protected]>
---
.../ethernet/marvell/octeontx2/af/rvu_nix.c | 11 ++---------
.../marvell/octeontx2/af/rvu_npc_hash.c | 19 +++++++++++++++++--
2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 0d745ae1cc9a..04b0e885f9d2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -4069,21 +4069,14 @@ int rvu_mbox_handler_nix_set_rx_mode(struct rvu *rvu, struct nix_rx_mode *req,
}

/* install/uninstall promisc entry */
- if (promisc) {
+ if (promisc)
rvu_npc_install_promisc_entry(rvu, pcifunc, nixlf,
pfvf->rx_chan_base,
pfvf->rx_chan_cnt);
-
- if (rvu_npc_exact_has_match_table(rvu))
- rvu_npc_exact_promisc_enable(rvu, pcifunc);
- } else {
+ else
if (!nix_rx_multicast)
rvu_npc_enable_promisc_entry(rvu, pcifunc, nixlf, false);

- if (rvu_npc_exact_has_match_table(rvu))
- rvu_npc_exact_promisc_disable(rvu, pcifunc);
- }
-
return 0;
}

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
index 9f11c1e40737..7ee7bc256bde 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_hash.c
@@ -1164,8 +1164,10 @@ static u16 __rvu_npc_exact_cmd_rules_cnt_update(struct rvu *rvu, int drop_mcam_i
{
struct npc_exact_table *table;
u16 *cnt, old_cnt;
+ bool promisc;

table = rvu->hw->table;
+ promisc = table->promisc_mode[drop_mcam_idx];

cnt = &table->cnt_cmd_rules[drop_mcam_idx];
old_cnt = *cnt;
@@ -1177,13 +1179,16 @@ static u16 __rvu_npc_exact_cmd_rules_cnt_update(struct rvu *rvu, int drop_mcam_i

*enable_or_disable_cam = false;

- /* If all rules are deleted, disable cam */
+ if (promisc)
+ goto done;
+
+ /* If all rules are deleted and not already in promisc mode; disable cam */
if (!*cnt && val < 0) {
*enable_or_disable_cam = true;
goto done;
}

- /* If rule got added, enable cam */
+ /* If rule got added and not already in promisc mode; enable cam */
if (!old_cnt && val > 0) {
*enable_or_disable_cam = true;
goto done;
@@ -1462,6 +1467,11 @@ int rvu_npc_exact_promisc_disable(struct rvu *rvu, u16 pcifunc)
*promisc = false;
mutex_unlock(&table->lock);

+ /* Enable drop rule */
+ rvu_npc_enable_mcam_by_entry_index(rvu, drop_mcam_idx, NIX_INTF_RX, true);
+
+ dev_dbg(rvu->dev, "%s: disabled promisc mode (cgx=%d lmac=%d)\n",
+ __func__, cgx_id, lmac_id);
return 0;
}

@@ -1503,6 +1513,11 @@ int rvu_npc_exact_promisc_enable(struct rvu *rvu, u16 pcifunc)
*promisc = true;
mutex_unlock(&table->lock);

+ /* disable drop rule */
+ rvu_npc_enable_mcam_by_entry_index(rvu, drop_mcam_idx, NIX_INTF_RX, false);
+
+ dev_dbg(rvu->dev, "%s: Enabled promisc mode (cgx=%d lmac=%d)\n",
+ __func__, cgx_id, lmac_id);
return 0;
}

--
2.25.1



2023-07-05 13:45:20

by Michal Kubiak

[permalink] [raw]
Subject: Re: [PATCH net] octeontx2-af: Promisc enable/disable through mbox

On Wed, Jul 05, 2023 at 09:08:13AM +0530, Ratheesh Kannoth wrote:
> In Legacy silicon, promisc mode is only modified
> through CGX mbox messages. In CN10KB silicon, it modified
> from CGX mbox and NIX. This breaks legacy application
> behaviour. Fix this by removing call from NIX.
>
> Fixes: d6c9784baf59 ("octeontx2-af: Invoke exact match functions if supported")
> Signed-off-by: Ratheesh Kannoth <[email protected]>

Please follow the Patchwork report.
There are several lines exceeding 80 characters.

Thanks,
Michal

> @@ -1177,13 +1179,16 @@ static u16 __rvu_npc_exact_cmd_rules_cnt_update(struct rvu *rvu, int drop_mcam_i
>
> *enable_or_disable_cam = false;
>
> - /* If all rules are deleted, disable cam */
> + if (promisc)
> + goto done;
> +
> + /* If all rules are deleted and not already in promisc mode; disable cam */

Line length above 80 characters.

> if (!*cnt && val < 0) {
> *enable_or_disable_cam = true;
> goto done;
> }
>
> - /* If rule got added, enable cam */
> + /* If rule got added and not already in promisc mode; enable cam */
> if (!old_cnt && val > 0) {
> *enable_or_disable_cam = true;
> goto done;
> @@ -1462,6 +1467,11 @@ int rvu_npc_exact_promisc_disable(struct rvu *rvu, u16 pcifunc)
> *promisc = false;
> mutex_unlock(&table->lock);
>
> + /* Enable drop rule */
> + rvu_npc_enable_mcam_by_entry_index(rvu, drop_mcam_idx, NIX_INTF_RX, true);

Line length above 80 characters.

> +
> + dev_dbg(rvu->dev, "%s: disabled promisc mode (cgx=%d lmac=%d)\n",
> + __func__, cgx_id, lmac_id);
> return 0;
> }
>
> @@ -1503,6 +1513,11 @@ int rvu_npc_exact_promisc_enable(struct rvu *rvu, u16 pcifunc)
> *promisc = true;
> mutex_unlock(&table->lock);
>
> + /* disable drop rule */
> + rvu_npc_enable_mcam_by_entry_index(rvu, drop_mcam_idx, NIX_INTF_RX, false);

Line length above 80 characters.


2023-07-06 03:30:34

by Ratheesh Kannoth

[permalink] [raw]
Subject: RE: Re: [PATCH net] octeontx2-af: Promisc enable/disable through mbox

> From: Michal Kubiak <[email protected]>
> Sent: Wednesday, July 5, 2023 7:04 PM
> Subject: [EXT] Re: [PATCH net] octeontx2-af: Promisc enable/disable through
> mbox
> Please follow the Patchwork report.
> There are several lines exceeding 80 characters.

I cloned https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git and ./scripts/checkpatch.pl --strict --codespell --patch <patchname> is not showing these errors/warnings.
Could you please help in understanding why this restriction is removed in checkpatch.pl and kept in patchwork script ?.
Is it mandatory to fix these ? ASFAIK, 80 lines restriction is old one when screen size were small.

-Ratheesh



2023-07-06 03:32:57

by Jakub Kicinski

[permalink] [raw]
Subject: Re: Re: [PATCH net] octeontx2-af: Promisc enable/disable through mbox

On Thu, 6 Jul 2023 03:18:55 +0000 Ratheesh Kannoth wrote:
> Is it mandatory to fix these ? ASFAIK, 80 lines restriction is old
> one when screen size were small.

More of an old rule from when people cared if the code was readable.
We use checkpatch with --max-line-length=80 in networking.