On Tue, Jul 30, 2019 at 4:08 PM Saeed Mahameed [off-list ref] wrote:

On Tue, 2019-07-30 at 11:52 -0400, Willem de Bruijn wrote:

quoted

On Mon, Jul 29, 2019 at 7:50 PM Saeed Mahameed [off-list ref]
wrote:

quoted

From: Huy Nguyen <redacted>

When user enables LRO via ethtool and if the RQ mode is legacy,
mlx5e_fix_features drops the request without any explanation.
Add netdev_warn to cover this case.

Fixes: 6c3a823e1e9c ("net/mlx5e: RX, Remove HW LRO support in
legacy RQ")
Signed-off-by: Huy Nguyen <redacted>
Signed-off-by: Saeed Mahameed <redacted>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 47eea6b3a1c3..776eb46d263d 100644

--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c

@@ -3788,9 +3788,10 @@ static netdev_features_t

mlx5e_fix_features(struct net_device *netdev,
                        netdev_warn(netdev, "Dropping C-tag vlan
stripping offload due to S-tag vlan\n");
        }
        if (!MLX5E_GET_PFLAG(params, MLX5E_PFLAG_RX_STRIDING_RQ)) {
-               features &= ~NETIF_F_LRO;
-               if (params->lro_en)
+               if (features & NETIF_F_LRO) {
                        netdev_warn(netdev, "Disabling LRO, not
supported in legacy RQ\n");

This warns about "Disabling LRO" on an enable request?

no, this warning appears only when lro is already enabled and might
conflict with any other feature requested by user (hence
mlx5e_fix_features), e.g when moving away from striding rq in this
example, we will force lro to off.

Ok. The previous commit mentioned "totally remove LRO support in
Legacy RQ". This handles the additional case when moving a queue into
legacy mode that still had LRO enabled. I see.

quoted

More fundamentally, it appears that the device does not advertise
the feature as configurable in netdev_hw_features as of commit
6c3a823e1e9c ("net/mlx5e: RX, Remove HW LRO support in
legacy RQ"), so shouldn't this be caught by the device driver
independent ethtool code?

when hw doesn't support MLX5E_PFLAG_RX_STRIDING_RQ then yes, you will
never hit this code path, but when hw does support
MLX5E_PFLAG_RX_STRIDING_RQ and you want to turn striding rq off, then
lro will be forced to off (if it was enabled in first space) and a
warning msg will be shown.

Got it, thanks.