All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU
@ 2020-09-13 15:46 Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 1/5] mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes Ido Schimmel
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Ido Schimmel <idosch@nvidia.com>

Petr says:

Internal buffer is a part of port headroom used for packets that are
mirrored due to triggers that the Spectrum ASIC considers "egress". Besides
ACL mirroring on port egresss this includes also packets mirrored due to
ECN marking.

This patchset changes the way the internal mirroring buffer is reserved.
Currently the buffer reflects port MTU and speed accurately. In the future,
mlxsw should support dcbnl_setbuffer hook to allow the users to set buffer
sizes by hand. In that case, there might not be enough space for growth of
the internal mirroring buffer due to MTU and speed changes. While vetoing
MTU changes would be merely confusing, port speed changes cannot be vetoed,
and such change would simply lead to issues in packet mirroring.

For these reasons, with these patches the internal mirroring buffer is
derived from maximum MTU and maximum speed achievable on the port.

Patches #1 and #2 introduce a new callback to determine the maximum speed a
given port can achieve.

With patches #3 and #4, the information about, respectively, maximum MTU
and maximum port speed, is kept in struct mlxsw_sp_port.

In patch #5, maximum MTU and maximum speed are used to determine the size
of the internal buffer. MTU update and speed update hooks are dropped,
because they are no longer necessary.

Petr Machata (5):
  mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes
  mlxsw: spectrum_ethtool: Introduce ptys_max_speed callback
  mlxsw: spectrum: Keep maximum MTU around
  mlxsw: spectrum: Keep maximum speed around
  mlxsw: spectrum_span: Derive SBIB from maximum port speed & MTU

 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 43 ++++++----
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  6 +-
 .../mellanox/mlxsw/spectrum_ethtool.c         | 82 ++++++++++++++++---
 .../ethernet/mellanox/mlxsw/spectrum_span.c   | 59 +------------
 4 files changed, 108 insertions(+), 82 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/5] mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
@ 2020-09-13 15:46 ` Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 2/5] mlxsw: spectrum_ethtool: Introduce ptys_max_speed callback Ido Schimmel
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@nvidia.com>

In order to allow reusing the logic, extract from
mlxsw_sp_port_get_link_ksettings() the code to obtain Ethernet protocol
attributes, mlxsw_sp_port_ptys_query().

Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../mellanox/mlxsw/spectrum_ethtool.c         | 38 ++++++++++++++-----
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index f08cad5b5657..f007e58950da 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -842,6 +842,29 @@ mlxsw_sp_port_connector_port(enum mlxsw_reg_ptys_connector_type connector_type)
 	}
 }
 
+static int mlxsw_sp_port_ptys_query(struct mlxsw_sp_port *mlxsw_sp_port,
+				    u32 *p_eth_proto_cap, u32 *p_eth_proto_admin,
+				    u32 *p_eth_proto_oper, u8 *p_connector_type)
+{
+	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+	const struct mlxsw_sp_port_type_speed_ops *ops;
+	char ptys_pl[MLXSW_REG_PTYS_LEN];
+	int err;
+
+	ops = mlxsw_sp->port_type_speed_ops;
+
+	ops->reg_ptys_eth_pack(mlxsw_sp, ptys_pl, mlxsw_sp_port->local_port, 0, false);
+	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ptys), ptys_pl);
+	if (err)
+		return err;
+
+	ops->reg_ptys_eth_unpack(mlxsw_sp, ptys_pl, p_eth_proto_cap, p_eth_proto_admin,
+				 p_eth_proto_oper);
+	if (p_connector_type)
+		*p_connector_type = mlxsw_reg_ptys_connector_type_get(ptys_pl);
+	return 0;
+}
+
 static int mlxsw_sp_port_get_link_ksettings(struct net_device *dev,
 					    struct ethtool_link_ksettings *cmd)
 {
@@ -849,21 +872,17 @@ static int mlxsw_sp_port_get_link_ksettings(struct net_device *dev,
 	struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	const struct mlxsw_sp_port_type_speed_ops *ops;
-	char ptys_pl[MLXSW_REG_PTYS_LEN];
 	u8 connector_type;
 	bool autoneg;
 	int err;
 
-	ops = mlxsw_sp->port_type_speed_ops;
-
-	autoneg = mlxsw_sp_port->link.autoneg;
-	ops->reg_ptys_eth_pack(mlxsw_sp, ptys_pl, mlxsw_sp_port->local_port,
-			       0, false);
-	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(ptys), ptys_pl);
+	err = mlxsw_sp_port_ptys_query(mlxsw_sp_port, &eth_proto_cap, &eth_proto_admin,
+				       &eth_proto_oper, &connector_type);
 	if (err)
 		return err;
-	ops->reg_ptys_eth_unpack(mlxsw_sp, ptys_pl, &eth_proto_cap,
-				 &eth_proto_admin, &eth_proto_oper);
+
+	ops = mlxsw_sp->port_type_speed_ops;
+	autoneg = mlxsw_sp_port->link.autoneg;
 
 	mlxsw_sp_port_get_link_supported(mlxsw_sp, eth_proto_cap,
 					 mlxsw_sp_port->mapping.width, cmd);
@@ -872,7 +891,6 @@ static int mlxsw_sp_port_get_link_ksettings(struct net_device *dev,
 					 mlxsw_sp_port->mapping.width, cmd);
 
 	cmd->base.autoneg = autoneg ? AUTONEG_ENABLE : AUTONEG_DISABLE;
-	connector_type = mlxsw_reg_ptys_connector_type_get(ptys_pl);
 	cmd->base.port = mlxsw_sp_port_connector_port(connector_type);
 	ops->from_ptys_speed_duplex(mlxsw_sp, netif_carrier_ok(dev),
 				    eth_proto_oper, cmd);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/5] mlxsw: spectrum_ethtool: Introduce ptys_max_speed callback
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 1/5] mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes Ido Schimmel
@ 2020-09-13 15:46 ` Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 3/5] mlxsw: spectrum: Keep maximum MTU around Ido Schimmel
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@nvidia.com>

The SBIB register configures the size of an internal buffer that the
Spectrum ASICs use when mirroring traffic on egress. This size should be
taken into account when validating that the port headroom buffers are not
larger than the chip can handle. Up until now this was not done, which is
incidentally not a problem, because the priority group buffers that mlxsw
auto-configures are small enough that the boundary condition could not be
violated.

When dcbnl_setbuffer is implemented, the user gets control over sizes of PG
buffers, and they might overshoot the headroom capacity. However the size
of the SBIB buffer depends on port speed, which cannot be vetoed. There is
obviously no way to retroactively push back on requests for overlarge PG
buffers, or reject an overlarge MTU, or cancel losslessness of a certain
PG.

Therefore, instead of taking into account the current speed when
calculating SBIB buffer size, take into account the maximum speed that a
port with given Ethernet protocol capabilities can have.

To that end, add a new ethtool callback, ptys_max_speed, which determines
this maximum speed.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  1 +
 .../mellanox/mlxsw/spectrum_ethtool.c         | 44 +++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 5240bf11b6c4..007e97e99ec8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -331,6 +331,7 @@ struct mlxsw_sp_port_type_speed_ops {
 	void (*from_ptys_speed_duplex)(struct mlxsw_sp *mlxsw_sp,
 				       bool carrier_ok, u32 ptys_eth_proto,
 				       struct ethtool_link_ksettings *cmd);
+	int (*ptys_max_speed)(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed);
 	u32 (*to_ptys_advert_link)(struct mlxsw_sp *mlxsw_sp, u8 width,
 				   const struct ethtool_link_ksettings *cmd);
 	u32 (*to_ptys_speed)(struct mlxsw_sp *mlxsw_sp, u8 width, u32 speed);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
index f007e58950da..6ee0479b189f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_ethtool.c
@@ -1162,6 +1162,27 @@ mlxsw_sp1_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
 		cmd->base.duplex = DUPLEX_FULL;
 }
 
+static int mlxsw_sp1_ptys_max_speed(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed)
+{
+	u32 eth_proto_cap;
+	u32 max_speed = 0;
+	int err;
+	int i;
+
+	err = mlxsw_sp_port_ptys_query(mlxsw_sp_port, &eth_proto_cap, NULL, NULL, NULL);
+	if (err)
+		return err;
+
+	for (i = 0; i < MLXSW_SP1_PORT_LINK_MODE_LEN; i++) {
+		if ((eth_proto_cap & mlxsw_sp1_port_link_mode[i].mask) &&
+		    mlxsw_sp1_port_link_mode[i].speed > max_speed)
+			max_speed = mlxsw_sp1_port_link_mode[i].speed;
+	}
+
+	*p_max_speed = max_speed;
+	return 0;
+}
+
 static u32
 mlxsw_sp1_to_ptys_advert_link(struct mlxsw_sp *mlxsw_sp, u8 width,
 			      const struct ethtool_link_ksettings *cmd)
@@ -1211,6 +1232,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp1_port_type_speed_ops = {
 	.from_ptys_link			= mlxsw_sp1_from_ptys_link,
 	.from_ptys_speed		= mlxsw_sp1_from_ptys_speed,
 	.from_ptys_speed_duplex		= mlxsw_sp1_from_ptys_speed_duplex,
+	.ptys_max_speed			= mlxsw_sp1_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp1_to_ptys_advert_link,
 	.to_ptys_speed			= mlxsw_sp1_to_ptys_speed,
 	.reg_ptys_eth_pack		= mlxsw_sp1_reg_ptys_eth_pack,
@@ -1548,6 +1570,27 @@ mlxsw_sp2_from_ptys_speed_duplex(struct mlxsw_sp *mlxsw_sp, bool carrier_ok,
 		cmd->base.duplex = DUPLEX_FULL;
 }
 
+static int mlxsw_sp2_ptys_max_speed(struct mlxsw_sp_port *mlxsw_sp_port, u32 *p_max_speed)
+{
+	u32 eth_proto_cap;
+	u32 max_speed = 0;
+	int err;
+	int i;
+
+	err = mlxsw_sp_port_ptys_query(mlxsw_sp_port, &eth_proto_cap, NULL, NULL, NULL);
+	if (err)
+		return err;
+
+	for (i = 0; i < MLXSW_SP2_PORT_LINK_MODE_LEN; i++) {
+		if ((eth_proto_cap & mlxsw_sp2_port_link_mode[i].mask) &&
+		    mlxsw_sp2_port_link_mode[i].speed > max_speed)
+			max_speed = mlxsw_sp2_port_link_mode[i].speed;
+	}
+
+	*p_max_speed = max_speed;
+	return 0;
+}
+
 static bool
 mlxsw_sp2_test_bit_ethtool(const struct mlxsw_sp2_port_link_mode *link_mode,
 			   const unsigned long *mode)
@@ -1617,6 +1660,7 @@ const struct mlxsw_sp_port_type_speed_ops mlxsw_sp2_port_type_speed_ops = {
 	.from_ptys_link			= mlxsw_sp2_from_ptys_link,
 	.from_ptys_speed		= mlxsw_sp2_from_ptys_speed,
 	.from_ptys_speed_duplex		= mlxsw_sp2_from_ptys_speed_duplex,
+	.ptys_max_speed			= mlxsw_sp2_ptys_max_speed,
 	.to_ptys_advert_link		= mlxsw_sp2_to_ptys_advert_link,
 	.to_ptys_speed			= mlxsw_sp2_to_ptys_speed,
 	.reg_ptys_eth_pack		= mlxsw_sp2_reg_ptys_eth_pack,
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 3/5] mlxsw: spectrum: Keep maximum MTU around
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 1/5] mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 2/5] mlxsw: spectrum_ethtool: Introduce ptys_max_speed callback Ido Schimmel
@ 2020-09-13 15:46 ` Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 4/5] mlxsw: spectrum: Keep maximum speed around Ido Schimmel
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@nvidia.com>

The maximum port MTU depends on port type. On Spectrum, mlxsw configures
all ports as Ethernet ports, and the maximum MTU therefore never changes.
Besides checking MTU configuration, maximum MTU will also be handy when
setting SBIB, the internal buffer used for traffic mirroring. Therefore,
keep it in struct mlxsw_sp_port for easy access.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.c    | 25 +++++++++++++++----
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  1 +
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 4186e29119c2..a68e62256bee 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -590,21 +590,28 @@ static int mlxsw_sp_port_dev_addr_init(struct mlxsw_sp_port *mlxsw_sp_port)
 	return mlxsw_sp_port_dev_addr_set(mlxsw_sp_port, addr);
 }
 
-static int mlxsw_sp_port_mtu_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 mtu)
+static int mlxsw_sp_port_max_mtu_get(struct mlxsw_sp_port *mlxsw_sp_port, int *p_max_mtu)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	char pmtu_pl[MLXSW_REG_PMTU_LEN];
-	int max_mtu;
 	int err;
 
-	mtu += MLXSW_TXHDR_LEN + ETH_HLEN;
 	mlxsw_reg_pmtu_pack(pmtu_pl, mlxsw_sp_port->local_port, 0);
 	err = mlxsw_reg_query(mlxsw_sp->core, MLXSW_REG(pmtu), pmtu_pl);
 	if (err)
 		return err;
-	max_mtu = mlxsw_reg_pmtu_max_mtu_get(pmtu_pl);
 
-	if (mtu > max_mtu)
+	*p_max_mtu = mlxsw_reg_pmtu_max_mtu_get(pmtu_pl);
+	return 0;
+}
+
+static int mlxsw_sp_port_mtu_set(struct mlxsw_sp_port *mlxsw_sp_port, u16 mtu)
+{
+	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
+	char pmtu_pl[MLXSW_REG_PMTU_LEN];
+
+	mtu += MLXSW_TXHDR_LEN + ETH_HLEN;
+	if (mtu > mlxsw_sp_port->max_mtu)
 		return -EINVAL;
 
 	mlxsw_reg_pmtu_pack(pmtu_pl, mlxsw_sp_port->local_port, mtu);
@@ -1842,6 +1849,13 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 		goto err_port_speed_by_width_set;
 	}
 
+	err = mlxsw_sp_port_max_mtu_get(mlxsw_sp_port, &mlxsw_sp_port->max_mtu);
+	if (err) {
+		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to get maximum MTU\n",
+			mlxsw_sp_port->local_port);
+		goto err_port_max_mtu_get;
+	}
+
 	err = mlxsw_sp_port_mtu_set(mlxsw_sp_port, ETH_DATA_LEN);
 	if (err) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to set MTU\n",
@@ -1966,6 +1980,7 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 err_port_buffers_init:
 err_port_admin_status_set:
 err_port_mtu_set:
+err_port_max_mtu_get:
 err_port_speed_by_width_set:
 err_port_system_port_mapping_set:
 err_dev_addr_init:
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 007e97e99ec8..69e59cf7812f 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -319,6 +319,7 @@ struct mlxsw_sp_port {
 	struct {
 		struct delayed_work speed_update_dw;
 	} span;
+	int max_mtu;
 };
 
 struct mlxsw_sp_port_type_speed_ops {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 4/5] mlxsw: spectrum: Keep maximum speed around
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
                   ` (2 preceding siblings ...)
  2020-09-13 15:46 ` [PATCH net-next 3/5] mlxsw: spectrum: Keep maximum MTU around Ido Schimmel
@ 2020-09-13 15:46 ` Ido Schimmel
  2020-09-13 15:46 ` [PATCH net-next 5/5] mlxsw: spectrum_span: Derive SBIB from maximum port speed & MTU Ido Schimmel
  2020-09-14 21:37 ` [PATCH net-next 0/5] mlxsw: " David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@nvidia.com>

The maximum port speed depends on link modes supported by the port, and for
Ethernet ports is constant. The maximum speed will be handy when setting
SBIB, the internal buffer used for traffic mirroring. Therefore, keep it in
struct mlxsw_sp_port for easy access.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 9 +++++++++
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index a68e62256bee..439f3302c4ff 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1849,6 +1849,14 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 		goto err_port_speed_by_width_set;
 	}
 
+	err = mlxsw_sp->port_type_speed_ops->ptys_max_speed(mlxsw_sp_port,
+							    &mlxsw_sp_port->max_speed);
+	if (err) {
+		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to get maximum speed\n",
+			mlxsw_sp_port->local_port);
+		goto err_max_speed_get;
+	}
+
 	err = mlxsw_sp_port_max_mtu_get(mlxsw_sp_port, &mlxsw_sp_port->max_mtu);
 	if (err) {
 		dev_err(mlxsw_sp->bus_info->dev, "Port %d: Failed to get maximum MTU\n",
@@ -1981,6 +1989,7 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 err_port_admin_status_set:
 err_port_mtu_set:
 err_port_max_mtu_get:
+err_max_speed_get:
 err_port_speed_by_width_set:
 err_port_system_port_mapping_set:
 err_dev_addr_init:
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 69e59cf7812f..824ca4507c7e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -320,6 +320,7 @@ struct mlxsw_sp_port {
 		struct delayed_work speed_update_dw;
 	} span;
 	int max_mtu;
+	u32 max_speed;
 };
 
 struct mlxsw_sp_port_type_speed_ops {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 5/5] mlxsw: spectrum_span: Derive SBIB from maximum port speed & MTU
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
                   ` (3 preceding siblings ...)
  2020-09-13 15:46 ` [PATCH net-next 4/5] mlxsw: spectrum: Keep maximum speed around Ido Schimmel
@ 2020-09-13 15:46 ` Ido Schimmel
  2020-09-14 21:37 ` [PATCH net-next 0/5] mlxsw: " David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Ido Schimmel @ 2020-09-13 15:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, kuba, jiri, petrm, mlxsw, Ido Schimmel

From: Petr Machata <petrm@nvidia.com>

The SBIB register configures the size of an internal buffer that the
Spectrum ASICs use when mirroring traffic on egress. This size should be
taken into account when validating that the port headroom buffers are not
larger than the chip can handle. Up until now this was not done, which is
incidentally not a problem, because the priority group buffers that mlxsw
auto-configures are small enough that the boundary condition could not be
violated.

However when dcbnl_setbuffer is implemented, the user has control over
sizes of PG buffers, and they might overshoot the headroom capacity.
However the size of the SBIB buffer depends on port speed, and that cannot
be vetoed. Therefore SBIB size should be deduced from maximum port speed.

Additionally, once the buffers are configured by hand, the user could get
into an uncomfortable situation where their MTU change requests get vetoed,
because the SBIB does not fit anymore. Therefore derive SBIB size from
maximum permissible MTU as well.

Remove all the code that adjusted the SBIB size whenever speed or MTU
changed.

Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum.c    |  9 ---
 .../net/ethernet/mellanox/mlxsw/spectrum.h    |  3 -
 .../ethernet/mellanox/mlxsw/spectrum_span.c   | 59 ++-----------------
 3 files changed, 4 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 439f3302c4ff..0097c18d0d67 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -1003,9 +1003,6 @@ static int mlxsw_sp_port_change_mtu(struct net_device *dev, int mtu)
 	err = mlxsw_sp_port_headroom_set(mlxsw_sp_port, mtu, pause_en);
 	if (err)
 		return err;
-	err = mlxsw_sp_span_port_mtu_update(mlxsw_sp_port, mtu);
-	if (err)
-		goto err_span_port_mtu_update;
 	err = mlxsw_sp_port_mtu_set(mlxsw_sp_port, mtu);
 	if (err)
 		goto err_port_mtu_set;
@@ -1013,8 +1010,6 @@ static int mlxsw_sp_port_change_mtu(struct net_device *dev, int mtu)
 	return 0;
 
 err_port_mtu_set:
-	mlxsw_sp_span_port_mtu_update(mlxsw_sp_port, dev->mtu);
-err_span_port_mtu_update:
 	mlxsw_sp_port_headroom_set(mlxsw_sp_port, dev->mtu, pause_en);
 	return err;
 }
@@ -1952,8 +1947,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 
 	INIT_DELAYED_WORK(&mlxsw_sp_port->ptp.shaper_dw,
 			  mlxsw_sp->ptp_ops->shaper_work);
-	INIT_DELAYED_WORK(&mlxsw_sp_port->span.speed_update_dw,
-			  mlxsw_sp_span_speed_update_work);
 
 	mlxsw_sp->ports[local_port] = mlxsw_sp_port;
 	err = register_netdev(dev);
@@ -2010,7 +2003,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp->ports[local_port];
 
 	cancel_delayed_work_sync(&mlxsw_sp_port->periodic_hw_stats.update_dw);
-	cancel_delayed_work_sync(&mlxsw_sp_port->span.speed_update_dw);
 	cancel_delayed_work_sync(&mlxsw_sp_port->ptp.shaper_dw);
 	mlxsw_sp_port_ptp_clear(mlxsw_sp_port);
 	mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
@@ -2414,7 +2406,6 @@ static void mlxsw_sp_pude_event_func(const struct mlxsw_reg_info *reg,
 		netdev_info(mlxsw_sp_port->dev, "link up\n");
 		netif_carrier_on(mlxsw_sp_port->dev);
 		mlxsw_core_schedule_dw(&mlxsw_sp_port->ptp.shaper_dw, 0);
-		mlxsw_core_schedule_dw(&mlxsw_sp_port->span.speed_update_dw, 0);
 	} else {
 		netdev_info(mlxsw_sp_port->dev, "link down\n");
 		netif_carrier_off(mlxsw_sp_port->dev);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 824ca4507c7e..c8077eddf0a8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -316,9 +316,6 @@ struct mlxsw_sp_port {
 		struct mlxsw_sp_ptp_port_stats stats;
 	} ptp;
 	u8 split_base_local_port;
-	struct {
-		struct delayed_work speed_update_dw;
-	} span;
 	int max_mtu;
 	u32 max_speed;
 };
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
index 1d18e41ab255..38b3131c4027 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_span.c
@@ -977,21 +977,14 @@ static u32 mlxsw_sp_span_buffsize_get(struct mlxsw_sp *mlxsw_sp, int mtu,
 }
 
 static int
-mlxsw_sp_span_port_buffer_update(struct mlxsw_sp_port *mlxsw_sp_port, u16 mtu)
+mlxsw_sp_span_port_buffer_enable(struct mlxsw_sp_port *mlxsw_sp_port)
 {
 	struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
 	char sbib_pl[MLXSW_REG_SBIB_LEN];
 	u32 buffsize;
-	u32 speed;
-	int err;
-
-	err = mlxsw_sp_port_speed_get(mlxsw_sp_port, &speed);
-	if (err)
-		return err;
-	if (speed == SPEED_UNKNOWN)
-		speed = 0;
 
-	buffsize = mlxsw_sp_span_buffsize_get(mlxsw_sp, speed, mtu);
+	buffsize = mlxsw_sp_span_buffsize_get(mlxsw_sp, mlxsw_sp_port->max_speed,
+					      mlxsw_sp_port->max_mtu);
 	buffsize = mlxsw_sp_port_headroom_8x_adjust(mlxsw_sp_port, buffsize);
 	mlxsw_reg_sbib_pack(sbib_pl, mlxsw_sp_port->local_port, buffsize);
 	return mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(sbib), sbib_pl);
@@ -1021,48 +1014,6 @@ mlxsw_sp_span_analyzed_port_find(struct mlxsw_sp_span *span, u8 local_port,
 	return NULL;
 }
 
-int mlxsw_sp_span_port_mtu_update(struct mlxsw_sp_port *port, u16 mtu)
-{
-	struct mlxsw_sp *mlxsw_sp = port->mlxsw_sp;
-	int err = 0;
-
-	/* If port is egress mirrored, the shared buffer size should be
-	 * updated according to the mtu value
-	 */
-	mutex_lock(&mlxsw_sp->span->analyzed_ports_lock);
-
-	if (mlxsw_sp_span_analyzed_port_find(mlxsw_sp->span, port->local_port,
-					     false))
-		err = mlxsw_sp_span_port_buffer_update(port, mtu);
-
-	mutex_unlock(&mlxsw_sp->span->analyzed_ports_lock);
-
-	return err;
-}
-
-void mlxsw_sp_span_speed_update_work(struct work_struct *work)
-{
-	struct delayed_work *dwork = to_delayed_work(work);
-	struct mlxsw_sp_port *mlxsw_sp_port;
-	struct mlxsw_sp *mlxsw_sp;
-
-	mlxsw_sp_port = container_of(dwork, struct mlxsw_sp_port,
-				     span.speed_update_dw);
-
-	/* If port is egress mirrored, the shared buffer size should be
-	 * updated according to the speed value.
-	 */
-	mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
-	mutex_lock(&mlxsw_sp->span->analyzed_ports_lock);
-
-	if (mlxsw_sp_span_analyzed_port_find(mlxsw_sp->span,
-					     mlxsw_sp_port->local_port, false))
-		mlxsw_sp_span_port_buffer_update(mlxsw_sp_port,
-						 mlxsw_sp_port->dev->mtu);
-
-	mutex_unlock(&mlxsw_sp->span->analyzed_ports_lock);
-}
-
 static const struct mlxsw_sp_span_entry_ops *
 mlxsw_sp_span_entry_ops(struct mlxsw_sp *mlxsw_sp,
 			const struct net_device *to_dev)
@@ -1180,9 +1131,7 @@ mlxsw_sp_span_analyzed_port_create(struct mlxsw_sp_span *span,
 	 * does the mirroring.
 	 */
 	if (!ingress) {
-		u16 mtu = mlxsw_sp_port->dev->mtu;
-
-		err = mlxsw_sp_span_port_buffer_update(mlxsw_sp_port, mtu);
+		err = mlxsw_sp_span_port_buffer_enable(mlxsw_sp_port);
 		if (err)
 			goto err_buffer_update;
 	}
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU
  2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
                   ` (4 preceding siblings ...)
  2020-09-13 15:46 ` [PATCH net-next 5/5] mlxsw: spectrum_span: Derive SBIB from maximum port speed & MTU Ido Schimmel
@ 2020-09-14 21:37 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2020-09-14 21:37 UTC (permalink / raw)
  To: idosch; +Cc: netdev, kuba, jiri, petrm, mlxsw, idosch

From: Ido Schimmel <idosch@idosch.org>
Date: Sun, 13 Sep 2020 18:46:04 +0300

> From: Ido Schimmel <idosch@nvidia.com>
> 
> Petr says:
> 
> Internal buffer is a part of port headroom used for packets that are
> mirrored due to triggers that the Spectrum ASIC considers "egress". Besides
> ACL mirroring on port egresss this includes also packets mirrored due to
> ECN marking.
> 
> This patchset changes the way the internal mirroring buffer is reserved.
> Currently the buffer reflects port MTU and speed accurately. In the future,
> mlxsw should support dcbnl_setbuffer hook to allow the users to set buffer
> sizes by hand. In that case, there might not be enough space for growth of
> the internal mirroring buffer due to MTU and speed changes. While vetoing
> MTU changes would be merely confusing, port speed changes cannot be vetoed,
> and such change would simply lead to issues in packet mirroring.
> 
> For these reasons, with these patches the internal mirroring buffer is
> derived from maximum MTU and maximum speed achievable on the port.
> 
> Patches #1 and #2 introduce a new callback to determine the maximum speed a
> given port can achieve.
> 
> With patches #3 and #4, the information about, respectively, maximum MTU
> and maximum port speed, is kept in struct mlxsw_sp_port.
> 
> In patch #5, maximum MTU and maximum speed are used to determine the size
> of the internal buffer. MTU update and speed update hooks are dropped,
> because they are no longer necessary.

Series applied, thank you.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-14 21:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-13 15:46 [PATCH net-next 0/5] mlxsw: Derive SBIB from maximum port speed & MTU Ido Schimmel
2020-09-13 15:46 ` [PATCH net-next 1/5] mlxsw: spectrum_ethtool: Extract a helper to get Ethernet attributes Ido Schimmel
2020-09-13 15:46 ` [PATCH net-next 2/5] mlxsw: spectrum_ethtool: Introduce ptys_max_speed callback Ido Schimmel
2020-09-13 15:46 ` [PATCH net-next 3/5] mlxsw: spectrum: Keep maximum MTU around Ido Schimmel
2020-09-13 15:46 ` [PATCH net-next 4/5] mlxsw: spectrum: Keep maximum speed around Ido Schimmel
2020-09-13 15:46 ` [PATCH net-next 5/5] mlxsw: spectrum_span: Derive SBIB from maximum port speed & MTU Ido Schimmel
2020-09-14 21:37 ` [PATCH net-next 0/5] mlxsw: " David Miller

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.