All of lore.kernel.org
 help / color / mirror / Atom feed
From: Will Mortensen <will@extrahop.com>
To: netdev@vger.kernel.org
Cc: Will Mortensen <will@extrahop.com>,
	Charlotte Tan <charlotte@extrahop.com>,
	Adham Faris <afaris@nvidia.com>, Aya Levin <ayal@nvidia.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	Moshe Shemesh <moshe@nvidia.com>,
	Saeed Mahameed <saeedm@nvidia.com>
Subject: [PATCH] net/mlx5e: Again mutually exclude RX-FCS and RX-port-timestamp
Date: Thu,  5 Oct 2023 22:37:06 -0700	[thread overview]
Message-ID: <20231006053706.514618-1-will@extrahop.com> (raw)

Commit 1e66220948df8 ("net/mlx5e: Update rx ring hw mtu upon each rx-fcs
flag change") seems to have accidentally inverted the logic added in
commit 0bc73ad46a76 ("net/mlx5e: Mutually exclude RX-FCS and
RX-port-timestamp").

The impact of this is a little unclear since it seems the FCS scattered
with RX-FCS is (usually?) correct regardless.

Fixes: 1e66220948df8 ("net/mlx5e: Update rx ring hw mtu upon each rx-fcs flag change")
Tested-by: Charlotte Tan <charlotte@extrahop.com>
Reviewed-by: Charlotte Tan <charlotte@extrahop.com>
Cc: Adham Faris <afaris@nvidia.com>
Cc: Aya Levin <ayal@nvidia.com>
Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Moshe Shemesh <moshe@nvidia.com>
Cc: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Will Mortensen <will@extrahop.com>
---
For what it's worth, regardless of this change the PCMR register behaves
unexpectedly in our testing on NICs where rx_ts_over_crc_cap is 1 (i.e.
where rx_ts_over_crc is supported), such as ConnectX-7 running firmware
28.37.1014. For example, fcs_chk is always 0, and rx_ts_over_crc can
never be set to 1 after being set to 0. On ConnectX-5, where
rx_ts_over_crc_cap is 0, fcs_chk behaves as expected.

We'll probably be opening a support case about that after we test more,
but I mention it here because it makes FCS-related testing confusing.

 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a2ae791538ed..acb40770cf0c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3952,13 +3952,14 @@ static int set_feature_rx_fcs(struct net_device *netdev, bool enable)
 	struct mlx5e_channels *chs = &priv->channels;
 	struct mlx5e_params new_params;
 	int err;
+	bool rx_ts_over_crc = !enable;
 
 	mutex_lock(&priv->state_lock);
 
 	new_params = chs->params;
 	new_params.scatter_fcs_en = enable;
 	err = mlx5e_safe_switch_params(priv, &new_params, mlx5e_set_rx_port_ts_wrap,
-				       &new_params.scatter_fcs_en, true);
+				       &rx_ts_over_crc, true);
 	mutex_unlock(&priv->state_lock);
 	return err;
 }
-- 
2.34.1


             reply	other threads:[~2023-10-06  5:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06  5:37 Will Mortensen [this message]
2023-10-10  6:31 ` [PATCH] net/mlx5e: Again mutually exclude RX-FCS and RX-port-timestamp Tariq Toukan
2023-10-10  8:52   ` Paolo Abeni
2023-10-10 10:50     ` Tariq Toukan
2023-10-10 13:30 ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231006053706.514618-1-will@extrahop.com \
    --to=will@extrahop.com \
    --cc=afaris@nvidia.com \
    --cc=ayal@nvidia.com \
    --cc=charlotte@extrahop.com \
    --cc=moshe@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.