Re: [net-next 07/10] net/mlx5: Enable PCIe buffer congestion handling workaround via devlink

Alexander Duyck
2018-08-02T00:06:19Z

On Wed, Aug 1, 2018 at 2:52 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Add support for two driver parameters via devlink params interface:
> - Congestion action
> HW mechanism in the PCIe buffer which monitors the amount of
> consumed PCIe buffer per host. This mechanism supports the
> following actions in case of threshold overflow:
> - disabled - NOP (Default)
> - drop
> - mark - Mark CE bit in the CQE of received packet

Any chance you could clarify the differences between "disabled" and
"drop"? I am assuming the "drop" is a head-of-line drop versus the
"disabled" being a incoming packet drop?

Also I still don't see this as necessarily being all that unique of a
feature/issue. Basically being PCIe bus limited is not all that
uncommon of a thing and has existed since the early days of PCI. In
the case of the Intel NICs we just throw a warning and end up dropping
the incoming packets instead of providing the two other options you
have listed.

> - Congestion mode
> - aggressive - Aggressive static trigger threshold (Default)
> - dynamic - Dynamically change the trigger threshold
>
> These driver-specific params enable the NIC HW workaround to handle
> buffer congestion on the current NIC generation.

Is there any documentation anywhere for any of these features? In the
patch set I see you adding interfaces, but I don't see them documented
anywhere.

> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Reviewed-by: Moshe Shemesh <moshe@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/devlink.c | 204 +++++++++++++++++-
> 1 file changed, 203 insertions(+), 1 deletion(-)

Ideally there should be some documentation going into the kernel when
you extend the devlink interface at least so that I know how to use
your new interfaces when you define them. Just updating devlink.c
seems like a messy way to do things.

- Alex