Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 3/5] net: aquantia: Implement rx/tx flow control ethtools callback
From: Igor Russkikh @ 2018-05-29 12:57 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, David Arcari, Pavel Belous, Igor Russkikh
In-Reply-To: <cover.1527596210.git.igor.russkikh@aquantia.com>

Runtime change of pause frame configuration (rx/tx flow control)
via ethtool.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 .../net/ethernet/aquantia/atlantic/aq_ethtool.c    | 42 ++++++++++++++++++++++
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    |  6 +++-
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c        |  1 +
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 26 ++++++++++++++
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index bc43d29..c679203 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
@@ -285,6 +285,46 @@ static int aq_ethtool_set_coalesce(struct net_device *ndev,
 	return aq_nic_update_interrupt_moderation_settings(aq_nic);
 }
 
+static void aq_ethtool_get_pauseparam(struct net_device *ndev,
+				      struct ethtool_pauseparam *pause)
+{
+	struct aq_nic_s *aq_nic = netdev_priv(ndev);
+
+	pause->autoneg = 0;
+
+	if (aq_nic->aq_hw->aq_nic_cfg->flow_control & AQ_NIC_FC_RX)
+		pause->rx_pause = 1;
+	if (aq_nic->aq_hw->aq_nic_cfg->flow_control & AQ_NIC_FC_TX)
+		pause->tx_pause = 1;
+}
+
+static int aq_ethtool_set_pauseparam(struct net_device *ndev,
+				     struct ethtool_pauseparam *pause)
+{
+	struct aq_nic_s *aq_nic = netdev_priv(ndev);
+	int err = 0;
+
+	if (!aq_nic->aq_fw_ops->set_flow_control)
+		return -EOPNOTSUPP;
+
+	if (pause->autoneg == AUTONEG_ENABLE)
+		return -EOPNOTSUPP;
+
+	if (pause->rx_pause)
+		aq_nic->aq_hw->aq_nic_cfg->flow_control |= AQ_NIC_FC_RX;
+	else
+		aq_nic->aq_hw->aq_nic_cfg->flow_control &= ~AQ_NIC_FC_RX;
+
+	if (pause->tx_pause)
+		aq_nic->aq_hw->aq_nic_cfg->flow_control |= AQ_NIC_FC_TX;
+	else
+		aq_nic->aq_hw->aq_nic_cfg->flow_control &= ~AQ_NIC_FC_TX;
+
+	err = aq_nic->aq_fw_ops->set_flow_control(aq_nic->aq_hw);
+
+	return err;
+}
+
 static void aq_get_ringparam(struct net_device *ndev,
 			     struct ethtool_ringparam *ring)
 {
@@ -353,6 +393,8 @@ const struct ethtool_ops aq_ethtool_ops = {
 	.get_rxfh_indir_size = aq_ethtool_get_rss_indir_size,
 	.get_ringparam       = aq_get_ringparam,
 	.set_ringparam       = aq_set_ringparam,
+	.get_pauseparam      = aq_ethtool_get_pauseparam,
+	.set_pauseparam      = aq_ethtool_set_pauseparam,
 	.get_rxfh_key_size   = aq_ethtool_get_rss_key_size,
 	.get_rxfh            = aq_ethtool_get_rss,
 	.get_rxnfc           = aq_ethtool_get_rxnfc,
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index bbafa4e..14fa76a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -766,10 +766,14 @@ void aq_nic_get_link_ksettings(struct aq_nic_s *self,
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     100baseT_Full);
 
-	if (self->aq_nic_cfg.flow_control)
+	if (self->aq_nic_cfg.flow_control & AQ_NIC_FC_RX)
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     Pause);
 
+	if (self->aq_nic_cfg.flow_control & AQ_NIC_FC_TX)
+		ethtool_link_ksettings_add_link_mode(cmd, advertising,
+						     Asym_Pause);
+
 	if (self->aq_nic_cfg.aq_hw_caps->media_type == AQ_HW_MEDIA_TYPE_FIBRE)
 		ethtool_link_ksettings_add_link_mode(cmd, advertising, FIBRE);
 	else
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 9d0a96d..e1feba5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -834,4 +834,5 @@ const struct aq_fw_ops aq_fw_1x_ops = {
 	.set_state = hw_atl_utils_mpi_set_state,
 	.update_link_status = hw_atl_utils_mpi_get_link_status,
 	.update_stats = hw_atl_utils_update_stats,
+	.set_flow_control = NULL,
 };
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index a4ac592..d2d030a 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -87,6 +87,19 @@ static int aq_fw2x_set_link_speed(struct aq_hw_s *self, u32 speed)
 	return 0;
 }
 
+static void aq_fw2x_set_mpi_flow_control(struct aq_hw_s *self, u32 *mpi_state)
+{
+	if (self->aq_nic_cfg->flow_control & AQ_NIC_FC_RX)
+		*mpi_state |= BIT(CAPS_HI_PAUSE);
+	else
+		*mpi_state &= ~BIT(CAPS_HI_PAUSE);
+
+	if (self->aq_nic_cfg->flow_control & AQ_NIC_FC_TX)
+		*mpi_state |= BIT(CAPS_HI_ASYMMETRIC_PAUSE);
+	else
+		*mpi_state &= ~BIT(CAPS_HI_ASYMMETRIC_PAUSE);
+}
+
 static int aq_fw2x_set_state(struct aq_hw_s *self,
 			     enum hal_atl_utils_fw_state_e state)
 {
@@ -95,6 +108,7 @@ static int aq_fw2x_set_state(struct aq_hw_s *self,
 	switch (state) {
 	case MPI_INIT:
 		mpi_state &= ~BIT(CAPS_HI_LINK_DROP);
+		aq_fw2x_set_mpi_flow_control(self, &mpi_state);
 		break;
 	case MPI_DEINIT:
 		mpi_state |= BIT(CAPS_HI_LINK_DROP);
@@ -201,6 +215,17 @@ static int aq_fw2x_update_stats(struct aq_hw_s *self)
 	return hw_atl_utils_update_stats(self);
 }
 
+static int aq_fw2x_set_flow_control(struct aq_hw_s *self)
+{
+	u32 mpi_state = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR);
+
+	aq_fw2x_set_mpi_flow_control(self, &mpi_state);
+
+	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, mpi_state);
+
+	return 0;
+}
+
 const struct aq_fw_ops aq_fw_2x_ops = {
 	.init = aq_fw2x_init,
 	.deinit = aq_fw2x_deinit,
@@ -210,4 +235,5 @@ const struct aq_fw_ops aq_fw_2x_ops = {
 	.set_state = aq_fw2x_set_state,
 	.update_link_status = aq_fw2x_update_link_status,
 	.update_stats = aq_fw2x_update_stats,
+	.set_flow_control   = aq_fw2x_set_flow_control,
 };
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 2/5] net: aquantia: Improve adapter init/deinit logic
From: Igor Russkikh @ 2018-05-29 12:56 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, David Arcari, Pavel Belous, Igor Russkikh
In-Reply-To: <cover.1527596210.git.igor.russkikh@aquantia.com>

We now pass link drop status to FW on init/deinit. This is required
to inform FW that driver took/released a control on link.
FW then will manage its own state and device power profile based
on this information. To improve management we remove mpi_set
function which ambiguously took both state and speed parameters.

Deinit callback is now a part of FW ops, as it actually manages the FW.

Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h     |  9 ++--
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    |  2 +-
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c  |  1 -
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  |  1 -
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c        | 53 ++++++++++++----------
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c   | 31 ++++++++++++-
 6 files changed, 66 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
index 904cdfd..3aa36d5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
@@ -202,25 +202,28 @@ struct aq_hw_ops {
 
 	int (*hw_get_fw_version)(struct aq_hw_s *self, u32 *fw_version);
 
-	int (*hw_deinit)(struct aq_hw_s *self);
-
 	int (*hw_set_power)(struct aq_hw_s *self, unsigned int power_state);
 };
 
 struct aq_fw_ops {
 	int (*init)(struct aq_hw_s *self);
 
+	int (*deinit)(struct aq_hw_s *self);
+
 	int (*reset)(struct aq_hw_s *self);
 
 	int (*get_mac_permanent)(struct aq_hw_s *self, u8 *mac);
 
 	int (*set_link_speed)(struct aq_hw_s *self, u32 speed);
 
-	int (*set_state)(struct aq_hw_s *self, enum hal_atl_utils_fw_state_e state);
+	int (*set_state)(struct aq_hw_s *self,
+			 enum hal_atl_utils_fw_state_e state);
 
 	int (*update_link_status)(struct aq_hw_s *self);
 
 	int (*update_stats)(struct aq_hw_s *self);
+
+	int (*set_flow_control)(struct aq_hw_s *self);
 };
 
 #endif /* AQ_HW_H */
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 05d4e28..bbafa4e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -884,7 +884,7 @@ void aq_nic_deinit(struct aq_nic_s *self)
 		aq_vec_deinit(aq_vec);
 
 	if (self->power_state == AQ_HW_POWER_STATE_D0) {
-		(void)self->aq_hw_ops->hw_deinit(self->aq_hw);
+		(void)self->aq_fw_ops->deinit(self->aq_hw);
 	} else {
 		(void)self->aq_hw_ops->hw_set_power(self->aq_hw,
 						   self->power_state);
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 7fd6a7e..ed7fe6f 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -877,7 +877,6 @@ static int hw_atl_a0_hw_ring_rx_stop(struct aq_hw_s *self,
 const struct aq_hw_ops hw_atl_ops_a0 = {
 	.hw_set_mac_address   = hw_atl_a0_hw_mac_addr_set,
 	.hw_init              = hw_atl_a0_hw_init,
-	.hw_deinit            = hw_atl_utils_hw_deinit,
 	.hw_set_power         = hw_atl_utils_hw_set_power,
 	.hw_reset             = hw_atl_a0_hw_reset,
 	.hw_start             = hw_atl_a0_hw_start,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 4ea15b9..9dd4f49 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -935,7 +935,6 @@ static int hw_atl_b0_hw_ring_rx_stop(struct aq_hw_s *self,
 const struct aq_hw_ops hw_atl_ops_b0 = {
 	.hw_set_mac_address   = hw_atl_b0_hw_mac_addr_set,
 	.hw_init              = hw_atl_b0_hw_init,
-	.hw_deinit            = hw_atl_utils_hw_deinit,
 	.hw_set_power         = hw_atl_utils_hw_set_power,
 	.hw_reset             = hw_atl_b0_hw_reset,
 	.hw_start             = hw_atl_b0_hw_start,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index e652d86..9d0a96d 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -30,10 +30,11 @@
 #define HW_ATL_MPI_CONTROL_ADR  0x0368U
 #define HW_ATL_MPI_STATE_ADR    0x036CU
 
-#define HW_ATL_MPI_STATE_MSK    0x00FFU
-#define HW_ATL_MPI_STATE_SHIFT  0U
-#define HW_ATL_MPI_SPEED_MSK    0xFFFF0000U
-#define HW_ATL_MPI_SPEED_SHIFT  16U
+#define HW_ATL_MPI_STATE_MSK      0x00FFU
+#define HW_ATL_MPI_STATE_SHIFT    0U
+#define HW_ATL_MPI_SPEED_MSK      0x00FF0000U
+#define HW_ATL_MPI_SPEED_SHIFT    16U
+#define HW_ATL_MPI_DIRTY_WAKE_MSK 0x02000000U
 
 #define HW_ATL_MPI_DAISY_CHAIN_STATUS	0x704
 #define HW_ATL_MPI_BOOT_EXIT_CODE	0x388
@@ -521,23 +522,24 @@ void hw_atl_utils_mpi_read_stats(struct aq_hw_s *self,
 err_exit:;
 }
 
-static int hw_atl_utils_mpi_set_speed(struct aq_hw_s *self, u32 speed)
+int hw_atl_utils_mpi_set_speed(struct aq_hw_s *self, u32 speed)
 {
 	u32 val = aq_hw_read_reg(self, HW_ATL_MPI_CONTROL_ADR);
 
-	val = (val & HW_ATL_MPI_STATE_MSK) | (speed << HW_ATL_MPI_SPEED_SHIFT);
+	val = val & ~HW_ATL_MPI_SPEED_MSK;
+	val |= speed << HW_ATL_MPI_SPEED_SHIFT;
 	aq_hw_write_reg(self, HW_ATL_MPI_CONTROL_ADR, val);
 
 	return 0;
 }
 
-void hw_atl_utils_mpi_set(struct aq_hw_s *self,
-			  enum hal_atl_utils_fw_state_e state,
-			  u32 speed)
+int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
+			       enum hal_atl_utils_fw_state_e state)
 {
 	int err = 0;
 	u32 transaction_id = 0;
 	struct hw_aq_atl_utils_mbox_header mbox;
+	u32 val = aq_hw_read_reg(self, HW_ATL_MPI_CONTROL_ADR);
 
 	if (state == MPI_RESET) {
 		hw_atl_utils_mpi_read_mbox(self, &mbox);
@@ -551,21 +553,21 @@ void hw_atl_utils_mpi_set(struct aq_hw_s *self,
 		if (err < 0)
 			goto err_exit;
 	}
+	/* On interface DEINIT we disable DW (raise bit)
+	 * Otherwise enable DW (clear bit)
+	 */
+	if (state == MPI_DEINIT || state == MPI_POWER)
+		val |= HW_ATL_MPI_DIRTY_WAKE_MSK;
+	else
+		val &= ~HW_ATL_MPI_DIRTY_WAKE_MSK;
 
-	aq_hw_write_reg(self, HW_ATL_MPI_CONTROL_ADR,
-			(speed << HW_ATL_MPI_SPEED_SHIFT) | state);
+	/* Set new state bits */
+	val = val & ~HW_ATL_MPI_STATE_MSK;
+	val |= state & HW_ATL_MPI_STATE_MSK;
 
-err_exit:;
-}
-
-static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
-				      enum hal_atl_utils_fw_state_e state)
-{
-	u32 val = aq_hw_read_reg(self, HW_ATL_MPI_CONTROL_ADR);
-
-	val = state | (val & HW_ATL_MPI_SPEED_MSK);
 	aq_hw_write_reg(self, HW_ATL_MPI_CONTROL_ADR, val);
-	return 0;
+err_exit:
+	return err;
 }
 
 int hw_atl_utils_mpi_get_link_status(struct aq_hw_s *self)
@@ -721,16 +723,18 @@ void hw_atl_utils_hw_chip_features_init(struct aq_hw_s *self, u32 *p)
 	*p = chip_features;
 }
 
-int hw_atl_utils_hw_deinit(struct aq_hw_s *self)
+static int hw_atl_fw1x_deinit(struct aq_hw_s *self)
 {
-	hw_atl_utils_mpi_set(self, MPI_DEINIT, 0x0U);
+	hw_atl_utils_mpi_set_speed(self, 0);
+	hw_atl_utils_mpi_set_state(self, MPI_DEINIT);
 	return 0;
 }
 
 int hw_atl_utils_hw_set_power(struct aq_hw_s *self,
 			      unsigned int power_state)
 {
-	hw_atl_utils_mpi_set(self, MPI_POWER, 0x0U);
+	hw_atl_utils_mpi_set_speed(self, 0);
+	hw_atl_utils_mpi_set_state(self, MPI_POWER);
 	return 0;
 }
 
@@ -823,6 +827,7 @@ int hw_atl_utils_get_fw_version(struct aq_hw_s *self, u32 *fw_version)
 
 const struct aq_fw_ops aq_fw_1x_ops = {
 	.init = hw_atl_utils_mpi_create,
+	.deinit = hw_atl_fw1x_deinit,
 	.reset = NULL,
 	.get_mac_permanent = hw_atl_utils_get_mac_permanent,
 	.set_link_speed = hw_atl_utils_mpi_set_speed,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 8cfce95..a4ac592 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -28,6 +28,10 @@
 #define HW_ATL_FW2X_MPI_STATE_ADDR	0x370
 #define HW_ATL_FW2X_MPI_STATE2_ADDR	0x374
 
+static int aq_fw2x_set_link_speed(struct aq_hw_s *self, u32 speed);
+static int aq_fw2x_set_state(struct aq_hw_s *self,
+			     enum hal_atl_utils_fw_state_e state);
+
 static int aq_fw2x_init(struct aq_hw_s *self)
 {
 	int err = 0;
@@ -39,6 +43,16 @@ static int aq_fw2x_init(struct aq_hw_s *self)
 	return err;
 }
 
+static int aq_fw2x_deinit(struct aq_hw_s *self)
+{
+	int err = aq_fw2x_set_link_speed(self, 0);
+
+	if (!err)
+		err = aq_fw2x_set_state(self, MPI_DEINIT);
+
+	return err;
+}
+
 static enum hw_atl_fw2x_rate link_speed_mask_2fw2x_ratemask(u32 speed)
 {
 	enum hw_atl_fw2x_rate rate = 0;
@@ -76,7 +90,21 @@ static int aq_fw2x_set_link_speed(struct aq_hw_s *self, u32 speed)
 static int aq_fw2x_set_state(struct aq_hw_s *self,
 			     enum hal_atl_utils_fw_state_e state)
 {
-	/* No explicit state in 2x fw */
+	u32 mpi_state = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR);
+
+	switch (state) {
+	case MPI_INIT:
+		mpi_state &= ~BIT(CAPS_HI_LINK_DROP);
+		break;
+	case MPI_DEINIT:
+		mpi_state |= BIT(CAPS_HI_LINK_DROP);
+		break;
+	case MPI_RESET:
+	case MPI_POWER:
+		/* No actions */
+		break;
+	}
+	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, mpi_state);
 	return 0;
 }
 
@@ -175,6 +203,7 @@ static int aq_fw2x_update_stats(struct aq_hw_s *self)
 
 const struct aq_fw_ops aq_fw_2x_ops = {
 	.init = aq_fw2x_init,
+	.deinit = aq_fw2x_deinit,
 	.reset = NULL,
 	.get_mac_permanent = aq_fw2x_get_mac_permanent,
 	.set_link_speed = aq_fw2x_set_link_speed,
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 1/5] net: aquantia: Ethtool based ring size configuration
From: Igor Russkikh @ 2018-05-29 12:56 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, David Arcari, Pavel Belous, Igor Russkikh, Anton Mikaev
In-Reply-To: <cover.1527596210.git.igor.russkikh@aquantia.com>

From: Anton Mikaev <amikaev@aquantia.com>

Implemented ring size setup, min/max validation and reconfiguration in
runtime. NIC level lock is used to prevent collisions on parallel
reconfiguration and interference with periodic service timer job.

Signed-off-by: Anton Mikaev <amikaev@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
 .../net/ethernet/aquantia/atlantic/aq_ethtool.c    | 62 ++++++++++++++++++++++
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h     |  9 +++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    |  9 +++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.h    |  2 +
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c  | 46 ++++++++--------
 .../aquantia/atlantic/hw_atl/hw_atl_a0_internal.h  |  8 +++
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  | 50 ++++++++---------
 .../aquantia/atlantic/hw_atl/hw_atl_b0_internal.h  |  8 +++
 8 files changed, 144 insertions(+), 50 deletions(-)

diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index f2d8063..bc43d29 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
@@ -11,6 +11,7 @@
 
 #include "aq_ethtool.h"
 #include "aq_nic.h"
+#include "aq_vec.h"
 
 static void aq_ethtool_get_regs(struct net_device *ndev,
 				struct ethtool_regs *regs, void *p)
@@ -284,6 +285,65 @@ static int aq_ethtool_set_coalesce(struct net_device *ndev,
 	return aq_nic_update_interrupt_moderation_settings(aq_nic);
 }
 
+static void aq_get_ringparam(struct net_device *ndev,
+			     struct ethtool_ringparam *ring)
+{
+	struct aq_nic_s *aq_nic = netdev_priv(ndev);
+	struct aq_nic_cfg_s *aq_nic_cfg = aq_nic_get_cfg(aq_nic);
+
+	ring->rx_pending = aq_nic_cfg->rxds;
+	ring->tx_pending = aq_nic_cfg->txds;
+
+	ring->rx_max_pending = aq_nic_cfg->aq_hw_caps->rxds_max;
+	ring->tx_max_pending = aq_nic_cfg->aq_hw_caps->txds_max;
+}
+
+static int aq_set_ringparam(struct net_device *ndev,
+			    struct ethtool_ringparam *ring)
+{
+	int err = 0;
+	struct aq_nic_s *aq_nic = netdev_priv(ndev);
+	struct aq_nic_cfg_s *aq_nic_cfg = aq_nic_get_cfg(aq_nic);
+	const struct aq_hw_caps_s *hw_caps = aq_nic_cfg->aq_hw_caps;
+
+	if (ring->rx_mini_pending || ring->rx_jumbo_pending) {
+		err = -EOPNOTSUPP;
+		goto err_exit;
+	}
+
+	spin_lock(&aq_nic->aq_spinlock);
+
+	if (netif_running(ndev))
+		dev_close(ndev);
+
+	aq_nic_free_vectors(aq_nic);
+
+	aq_nic_cfg->rxds = max(ring->rx_pending, hw_caps->rxds_min);
+	aq_nic_cfg->rxds = min(aq_nic_cfg->rxds, hw_caps->rxds_max);
+	aq_nic_cfg->rxds = ALIGN(aq_nic_cfg->rxds, AQ_HW_RXD_MULTIPLE);
+
+	aq_nic_cfg->txds = max(ring->tx_pending, hw_caps->txds_min);
+	aq_nic_cfg->txds = min(aq_nic_cfg->txds, hw_caps->txds_max);
+	aq_nic_cfg->txds = ALIGN(aq_nic_cfg->txds, AQ_HW_TXD_MULTIPLE);
+
+	for (aq_nic->aq_vecs = 0; aq_nic->aq_vecs < aq_nic_cfg->vecs;
+	     aq_nic->aq_vecs++) {
+		aq_nic->aq_vec[aq_nic->aq_vecs] =
+		    aq_vec_alloc(aq_nic, aq_nic->aq_vecs, aq_nic_cfg);
+		if (unlikely(!aq_nic->aq_vec[aq_nic->aq_vecs])) {
+			err = -ENOMEM;
+			goto err_unlock;
+		}
+	}
+	if (!netif_running(ndev))
+		err = dev_open(ndev);
+
+err_unlock:
+	spin_unlock(&aq_nic->aq_spinlock);
+err_exit:
+	return err;
+}
+
 const struct ethtool_ops aq_ethtool_ops = {
 	.get_link            = aq_ethtool_get_link,
 	.get_regs_len        = aq_ethtool_get_regs_len,
@@ -291,6 +351,8 @@ const struct ethtool_ops aq_ethtool_ops = {
 	.get_drvinfo         = aq_ethtool_get_drvinfo,
 	.get_strings         = aq_ethtool_get_strings,
 	.get_rxfh_indir_size = aq_ethtool_get_rss_indir_size,
+	.get_ringparam       = aq_get_ringparam,
+	.set_ringparam       = aq_set_ringparam,
 	.get_rxfh_key_size   = aq_ethtool_get_rss_key_size,
 	.get_rxfh            = aq_ethtool_get_rss,
 	.get_rxnfc           = aq_ethtool_get_rxnfc,
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
index a2d416b..904cdfd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw.h
@@ -24,8 +24,10 @@ struct aq_hw_caps_s {
 	u64 link_speed_msk;
 	unsigned int hw_priv_flags;
 	u32 media_type;
-	u32 rxds;
-	u32 txds;
+	u32 rxds_max;
+	u32 txds_max;
+	u32 rxds_min;
+	u32 txds_min;
 	u32 txhwb_alignment;
 	u32 irq_mask;
 	u32 vecs;
@@ -98,6 +100,9 @@ struct aq_stats_s {
 #define AQ_HW_MEDIA_TYPE_TP    1U
 #define AQ_HW_MEDIA_TYPE_FIBRE 2U
 
+#define AQ_HW_TXD_MULTIPLE 8U
+#define AQ_HW_RXD_MULTIPLE 8U
+
 struct aq_hw_s {
 	atomic_t flags;
 	u8 rbl_enabled:1;
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 1a1a638..05d4e28 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -89,8 +89,8 @@ void aq_nic_cfg_start(struct aq_nic_s *self)
 	aq_nic_rss_init(self, cfg->num_rss_queues);
 
 	/*descriptors */
-	cfg->rxds = min(cfg->aq_hw_caps->rxds, AQ_CFG_RXDS_DEF);
-	cfg->txds = min(cfg->aq_hw_caps->txds, AQ_CFG_TXDS_DEF);
+	cfg->rxds = min(cfg->aq_hw_caps->rxds_max, AQ_CFG_RXDS_DEF);
+	cfg->txds = min(cfg->aq_hw_caps->txds_max, AQ_CFG_TXDS_DEF);
 
 	/*rss rings */
 	cfg->vecs = min(cfg->aq_hw_caps->vecs, AQ_CFG_VECS_DEF);
@@ -158,6 +158,8 @@ static void aq_nic_service_timer_cb(struct timer_list *t)
 	int ctimer = AQ_CFG_SERVICE_TIMER_INTERVAL;
 	int err = 0;
 
+	spin_lock(&self->aq_spinlock);
+
 	if (aq_utils_obj_test(&self->flags, AQ_NIC_FLAGS_IS_NOT_READY))
 		goto err_exit;
 
@@ -175,6 +177,7 @@ static void aq_nic_service_timer_cb(struct timer_list *t)
 		ctimer = max(ctimer / 2, 1);
 
 err_exit:
+	spin_unlock(&self->aq_spinlock);
 	mod_timer(&self->service_timer, jiffies + ctimer);
 }
 
@@ -288,6 +291,8 @@ int aq_nic_init(struct aq_nic_s *self)
 		self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
 		aq_vec_init(aq_vec, self->aq_hw_ops, self->aq_hw);
 
+	spin_lock_init(&self->aq_spinlock);
+
 	netif_carrier_off(self->ndev);
 
 err_exit:
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
index faa533a..aa1cef7 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.h
@@ -81,6 +81,8 @@ struct aq_nic_s {
 	struct pci_dev *pdev;
 	unsigned int msix_entry_mask;
 	u32 irqvecs;
+	/* NIC reconfiguration synchronization */
+	spinlock_t aq_spinlock;
 };
 
 static inline struct device *aq_nic_get_dev(struct aq_nic_s *self)
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 67e2f9f..7fd6a7e 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -19,29 +19,31 @@
 #include "hw_atl_a0_internal.h"
 
 #define DEFAULT_A0_BOARD_BASIC_CAPABILITIES \
-	.is_64_dma = true, \
-	.msix_irqs = 4U, \
-	.irq_mask = ~0U, \
-	.vecs = HW_ATL_A0_RSS_MAX, \
-	.tcs = HW_ATL_A0_TC_MAX, \
-	.rxd_alignment = 1U, \
-	.rxd_size = HW_ATL_A0_RXD_SIZE, \
-	.rxds = 248U, \
-	.txd_alignment = 1U, \
-	.txd_size = HW_ATL_A0_TXD_SIZE, \
-	.txds = 8U * 1024U, \
-	.txhwb_alignment = 4096U, \
-	.tx_rings = HW_ATL_A0_TX_RINGS, \
-	.rx_rings = HW_ATL_A0_RX_RINGS, \
-	.hw_features = NETIF_F_HW_CSUM | \
-			NETIF_F_RXHASH | \
-			NETIF_F_RXCSUM | \
-			NETIF_F_SG | \
-			NETIF_F_TSO, \
+	.is_64_dma = true,		  \
+	.msix_irqs = 4U,		  \
+	.irq_mask = ~0U,		  \
+	.vecs = HW_ATL_A0_RSS_MAX,	  \
+	.tcs = HW_ATL_A0_TC_MAX,	  \
+	.rxd_alignment = 1U,		  \
+	.rxd_size = HW_ATL_A0_RXD_SIZE,   \
+	.rxds_max = HW_ATL_A0_MAX_RXD,    \
+	.rxds_min = HW_ATL_A0_MIN_RXD,    \
+	.txd_alignment = 1U,		  \
+	.txd_size = HW_ATL_A0_TXD_SIZE,   \
+	.txds_max = HW_ATL_A0_MAX_TXD,    \
+	.txds_min = HW_ATL_A0_MIN_RXD,    \
+	.txhwb_alignment = 4096U,	  \
+	.tx_rings = HW_ATL_A0_TX_RINGS,   \
+	.rx_rings = HW_ATL_A0_RX_RINGS,   \
+	.hw_features = NETIF_F_HW_CSUM |  \
+			NETIF_F_RXHASH |  \
+			NETIF_F_RXCSUM |  \
+			NETIF_F_SG |	  \
+			NETIF_F_TSO,	  \
 	.hw_priv_flags = IFF_UNICAST_FLT, \
-	.flow_control = true, \
-	.mtu = HW_ATL_A0_MTU_JUMBO, \
-	.mac_regs_count = 88, \
+	.flow_control = true,		  \
+	.mtu = HW_ATL_A0_MTU_JUMBO,       \
+	.mac_regs_count = 88,		  \
 	.hw_alive_check_addr = 0x10U
 
 const struct aq_hw_caps_s hw_atl_a0_caps_aqc100 = {
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0_internal.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0_internal.h
index 1d88555..3c94cff 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0_internal.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0_internal.h
@@ -88,4 +88,12 @@
 
 #define HW_ATL_A0_FW_VER_EXPECTED 0x01050006U
 
+#define HW_ATL_A0_MIN_RXD \
+	(ALIGN(AQ_CFG_SKB_FRAGS_MAX + 1U, AQ_HW_RXD_MULTIPLE))
+#define HW_ATL_A0_MIN_TXD \
+	(ALIGN(AQ_CFG_SKB_FRAGS_MAX + 1U, AQ_HW_TXD_MULTIPLE))
+
+#define HW_ATL_A0_MAX_RXD 8184U
+#define HW_ATL_A0_MAX_TXD 8184U
+
 #endif /* HW_ATL_A0_INTERNAL_H */
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index 819f6bc..4ea15b9 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -20,30 +20,32 @@
 #include "hw_atl_llh_internal.h"
 
 #define DEFAULT_B0_BOARD_BASIC_CAPABILITIES \
-	.is_64_dma = true,	\
-	.msix_irqs = 4U,	\
-	.irq_mask = ~0U,	\
-	.vecs = HW_ATL_B0_RSS_MAX,	\
-	.tcs = HW_ATL_B0_TC_MAX,	\
-	.rxd_alignment = 1U,		\
-	.rxd_size = HW_ATL_B0_RXD_SIZE, \
-	.rxds = 4U * 1024U,		\
-	.txd_alignment = 1U,		\
-	.txd_size = HW_ATL_B0_TXD_SIZE, \
-	.txds = 8U * 1024U,		\
-	.txhwb_alignment = 4096U,	\
-	.tx_rings = HW_ATL_B0_TX_RINGS, \
-	.rx_rings = HW_ATL_B0_RX_RINGS, \
-	.hw_features = NETIF_F_HW_CSUM | \
-			NETIF_F_RXCSUM | \
-			NETIF_F_RXHASH | \
-			NETIF_F_SG |  \
-			NETIF_F_TSO | \
-			NETIF_F_LRO,  \
-	.hw_priv_flags = IFF_UNICAST_FLT,   \
-	.flow_control = true,		\
-	.mtu = HW_ATL_B0_MTU_JUMBO,	\
-	.mac_regs_count = 88,		\
+	.is_64_dma = true,		  \
+	.msix_irqs = 4U,		  \
+	.irq_mask = ~0U,		  \
+	.vecs = HW_ATL_B0_RSS_MAX,	  \
+	.tcs = HW_ATL_B0_TC_MAX,	  \
+	.rxd_alignment = 1U,		  \
+	.rxd_size = HW_ATL_B0_RXD_SIZE,   \
+	.rxds_max = HW_ATL_B0_MAX_RXD,    \
+	.rxds_min = HW_ATL_B0_MIN_RXD,    \
+	.txd_alignment = 1U,		  \
+	.txd_size = HW_ATL_B0_TXD_SIZE,   \
+	.txds_max = HW_ATL_B0_MAX_TXD,    \
+	.txds_min = HW_ATL_B0_MIN_TXD,    \
+	.txhwb_alignment = 4096U,	  \
+	.tx_rings = HW_ATL_B0_TX_RINGS,   \
+	.rx_rings = HW_ATL_B0_RX_RINGS,   \
+	.hw_features = NETIF_F_HW_CSUM |  \
+			NETIF_F_RXCSUM |  \
+			NETIF_F_RXHASH |  \
+			NETIF_F_SG |      \
+			NETIF_F_TSO |     \
+			NETIF_F_LRO,      \
+	.hw_priv_flags = IFF_UNICAST_FLT, \
+	.flow_control = true,		  \
+	.mtu = HW_ATL_B0_MTU_JUMBO,	  \
+	.mac_regs_count = 88,		  \
 	.hw_alive_check_addr = 0x10U
 
 const struct aq_hw_caps_s hw_atl_b0_caps_aqc100 = {
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0_internal.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0_internal.h
index 405d145..28568f5 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0_internal.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0_internal.h
@@ -142,6 +142,14 @@
 #define HW_ATL_INTR_MODER_MAX  0x1FF
 #define HW_ATL_INTR_MODER_MIN  0xFF
 
+#define HW_ATL_B0_MIN_RXD \
+	(ALIGN(AQ_CFG_SKB_FRAGS_MAX + 1U, AQ_HW_RXD_MULTIPLE))
+#define HW_ATL_B0_MIN_TXD \
+	(ALIGN(AQ_CFG_SKB_FRAGS_MAX + 1U, AQ_HW_TXD_MULTIPLE))
+
+#define HW_ATL_B0_MAX_RXD 8184U
+#define HW_ATL_B0_MAX_TXD 8184U
+
 /* HW layer capabilities */
 
 #endif /* HW_ATL_B0_INTERNAL_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 0/5] net: aquantia: various ethtool ops implementation
From: Igor Russkikh @ 2018-05-29 12:56 UTC (permalink / raw)
  To: David S . Miller; +Cc: netdev, David Arcari, Pavel Belous, Igor Russkikh

In this patchset Anton Mikaev and I added some useful ethtool operations:
- ring size changes
- link renegotioation
- flow control management

The patch also improves init/deinit sequence.

Igor Russkikh (5):
  net: aquantia: Ethtool based ring size configuration
  net: aquantia: Improve adapter init/deinit logic
  net: aquantia: Implement rx/tx flow control ethtools callback
  net: aquantia: Add renegotiate ethtool operation support
  net: aquantia: bump driver version

 .../net/ethernet/aquantia/atlantic/aq_ethtool.c    | 118 +++++++++++++++++++++
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h     |  20 +++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    |  17 ++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.h    |   2 +
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c  |  47 ++++----
 .../aquantia/atlantic/hw_atl/hw_atl_a0_internal.h  |   8 ++
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  |  51 ++++-----
 .../aquantia/atlantic/hw_atl/hw_atl_b0_internal.h  |   8 ++
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c        |  54 +++++-----
 .../aquantia/atlantic/hw_atl/hw_atl_utils.h        |  35 ++++++
 .../aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c   |  69 +++++++++++-
 drivers/net/ethernet/aquantia/atlantic/ver.h       |   4 +-
 12 files changed, 349 insertions(+), 84 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH net-next v3 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()
From: Vivien Didelot @ 2018-05-29 12:42 UTC (permalink / raw)
  To: Petr Machata, netdev, devel, bridge
  Cc: f.fainelli, andrew, nikolay, gregkh, idosch, jiri,
	razvan.stefanescu, davem
In-Reply-To: <85401f20b801fa1bae3025bf1df991a9d475fe85.1527519997.git.petrm@mellanox.com>

Hi Petr,

Petr Machata <petrm@mellanox.com> writes:

> A call to switchdev_port_obj_add() or switchdev_port_obj_del() involves
> initializing a struct switchdev_obj_port_vlan, a piece of code that
> repeats on each call site almost verbatim. While in the current codebase
> there is just one duplicated add call, the follow-up patches add more of
> both add and del calls.
>
> Thus to remove the duplication, extract the repetition into named
> functions and reuse.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>

Considering Dan's comment as well:

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Thanks,

        Vivien

^ permalink raw reply

* Re: [PATCH 1/4 RFCv2] net: phy: realtek: Support RTL8366RB variant
From: Andrew Lunn @ 2018-05-29 12:34 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vivien Didelot, Florian Fainelli, netdev, openwrt-devel,
	LEDE Development List, Antti Seppälä, Roman Yeryomin,
	Colin Leitner, Gabor Juhos
In-Reply-To: <20180528174752.6806-2-linus.walleij@linaro.org>

On Mon, May 28, 2018 at 07:47:49PM +0200, Linus Walleij wrote:
> The RTL8366RB is an ASIC with five internal PHYs for
> LAN0..LAN3 and WAN. The PHYs are spawn off the main
> device so they can be handled in a distributed manner
> by the Realtek PHY driver. All that is really needed
> is the power save feature enablement and letting the
> PHY driver core pick up the IRQ from the switch chip.
> 
> Cc: Antti Seppälä <a.seppala@gmail.com>
> Cc: Roman Yeryomin <roman@advem.lv>
> Cc: Colin Leitner <colin.leitner@googlemail.com>
> Cc: Gabor Juhos <juhosg@openwrt.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog RFCv1->RFCv2:
> - No real changes.
> ---
>  drivers/net/phy/realtek.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 9f48ecf9c627..21624d1c9d38 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -37,6 +37,9 @@
>  #define RTL8201F_ISR				0x1e
>  #define RTL8201F_IER				0x13
>  
> +#define RTL8366RB_POWER_SAVE	0x21
> +#define RTL8366RB_POWER_SAVE_ON 0x1000
> +
>  MODULE_DESCRIPTION("Realtek PHY driver");
>  MODULE_AUTHOR("Johnson Leung");
>  MODULE_LICENSE("GPL");
> @@ -145,6 +148,22 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  	return phy_modify_paged(phydev, 0xd08, 0x11, RTL8211F_TX_DELAY, val);
>  }
>  
> +static int rtl8366rb_config_init(struct phy_device *phydev)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	ret = genphy_config_init(phydev);
> +	if (ret < 0)
> +		return ret;
> +
> +	reg = phy_read(phydev, RTL8366RB_POWER_SAVE);
> +	reg |= RTL8366RB_POWER_SAVE_ON;
> +	phy_write(phydev, RTL8366RB_POWER_SAVE, reg);
> +
> +	return 0;
> +}
> +
>  static struct phy_driver realtek_drvs[] = {
>  	{
>  		.phy_id         = 0x00008201,
> @@ -207,6 +226,18 @@ static struct phy_driver realtek_drvs[] = {
>  		.resume		= genphy_resume,
>  		.read_page	= rtl821x_read_page,
>  		.write_page	= rtl821x_write_page,
> +	}, {
> +		/* The main part of this DSA is in drivers/net/dsa */

Hi Linus

I would not bother with this comment. We don't say, The main part of
this driver is in drivers/net/ethernet/... PHY drivers should be
completely separate to MAC drivers.

Otherwise this looks good.

	Andrew



> +		.phy_id		= 0x001cc961,
> +		.name		= "RTL8366RB Gigabit Ethernet",
> +		.phy_id_mask	= 0x001fffff,
> +		.features	= PHY_GBIT_FEATURES,
> +		.flags		= PHY_HAS_INTERRUPT,
> +		.config_aneg	= &genphy_config_aneg,
> +		.config_init	= &rtl8366rb_config_init,
> +		.read_status	= &genphy_read_status,
> +		.suspend	= genphy_suspend,
> +		.resume		= genphy_resume,
>  	},
>  };
>  
> @@ -218,6 +249,7 @@ static struct mdio_device_id __maybe_unused realtek_tbl[] = {
>  	{ 0x001cc914, 0x001fffff },
>  	{ 0x001cc915, 0x001fffff },
>  	{ 0x001cc916, 0x001fffff },
> +	{ 0x001cc961, 0x001fffff },
>  	{ }
>  };
>  
> -- 
> 2.17.0
> 

^ permalink raw reply

* Re: [PATCH 0/4 RFCv2] Realtek SMI RTL836x DSA driver
From: Andrew Lunn @ 2018-05-29 12:24 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Vivien Didelot, Florian Fainelli, netdev,
	OpenWrt Development List, LEDE Development List
In-Reply-To: <CACRpkdbnO4vhGHTodkMkP+Zgrr71jp2pGuyXCg03YzjK1D1NTw@mail.gmail.com>

On Tue, May 29, 2018 at 10:49:46AM +0200, Linus Walleij wrote:
> On Mon, May 28, 2018 at 8:20 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, May 28, 2018 at 07:47:48PM +0200, Linus Walleij wrote:
> >> This is a second RFC version of the DSA driver for Realtek
> >> RTL8366x especially RTL8366RB.
> >>
> >> I've been beating my head against this one and I'm not really
> >> clear on why my ethernet frames are not coming through to the
> >> CPU port on the chip.
> >>
> >> It appears when using ethtool -S on the ports that packets
> >> are passing fine into the router fabric and through to the
> >> CPU port but the ethernet driver where the fixed link is
> >> connected refuse to accept the packages.
> >
> > Hi Linus
> >
> > Have you played with RGMII delays?
> 
> No not like I changed them or anything... the SoC has some
> set-up for skew and delay on the nanosecond level, but I used the
> vendor defaults, verified to be the same in their custom
> kernel tree.

Hi Linus

Did you look at the switch end? I found a datasheet for the
8366/8369. Register at 0x0050, P8GCR. It has two bits for RGMII
delays.

With RGMII delays, you have 3 'choices'.

1) The hardware design includes the delay, by zig-zagging the clock
line to make it longer.
2) The 'MAC' side does the delay.
3) The 'PHY' side does the delay.

I normally recommend the PHY side doing it, because that's what most
board do. Gives us some consistency. But it does not really
matter. Just make sure one side, and only once side is inserting the
delays.

	Andrew

^ permalink raw reply

* Re: [PATCH bpf-next 06/11] bpf: add bpf_skb_cgroup_id helper
From: Quentin Monnet @ 2018-05-29 12:15 UTC (permalink / raw)
  To: Daniel Borkmann, ast; +Cc: netdev
In-Reply-To: <20180528004344.3606-7-daniel@iogearbox.net>

Hi Daniel,

2018-05-28 02:43 UTC+0200 ~ Daniel Borkmann <daniel@iogearbox.net>
> Add a new bpf_skb_cgroup_id() helper that allows to retrieve the
> cgroup id from the skb's socket. This is useful in particular to
> enable bpf_get_cgroup_classid()-like behavior for cgroup v1 in
> cgroup v2 by allowing ID based matching on egress. This can in
> particular be used in combination with applying policy e.g. from
> map lookups, and also complements the older bpf_skb_under_cgroup()
> interface. In user space the cgroup id for a given path can be
> retrieved through the f_handle as demonstrated in [0] recently.
> 
>   [0] https://lkml.org/lkml/2018/5/22/1190
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  include/uapi/linux/bpf.h | 17 ++++++++++++++++-
>  net/core/filter.c        | 29 +++++++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 9b8c6e3..e2853aa 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2004,6 +2004,20 @@ union bpf_attr {
>   * 		direct packet access.
>   *	Return
>   * 		0 on success, or a negative error in case of failure.
> + *
> + * uint64_t bpf_skb_cgroup_id(struct sk_buff *skb)
> + * 	Description
> + * 		Return the cgroup v2 id of the socket associated with the *skb*.
> + * 		This is roughly similar to the **bpf_get_cgroup_classid**\ ()
> + * 		helper for cgroup v1 by providing a tag resp. identifier that
> + * 		can be matched on or used for map lookups e.g. to implement
> + * 		policy. The cgroup v2 id of a given path in the hierarchy is
> + * 		exposed in user space through the f_handle API in order to get
> + * 		to the same 64-bit id.
> + *
> + * 		This helper can be used on TC egress path, but not on ingress.

Nitpick: Maybe mention that the kernel must be built with
CONFIG_SOCK_CGROUP_DATA option for the helper to be available?

Best,
Quentin


> + * 	Return
> + * 		The id is returned or 0 in case the id could not be retrieved.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2082,7 +2096,8 @@ union bpf_attr {
>  	FN(lwt_push_encap),		\
>  	FN(lwt_seg6_store_bytes),	\
>  	FN(lwt_seg6_adjust_srh),	\
> -	FN(lwt_seg6_action),
> +	FN(lwt_seg6_action),		\
> +	FN(skb_cgroup_id),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index acf1f4f..717c740 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3661,6 +3661,27 @@ static const struct bpf_func_proto bpf_skb_under_cgroup_proto = {
>  	.arg3_type	= ARG_ANYTHING,
>  };
>  
> +#ifdef CONFIG_SOCK_CGROUP_DATA
> +BPF_CALL_1(bpf_skb_cgroup_id, const struct sk_buff *, skb)
> +{
> +	struct sock *sk = skb_to_full_sk(skb);
> +	struct cgroup *cgrp;
> +
> +	if (!sk || !sk_fullsock(sk))
> +		return 0;
> +
> +	cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
> +	return cgrp->kn->id.id;
> +}
> +
> +static const struct bpf_func_proto bpf_skb_cgroup_id_proto = {
> +	.func           = bpf_skb_cgroup_id,
> +	.gpl_only       = false,
> +	.ret_type       = RET_INTEGER,
> +	.arg1_type      = ARG_PTR_TO_CTX,
> +};
> +#endif
> +
>  static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
>  				  unsigned long off, unsigned long len)
>  {
> @@ -4741,12 +4762,16 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_socket_cookie_proto;
>  	case BPF_FUNC_get_socket_uid:
>  		return &bpf_get_socket_uid_proto;
> +	case BPF_FUNC_fib_lookup:
> +		return &bpf_skb_fib_lookup_proto;
>  #ifdef CONFIG_XFRM
>  	case BPF_FUNC_skb_get_xfrm_state:
>  		return &bpf_skb_get_xfrm_state_proto;
>  #endif
> -	case BPF_FUNC_fib_lookup:
> -		return &bpf_skb_fib_lookup_proto;
> +#ifdef CONFIG_SOCK_CGROUP_DATA
> +	case BPF_FUNC_skb_cgroup_id:
> +		return &bpf_skb_cgroup_id_proto;
> +#endif
>  	default:
>  		return bpf_base_func_proto(func_id);
>  	}
> 

^ permalink raw reply

* Re: [PATCH] bpfilter: fix building without CONFIG_INET
From: David Miller @ 2018-05-29 12:14 UTC (permalink / raw)
  To: arnd; +Cc: ast, netdev, linux-kernel
In-Reply-To: <20180529095535.81934-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 29 May 2018 11:55:06 +0200

> bpfilter_process_sockopt is a callback that gets called from
> ip_setsockopt() and ip_getsockopt(). However, when CONFIG_INET is
> disabled, it never gets called at all, and assigning a function to the
> callback pointer results in a link failure:
> 
> net/bpfilter/bpfilter_kern.o: In function `__stop_umh':
> bpfilter_kern.c:(.text.unlikely+0x3): undefined reference to `bpfilter_process_sockopt'
> net/bpfilter/bpfilter_kern.o: In function `load_umh':
> bpfilter_kern.c:(.init.text+0x73): undefined reference to `bpfilter_process_sockopt'
> 
> Since there is no caller in this configuration, I assume we can
> simply make the assignment conditional.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thanks Arnd.

^ permalink raw reply

* [PATCH net-next] net: davinci_mdio: fix building error without CONFIG_OF
From: YueHaibing @ 2018-05-29 11:56 UTC (permalink / raw)
  To: davem, grygorii.strashko, muvarov
  Cc: netdev, linux-kernel, andrew, fugang.duan, linux-omap, YueHaibing

gcc report a build error when compiling without CONFIG_OF
drivers/net/ethernet/ti/davinci_mdio.c: In function ‘davinci_mdio_probe’:
drivers/net/ethernet/ti/davinci_mdio.c:380:9: error: implicit declaration of function ‘davinci_mdio_probe_dt’ [-Werror=implicit-function-declaration]
   ret = davinci_mdio_probe_dt(&data->pdata, pdev);
         ^
Fixes: 9eae9c7d0875 ("drivers: net: davinci_mdio: enable pm runtime auto for ti cpsw-mdio")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/ti/davinci_mdio.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 8ac7283..6e544d9 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -339,9 +339,7 @@ static int davinci_mdio_probe_dt(struct mdio_platform_data *data,
 
 	return 0;
 }
-#endif
 
-#if IS_ENABLED(CONFIG_OF)
 static const struct davinci_mdio_of_param of_cpsw_mdio_data = {
 	.autosuspend_delay_ms = 100,
 };
@@ -352,6 +350,12 @@ static const struct of_device_id davinci_mdio_of_mtable[] = {
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, davinci_mdio_of_mtable);
+#else
+static int davinci_mdio_probe_dt(struct mdio_platform_data *data,
+				 struct platform_device *pdev)
+{
+	return -EINVAL;
+}
 #endif
 
 static int davinci_mdio_probe(struct platform_device *pdev)
-- 
2.7.0

^ permalink raw reply related

* Re: INFO: rcu detected stall in is_bpf_text_address
From: Andrey Konovalov @ 2018-05-29 11:53 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Xin Long, Eric Dumazet, syzbot, ast, Daniel Borkmann, LKML,
	network dev, syzkaller-bugs
In-Reply-To: <20180528182230.GE3787@localhost.localdomain>

On Mon, May 28, 2018 at 8:22 PM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> The reproducer has:
> r0 = socket$inet6(0xa, 0x1, 0x8010000000000084)
>
> Considering socket() prototype uses an int for the 3rd argument, how
> should I interpret this 64b value?
>
> 0x84 is IPPROTO_SCTP, but don't know about the 0x8010000000000000 in
> there.

Hi Marcelo,

It gets stripped to int the same way C does it, so the value would be 0x84.

Thanks!

^ permalink raw reply

* Re: [PATCH net] sctp: not allow to set rto_min with a value below 200 msecs
From: Neil Horman @ 2018-05-29 11:41 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Dmitry Vyukov, Michael Tuexen, Xin Long, network dev, linux-sctp,
	David Miller, David Ahern, Eric Dumazet, syzkaller
In-Reply-To: <20180528194315.GB3788@localhost.localdomain>

On Mon, May 28, 2018 at 04:43:15PM -0300, Marcelo Ricardo Leitner wrote:
> On Sat, May 26, 2018 at 09:01:00PM -0400, Neil Horman wrote:
> > On Sat, May 26, 2018 at 05:50:39PM +0200, Dmitry Vyukov wrote:
> > > On Sat, May 26, 2018 at 5:42 PM, Michael Tuexen
> > > <michael.tuexen@lurchi.franken.de> wrote:
> > > >> On 25. May 2018, at 21:13, Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >>
> > > >> On Sat, May 26, 2018 at 01:41:02AM +0800, Xin Long wrote:
> > > >>> syzbot reported a rcu_sched self-detected stall on CPU which is caused
> > > >>> by too small value set on rto_min with SCTP_RTOINFO sockopt. With this
> > > >>> value, hb_timer will get stuck there, as in its timer handler it starts
> > > >>> this timer again with this value, then goes to the timer handler again.
> > > >>>
> > > >>> This problem is there since very beginning, and thanks to Eric for the
> > > >>> reproducer shared from a syzbot mail.
> > > >>>
> > > >>> This patch fixes it by not allowing to set rto_min with a value below
> > > >>> 200 msecs, which is based on TCP's, by either setsockopt or sysctl.
> > > >>>
> > > >>> Reported-by: syzbot+3dcd59a1f907245f891f@syzkaller.appspotmail.com
> > > >>> Suggested-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > >>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > >>> ---
> > > >>> include/net/sctp/constants.h |  1 +
> > > >>> net/sctp/socket.c            | 10 +++++++---
> > > >>> net/sctp/sysctl.c            |  3 ++-
> > > >>> 3 files changed, 10 insertions(+), 4 deletions(-)
> > > >>>
> > > >>> diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
> > > >>> index 20ff237..2ee7a7b 100644
> > > >>> --- a/include/net/sctp/constants.h
> > > >>> +++ b/include/net/sctp/constants.h
> > > >>> @@ -277,6 +277,7 @@ enum { SCTP_MAX_GABS = 16 };
> > > >>> #define SCTP_RTO_INITIAL     (3 * 1000)
> > > >>> #define SCTP_RTO_MIN         (1 * 1000)
> > > >>> #define SCTP_RTO_MAX         (60 * 1000)
> > > >>> +#define SCTP_RTO_HARD_MIN   200
> > > >>>
> > > >>> #define SCTP_RTO_ALPHA          3   /* 1/8 when converted to right shifts. */
> > > >>> #define SCTP_RTO_BETA           2   /* 1/4 when converted to right shifts. */
> > > >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > > >>> index ae7e7c6..6ef12c7 100644
> > > >>> --- a/net/sctp/socket.c
> > > >>> +++ b/net/sctp/socket.c
> > > >>> @@ -3029,7 +3029,8 @@ static int sctp_setsockopt_nodelay(struct sock *sk, char __user *optval,
> > > >>>  * be changed.
> > > >>>  *
> > > >>>  */
> > > >>> -static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigned int optlen)
> > > >>> +static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval,
> > > >>> +                               unsigned int optlen)
> > > >>> {
> > > >>>      struct sctp_rtoinfo rtoinfo;
> > > >>>      struct sctp_association *asoc;
> > > >>> @@ -3056,10 +3057,13 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne
> > > >>>      else
> > > >>>              rto_max = asoc ? asoc->rto_max : sp->rtoinfo.srto_max;
> > > >>>
> > > >>> -    if (rto_min)
> > > >>> +    if (rto_min) {
> > > >>> +            if (rto_min < SCTP_RTO_HARD_MIN)
> > > >>> +                    return -EINVAL;
> > > >>>              rto_min = asoc ? msecs_to_jiffies(rto_min) : rto_min;
> > > >>> -    else
> > > >>> +    } else {
> > > >>>              rto_min = asoc ? asoc->rto_min : sp->rtoinfo.srto_min;
> > > >>> +    }
> > > >>>
> > > >>>      if (rto_min > rto_max)
> > > >>>              return -EINVAL;
> > > >>> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> > > >>> index 33ca5b7..7ec854a 100644
> > > >>> --- a/net/sctp/sysctl.c
> > > >>> +++ b/net/sctp/sysctl.c
> > > >>> @@ -52,6 +52,7 @@ static int rto_alpha_min = 0;
> > > >>> static int rto_beta_min = 0;
> > > >>> static int rto_alpha_max = 1000;
> > > >>> static int rto_beta_max = 1000;
> > > >>> +static int rto_hard_min = SCTP_RTO_HARD_MIN;
> > > >>>
> > > >>> static unsigned long max_autoclose_min = 0;
> > > >>> static unsigned long max_autoclose_max =
> > > >>> @@ -116,7 +117,7 @@ static struct ctl_table sctp_net_table[] = {
> > > >>>              .maxlen         = sizeof(unsigned int),
> > > >>>              .mode           = 0644,
> > > >>>              .proc_handler   = proc_sctp_do_rto_min,
> > > >>> -            .extra1         = &one,
> > > >>> +            .extra1         = &rto_hard_min,
> > > >>>              .extra2         = &init_net.sctp.rto_max
> > > >>>      },
> > > >>>      {
> > > >>> --
> > > >>> 2.1.0
> > > >>>
> > > >>> --
> > > >>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> > > >>> the body of a message to majordomo@vger.kernel.org
> > > >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > >>>
> > > >> Patch looks fine, you probably want to note this hard minimum in man(7) sctp as
> > > >> well
> > > >>
> > > > I'm aware of some signalling networks which use RTO.min of smaller values than 200ms.
> > > > So could this be reduced?
> > > 
> > > Hi Michael,
> > > 
> > > What value do they use?
> > > 
> > > Xin, Neil, is there more principled way of ensuring that a timer won't
> > > cause a hard CPU stall? There are slow machines and there are slow
> > > kernels (in particular syzbot kernel has tons of debug configs
> > > enabled). 200ms _should_ not cause problems because we did not see
> > > them with tcp. But it's hard to say what's the low limit as we are
> > > trying to put a hard upper bound on execution time of a complex
> > > section of code. Is there something like cond_resched for timers?
> > Unfortunately, Theres not really a way to do conditional rescheduling of timers,
> > additionally, we have a problem because the timer is reset as a side effect of
> > the SCTP state machine, and so the execution time between timer updates has a
> > signifcant amount of jitter (meaning its a pretty hard value to calibrate,
> > unless you just select a 'safe' large value for the floor).
> > 
> > What we might could do (though this might impact the protocol function is change
> > the timer update side effects to simply set a flag, and consistently update the
> > timers on exit from sctp_do_sm, so they don't re-arm until all state machine
> > processing is complete.  Anyone have any thoughts on that?
> 
> I was reviewing all this again and I'm thinking that we are missing
> the real point. With the parameters that reproducer [1] has, setting
> those very low RTO parameters, it causes the timer to actually
> busyloop on the heartbeats, as Xin had explained.
> 
> But thing is, it busy loops not just because RTO is too low, but
> because hbinterval was not accounted.
> 
> /* What is the next timeout value for this transport? */
> unsigned long sctp_transport_timeout(struct sctp_transport *trans)
> {
>         /* RTO + timer slack +/- 50% of RTO */
>         unsigned long timeout = trans->rto >> 1;  <-- [a]
> 
>         if (trans->state != SCTP_UNCONFIRMED &&
>             trans->state != SCTP_PF)             <--- [2]
>                 timeout += trans->hbinterval;
> 
>         return timeout;
> }
> 
> The if() in [2] is to speed up path verification before using them, as
> per the commit changelog. Secondary paths added on processing the
> cookie are created with status SCTP_UNCONFIRMED, and HB timers are
> started in the sequence:
>  sctp_sf_do_5_1D_ce
>    -> sctp_process_init
>      |> sctp_process_param
>      | -> sctp_assoc_add_peer(asoc, &addr, gfp, SCTP_UNCONFIRMED)
>      '> sctp_add_cmd_sf(commands, SCTP_CMD_HB_TIMERS_START, SCTP_NULL());
> 
> which starts the timer using only the small RTO for secondary paths:
> static void sctp_cmd_hb_timers_start(struct sctp_cmd_seq *cmds,
>                                      struct sctp_association *asoc)
> {
>         struct sctp_transport *t;
> 
>         /* Start a heartbeat timer for each transport on the association.
>          * hold a reference on the transport to make sure none of
>          * the needed data structures go away.
>          */
>         list_for_each_entry(t, &asoc->peer.transport_addr_list, transports)
>                 sctp_transport_reset_hb_timer(t);
> }
> 
> But if the system is too busy generating HBs, it likely won't process
> incoming HB ACKs, which would stop the loop as it would mark the
> transport as Active.
> 
> I'm now thinking a better fix would be to have a specific way to
> kickstart these initial heartbeets, and then always use hbinterval on
> subsequent ones.
> 
I like the idea, but I don't think we can just use the hbinterval to set the
timeout.  That said, it seems like we should always be using the HB interval,
not just on unconfirmed or partially failed transports.  From the RFC:

On an idle destination address that is allowed to heartbeat, it is
   recommended that a HEARTBEAT chunk is sent once per RTO of that
   destination address plus the protocol parameter 'HB.interval', with
   jittering of +/- 50% of the RTO value, and exponential backoff of the
   RTO if the previous HEARTBEAT is unanswered

It seems like we should be adding it to the timer expiration universally.  By my
read, we've never done this quite right.  And yes, I agree, if we account this
properly, we will avoid this issue.

Its also probably important to note here, that, like RTO.min currently, there is
no hard floor to the heartbeat interval, and the RFC is silent on what it should
be.  So it would be possible to still find ourselves in this situation if we set
the interval to 0 from userspace.  Is it worth considering a floor on the
minimum hb interval of the rto is to have no floor?

Neil
 

> This would not only fix the issue, but also improve the time we need
> to identify the transports as Active upon association start, which is
> currently RTO/2 (equals to 500ms by default).
> 
> While working on this, I got myself wondering how HZ can affect the
> stack with such small RTO. If we have HZ=250, for example, we probably
> should be careful when doing calcs such as in mark [a] to not let it
> tend to 0. This should not be related to the reported issue as
> syzkaller was using HZ=1000.
> 
> (I didn't do any tests, this is only based on code review so far)
> 
> 1. https://syzkaller.appspot.com/x/repro.syz?x=1079cf8f800000
> 2. ad8fec1720e0 ("[SCTP]: Verify all the paths to a peer via heartbeat before using them.")
> b. https://syzkaller.appspot.com/x/.config?x=f3b4e30da84ec1ed
> 

^ permalink raw reply

* [PATCH bpf-next] bpf: clean up eBPF helpers documentation
From: Quentin Monnet @ 2018-05-29 11:27 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, oss-drivers, quentin.monnet

These are minor edits for the eBPF helpers documentation in
include/uapi/linux/bpf.h.

The main fix consists in removing "BPF_FIB_LOOKUP_", because it ends
with a non-escaped underscore that gets interpreted by rst2man and
produces the following message in the resulting manual page:

    DOCUTILS SYSTEM MESSAGES
           System Message: ERROR/3 (/tmp/bpf-helpers.rst:, line 1514)
                  Unknown target name: "bpf_fib_lookup".

Other edits consist in:

- Improving formatting for flag values for "bpf_fib_lookup()" helper.
- Emphasising a parameter name in description of the return value for
  "bpf_get_stack()" helper.
- Removing unnecessary blank lines between "Description" and "Return"
  sections for the few helpers that would use it, for consistency.

Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 include/uapi/linux/bpf.h | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index cc68787f2d97..3f556b35ac8d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1010,7 +1010,6 @@ union bpf_attr {
  * 		::
  *
  * 			# sysctl kernel.perf_event_max_stack=<new value>
- *
  * 	Return
  * 		The positive or null stack id on success, or a negative error
  * 		in case of failure.
@@ -1821,10 +1820,9 @@ union bpf_attr {
  * 		::
  *
  * 			# sysctl kernel.perf_event_max_stack=<new value>
- *
  * 	Return
- * 		a non-negative value equal to or less than size on success, or
- * 		a negative error in case of failure.
+ * 		A non-negative value equal to or less than *size* on success,
+ * 		or a negative error in case of failure.
  *
  * int skb_load_bytes_relative(const struct sk_buff *skb, u32 offset, void *to, u32 len, u32 start_header)
  * 	Description
@@ -1845,7 +1843,6 @@ union bpf_attr {
  * 		in socket filters where *skb*\ **->data** does not always point
  * 		to the start of the mac header and where "direct packet access"
  * 		is not available.
- *
  * 	Return
  * 		0 on success, or a negative error in case of failure.
  *
@@ -1861,16 +1858,18 @@ union bpf_attr {
  *		rt_metric is set to metric from route.
  *
  *             *plen* argument is the size of the passed in struct.
- *             *flags* argument can be one or more BPF_FIB_LOOKUP_ flags:
+ *             *flags* argument can be a combination of one or more of the
+ *             following values:
  *
- *             **BPF_FIB_LOOKUP_DIRECT** means do a direct table lookup vs
- *             full lookup using FIB rules
- *             **BPF_FIB_LOOKUP_OUTPUT** means do lookup from an egress
- *             perspective (default is ingress)
+ *		**BPF_FIB_LOOKUP_DIRECT**
+ *			Do a direct table lookup vs full lookup using FIB
+ *			rules.
+ *		**BPF_FIB_LOOKUP_OUTPUT**
+ *			Perform lookup from an egress perspective (default is
+ *			ingress).
  *
  *             *ctx* is either **struct xdp_md** for XDP programs or
  *             **struct sk_buff** tc cls_act programs.
- *
  *     Return
  *             Egress device index on success, 0 if packet needs to continue
  *             up the stack for further processing or a negative error in case
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH v3 net-next 2/2] tcp: minor optimization around tcp_hdr() usage in tcp receive path
From: Yafang Shao @ 2018-05-29 11:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Eric Dumazet, Song Liu, David Miller, netdev, LKML
In-Reply-To: <68b62912-21d5-c238-7536-12928670a08a@gmail.com>

On Tue, May 29, 2018 at 12:53 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
> On 05/28/2018 05:41 PM, Yafang Shao wrote:
>
>> OK.
>>
>> And what about introducing a new helper tcp_hdr_fast() ?
>>
>> /* use it when tcp header has not been pulled yet */
>> static inline struct tcphdr *tcp_hdr_fast(const struct sk_buff *skb)
>>
>> {
>>
>>         return (const struct tcphdr *)skb->data;
>>
>> }
>>
>>
>> That could help us to use this optimized one instead of the original
>> one if possilbe.
>
>
> I would rather not add such macro...
>
> The call site needs to know what is going on, so having a macro like that would not help.

OK

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs
From: Dan Carpenter @ 2018-05-29 11:02 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: devel, andrew, f.fainelli, vivien.didelot, nikolay, netdev,
	bridge, idosch, jiri, razvan.stefanescu, gregkh, Petr Machata,
	davem
In-Reply-To: <20180529105853.GA13795@splinter.mtl.com>

On Tue, May 29, 2018 at 01:58:53PM +0300, Ido Schimmel wrote:
> On Tue, May 29, 2018 at 01:46:09PM +0300, Dan Carpenter wrote:
> > It occured to me that I should read the cover letter and here are the
> > answers I was looking for.  But the cover letter isn't saved after the
> > commits are merged.
> 
> DaveM adds the cover letter to the merge commit.

That's true but it doesn't help.  When I want to find why line of code
is there I look up the commit message, not the merge commit.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs
From: Ido Schimmel @ 2018-05-29 10:58 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Petr Machata, netdev, devel, bridge, f.fainelli, andrew, nikolay,
	gregkh, vivien.didelot, idosch, jiri, razvan.stefanescu, davem
In-Reply-To: <20180529104609.jqbql6ts2rbb7k3l@mwanda>

On Tue, May 29, 2018 at 01:46:09PM +0300, Dan Carpenter wrote:
> It occured to me that I should read the cover letter and here are the
> answers I was looking for.  But the cover letter isn't saved after the
> commits are merged.

DaveM adds the cover letter to the merge commit.

^ permalink raw reply

* Re: [PATCH net-next v3 6/7] net: bridge: Notify about bridge VLANs
From: Dan Carpenter @ 2018-05-29 10:55 UTC (permalink / raw)
  To: Petr Machata
  Cc: devel, andrew, f.fainelli, vivien.didelot, nikolay, netdev,
	bridge, idosch, jiri, razvan.stefanescu, gregkh, davem
In-Reply-To: <645c0bde19cffebb0dc603872ef6ab45e9fbf0b6.1527519997.git.petrm@mellanox.com>

On Mon, May 28, 2018 at 05:11:04PM +0200, Petr Machata wrote:
> @@ -580,6 +591,9 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags, bool *changed)
>  			vg->num_vlans++;
>  			*changed = true;
>  		}
> +		ret = br_switchdev_port_vlan_add(br->dev, vid, flags);
> +		if (ret && ret != -EOPNOTSUPP)
> +			return ret;

We should probably do some error handling instead of returning directly?

>  		if (__vlan_add_flags(vlan, flags))
>  			*changed = true;
>  

regards,
dan caprenter

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: bridge: Notify about bridge VLANs
From: Dan Carpenter @ 2018-05-29 10:46 UTC (permalink / raw)
  To: Petr Machata
  Cc: devel, andrew, f.fainelli, vivien.didelot, nikolay, netdev,
	bridge, idosch, jiri, razvan.stefanescu, gregkh, davem
In-Reply-To: <cover.1527519997.git.petrm@mellanox.com>

On Mon, May 28, 2018 at 05:10:22PM +0200, Petr Machata wrote:
> In commit 946a11e7408e ("mlxsw: spectrum_span: Allow bridge for gretap
> mirror"), mlxsw got support for offloading mirror-to-gretap such that
> the underlay packet path involves a bridge. In that case, the offload is
> also influenced by PVID setting of said bridge. However, changes to VLAN
> configuration of the bridge itself do not generate switchdev
> notifications, so there's no mechanism to prod mlxsw to update the
> offload when these settings change.
> 
> In this patchset, the problem is resolved by distributing the switchdev
> notification SWITCHDEV_OBJ_ID_PORT_VLAN also for configuration changes
> on bridge VLANs. Since stacked devices distribute the notification to
> lower devices, such event eventually reaches the driver, which can
> determine whether it's a bridge or port VLAN by inspecting orig_dev.
> 
> To keep things consistent, the newly-distributed notifications observe
> the same protocol as the existing ones: dual prepare/commit, with
> -EOPNOTSUPP indicating lack of support, even though there's currently
> nothing to prepare for and nothing to support. Correspondingly, all
> switchdev drivers have been updated to return -EOPNOTSUPP for bridge
> VLAN notifications.
> 
> In patch #1, the code to send notifications for adding and deleting is
> factored out into two named functions.
> 
> In patches #2-#5, respectively for mlxsw, rocker, DSA and DPAA2 ethsw,
> the new notifications (which are not enabled yet) are ignored to
> maintain the current behavior.
> 

It occured to me that I should read the cover letter and here are the
answers I was looking for.  But the cover letter isn't saved after the
commits are merged.  This should really be in the commit messages itself
so that we can look it up in the git history.

> In patch #6, the notification is actually enabled.
> 
> In patch #7, mlxsw is changed to update offloads of mirror-to-gre also
> for bridge-related notifications.

regards,
dan carpenter

^ permalink raw reply

* [PATCH net-next] net: qcom/emac: fix unused variable
From: YueHaibing @ 2018-05-29 10:43 UTC (permalink / raw)
  To: davem, timur; +Cc: netdev, linux-kernel, YueHaibing

When CONFIG_ACPI isn't set, variable qdf2400_ops/qdf2432_ops isn't used.
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c:284:25: warning: ‘qdf2400_ops’ defined but not used [-Wunused-variable]
 static struct sgmii_ops qdf2400_ops = {
                         ^~~~~~~~~~~
drivers/net/ethernet/qualcomm/emac/emac-sgmii.c:276:25: warning: ‘qdf2432_ops’ defined but not used [-Wunused-variable]
 static struct sgmii_ops qdf2432_ops = {
                         ^~~~~~~~~~~

Move the declaration and functions inside the CONFIG_ACPI ifdef 
to fix the warning.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/qualcomm/emac/emac-sgmii.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
index 562420b..15609cb 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac-sgmii.c
@@ -108,6 +108,7 @@ static void emac_sgmii_link_init(struct emac_adapter *adpt)
 	writel(val, phy->base + EMAC_SGMII_PHY_AUTONEG_CFG2);
 }
 
+#ifdef CONFIG_ACPI
 static int emac_sgmii_irq_clear(struct emac_adapter *adpt, u8 irq_bits)
 {
 	struct emac_sgmii *phy = &adpt->phy;
@@ -291,7 +292,6 @@ static struct sgmii_ops qdf2400_ops = {
 
 static int emac_sgmii_acpi_match(struct device *dev, void *data)
 {
-#ifdef CONFIG_ACPI
 	static const struct acpi_device_id match_table[] = {
 		{
 			.id = "QCOM8071",
@@ -327,10 +327,16 @@ static int emac_sgmii_acpi_match(struct device *dev, void *data)
 			return 1;
 		}
 	}
-#endif
 
 	return 0;
 }
+#else
+static int emac_sgmii_acpi_match(struct device *dev, void *data)
+{
+	return 0;
+}
+
+#endif
 
 static const struct of_device_id emac_sgmii_dt_match[] = {
 	{
-- 
2.7.0

^ permalink raw reply related

* Re: [PATCH net-next v3 2/7] mlxsw: spectrum_switchdev: Ignore bridge VLAN events
From: Dan Carpenter @ 2018-05-29 10:43 UTC (permalink / raw)
  To: Petr Machata
  Cc: devel, andrew, f.fainelli, vivien.didelot, nikolay, netdev,
	bridge, idosch, jiri, razvan.stefanescu, gregkh, davem
In-Reply-To: <67c3f567cd774cae8935873072c81e5ea62dc521.1527519997.git.petrm@mellanox.com>

On Mon, May 28, 2018 at 05:10:40PM +0200, Petr Machata wrote:
> Ignore VLAN events where the orig_dev is the bridge device itself.
> 

I don't know this code, and I have not looked at it either...  But this
changelog just says what you are doing and not why.  Is this a bug fix?
What are the user visible effects of this patch?  Perhaps it is
something which will be required in the future but has no effect now?

You know how the New York Times is written for people with at least a
10th grade education?  I feel like maybe if I knew this code I would
know the answers to my question, but kernel commit descriptions should
generally be written to Dan Carpenter level of education.  Which is
pretty darn ignorant...  Can you please resend?

regards,
dan carpenter

^ permalink raw reply

* Re: Unable to create ip alias on bridge interface
From: Akshat Kakkar @ 2018-05-29 10:41 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: netdev
In-Reply-To: <20180529102954.r7ayncpopip6dcpr@unicorn.suse.cz>

Thanks.
Thanks a lot for clarifying all this.

On Tue, May 29, 2018 at 3:59 PM, Michal Kubecek <mkubecek@suse.cz> wrote:
> On Tue, May 29, 2018 at 03:39:05PM +0530, Akshat Kakkar wrote:
>> For following commands,
>>   ip addr add 10.10.10.1/24 brd +  dev br0
>>   ip addr add 10.10.10.2/24 brd +  dev br0
>>   ip addr add 20.20.20.1/24 brd +  dev br0
>>   ip addr add 20.20.20.2/24 brd +  dev br0
>>
>> Both 10.10.10.1 and 20.20.20.1 becomes primary. Which one will be used
>> as source IP?
>>
>> Is it nextHop of route that will decide?
>
> Unless you have an unusual routing setup, yes. When unsure, you can use
> "ip route get ..." which should tell you which route and which source
> address will be used.
>
>> And what about communication in local subnet, say ping to 10.10.10.200
>> and 20.20.20.200? Will source for both will change according to
>> destination IP?
>
> This is the same. Under "normal" circumstances, 10.10.10.1 will be used
> for targets from 10.10.10.0/24 and 20.20.20.1 for targets from
> 20.20.20.0/24.
>
> Michal Kubecek

^ permalink raw reply

* Re: [PATCH net-next v3 1/7] net: bridge: Extract boilerplate around switchdev_port_obj_*()
From: Dan Carpenter @ 2018-05-29 10:35 UTC (permalink / raw)
  To: Petr Machata
  Cc: devel, andrew, f.fainelli, vivien.didelot, nikolay, netdev,
	bridge, idosch, jiri, razvan.stefanescu, gregkh, davem
In-Reply-To: <85401f20b801fa1bae3025bf1df991a9d475fe85.1527519997.git.petrm@mellanox.com>

On Mon, May 28, 2018 at 05:10:28PM +0200, Petr Machata wrote:
>  	/* Try switchdev op first. In case it is not supported, fallback to
>  	 * 8021q add.
>  	 */
> -	err = switchdev_port_obj_add(dev, &v.obj);
> +	int err = br_switchdev_port_vlan_add(dev, vid, flags);
>  	if (err == -EOPNOTSUPP)

This will introduce a checkpatch warning if you re-run with the --file
option.  (I haven't tested).  It's sort of ugly to put function logic in
the declaration block.  That's really just for initializing variables
like "int start = 0; struct foo *p = parent_of_bar();"  Do it like this
instead:

	int err;

	err = br_switchdev_port_vlan_add(dev, vid, flags);
	if (err == -EOPNOTSUPP)

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: hide the unused 'off' variable
From: Arnd Bergmann @ 2018-05-29 10:35 UTC (permalink / raw)
  To: YueHaibing
  Cc: David Miller, Alexei Starovoitov, Daniel Borkmann, Networking,
	Linux Kernel Mailing List
In-Reply-To: <20180529024018.12740-1-yuehaibing@huawei.com>

On Tue, May 29, 2018 at 4:40 AM, YueHaibing <yuehaibing@huawei.com> wrote:
> The local variable is only used while CONFIG_IPV6 enabled
>
> net/core/filter.c: In function ‘sk_msg_convert_ctx_access’:
> net/core/filter.c:6489:6: warning: unused variable ‘off’ [-Wunused-variable]
>   int off;
>       ^
> This puts it into #ifdef.
>
> Fixes: 303def35f64e ("bpf: allow sk_msg programs to read sock fields")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

I was about to send the same patch and found you had already sent one.

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: Unable to create ip alias on bridge interface
From: Michal Kubecek @ 2018-05-29 10:29 UTC (permalink / raw)
  To: netdev; +Cc: Akshat Kakkar
In-Reply-To: <CAA5aLPiuP8ToWLtqKvP-+X6urdQ92wbwt76SHDLn+D1oXdoOfA@mail.gmail.com>

On Tue, May 29, 2018 at 03:39:05PM +0530, Akshat Kakkar wrote:
> For following commands,
>   ip addr add 10.10.10.1/24 brd +  dev br0
>   ip addr add 10.10.10.2/24 brd +  dev br0
>   ip addr add 20.20.20.1/24 brd +  dev br0
>   ip addr add 20.20.20.2/24 brd +  dev br0
> 
> Both 10.10.10.1 and 20.20.20.1 becomes primary. Which one will be used
> as source IP?
> 
> Is it nextHop of route that will decide?

Unless you have an unusual routing setup, yes. When unsure, you can use
"ip route get ..." which should tell you which route and which source
address will be used.

> And what about communication in local subnet, say ping to 10.10.10.200
> and 20.20.20.200? Will source for both will change according to
> destination IP?

This is the same. Under "normal" circumstances, 10.10.10.1 will be used
for targets from 10.10.10.0/24 and 20.20.20.1 for targets from
20.20.20.0/24.

Michal Kubecek

^ permalink raw reply

* Re: [PATCH v3 11/11] net: sched: change action API to use array of pointers to actions
From: Vlad Buslov @ 2018-05-29 10:25 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: jiri, netdev, jhs, xiyou.wangcong, davem, ast, daniel, kliteyn
In-Reply-To: <20180528213147.GF3787@localhost.localdomain>


On Mon 28 May 2018 at 21:31, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> On Mon, May 28, 2018 at 12:17:29AM +0300, Vlad Buslov wrote:
> ...
>> -int tcf_action_destroy(struct list_head *actions, int bind)
>> +int tcf_action_destroy(struct tc_action *actions[], int bind)
>>  {
>>  	const struct tc_action_ops *ops;
>> -	struct tc_action *a, *tmp;
>> -	int ret = 0;
>> +	struct tc_action *a;
>> +	int ret = 0, i;
>>  
>> -	list_for_each_entry_safe(a, tmp, actions, list) {
>> +	for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
> ...
>> @@ -878,10 +881,9 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>>  	if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
>>  		err = tcf_action_goto_chain_init(a, tp);
>>  		if (err) {
>> -			LIST_HEAD(actions);
>> +			struct tc_action *actions[TCA_ACT_MAX_PRIO] = { a };
>
> Somewhat nit.. Considering tcf_action_destroy will stop at the first
> NULL, you need only 2 slots here.

Yes, I guess NULLing whole array when only first pointer is used, is
redundant. I didn't want to be too clever in this patch and made all
actions array of same size, but I don't see anything potentially
dangerous in reducing this one.

>
>>  
>> -			list_add_tail(&a->list, &actions);
>> -			tcf_action_destroy(&actions, bind);
>> +			tcf_action_destroy(actions, bind);
>>  			NL_SET_ERR_MSG(extack, "Failed to init TC action chain");
>>  			return ERR_PTR(err);
>>  		}

Thank you for reviewing my code!

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox