Netdev List
 help / color / mirror / Atom feed
* [PATCH V2 net-next 06/11] net: ena: remove old adaptive interrupt moderation code from ena_netdev
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

1. Out of the fields {per_napi_bytes, per_napi_packets} in struct ena_ring,
   only rx_ring->per_napi_packets are used to determine if napi did work
   for dim.
   This commit removes all other uses of these fields.
2. Remove ena_ring->moder_tbl_idx, which is not used by dim.
3. Remove all calls to ena_com_destroy_interrupt_moderation(), since all it
   did was to destroy the interrupt moderation table, which is removed as
   part of removing old interrupt moderation code.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 8 --------
 drivers/net/ethernet/amazon/ena/ena_netdev.h | 2 --
 2 files changed, 10 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index f19736493c01..1d8855ab8624 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -158,7 +158,6 @@ static void ena_init_io_rings_common(struct ena_adapter *adapter,
 	ring->adapter = adapter;
 	ring->ena_dev = adapter->ena_dev;
 	ring->per_napi_packets = 0;
-	ring->per_napi_bytes = 0;
 	ring->cpu = 0;
 	ring->first_interrupt = false;
 	ring->no_interrupt_event_cnt = 0;
@@ -834,9 +833,6 @@ static int ena_clean_tx_irq(struct ena_ring *tx_ring, u32 budget)
 		__netif_tx_unlock(txq);
 	}
 
-	tx_ring->per_napi_bytes += tx_bytes;
-	tx_ring->per_napi_packets += tx_pkts;
-
 	return tx_pkts;
 }
 
@@ -1120,7 +1116,6 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
 	} while (likely(res_budget));
 
 	work_done = budget - res_budget;
-	rx_ring->per_napi_bytes += total_len;
 	rx_ring->per_napi_packets += work_done;
 	u64_stats_update_begin(&rx_ring->syncp);
 	rx_ring->rx_stats.bytes += total_len;
@@ -3641,7 +3636,6 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	ena_free_mgmnt_irq(adapter);
 	ena_disable_msix(adapter);
 err_worker_destroy:
-	ena_com_destroy_interrupt_moderation(ena_dev);
 	del_timer(&adapter->timer_service);
 err_netdev_destroy:
 	free_netdev(netdev);
@@ -3702,8 +3696,6 @@ static void ena_remove(struct pci_dev *pdev)
 
 	pci_disable_device(pdev);
 
-	ena_com_destroy_interrupt_moderation(ena_dev);
-
 	vfree(ena_dev);
 }
 
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index f67ecab3389a..e9450991ab74 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -280,8 +280,6 @@ struct ena_ring {
 	struct ena_com_rx_buf_info ena_bufs[ENA_PKT_MAX_BUFS];
 	u32  smoothed_interval;
 	u32  per_napi_packets;
-	u32  per_napi_bytes;
-	enum ena_intr_moder_level moder_tbl_idx;
 	u16 non_empty_napi_events;
 	struct u64_stats_sync syncp;
 	union {
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 07/11] net: ena: remove ena_restore_ethtool_params() and relevant fields
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

Deleted unused 4 fields from struct ena_adapter and their only user
ena_restore_ethtool_params().

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 10 ----------
 drivers/net/ethernet/amazon/ena/ena_netdev.h |  3 ---
 2 files changed, 13 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 1d8855ab8624..8b9f8b90e525 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1563,14 +1563,6 @@ static void ena_napi_enable_all(struct ena_adapter *adapter)
 		napi_enable(&adapter->ena_napi[i].napi);
 }
 
-static void ena_restore_ethtool_params(struct ena_adapter *adapter)
-{
-	adapter->tx_usecs = 0;
-	adapter->rx_usecs = 0;
-	adapter->tx_frames = 1;
-	adapter->rx_frames = 1;
-}
-
 /* Configure the Rx forwarding */
 static int ena_rss_configure(struct ena_adapter *adapter)
 {
@@ -1620,8 +1612,6 @@ static int ena_up_complete(struct ena_adapter *adapter)
 	/* enable transmits */
 	netif_tx_start_all_queues(adapter->netdev);
 
-	ena_restore_ethtool_params(adapter);
-
 	ena_napi_enable_all(adapter);
 
 	return 0;
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index e9450991ab74..72ee51a82ec7 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -330,9 +330,6 @@ struct ena_adapter {
 
 	u32 missing_tx_completion_threshold;
 
-	u32 tx_usecs, rx_usecs; /* interrupt moderation */
-	u32 tx_frames, rx_frames; /* interrupt moderation */
-
 	u32 requested_tx_ring_size;
 	u32 requested_rx_ring_size;
 
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 09/11] net: ena: fix update of interrupt moderation register
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

Current implementation always updates the interrupt register with
the smoothed_interval of the rx_ring. However this should be
done only in case of adaptive interrupt moderation. If non-adaptive
interrupt moderation is used, the non-adaptive interrupt moderation
interval should be used. This commit fixes that.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 8b9f8b90e525..54539e57aa73 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -1187,12 +1187,15 @@ static void ena_unmask_interrupt(struct ena_ring *tx_ring,
 					struct ena_ring *rx_ring)
 {
 	struct ena_eth_io_intr_reg intr_reg;
+	u32 rx_interval = ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev) ?
+		rx_ring->smoothed_interval :
+		ena_com_get_nonadaptive_moderation_interval_rx(rx_ring->ena_dev);
 
 	/* Update intr register: rx intr delay,
 	 * tx intr delay and interrupt unmask
 	 */
 	ena_com_update_intr_reg(&intr_reg,
-				rx_ring->smoothed_interval,
+				rx_interval,
 				tx_ring->smoothed_interval,
 				true);
 
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 08/11] net: ena: remove all old adaptive rx interrupt moderation code from ena_com
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

Remove previous implementation of adaptive rx interrupt moderation
from ena_com files.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_com.c | 110 -----------------
 drivers/net/ethernet/amazon/ena/ena_com.h | 142 ----------------------
 2 files changed, 252 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index bbb59fd220a1..621b747f062b 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -1278,22 +1278,6 @@ static int ena_com_ind_tbl_convert_from_device(struct ena_com_dev *ena_dev)
 	return 0;
 }
 
-static int ena_com_init_interrupt_moderation_table(struct ena_com_dev *ena_dev)
-{
-	size_t size;
-
-	size = sizeof(struct ena_intr_moder_entry) * ENA_INTR_MAX_NUM_OF_LEVELS;
-
-	ena_dev->intr_moder_tbl =
-		devm_kzalloc(ena_dev->dmadev, size, GFP_KERNEL);
-	if (!ena_dev->intr_moder_tbl)
-		return -ENOMEM;
-
-	ena_com_config_default_interrupt_moderation_table(ena_dev);
-
-	return 0;
-}
-
 static void ena_com_update_intr_delay_resolution(struct ena_com_dev *ena_dev,
 						 u16 intr_delay_resolution)
 {
@@ -2802,13 +2786,6 @@ int ena_com_update_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_de
 							      &ena_dev->intr_moder_rx_interval);
 }
 
-void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev)
-{
-	if (ena_dev->intr_moder_tbl)
-		devm_kfree(ena_dev->dmadev, ena_dev->intr_moder_tbl);
-	ena_dev->intr_moder_tbl = NULL;
-}
-
 int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 {
 	struct ena_admin_get_feat_resp get_resp;
@@ -2833,10 +2810,6 @@ int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 		return rc;
 	}
 
-	rc = ena_com_init_interrupt_moderation_table(ena_dev);
-	if (rc)
-		goto err;
-
 	/* if moderation is supported by device we set adaptive moderation */
 	delay_resolution = get_resp.u.intr_moderation.intr_delay_resolution;
 	ena_com_update_intr_delay_resolution(ena_dev, delay_resolution);
@@ -2845,52 +2818,6 @@ int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 	ena_com_disable_adaptive_moderation(ena_dev);
 
 	return 0;
-err:
-	ena_com_destroy_interrupt_moderation(ena_dev);
-	return rc;
-}
-
-void ena_com_config_default_interrupt_moderation_table(struct ena_com_dev *ena_dev)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (!intr_moder_tbl)
-		return;
-
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].intr_moder_interval =
-		ENA_INTR_LOWEST_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].pkts_per_interval =
-		ENA_INTR_LOWEST_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_LOWEST].bytes_per_interval =
-		ENA_INTR_LOWEST_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_LOW].intr_moder_interval =
-		ENA_INTR_LOW_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_LOW].pkts_per_interval =
-		ENA_INTR_LOW_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_LOW].bytes_per_interval =
-		ENA_INTR_LOW_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_MID].intr_moder_interval =
-		ENA_INTR_MID_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_MID].pkts_per_interval =
-		ENA_INTR_MID_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_MID].bytes_per_interval =
-		ENA_INTR_MID_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].intr_moder_interval =
-		ENA_INTR_HIGH_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].pkts_per_interval =
-		ENA_INTR_HIGH_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGH].bytes_per_interval =
-		ENA_INTR_HIGH_BYTES;
-
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].intr_moder_interval =
-		ENA_INTR_HIGHEST_USECS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].pkts_per_interval =
-		ENA_INTR_HIGHEST_PKTS;
-	intr_moder_tbl[ENA_INTR_MODER_HIGHEST].bytes_per_interval =
-		ENA_INTR_HIGHEST_BYTES;
 }
 
 unsigned int ena_com_get_nonadaptive_moderation_interval_tx(struct ena_com_dev *ena_dev)
@@ -2903,43 +2830,6 @@ unsigned int ena_com_get_nonadaptive_moderation_interval_rx(struct ena_com_dev *
 	return ena_dev->intr_moder_rx_interval;
 }
 
-void ena_com_init_intr_moderation_entry(struct ena_com_dev *ena_dev,
-					enum ena_intr_moder_level level,
-					struct ena_intr_moder_entry *entry)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (level >= ENA_INTR_MAX_NUM_OF_LEVELS)
-		return;
-
-	intr_moder_tbl[level].intr_moder_interval = entry->intr_moder_interval;
-	if (ena_dev->intr_delay_resolution)
-		intr_moder_tbl[level].intr_moder_interval /=
-			ena_dev->intr_delay_resolution;
-	intr_moder_tbl[level].pkts_per_interval = entry->pkts_per_interval;
-
-	/* use hardcoded value until ethtool supports bytecount parameter */
-	if (entry->bytes_per_interval != ENA_INTR_BYTE_COUNT_NOT_SUPPORTED)
-		intr_moder_tbl[level].bytes_per_interval = entry->bytes_per_interval;
-}
-
-void ena_com_get_intr_moderation_entry(struct ena_com_dev *ena_dev,
-				       enum ena_intr_moder_level level,
-				       struct ena_intr_moder_entry *entry)
-{
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-
-	if (level >= ENA_INTR_MAX_NUM_OF_LEVELS)
-		return;
-
-	entry->intr_moder_interval = intr_moder_tbl[level].intr_moder_interval;
-	if (ena_dev->intr_delay_resolution)
-		entry->intr_moder_interval *= ena_dev->intr_delay_resolution;
-	entry->pkts_per_interval =
-	intr_moder_tbl[level].pkts_per_interval;
-	entry->bytes_per_interval = intr_moder_tbl[level].bytes_per_interval;
-}
-
 int ena_com_config_dev_mode(struct ena_com_dev *ena_dev,
 			    struct ena_admin_feature_llq_desc *llq_features,
 			    struct ena_llq_configurations *llq_default_cfg)
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
index baeefc6af4f3..ddc2a8c50333 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_com.h
@@ -72,46 +72,13 @@
 /*****************************************************************************/
 /* ENA adaptive interrupt moderation settings */
 
-#define ENA_INTR_LOWEST_USECS           (0)
-#define ENA_INTR_LOWEST_PKTS            (3)
-#define ENA_INTR_LOWEST_BYTES           (2 * 1524)
-
-#define ENA_INTR_LOW_USECS              (32)
-#define ENA_INTR_LOW_PKTS               (12)
-#define ENA_INTR_LOW_BYTES              (16 * 1024)
-
-#define ENA_INTR_MID_USECS              (80)
-#define ENA_INTR_MID_PKTS               (48)
-#define ENA_INTR_MID_BYTES              (64 * 1024)
-
-#define ENA_INTR_HIGH_USECS             (128)
-#define ENA_INTR_HIGH_PKTS              (96)
-#define ENA_INTR_HIGH_BYTES             (128 * 1024)
-
-#define ENA_INTR_HIGHEST_USECS          (192)
-#define ENA_INTR_HIGHEST_PKTS           (128)
-#define ENA_INTR_HIGHEST_BYTES          (192 * 1024)
-
 #define ENA_INTR_INITIAL_TX_INTERVAL_USECS		196
 #define ENA_INTR_INITIAL_RX_INTERVAL_USECS		0
-#define ENA_INTR_DELAY_OLD_VALUE_WEIGHT			6
-#define ENA_INTR_DELAY_NEW_VALUE_WEIGHT			4
-#define ENA_INTR_MODER_LEVEL_STRIDE			2
-#define ENA_INTR_BYTE_COUNT_NOT_SUPPORTED		0xFFFFFF
 
 #define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
 
 #define ENA_FEATURE_MAX_QUEUE_EXT_VER	1
 
-enum ena_intr_moder_level {
-	ENA_INTR_MODER_LOWEST = 0,
-	ENA_INTR_MODER_LOW,
-	ENA_INTR_MODER_MID,
-	ENA_INTR_MODER_HIGH,
-	ENA_INTR_MODER_HIGHEST,
-	ENA_INTR_MAX_NUM_OF_LEVELS,
-};
-
 struct ena_llq_configurations {
 	enum ena_admin_llq_header_location llq_header_location;
 	enum ena_admin_llq_ring_entry_size llq_ring_entry_size;
@@ -120,12 +87,6 @@ struct ena_llq_configurations {
 	u16 llq_ring_entry_size_value;
 };
 
-struct ena_intr_moder_entry {
-	unsigned int intr_moder_interval;
-	unsigned int pkts_per_interval;
-	unsigned int bytes_per_interval;
-};
-
 enum queue_direction {
 	ENA_COM_IO_QUEUE_DIRECTION_TX,
 	ENA_COM_IO_QUEUE_DIRECTION_RX
@@ -920,11 +881,6 @@ int ena_com_execute_admin_command(struct ena_com_admin_queue *admin_queue,
  */
 int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev);
 
-/* ena_com_destroy_interrupt_moderation - Destroy interrupt moderation resources
- * @ena_dev: ENA communication layer struct
- */
-void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev);
-
 /* ena_com_interrupt_moderation_supported - Return if interrupt moderation
  * capability is supported by the device.
  *
@@ -932,12 +888,6 @@ void ena_com_destroy_interrupt_moderation(struct ena_com_dev *ena_dev);
  */
 bool ena_com_interrupt_moderation_supported(struct ena_com_dev *ena_dev);
 
-/* ena_com_config_default_interrupt_moderation_table - Restore the interrupt
- * moderation table back to the default parameters.
- * @ena_dev: ENA communication layer struct
- */
-void ena_com_config_default_interrupt_moderation_table(struct ena_com_dev *ena_dev);
-
 /* ena_com_update_nonadaptive_moderation_interval_tx - Update the
  * non-adaptive interval in Tx direction.
  * @ena_dev: ENA communication layer struct
@@ -974,29 +924,6 @@ unsigned int ena_com_get_nonadaptive_moderation_interval_tx(struct ena_com_dev *
  */
 unsigned int ena_com_get_nonadaptive_moderation_interval_rx(struct ena_com_dev *ena_dev);
 
-/* ena_com_init_intr_moderation_entry - Update a single entry in the interrupt
- * moderation table.
- * @ena_dev: ENA communication layer struct
- * @level: Interrupt moderation table level
- * @entry: Entry value
- *
- * Update a single entry in the interrupt moderation table.
- */
-void ena_com_init_intr_moderation_entry(struct ena_com_dev *ena_dev,
-					enum ena_intr_moder_level level,
-					struct ena_intr_moder_entry *entry);
-
-/* ena_com_get_intr_moderation_entry - Init ena_intr_moder_entry.
- * @ena_dev: ENA communication layer struct
- * @level: Interrupt moderation table level
- * @entry: Entry to fill.
- *
- * Initialize the entry according to the adaptive interrupt moderation table.
- */
-void ena_com_get_intr_moderation_entry(struct ena_com_dev *ena_dev,
-				       enum ena_intr_moder_level level,
-				       struct ena_intr_moder_entry *entry);
-
 /* ena_com_config_dev_mode - Configure the placement policy of the device.
  * @ena_dev: ENA communication layer struct
  * @llq_features: LLQ feature descriptor, retrieve via
@@ -1022,75 +949,6 @@ static inline void ena_com_disable_adaptive_moderation(struct ena_com_dev *ena_d
 	ena_dev->adaptive_coalescing = false;
 }
 
-/* ena_com_calculate_interrupt_delay - Calculate new interrupt delay
- * @ena_dev: ENA communication layer struct
- * @pkts: Number of packets since the last update
- * @bytes: Number of bytes received since the last update.
- * @smoothed_interval: Returned interval
- * @moder_tbl_idx: Current table level as input update new level as return
- * value.
- */
-static inline void ena_com_calculate_interrupt_delay(struct ena_com_dev *ena_dev,
-						     unsigned int pkts,
-						     unsigned int bytes,
-						     unsigned int *smoothed_interval,
-						     unsigned int *moder_tbl_idx)
-{
-	enum ena_intr_moder_level curr_moder_idx, new_moder_idx;
-	struct ena_intr_moder_entry *curr_moder_entry;
-	struct ena_intr_moder_entry *pred_moder_entry;
-	struct ena_intr_moder_entry *new_moder_entry;
-	struct ena_intr_moder_entry *intr_moder_tbl = ena_dev->intr_moder_tbl;
-	unsigned int interval;
-
-	/* We apply adaptive moderation on Rx path only.
-	 * Tx uses static interrupt moderation.
-	 */
-	if (!pkts || !bytes)
-		/* Tx interrupt, or spurious interrupt,
-		 * in both cases we just use same delay values
-		 */
-		return;
-
-	curr_moder_idx = (enum ena_intr_moder_level)(*moder_tbl_idx);
-	if (unlikely(curr_moder_idx >= ENA_INTR_MAX_NUM_OF_LEVELS)) {
-		pr_err("Wrong moderation index %u\n", curr_moder_idx);
-		return;
-	}
-
-	curr_moder_entry = &intr_moder_tbl[curr_moder_idx];
-	new_moder_idx = curr_moder_idx;
-
-	if (curr_moder_idx == ENA_INTR_MODER_LOWEST) {
-		if ((pkts > curr_moder_entry->pkts_per_interval) ||
-		    (bytes > curr_moder_entry->bytes_per_interval))
-			new_moder_idx =
-				(enum ena_intr_moder_level)(curr_moder_idx + ENA_INTR_MODER_LEVEL_STRIDE);
-	} else {
-		pred_moder_entry = &intr_moder_tbl[curr_moder_idx - ENA_INTR_MODER_LEVEL_STRIDE];
-
-		if ((pkts <= pred_moder_entry->pkts_per_interval) ||
-		    (bytes <= pred_moder_entry->bytes_per_interval))
-			new_moder_idx =
-				(enum ena_intr_moder_level)(curr_moder_idx - ENA_INTR_MODER_LEVEL_STRIDE);
-		else if ((pkts > curr_moder_entry->pkts_per_interval) ||
-			 (bytes > curr_moder_entry->bytes_per_interval)) {
-			if (curr_moder_idx != ENA_INTR_MODER_HIGHEST)
-				new_moder_idx =
-					(enum ena_intr_moder_level)(curr_moder_idx + ENA_INTR_MODER_LEVEL_STRIDE);
-		}
-	}
-	new_moder_entry = &intr_moder_tbl[new_moder_idx];
-
-	interval = new_moder_entry->intr_moder_interval;
-	*smoothed_interval = (
-		(interval * ENA_INTR_DELAY_NEW_VALUE_WEIGHT +
-		ENA_INTR_DELAY_OLD_VALUE_WEIGHT * (*smoothed_interval)) + 5) /
-		10;
-
-	*moder_tbl_idx = new_moder_idx;
-}
-
 /* ena_com_update_intr_reg - Prepare interrupt register
  * @intr_reg: interrupt register to update.
  * @rx_delay_interval: Rx interval in usecs
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 02/11] net: ena: switch to dim algorithm for rx adaptive interrupt moderation
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

Use the dim library for the rx adaptive interrupt moderation implementation

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_com.c    |  4 +-
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 55 +++++++++++++-------
 drivers/net/ethernet/amazon/ena/ena_netdev.h |  3 ++
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 4144936bf8a2..f3c67c27c19c 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -2839,9 +2839,7 @@ int ena_com_init_interrupt_moderation(struct ena_com_dev *ena_dev)
 	delay_resolution = get_resp.u.intr_moderation.intr_delay_resolution;
 	ena_com_update_intr_delay_resolution(ena_dev, delay_resolution);
 
-	/* Disable adaptive moderation by default - can be enabled from
-	 * ethtool
-	 */
+	/* Disable adaptive moderation by default - can be enabled later */
 	ena_com_disable_adaptive_moderation(ena_dev);
 
 	return 0;
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 233b252ceb9e..cdcc169b87fa 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -196,6 +196,7 @@ static void ena_init_io_rings(struct ena_adapter *adapter)
 		rxr->smoothed_interval =
 			ena_com_get_nonadaptive_moderation_interval_rx(ena_dev);
 		rxr->empty_rx_queue = 0;
+		adapter->ena_napi[i].dim.mode = DIM_CQ_PERIOD_MODE_START_FROM_EQE;
 	}
 }
 
@@ -712,6 +713,7 @@ static void ena_destroy_all_rx_queues(struct ena_adapter *adapter)
 
 	for (i = 0; i < adapter->num_queues; i++) {
 		ena_qid = ENA_IO_RXQ_IDX(i);
+		cancel_work_sync(&adapter->ena_napi[i].dim.work);
 		ena_com_destroy_io_queue(adapter->ena_dev, ena_qid);
 	}
 }
@@ -1155,23 +1157,35 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
 	return 0;
 }
 
-void ena_adjust_intr_moderation(struct ena_ring *rx_ring,
-				       struct ena_ring *tx_ring)
+static void ena_dim_work(struct work_struct *w)
 {
-	/* We apply adaptive moderation on Rx path only.
-	 * Tx uses static interrupt moderation.
-	 */
-	ena_com_calculate_interrupt_delay(rx_ring->ena_dev,
-					  rx_ring->per_napi_packets,
-					  rx_ring->per_napi_bytes,
-					  &rx_ring->smoothed_interval,
-					  &rx_ring->moder_tbl_idx);
-
-	/* Reset per napi packets/bytes */
-	tx_ring->per_napi_packets = 0;
-	tx_ring->per_napi_bytes = 0;
+	struct dim *dim = container_of(w, struct dim, work);
+	struct dim_cq_moder cur_moder =
+		net_dim_get_rx_moderation(dim->mode, dim->profile_ix);
+	struct ena_napi *ena_napi = container_of(dim, struct ena_napi, dim);
+
+	ena_napi->rx_ring->smoothed_interval = cur_moder.usec;
+	dim->state = DIM_START_MEASURE;
+}
+
+static void ena_adjust_adaptive_rx_intr_moderation(struct ena_napi *ena_napi)
+{
+	struct dim_sample dim_sample;
+	struct ena_ring *rx_ring = ena_napi->rx_ring;
+
+	if (!rx_ring->per_napi_packets)
+		return;
+
+	rx_ring->non_empty_napi_events++;
+
+	dim_update_sample(rx_ring->non_empty_napi_events,
+			  rx_ring->rx_stats.cnt,
+			  rx_ring->rx_stats.bytes,
+			  &dim_sample);
+
+	net_dim(&ena_napi->dim, dim_sample);
+
 	rx_ring->per_napi_packets = 0;
-	rx_ring->per_napi_bytes = 0;
 }
 
 static void ena_unmask_interrupt(struct ena_ring *tx_ring,
@@ -1260,9 +1274,11 @@ static int ena_io_poll(struct napi_struct *napi, int budget)
 		 * from the interrupt context (vs from sk_busy_loop)
 		 */
 		if (napi_complete_done(napi, rx_work_done)) {
-			/* Tx and Rx share the same interrupt vector */
+			/* We apply adaptive moderation on Rx path only.
+			 * Tx uses static interrupt moderation.
+			 */
 			if (ena_com_get_adaptive_moderation_enabled(rx_ring->ena_dev))
-				ena_adjust_intr_moderation(rx_ring, tx_ring);
+				ena_adjust_adaptive_rx_intr_moderation(ena_napi);
 
 			ena_unmask_interrupt(tx_ring, rx_ring);
 		}
@@ -1740,13 +1756,16 @@ static int ena_create_all_io_rx_queues(struct ena_adapter *adapter)
 		rc = ena_create_io_rx_queue(adapter, i);
 		if (rc)
 			goto create_err;
+		INIT_WORK(&adapter->ena_napi[i].dim.work, ena_dim_work);
 	}
 
 	return 0;
 
 create_err:
-	while (i--)
+	while (i--) {
+		cancel_work_sync(&adapter->ena_napi[i].dim.work);
 		ena_com_destroy_io_queue(ena_dev, ENA_IO_RXQ_IDX(i));
+	}
 
 	return rc;
 }
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.h b/drivers/net/ethernet/amazon/ena/ena_netdev.h
index efbcffd22215..f67ecab3389a 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.h
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.h
@@ -34,6 +34,7 @@
 #define ENA_H
 
 #include <linux/bitops.h>
+#include <linux/dim.h>
 #include <linux/etherdevice.h>
 #include <linux/inetdevice.h>
 #include <linux/interrupt.h>
@@ -153,6 +154,7 @@ struct ena_napi {
 	struct ena_ring *tx_ring;
 	struct ena_ring *rx_ring;
 	u32 qid;
+	struct dim dim;
 };
 
 struct ena_calc_queue_size_ctx {
@@ -280,6 +282,7 @@ struct ena_ring {
 	u32  per_napi_packets;
 	u32  per_napi_bytes;
 	enum ena_intr_moder_level moder_tbl_idx;
+	u16 non_empty_napi_events;
 	struct u64_stats_sync syncp;
 	union {
 		struct ena_stats_tx tx_stats;
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 11/11] net: ena: fix incorrect update of intr_delay_resolution
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

ena_dev->intr_moder_rx/tx_interval save the intervals received from the
user after dividing them by ena_dev->intr_delay_resolution. Therefore
when intr_delay_resolution changes, the code needs to first mutiply
intr_moder_rx/tx_interval by the previous intr_delay_resolution to get
the value originally given by the user, and only then divide it by the
new intr_delay_resolution.

Current code does not first multiply intr_moder_rx/tx_interval by the old
intr_delay_resolution. This commit fixes it.

Also initialize ena_dev->intr_delay_resolution to be 1.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_com.c    | 21 ++++++++++++++++----
 drivers/net/ethernet/amazon/ena/ena_com.h    |  1 +
 drivers/net/ethernet/amazon/ena/ena_netdev.c |  1 +
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_com.c b/drivers/net/ethernet/amazon/ena/ena_com.c
index 621b747f062b..ea62604fdf8c 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.c
+++ b/drivers/net/ethernet/amazon/ena/ena_com.c
@@ -1281,17 +1281,30 @@ static int ena_com_ind_tbl_convert_from_device(struct ena_com_dev *ena_dev)
 static void ena_com_update_intr_delay_resolution(struct ena_com_dev *ena_dev,
 						 u16 intr_delay_resolution)
 {
+	/* Initial value of intr_delay_resolution might be 0 */
+	u16 prev_intr_delay_resolution =
+		ena_dev->intr_delay_resolution ?
+		ena_dev->intr_delay_resolution :
+		ENA_DEFAULT_INTR_DELAY_RESOLUTION;
+
 	if (!intr_delay_resolution) {
 		pr_err("Illegal intr_delay_resolution provided. Going to use default 1 usec resolution\n");
-		intr_delay_resolution = 1;
+		intr_delay_resolution = ENA_DEFAULT_INTR_DELAY_RESOLUTION;
 	}
-	ena_dev->intr_delay_resolution = intr_delay_resolution;
 
 	/* update Rx */
-	ena_dev->intr_moder_rx_interval /= intr_delay_resolution;
+	ena_dev->intr_moder_rx_interval =
+		ena_dev->intr_moder_rx_interval *
+		prev_intr_delay_resolution /
+		intr_delay_resolution;
 
 	/* update Tx */
-	ena_dev->intr_moder_tx_interval /= intr_delay_resolution;
+	ena_dev->intr_moder_tx_interval =
+		ena_dev->intr_moder_tx_interval *
+		prev_intr_delay_resolution /
+		intr_delay_resolution;
+
+	ena_dev->intr_delay_resolution = intr_delay_resolution;
 }
 
 /*****************************************************************************/
diff --git a/drivers/net/ethernet/amazon/ena/ena_com.h b/drivers/net/ethernet/amazon/ena/ena_com.h
index ddc2a8c50333..7c941eba0bc9 100644
--- a/drivers/net/ethernet/amazon/ena/ena_com.h
+++ b/drivers/net/ethernet/amazon/ena/ena_com.h
@@ -74,6 +74,7 @@
 
 #define ENA_INTR_INITIAL_TX_INTERVAL_USECS		196
 #define ENA_INTR_INITIAL_RX_INTERVAL_USECS		0
+#define ENA_DEFAULT_INTR_DELAY_RESOLUTION		1
 
 #define ENA_HW_HINTS_NO_TIMEOUT				0xFFFF
 
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 54539e57aa73..e4bf7a4af87a 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -3500,6 +3500,7 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	*/
 	ena_dev->intr_moder_tx_interval = ENA_INTR_INITIAL_TX_INTERVAL_USECS;
 	ena_dev->intr_moder_rx_interval = ENA_INTR_INITIAL_RX_INTERVAL_USECS;
+	ena_dev->intr_delay_resolution = ENA_DEFAULT_INTR_DELAY_RESOLUTION;
 	io_queue_num = ena_calc_io_queue_num(pdev, ena_dev, &get_feat_ctx);
 	rc = ena_calc_queue_size(&calc_queue_ctx);
 	if (rc || io_queue_num <= 0) {
-- 
2.17.2


^ permalink raw reply related

* [PATCH V2 net-next 00/11] net: ena: implement adaptive interrupt moderation using dim
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan

From: Arthur Kiyanovski <akiyano@amazon.com>

In this patchset we replace our adaptive interrupt moderation
implementation with the dim library implementation.
The dim library showed great improvement in throughput, latency
and CPU usage in different scenarios on ARM CPUs.
This patchset also includes a few bug fixes to the parts of the
old implementation of adaptive interrupt moderation that were left.

Changes from V1 patchset: 
Removed stray empty lines from patches 01/11, 09/11.

Arthur Kiyanovski (11):
  net: ena: add intr_moder_rx_interval to struct ena_com_dev and use it
  net: ena: switch to dim algorithm for rx adaptive interrupt moderation
  net: ena: reimplement set/get_coalesce()
  net: ena: enable the interrupt_moderation in driver_supported_features
  net: ena: remove code duplication in
    ena_com_update_nonadaptive_moderation_interval _*()
  net: ena: remove old adaptive interrupt moderation code from
    ena_netdev
  net: ena: remove ena_restore_ethtool_params() and relevant fields
  net: ena: remove all old adaptive rx interrupt moderation code from
    ena_com
  net: ena: fix update of interrupt moderation register
  net: ena: fix retrieval of nonadaptive interrupt moderation intervals
  net: ena: fix incorrect update of intr_delay_resolution

 .../net/ethernet/amazon/ena/ena_admin_defs.h  |   8 +
 drivers/net/ethernet/amazon/ena/ena_com.c     | 175 ++++--------------
 drivers/net/ethernet/amazon/ena/ena_com.h     | 151 +--------------
 drivers/net/ethernet/amazon/ena/ena_ethtool.c |  89 +++------
 drivers/net/ethernet/amazon/ena/ena_netdev.c  |  85 +++++----
 drivers/net/ethernet/amazon/ena/ena_netdev.h  |   8 +-
 6 files changed, 128 insertions(+), 388 deletions(-)

-- 
2.17.2


^ permalink raw reply

* [PATCH V2 net-next 10/11] net: ena: fix retrieval of nonadaptive interrupt moderation intervals
From: akiyano @ 2019-09-16 11:31 UTC (permalink / raw)
  To: davem, netdev
  Cc: Arthur Kiyanovski, dwmw, zorik, matua, saeedb, msw, aliguori,
	nafea, gtzalik, netanel, alisaidi, benh, sameehj, ndagan
In-Reply-To: <1568633496-4143-1-git-send-email-akiyano@amazon.com>

From: Arthur Kiyanovski <akiyano@amazon.com>

Nonadaptive interrupt moderation intervals are assigned the value set
by the user in ethtool -C divided by ena_dev->intr_delay_resolution.

Therefore when the user tries to get the nonadaptive interrupt moderation
intervals with ethtool -c the code needs to multiply the saved value
by ena_dev->intr_delay_resolution.

The current code erroneously divides instead of multiplying in ethtool -c.
This patch fixes this.

Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_ethtool.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_ethtool.c b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
index 0f90e2296630..16553d92fad2 100644
--- a/drivers/net/ethernet/amazon/ena/ena_ethtool.c
+++ b/drivers/net/ethernet/amazon/ena/ena_ethtool.c
@@ -310,14 +310,15 @@ static int ena_get_coalesce(struct net_device *net_dev,
 		/* the devie doesn't support interrupt moderation */
 		return -EOPNOTSUPP;
 	}
+
 	coalesce->tx_coalesce_usecs =
-		ena_com_get_nonadaptive_moderation_interval_tx(ena_dev) /
+		ena_com_get_nonadaptive_moderation_interval_tx(ena_dev) *
 			ena_dev->intr_delay_resolution;
 
 	if (!ena_com_get_adaptive_moderation_enabled(ena_dev))
 		coalesce->rx_coalesce_usecs =
 			ena_com_get_nonadaptive_moderation_interval_rx(ena_dev)
-			/ ena_dev->intr_delay_resolution;
+			* ena_dev->intr_delay_resolution;
 
 	coalesce->use_adaptive_rx_coalesce =
 		ena_com_get_adaptive_moderation_enabled(ena_dev);
-- 
2.17.2


^ permalink raw reply related

* RE: [PATCH V1 net-next 01/11] net: ena: add intr_moder_rx_interval to struct ena_com_dev and use it
From: Kiyanovski, Arthur @ 2019-09-16 11:39 UTC (permalink / raw)
  To: David Miller
  Cc: netdev@vger.kernel.org, Woodhouse, David, Machulsky, Zorik,
	Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
	Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
	Saidi, Ali, Herrenschmidt, Benjamin, Jubran, Samih, Dagan, Noam
In-Reply-To: <20190915.193241.878202512573492759.davem@davemloft.net>

> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Sunday, September 15, 2019 9:33 PM
> To: Kiyanovski, Arthur <akiyano@amazon.com>
> Cc: netdev@vger.kernel.org; Woodhouse, David <dwmw@amazon.co.uk>;
> Machulsky, Zorik <zorik@amazon.com>; Matushevsky, Alexander
> <matua@amazon.com>; Bshara, Saeed <saeedb@amazon.com>; Wilson, Matt
> <msw@amazon.com>; Liguori, Anthony <aliguori@amazon.com>; Bshara,
> Nafea <nafea@amazon.com>; Tzalik, Guy <gtzalik@amazon.com>; Belgazal,
> Netanel <netanel@amazon.com>; Saidi, Ali <alisaidi@amazon.com>;
> Herrenschmidt, Benjamin <benh@amazon.com>; Jubran, Samih
> <sameehj@amazon.com>; Dagan, Noam <ndagan@amazon.com>
> Subject: Re: [PATCH V1 net-next 01/11] net: ena: add intr_moder_rx_interval to
> struct ena_com_dev and use it
> 
> From: <akiyano@amazon.com>
> Date: Fri, 13 Sep 2019 01:08:38 +0300
> 
> > @@ -1307,8 +1304,8 @@ static void
> ena_com_update_intr_delay_resolution(struct ena_com_dev *ena_dev,
> >  	ena_dev->intr_delay_resolution = intr_delay_resolution;
> >
> >  	/* update Rx */
> > -	for (i = 0; i < ENA_INTR_MAX_NUM_OF_LEVELS; i++)
> > -		intr_moder_tbl[i].intr_moder_interval /= intr_delay_resolution;
> > +	ena_dev->intr_moder_rx_interval /= intr_delay_resolution;
> > +
> >
> >  	/* update Tx */
> 
> Now there are two empty lines here, please remove one of them.

Thanks.
For some reason checkpatch did not catch this and also another such extra space.
Sent out V2 of the patchset.

^ permalink raw reply

* Re: [PATCH v5 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable
From: Andrew Lunn @ 2019-09-16 11:57 UTC (permalink / raw)
  To: Alexandru Ardelean
  Cc: netdev, devicetree, linux-kernel, davem, robh+dt, mark.rutland,
	f.fainelli, hkallweit1, mkubecek
In-Reply-To: <20190916073526.24711-3-alexandru.ardelean@analog.com>

On Mon, Sep 16, 2019 at 10:35:26AM +0300, Alexandru Ardelean wrote:
> This driver becomes the first user of the kernel's `ETHTOOL_PHY_EDPD`
> phy-tunable feature.
> EDPD is also enabled by default on PHY config_init, but can be disabled via
> the phy-tunable control.
> 
> When enabling EDPD, it's also a good idea (for the ADIN PHYs) to enable TX
> periodic pulses, so that in case the other PHY is also on EDPD mode, there
> is no lock-up situation where both sides are waiting for the other to
> transmit.
> 
> Via the phy-tunable control, TX pulses can be disabled if specifying 0
> `tx-interval` via ethtool.
> 
> The ADIN PHY supports only fixed 1 second intervals; they cannot be
> configured. That is why the acceptable values are 1,
> ETHTOOL_PHY_EDPD_DFLT_TX_MSECS and ETHTOOL_PHY_EDPD_NO_TX (which disables
> TX pulses).
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* [PATCH] selftests: Add test cases for `ip nexthop flush proto XX`
From: Donald Sharp @ 2019-09-16 12:26 UTC (permalink / raw)
  To: netdev, dsahern

Add some test cases to allow the fib_nexthops.sh test code
to test the flushing of nexthops based upon the proto passed
in upon creation of the nexthop group.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
---
 tools/testing/selftests/net/fib_nexthops.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
index f9ebeac1e6f2..796670ebc65b 100755
--- a/tools/testing/selftests/net/fib_nexthops.sh
+++ b/tools/testing/selftests/net/fib_nexthops.sh
@@ -940,6 +940,20 @@ basic()
 	run_cmd "$IP nexthop add id 104 group 1 dev veth1"
 	log_test $? 2 "Nexthop group and device"
 
+	# Tests to ensure that flushing works as expected.
+	run_cmd "$IP nexthop add id 105 blackhole proto 99"
+	run_cmd "$IP nexthop add id 106 blackhole proto 100"
+	run_cmd "$IP nexthop add id 107 blackhole proto 99"
+	run_cmd "$IP nexthop flush proto 99"
+	check_nexthop "id 105" ""
+	check_nexthop "id 106" "id 106 blackhole proto 100"
+	check_nexthop "id 107" ""
+	run_cmd "$IP nexthop flush proto 100"
+	check_nexthop "id 106" ""
+
+	run_cmd "$IP nexthop flush proto 100"
+	log_test $? 0 "Test proto flush"
+
 	run_cmd "$IP nexthop add id 104 group 1 blackhole"
 	log_test $? 2 "Nexthop group and blackhole"
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH iproute2-next v2] bpf: replace snprintf with asprintf when dealing with long buffers
From: Andrea Claudi @ 2019-09-16 13:00 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern

This reduces stack usage, as asprintf allocates memory on the heap.

This indirectly fixes a snprintf truncation warning (from gcc v9.2.1):

bpf.c: In function ‘bpf_get_work_dir’:
bpf.c:784:49: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
  784 |  snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
      |                                                 ^
bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096
  784 |  snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fixes: e42256699cac ("bpf: make tc's bpf loader generic and move into lib")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 lib/bpf.c | 155 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 116 insertions(+), 39 deletions(-)

diff --git a/lib/bpf.c b/lib/bpf.c
index 7d2a322ffbaec..0fe2e9eb2cbf8 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -406,13 +406,21 @@ static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map,
 					  struct bpf_map_ext *ext)
 {
 	unsigned int val, owner_type = 0, owner_jited = 0;
-	char file[PATH_MAX], buff[4096];
+	char *file = NULL;
+	char buff[4096];
 	FILE *fp;
+	int ret;
 
-	snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
+	ret = asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		free(file);
+		return ret;
+	}
 	memset(map, 0, sizeof(*map));
 
 	fp = fopen(file, "r");
+	free(file);
 	if (!fp) {
 		fprintf(stderr, "No procfs support?!\n");
 		return -EIO;
@@ -600,8 +608,9 @@ int bpf_trace_pipe(void)
 		0,
 	};
 	int fd_in, fd_out = STDERR_FILENO;
-	char tpipe[PATH_MAX];
+	char *tpipe = NULL;
 	const char *mnt;
+	int ret;
 
 	mnt = bpf_find_mntpt("tracefs", TRACEFS_MAGIC, tracefs_mnt,
 			     sizeof(tracefs_mnt), tracefs_known_mnts);
@@ -610,9 +619,15 @@ int bpf_trace_pipe(void)
 		return -1;
 	}
 
-	snprintf(tpipe, sizeof(tpipe), "%s/trace_pipe", mnt);
+	ret = asprintf(&tpipe, "%s/trace_pipe", mnt);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		free(tpipe);
+		return ret;
+	}
 
 	fd_in = open(tpipe, O_RDONLY);
+	free(tpipe);
 	if (fd_in < 0)
 		return -1;
 
@@ -633,37 +648,50 @@ int bpf_trace_pipe(void)
 
 static int bpf_gen_global(const char *bpf_sub_dir)
 {
-	char bpf_glo_dir[PATH_MAX];
+	char *bpf_glo_dir = NULL;
 	int ret;
 
-	snprintf(bpf_glo_dir, sizeof(bpf_glo_dir), "%s/%s/",
-		 bpf_sub_dir, BPF_DIR_GLOBALS);
+	ret = asprintf(&bpf_glo_dir, "%s/%s/", bpf_sub_dir, BPF_DIR_GLOBALS);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
 
 	ret = mkdir(bpf_glo_dir, S_IRWXU);
 	if (ret && errno != EEXIST) {
 		fprintf(stderr, "mkdir %s failed: %s\n", bpf_glo_dir,
 			strerror(errno));
-		return ret;
+		goto out;
 	}
 
-	return 0;
+	ret = 0;
+out:
+	free(bpf_glo_dir);
+	return ret;
 }
 
 static int bpf_gen_master(const char *base, const char *name)
 {
-	char bpf_sub_dir[PATH_MAX + NAME_MAX + 1];
+	char *bpf_sub_dir = NULL;
 	int ret;
 
-	snprintf(bpf_sub_dir, sizeof(bpf_sub_dir), "%s%s/", base, name);
+	ret = asprintf(&bpf_sub_dir, "%s%s/", base, name);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
 
 	ret = mkdir(bpf_sub_dir, S_IRWXU);
 	if (ret && errno != EEXIST) {
 		fprintf(stderr, "mkdir %s failed: %s\n", bpf_sub_dir,
 			strerror(errno));
-		return ret;
+		goto out;
 	}
 
-	return bpf_gen_global(bpf_sub_dir);
+	ret = bpf_gen_global(bpf_sub_dir);
+out:
+	free(bpf_sub_dir);
+	return ret;
 }
 
 static int bpf_slave_via_bind_mnt(const char *full_name,
@@ -692,13 +720,22 @@ static int bpf_slave_via_bind_mnt(const char *full_name,
 static int bpf_gen_slave(const char *base, const char *name,
 			 const char *link)
 {
-	char bpf_lnk_dir[PATH_MAX + NAME_MAX + 1];
-	char bpf_sub_dir[PATH_MAX + NAME_MAX];
+	char *bpf_lnk_dir = NULL;
+	char *bpf_sub_dir = NULL;
 	struct stat sb = {};
 	int ret;
 
-	snprintf(bpf_lnk_dir, sizeof(bpf_lnk_dir), "%s%s/", base, link);
-	snprintf(bpf_sub_dir, sizeof(bpf_sub_dir), "%s%s",  base, name);
+	ret = asprintf(&bpf_lnk_dir, "%s%s/", base, link);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
+
+	ret = asprintf(&bpf_sub_dir, "%s%s",  base, name);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
 
 	ret = symlink(bpf_lnk_dir, bpf_sub_dir);
 	if (ret) {
@@ -706,25 +743,30 @@ static int bpf_gen_slave(const char *base, const char *name,
 			if (errno != EPERM) {
 				fprintf(stderr, "symlink %s failed: %s\n",
 					bpf_sub_dir, strerror(errno));
-				return ret;
+				goto out;
 			}
 
-			return bpf_slave_via_bind_mnt(bpf_sub_dir,
-						      bpf_lnk_dir);
+			ret = bpf_slave_via_bind_mnt(bpf_sub_dir, bpf_lnk_dir);
+			goto out;
 		}
 
 		ret = lstat(bpf_sub_dir, &sb);
 		if (ret) {
 			fprintf(stderr, "lstat %s failed: %s\n",
 				bpf_sub_dir, strerror(errno));
-			return ret;
+			goto out;
 		}
 
-		if ((sb.st_mode & S_IFMT) != S_IFLNK)
-			return bpf_gen_global(bpf_sub_dir);
+		if ((sb.st_mode & S_IFMT) != S_IFLNK) {
+			ret = bpf_gen_global(bpf_sub_dir);
+			goto out;
+		}
 	}
 
-	return 0;
+out:
+	free(bpf_lnk_dir);
+	free(bpf_sub_dir);
+	return ret;
 }
 
 static int bpf_gen_hierarchy(const char *base)
@@ -742,7 +784,7 @@ static int bpf_gen_hierarchy(const char *base)
 static const char *bpf_get_work_dir(enum bpf_prog_type type)
 {
 	static char bpf_tmp[PATH_MAX] = BPF_DIR_MNT;
-	static char bpf_wrk_dir[PATH_MAX];
+	static char *bpf_wrk_dir;
 	static const char *mnt;
 	static bool bpf_mnt_cached;
 	const char *mnt_env = getenv(BPF_ENV_MNT);
@@ -781,7 +823,12 @@ static const char *bpf_get_work_dir(enum bpf_prog_type type)
 		}
 	}
 
-	snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
+	ret = asprintf(&bpf_wrk_dir, "%s/", mnt);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		free(bpf_wrk_dir);
+		goto out;
+	}
 
 	ret = bpf_gen_hierarchy(bpf_wrk_dir);
 	if (ret) {
@@ -1438,31 +1485,48 @@ static int bpf_probe_pinned(const char *name, const struct bpf_elf_ctx *ctx,
 
 static int bpf_make_obj_path(const struct bpf_elf_ctx *ctx)
 {
-	char tmp[PATH_MAX];
+	char *tmp = NULL;
 	int ret;
 
-	snprintf(tmp, sizeof(tmp), "%s/%s", bpf_get_work_dir(ctx->type),
-		 ctx->obj_uid);
+	ret = asprintf(&tmp, "%s/%s", bpf_get_work_dir(ctx->type), ctx->obj_uid);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
 
 	ret = mkdir(tmp, S_IRWXU);
 	if (ret && errno != EEXIST) {
 		fprintf(stderr, "mkdir %s failed: %s\n", tmp, strerror(errno));
-		return ret;
+		goto out;
 	}
 
-	return 0;
+	ret = 0;
+out:
+	free(tmp);
+	return ret;
 }
 
 static int bpf_make_custom_path(const struct bpf_elf_ctx *ctx,
 				const char *todo)
 {
-	char tmp[PATH_MAX], rem[PATH_MAX], *sub;
+	char *tmp = NULL;
+	char *rem = NULL;
+	char *sub;
 	int ret;
 
-	snprintf(tmp, sizeof(tmp), "%s/../", bpf_get_work_dir(ctx->type));
-	snprintf(rem, sizeof(rem), "%s/", todo);
-	sub = strtok(rem, "/");
+	ret = asprintf(&tmp, "%s/../", bpf_get_work_dir(ctx->type));
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
 
+	ret = asprintf(&rem, "%s/", todo);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		goto out;
+	}
+
+	sub = strtok(rem, "/");
 	while (sub) {
 		if (strlen(tmp) + strlen(sub) + 2 > PATH_MAX)
 			return -EINVAL;
@@ -1474,13 +1538,17 @@ static int bpf_make_custom_path(const struct bpf_elf_ctx *ctx,
 		if (ret && errno != EEXIST) {
 			fprintf(stderr, "mkdir %s failed: %s\n", tmp,
 				strerror(errno));
-			return ret;
+			goto out;
 		}
 
 		sub = strtok(NULL, "/");
 	}
 
-	return 0;
+	ret = 0;
+out:
+	free(rem);
+	free(tmp);
+	return ret;
 }
 
 static int bpf_place_pinned(int fd, const char *name,
@@ -2581,14 +2649,23 @@ struct bpf_jited_aux {
 
 static int bpf_derive_prog_from_fdinfo(int fd, struct bpf_prog_data *prog)
 {
-	char file[PATH_MAX], buff[4096];
+	char *file = NULL;
+	char buff[4096];
 	unsigned int val;
 	FILE *fp;
+	int ret;
+
+	ret = asprintf(&file, "/proc/%d/fdinfo/%d", getpid(), fd);
+	if (ret < 0) {
+		fprintf(stderr, "asprintf failed: %s\n", strerror(errno));
+		free(file);
+		return ret;
+	}
 
-	snprintf(file, sizeof(file), "/proc/%d/fdinfo/%d", getpid(), fd);
 	memset(prog, 0, sizeof(*prog));
 
 	fp = fopen(file, "r");
+	free(file);
 	if (!fp) {
 		fprintf(stderr, "No procfs support?!\n");
 		return -EIO;
-- 
2.21.0


^ permalink raw reply related

* [PATCH bpf v2] libbpf: Remove getsockopt() check for XDP_OPTIONS
From: Toke Høiland-Jørgensen @ 2019-09-16 12:33 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, netdev, bpf
  Cc: Björn Töpel, Toke Høiland-Jørgensen,
	Yonghong Song

The xsk_socket__create() function fails and returns an error if it cannot
get the XDP_OPTIONS through getsockopt(). However, support for XDP_OPTIONS
was not added until kernel 5.3, so this means that creating XSK sockets
always fails on older kernels.

Since the option is just used to set the zero-copy flag in the xsk struct,
and that flag is not really used for anything yet, just remove the
getsockopt() call until a proper use for it is introduced.

Suggested-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
v2:
  - Remove the call entirely.
  
 tools/lib/bpf/xsk.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 680e63066cf3..14aea315c914 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -65,7 +65,6 @@ struct xsk_socket {
 	int xsks_map_fd;
 	__u32 queue_id;
 	char ifname[IFNAMSIZ];
-	bool zc;
 };
 
 struct xsk_nl_info {
@@ -481,7 +480,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 	void *rx_map = NULL, *tx_map = NULL;
 	struct sockaddr_xdp sxdp = {};
 	struct xdp_mmap_offsets off;
-	struct xdp_options opts;
 	struct xsk_socket *xsk;
 	socklen_t optlen;
 	int err;
@@ -601,15 +599,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 
 	xsk->prog_fd = -1;
 
-	optlen = sizeof(opts);
-	err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
-	if (err) {
-		err = -errno;
-		goto out_mmap_tx;
-	}
-
-	xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
-
 	if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
 		err = xsk_setup_xdp_prog(xsk);
 		if (err)
-- 
2.23.0


^ permalink raw reply related

* Re: [v3 2/3] samples: pktgen: add helper functions for IP(v4/v6) CIDR parsing
From: Toke Høiland-Jørgensen @ 2019-09-16 12:52 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Daniel T. Lee; +Cc: David S . Miller, netdev, brouer
In-Reply-To: <20190916085317.02e4d985@carbon>

Jesper Dangaard Brouer <brouer@redhat.com> writes:

> On Sun, 15 Sep 2019 00:13:52 +0900
> "Daniel T. Lee" <danieltimlee@gmail.com> wrote:
>
>> This commit adds CIDR parsing and IP validate helper function to parse
>> single IP or range of IP with CIDR. (e.g. 198.18.0.0/15)
>> 
>> Helpers will be used in prior to set target address in samples/pktgen.
>> 
>> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
>> ---
>> Changes since v3:
>>  * Set errexit option to stop script execution on error
>> 
>>  samples/pktgen/functions.sh | 124 ++++++++++++++++++++++++++++++++++++
>>  1 file changed, 124 insertions(+)
>> 
>> diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
>> index 4af4046d71be..87ae61701904 100644
>> --- a/samples/pktgen/functions.sh
>> +++ b/samples/pktgen/functions.sh
>> @@ -5,6 +5,8 @@
>>  # Author: Jesper Dangaaard Brouer
>>  # License: GPL
>>  
>> +set -o errexit
>
> Unfortunately, this breaks the scripts.
>
> The function proc_cmd are designed to grep after "Result: OK:" which
> might fail, and your patch/change makes the script stop immediately.
> We actually want to continue, and output what command that failed (and
> also grep again after "Result:" to provide the kernel reason).
>
> Even if you somehow "fix" function proc_cmd, then we in general want to
> catch different error situations by looking at status $?, and output
> meaning full errors via calling err() function.

Yeah, I can see that some functions do this, but I don't think that
would be too hard to fix. See sample diff below (will need some more
work to deal with grep failing, but you get the idea).

I'd argue that fixing this is the right thing to do... Maybe add set -o
nounset as well while we're at it? :)

> IHMO as minimum with errexit you need a 'trap' function that can
> help/inform the user of what went wrong.

Yeah, trap ERR (which would also need set -o errtrace to work inside the
functions) might be useful in any case.

-Toke




diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
index 4af4046d71be..d61a348f85f5 100644
--- a/samples/pktgen/functions.sh
+++ b/samples/pktgen/functions.sh
@@ -58,6 +58,7 @@ function pg_set() {
 function proc_cmd() {
     local result
     local proc_file=$1
+    local status=0
     # after shift, the remaining args are contained in $@
     shift
     local proc_ctrl=${PROC_DIR}/$proc_file
@@ -73,8 +74,7 @@ function proc_cmd() {
        echo "cmd: $@ > $proc_ctrl"
     fi
     # Quoting of "$@" is important for space expansion
-    echo "$@" > "$proc_ctrl"
-    local status=$?
+    echo "$@" > "$proc_ctrl" || status=$?
 
     result=$(grep "Result: OK:" $proc_ctrl)
     # Due to pgctrl, cannot use exit code $? from grep

^ permalink raw reply related

* Re: [PATCH] netfilter: bridge: drop a broken include
From: Jeremy Sowden @ 2019-09-16 13:08 UTC (permalink / raw)
  To: Adam Borowski
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Roopa Prabhu, Nikolay Aleksandrov, netfilter-devel, coreteam,
	netdev
In-Reply-To: <20190916000517.45028-1-kilobyte@angband.pl>

[-- Attachment #1: Type: text/plain, Size: 4755 bytes --]

On 2019-09-16, at 02:05:16 +0200, Adam Borowski wrote:
> This caused a build failure if CONFIG_NF_CONNTRACK_BRIDGE is set but
> CONFIG_NF_TABLES=n -- and appears to be unused anyway.
>
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
>  net/bridge/netfilter/nf_conntrack_bridge.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/net/bridge/netfilter/nf_conntrack_bridge.c
> b/net/bridge/netfilter/nf_conntrack_bridge.c
> index 4f5444d2a526..844ef5a53f87 100644
> --- a/net/bridge/netfilter/nf_conntrack_bridge.c
> +++ b/net/bridge/netfilter/nf_conntrack_bridge.c
> @@ -18,7 +18,6 @@
>
>  #include <linux/netfilter/nf_tables.h>
>  #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
> -#include <net/netfilter/nf_tables.h>
>
>  #include "../br_private.h"
>
> --
> 2.23.0

This is the build-failure (reproduced with v5.3):

  In file included from net/bridge/netfilter/nf_conntrack_bridge.c:21:
  ./include/net/netfilter/nf_tables.h: In function ‘nft_gencursor_next’:
  ./include/net/netfilter/nf_tables.h:1223:14: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1223 |  return net->nft.gencursor + 1 == 1 ? 1 : 0;
        |              ^~~
        |              nf
  In file included from ./include/linux/export.h:45,
                   from ./include/linux/linkage.h:7,
                   from ./include/linux/kernel.h:8,
                   from ./include/linux/skbuff.h:13,
                   from ./include/linux/ip.h:16,
                   from net/bridge/netfilter/nf_conntrack_bridge.c:3:
  ./include/net/netfilter/nf_tables.h: In function ‘nft_genmask_cur’:
  ./include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |                             ^~~
  ./include/linux/compiler.h:261:17: note: in definition of macro ‘__READ_ONCE’
    261 |  union { typeof(x) __val; char __c[1]; } __u;   \
        |                 ^
  ./include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |              ^~~~~~~~~
  ./include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |                             ^~~
  ./include/linux/compiler.h:263:22: note: in definition of macro ‘__READ_ONCE’
    263 |   __read_once_size(&(x), __u.__c, sizeof(x));  \
        |                      ^
  ./include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |              ^~~~~~~~~
  ./include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |                             ^~~
  ./include/linux/compiler.h:263:42: note: in definition of macro ‘__READ_ONCE’
    263 |   __read_once_size(&(x), __u.__c, sizeof(x));  \
        |                                          ^
  ./include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |              ^~~~~~~~~
  ./include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |                             ^~~
  ./include/linux/compiler.h:265:30: note: in definition of macro ‘__READ_ONCE’
    265 |   __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
        |                              ^
  ./include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |              ^~~~~~~~~
  ./include/net/netfilter/nf_tables.h:1234:29: error: ‘const struct net’ has no member named ‘nft’; did you mean ‘nf’?
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |                             ^~~
  ./include/linux/compiler.h:265:50: note: in definition of macro ‘__READ_ONCE’
    265 |   __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
        |                                                  ^
  ./include/net/netfilter/nf_tables.h:1234:14: note: in expansion of macro ‘READ_ONCE’
   1234 |  return 1 << READ_ONCE(net->nft.gencursor);
        |              ^~~~~~~~~

There are already changes in the net-next tree that will fix it.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] libbpf: Don't error out if getsockopt() fails for XDP_OPTIONS
From: Toke Høiland-Jørgensen @ 2019-09-16 12:31 UTC (permalink / raw)
  To: Daniel Borkmann, Yonghong Song, Alexei Starovoitov,
	netdev@vger.kernel.org, bpf@vger.kernel.org, maximmi@mellanox.com
In-Reply-To: <9271a44f-1bbf-1305-bff9-8cbb8bae9098@iogearbox.net>

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 9/13/19 8:53 PM, Yonghong Song wrote:
>> On 9/10/19 12:06 AM, Toke Høiland-Jørgensen wrote:
>>> Yonghong Song <yhs@fb.com> writes:
>>>> On 9/9/19 10:46 AM, Toke Høiland-Jørgensen wrote:
>>>>> The xsk_socket__create() function fails and returns an error if it cannot
>>>>> get the XDP_OPTIONS through getsockopt(). However, support for XDP_OPTIONS
>>>>> was not added until kernel 5.3, so this means that creating XSK sockets
>>>>> always fails on older kernels.
>>>>>
>>>>> Since the option is just used to set the zero-copy flag in the xsk struct,
>>>>> there really is no need to error out if the getsockopt() call fails.
>>>>>
>>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>>>>> ---
>>>>>     tools/lib/bpf/xsk.c | 8 ++------
>>>>>     1 file changed, 2 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
>>>>> index 680e63066cf3..598e487d9ce8 100644
>>>>> --- a/tools/lib/bpf/xsk.c
>>>>> +++ b/tools/lib/bpf/xsk.c
>>>>> @@ -603,12 +603,8 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>>>>>     
>>>>>     	optlen = sizeof(opts);
>>>>>     	err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
>>>>> -	if (err) {
>>>>> -		err = -errno;
>>>>> -		goto out_mmap_tx;
>>>>> -	}
>>>>> -
>>>>> -	xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
>>>>> +	if (!err)
>>>>> +		xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
>>>>>     
>>>>>     	if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
>>>>>     		err = xsk_setup_xdp_prog(xsk);
>>>>
>>>> Since 'zc' is not used by anybody, maybe all codes 'zc' related can be
>>>> removed? It can be added back back once there is an interface to use
>>>> 'zc'?
>>>
>>> Fine with me; up to the maintainers what they prefer, I guess? :)
>
> Given this is not exposed to applications at this point and we don't do anything
> useful with it, lets just remove the zc cruft until there is a proper interface
> added to libbpf. Toke, please respin with the suggested removal, thanks!

Sure, can do :)

-Toke


^ permalink raw reply

* Re: [PATCH net] net/sched: cls_api: Fix nooffloaddevcnt counter in indr block call success
From: wenxu @ 2019-09-16 13:28 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, netdev
In-Reply-To: <20190916102810.GN2286@nanopsycho.orion>


在 2019/9/16 18:28, Jiri Pirko 写道:
> Please use get_maintainers script to get list of ccs.
>
> Mon, Sep 16, 2019 at 12:15:34PM CEST, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> When a block bind with a dev which support indr block call(vxlan/gretap
>> device). It can bind success but with nooffloaddevcnt++. It will fail
>> when replace the hw filter in tc_setup_cb_call with skip_sw mode for
>> checkout the nooffloaddevcnt and skip_sw.
> I read this paragraph 5 times, I still don't understand :( Could you
> please re-phrase?
will do.
>> if (block->nooffloaddevcnt && err_stop)
>> 	return -EOPNOTSUPP;
>>
>> So with this patch, if the indr block call success, it will not modify
>> the nooffloaddevcnt counter.
>>
>> Fixes: 7f76fa36754b ("net: sched: register callbacks for indirect tc block binds")
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> ---
>> net/sched/cls_api.c | 27 +++++++++++++++------------
>> 1 file changed, 15 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>> index efd3cfb..8a1e3a5 100644
>> --- a/net/sched/cls_api.c
>> +++ b/net/sched/cls_api.c
>> @@ -766,10 +766,10 @@ void tc_indr_block_cb_unregister(struct net_device *dev,
>> }
>> EXPORT_SYMBOL_GPL(tc_indr_block_cb_unregister);
>>
>> -static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>> -			       struct tcf_block_ext_info *ei,
>> -			       enum flow_block_command command,
>> -			       struct netlink_ext_ack *extack)
>> +static int tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>> +			      struct tcf_block_ext_info *ei,
>> +			      enum flow_block_command command,
>> +			      struct netlink_ext_ack *extack)
>> {
>> 	struct tc_indr_block_cb *indr_block_cb;
>> 	struct tc_indr_block_dev *indr_dev;
>> @@ -785,7 +785,7 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>>
>> 	indr_dev = tc_indr_block_dev_lookup(dev);
>> 	if (!indr_dev)
>> -		return;
>> +		return -ENOENT;
>>
>> 	indr_dev->block = command == FLOW_BLOCK_BIND ? block : NULL;
>>
>> @@ -793,7 +793,10 @@ static void tc_indr_block_call(struct tcf_block *block, struct net_device *dev,
>> 		indr_block_cb->cb(dev, indr_block_cb->cb_priv, TC_SETUP_BLOCK,
>> 				  &bo);
>>
>> -	tcf_block_setup(block, &bo);
>> +	if (list_empty(&bo.cb_list))
>> +		return -EOPNOTSUPP;
> How is this part related to the rest of the patch?

This check the list is to make sure there is real hw-offload for the indr device( There maybe

get some wrong with indr_block_cb->cb in the driver).  It should a return err to indicate the

caller add the nooffloaddevcnt counter.

.


^ permalink raw reply

* divide error in usbnet_update_max_qlen
From: syzbot @ 2019-09-16 13:29 UTC (permalink / raw)
  To: andreyknvl, davem, linux-kernel, linux-usb, netdev, oneukum,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    f0df5c1b usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=117659fa600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=5c6633fa4ed00be5
dashboard link: https://syzkaller.appspot.com/bug?extid=6102c120be558c885f04
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12107ba9600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=146014e6600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6102c120be558c885f04@syzkaller.appspotmail.com

usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
cdc_ncm 1-1:1.0: MAC-Address: 42:42:42:42:42:42
cdc_ncm 1-1:1.0: dwNtbInMaxSize=0 is too small. Using 2048
cdc_ncm 1-1:1.0: setting rx_max = 2048
divide error: 0000 [#1] SMP KASAN
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
RIP: 0010:usbnet_update_max_qlen drivers/net/usb/usbnet.c:344 [inline]
RIP: 0010:usbnet_update_max_qlen+0x231/0x370 drivers/net/usb/usbnet.c:338
Code: 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 46 01  
00 00 48 8d bb f4 00 00 00 31 d2 b8 c8 63 01 00 48 89 f9 <48> f7 b3 a8 01  
00 00 48 ba 00 00 00 00 00 fc ff df 48 c1 e9 03 0f
RSP: 0018:ffff8881da20f010 EFLAGS: 00010246
RAX: 00000000000163c8 RBX: ffff8881d5ad8ac0 RCX: ffff8881d5ad8bb4
RDX: 0000000000000000 RSI: ffffffff8344ecde RDI: ffff8881d5ad8bb4
RBP: 0000000000000003 R08: ffff8881da1f9800 R09: ffffed103ab5b00a
R10: ffffed103ab5b009 R11: ffff8881d5ad804f R12: 0000000000000000
R13: ffff8881d5ad8c38 R14: ffff8881d5ad8ac0 R15: ffff8881d2820c80
FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f26d0180000 CR3: 00000001d38dd000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  cdc_ncm_update_rxtx_max+0x8e9/0xef0 drivers/net/usb/cdc_ncm.c:440
  cdc_ncm_setup drivers/net/usb/cdc_ncm.c:667 [inline]
  cdc_ncm_bind_common+0x1005/0x2570 drivers/net/usb/cdc_ncm.c:924
  cdc_ncm_bind+0x7c/0x1c0 drivers/net/usb/cdc_ncm.c:1038
  usbnet_probe+0xb43/0x23cf drivers/net/usb/usbnet.c:1722
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* divide error in cdc_ncm_update_rxtx_max
From: syzbot @ 2019-09-16 13:29 UTC (permalink / raw)
  To: andreyknvl, davem, linux-kernel, linux-usb, netdev, oliver,
	syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    f0df5c1b usb-fuzzer: main usb gadget fuzzer driver
git tree:       https://github.com/google/kasan.git usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=14cd1459600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=5c6633fa4ed00be5
dashboard link: https://syzkaller.appspot.com/bug?extid=ce366e2b8296e25d84f5
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12b4dd85600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=179b7fc1600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+ce366e2b8296e25d84f5@syzkaller.appspotmail.com

cdc_ncm 1-1:1.0: dwNtbInMaxSize=0 is too small. Using 2048
cdc_ncm 1-1:1.0: setting rx_max = 2048
cdc_ncm 1-1:1.0: setting tx_max = 16384
divide error: 0000 [#1] SMP KASAN
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
RIP: 0010:cdc_ncm_update_rxtx_max+0xc6e/0xef0 drivers/net/usb/cdc_ncm.c:419
Code: e0 07 38 c2 0f 9e c1 84 d2 0f 95 c0 84 c1 0f 85 35 02 00 00 0f b7 5b  
04 81 e3 ff 07 00 00 e8 29 47 f7 fd 31 d2 44 89 e0 31 ff <f7> f3 41 89 d5  
89 d6 e8 86 48 f7 fd 45 85 ed 0f 85 17 f7 ff ff e8
RSP: 0018:ffff8881da20f030 EFLAGS: 00010246
RAX: 0000000000004000 RBX: 0000000000000000 RCX: 0000000000000001
RDX: 0000000000000000 RSI: ffffffff83469377 RDI: 0000000000000000
RBP: ffff8881c7ba0000 R08: ffff8881da1f9800 R09: ffffed103b645d58
R10: ffffed103b645d57 R11: ffff8881db22eabf R12: 0000000000004000
R13: 0000000000000000 R14: ffff8881d35f3dc0 R15: ffff8881d2a70f00
FS:  0000000000000000(0000) GS:ffff8881db200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f1059879000 CR3: 00000001d49db000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  cdc_ncm_setup drivers/net/usb/cdc_ncm.c:667 [inline]
  cdc_ncm_bind_common+0x1005/0x2570 drivers/net/usb/cdc_ncm.c:924
  cdc_ncm_bind+0x7c/0x1c0 drivers/net/usb/cdc_ncm.c:1038
  usbnet_probe+0xb43/0x23cf drivers/net/usb/usbnet.c:1722
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  usb_set_configuration+0xdf6/0x1670 drivers/usb/core/message.c:2023
  generic_probe+0x9d/0xd5 drivers/usb/core/generic.c:210
  usb_probe_device+0x99/0x100 drivers/usb/core/driver.c:266
  really_probe+0x281/0x6d0 drivers/base/dd.c:548
  driver_probe_device+0x101/0x1b0 drivers/base/dd.c:721
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:828
  bus_for_each_drv+0x162/0x1e0 drivers/base/bus.c:454
  __device_attach+0x217/0x360 drivers/base/dd.c:894
  bus_probe_device+0x1e4/0x290 drivers/base/bus.c:514
  device_add+0xae6/0x16f0 drivers/base/core.c:2165
  usb_new_device.cold+0x6a4/0xe79 drivers/usb/core/hub.c:2536
  hub_port_connect drivers/usb/core/hub.c:5098 [inline]
  hub_port_connect_change drivers/usb/core/hub.c:5213 [inline]
  port_event drivers/usb/core/hub.c:5359 [inline]
  hub_event+0x1b5c/0x3640 drivers/usb/core/hub.c:5441
  process_one_work+0x92b/0x1530 kernel/workqueue.c:2269
  worker_thread+0x96/0xe20 kernel/workqueue.c:2415
  kthread+0x318/0x420 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace ab75cc10e099d8e9 ]---
RIP:


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* BUG: unable to handle kernel NULL pointer dereference in rds_bind
From: syzbot @ 2019-09-16 13:29 UTC (permalink / raw)
  To: arvid.brodin, davem, linux-kernel, linux-rdma, netdev, rds-devel,
	santosh.shilimkar, syzkaller-bugs, xiyou.wangcong

Hello,

syzbot found the following crash on:

HEAD commit:    f4b752a6 mlx4: fix spelling mistake "veify" -> "verify"
git tree:       net
console output: https://syzkaller.appspot.com/x/log.txt?x=16cbebe6600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b89bb446a3faaba4
dashboard link: https://syzkaller.appspot.com/bug?extid=fae39afd2101a17ec624
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=10753bc1600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=111dfc11600000

The bug was bisected to:

commit b9a1e627405d68d475a3c1f35e685ccfb5bbe668
Author: Cong Wang <xiyou.wangcong@gmail.com>
Date:   Thu Jul 4 00:21:13 2019 +0000

     hsr: implement dellink to clean up resources

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=11005fc1600000
final crash:    https://syzkaller.appspot.com/x/report.txt?x=13005fc1600000
console output: https://syzkaller.appspot.com/x/log.txt?x=15005fc1600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+fae39afd2101a17ec624@syzkaller.appspotmail.com
Fixes: b9a1e627405d ("hsr: implement dellink to clean up resources")

BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor instruction fetch in kernel mode
#PF: error_code(0x0010) - not-present page
PGD 9fef0067 P4D 9fef0067 PUD 9060d067 PMD 0
Oops: 0010 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 9884 Comm: syz-executor453 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:0x0
Code: Bad RIP value.
RSP: 0018:ffff8880a7c6fcd8 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffffffff8992bfe0 RCX: ffffffff86b9b636
RDX: 0000000000000000 RSI: ffff8880a7c6fd40 RDI: ffffffff897830c0
RBP: ffff8880a7c6fda8 R08: ffff888093150558 R09: 0000000000000000
R10: fffffbfff134af9f R11: ffff88809a008040 R12: ffff888093150080
R13: ffffffff897830c0 R14: 0000000000000000 R15: ffff8880a7c6fd40
FS:  000055555738a880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffffffffd6 CR3: 000000008f04a000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  rds_bind+0x420/0x800 net/rds/bind.c:247
  __sys_bind+0x239/0x290 net/socket.c:1647
  __do_sys_bind net/socket.c:1658 [inline]
  __se_sys_bind net/socket.c:1656 [inline]
  __x64_sys_bind+0x73/0xb0 net/socket.c:1656
  do_syscall_64+0xfd/0x6a0 arch/x86/entry/common.c:296
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4412b9
Code: e8 ac e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 eb 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc7c28b228 EFLAGS: 00000246 ORIG_RAX: 0000000000000031
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004412b9
RDX: 0000000000000010 RSI: 0000000020000200 RDI: 0000000000000003
RBP: 00000000006cb018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000246 R12: 0000000000402030
R13: 00000000004020c0 R14: 0000000000000000 R15: 0000000000000000
Modules linked in:
CR2: 0000000000000000
---[ end trace b47a1983319ea52a ]---
RIP: 0010:0x0
Code: Bad RIP value.
RSP: 0018:ffff8880a7c6fcd8 EFLAGS: 00010246
RAX: dffffc0000000000 RBX: ffffffff8992bfe0 RCX: ffffffff86b9b636
RDX: 0000000000000000 RSI: ffff8880a7c6fd40 RDI: ffffffff897830c0
RBP: ffff8880a7c6fda8 R08: ffff888093150558 R09: 0000000000000000
R10: fffffbfff134af9f R11: ffff88809a008040 R12: ffff888093150080
R13: ffffffff897830c0 R14: 0000000000000000 R15: ffff8880a7c6fd40
FS:  000055555738a880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004bf788 CR3: 000000008f04a000 CR4: 00000000001406e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* [PATCH net v3 00/11] net: fix nested device bugs
From: Taehee Yoo @ 2019-09-16 13:47 UTC (permalink / raw)
  To: davem, netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
  Cc: ap420073

This patchset fixes several bugs that are related to nesting
device infrastructure.
Current nesting infrastructure code doesn't limit the depth level of
devices. nested devices could be handled recursively. at that moment,
it needs huge memory and stack overflow could occur.
Below devices type have same bug.
VLAN, BONDING, TEAM, MACSEC, MACVLAN and VXLAN.

Test commands:
    ip link add dummy0 type dummy
    ip link add vlan1 link dummy0 type vlan id 1

    for i in {2..100}
    do
	    let A=$i-1
	    ip link add vlan$i link vlan$A type vlan id $i
    done
    ip link del dummy0

1st patch actually fixes the root cause.
It adds new common variables {upper/lower}_level that represent
depth level. upper_level variable is depth of upper devices.
lower_level variable is depth of lower devices.

      [U][L]       [U][L]
vlan1  1  5  vlan4  1  4
vlan2  2  4  vlan5  2  3
vlan3  3  3    |
  |            |
  +------------+
  |
vlan6  4  2
dummy0 5  1

After this patch, the nesting infrastructure code uses this variable to
check the depth level.

2, 4, 5, 6, 7 patches fix lockdep related problem.
Before this patch, devices use static lockdep map.
So, if devices that are same type is nested, lockdep will warn about
recursive situation.
These patches make these devices use dynamic lockdep key instead of
static lock or subclass.

3rd patch fixes unexpected IFF_BONDING bit unset.

8th patch fixes a refcnt leak in the macsec module.

9th patch adds ignore flag to an adjacent structure.
In order to exchange an adjacent node safely, ignore flag is needed.

10th patch makes vxlan add an adjacent link to limit depth level.

11th patch removes unnecessary variables and callback.

v2 -> v3 :
 - Modify nesting infrastructure code to use iterator instead of recursive.
v1 -> v2 :
 - Make the 3rd patch do not add a new priv_flag.

Taehee Yoo (11):
  net: core: limit nested device depth
  vlan: use dynamic lockdep key instead of subclass
  bonding: fix unexpected IFF_BONDING bit unset
  bonding: use dynamic lockdep key instead of subclass
  team: use dynamic lockdep key instead of static key
  macsec: use dynamic lockdep key instead of subclass
  macvlan: use dynamic lockdep key instead of subclass
  macsec: fix refcnt leak in module exit routine
  net: core: add ignore flag to netdev_adjacent structure
  vxlan: add adjacent link to limit depth level
  net: remove unnecessary variables and callback

 drivers/net/bonding/bond_alb.c                |   2 +-
 drivers/net/bonding/bond_main.c               |  81 +++-
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   |   2 +-
 drivers/net/macsec.c                          |  50 +-
 drivers/net/macvlan.c                         |  36 +-
 drivers/net/team/team.c                       |  61 ++-
 drivers/net/vxlan.c                           |  71 ++-
 include/linux/if_macvlan.h                    |   3 +-
 include/linux/if_team.h                       |   5 +
 include/linux/if_vlan.h                       |  13 +-
 include/linux/netdevice.h                     |  20 +-
 include/net/bonding.h                         |   4 +-
 include/net/vxlan.h                           |   1 +
 net/8021q/vlan.c                              |   1 -
 net/8021q/vlan_dev.c                          |  32 +-
 net/core/dev.c                                | 456 +++++++++++++++---
 net/core/dev_addr_lists.c                     |  12 +-
 net/smc/smc_core.c                            |   2 +-
 net/smc/smc_pnet.c                            |   2 +-
 19 files changed, 667 insertions(+), 187 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH net v3 01/11] net: core: limit nested device depth
From: Taehee Yoo @ 2019-09-16 13:47 UTC (permalink / raw)
  To: davem, netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
  Cc: ap420073
In-Reply-To: <20190916134802.8252-1-ap420073@gmail.com>

Current code doesn't limit the number of nested devices.
Nested devices would be handled recursively and this needs huge stack
memory. So, unlimited nested devices could make stack overflow.

This patch adds upper_level and lower_level, they are common variables
and represent maximum lower/upper depth.
When upper/lower device is attached or dettached,
{lower/upper}_level are updated. and if maximum depth is bigger than 8,
attach routine fails and returns -EMLINK.

In addition, this patch converts recursive routine of
netdev_walk_all_{lower/upper} to iterator routine.

Test commands:
    ip link add dummy0 type dummy
    ip link add link dummy0 name vlan1 type vlan id 1
    ip link set vlan1 up

    for i in {2..200}
    do
	    let A=$i-1

	    ip link add vlan$i link vlan$A type vlan id $i
    done
    ip link del vlan1

Splat looks like:
[  132.396918] Thread overran stack, or stack corrupted
[  132.397763] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[  132.398557] CPU: 1 PID: 1299 Comm: ip Not tainted 5.3.0-rc8+ #179
[  132.399241] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  132.400136] RIP: 0010:stack_depot_fetch+0x10/0x30
[  132.400683] Code: 00 75 10 48 8b 73 18 48 89 ef 5b 5d e9 79 8f 87 ff 0f 0b e8 c2 6d 9b ff eb e9 89 f8 c1 ef 110
[  132.402711] RSP: 0000:ffff8880b002eb78 EFLAGS: 00010006
[  132.404578] RAX: 00000000001fffff RBX: ffff8880b002eec0 RCX: 0000000000000000
[  132.405305] RDX: 000000000000001d RSI: ffff8880b002eb80 RDI: 0000000000003ff0
[  132.406022] RBP: ffffea0002c00a00 R08: ffffed101b53df23 R09: ffffed101b53df23
[  132.406776] R10: 0000000000000001 R11: ffffed101b53df22 R12: ffff8880d38dd900
[  132.407598] R13: ffff8880b002e600 R14: ffff8880b002eec0 R15: ffff8880b002ed20
[  132.408365] FS:  00007f5fca3c90c0(0000) GS:ffff8880da800000(0000) knlGS:0000000000000000
[  132.409213] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  132.409788] CR2: ffffffffa598b098 CR3: 00000000ccb86004 CR4: 00000000000606e0
[  132.410659] Call Trace:
[  132.410962] Modules linked in: 8021q garp stp mrp llc dummy veth openvswitch nsh nf_conncount nf_nat nf_conntrs
[  132.412410] CR2: ffffffffa598b098
[  132.412754] ---[ end trace 7f335fb982ddb2da ]---
[  132.413293] RIP: 0010:stack_depot_fetch+0x10/0x30
[  132.413851] Code: 00 75 10 48 8b 73 18 48 89 ef 5b 5d e9 79 8f 87 ff 0f 0b e8 c2 6d 9b ff eb e9 89 f8 c1 ef 110
[  132.415973] RSP: 0000:ffff8880b002eb78 EFLAGS: 00010006
[  132.416581] RAX: 00000000001fffff RBX: ffff8880b002eec0 RCX: 0000000000000000
[  132.417380] RDX: 000000000000001d RSI: ffff8880b002eb80 RDI: 0000000000003ff0
[  132.418211] RBP: ffffea0002c00a00 R08: ffffed101b53df23 R09: ffffed101b53df23
[  132.419036] R10: 0000000000000001 R11: ffffed101b53df22 R12: ffff8880d38dd900
[  132.419815] R13: ffff8880b002e600 R14: ffff8880b002eec0 R15: ffff8880b002ed20
[  132.420616] FS:  00007f5fca3c90c0(0000) GS:ffff8880da800000(0000) knlGS:0000000000000000
[  132.421489] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  132.422134] CR2: ffffffffa598b098 CR3: 00000000ccb86004 CR4: 00000000000606e0
[  132.422912] Kernel panic - not syncing: Fatal exception
[  132.423441] Kernel Offset: 0x1f000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff)
[  132.425455] Rebooting in 5 seconds..

Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

v2 -> v3 :
 - Modify nesting infra code to use iterator instead of recursive
v1 -> v2 :
 - This patch is not changed

 include/linux/netdevice.h |   4 +
 net/core/dev.c            | 286 ++++++++++++++++++++++++++++++++------
 2 files changed, 245 insertions(+), 45 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 88292953aa6f..5bb5756129af 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1624,6 +1624,8 @@ enum netdev_priv_flags {
  *	@type:		Interface hardware type
  *	@hard_header_len: Maximum hardware header length.
  *	@min_header_len:  Minimum hardware header length
+ *	@upper_level:	Maximum depth level of upper devices.
+ *	@lower_level:	Maximum depth level of lower devices.
  *
  *	@needed_headroom: Extra headroom the hardware may need, but not in all
  *			  cases can this be guaranteed
@@ -1854,6 +1856,8 @@ struct net_device {
 	unsigned short		type;
 	unsigned short		hard_header_len;
 	unsigned char		min_header_len;
+	unsigned char		upper_level;
+	unsigned char		lower_level;
 
 	unsigned short		needed_headroom;
 	unsigned short		needed_tailroom;
diff --git a/net/core/dev.c b/net/core/dev.c
index 5156c0edebe8..fa847ea957ee 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -146,6 +146,7 @@
 #include "net-sysfs.h"
 
 #define MAX_GRO_SKBS 8
+#define MAX_NEST_DEV 8
 
 /* This should be increased if a protocol with a bigger head is added. */
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
@@ -6602,6 +6603,21 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_get_next_dev_rcu);
 
+static struct net_device *netdev_next_upper_dev(struct net_device *dev,
+						struct list_head **iter)
+{
+	struct netdev_adjacent *upper;
+
+	upper = list_entry((*iter)->next, struct netdev_adjacent, list);
+
+	if (&upper->list == &dev->adj_list.upper)
+		return NULL;
+
+	*iter = &upper->list;
+
+	return upper->dev;
+}
+
 static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev,
 						    struct list_head **iter)
 {
@@ -6619,31 +6635,103 @@ static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev,
 	return upper->dev;
 }
 
+int netdev_walk_all_upper_dev(struct net_device *dev,
+			      int (*fn)(struct net_device *dev,
+					void *data),
+			      void *data)
+{
+	struct net_device *udev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
+	struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
+	int ret, cur = 0;
+
+	now = dev;
+	iter = &dev->adj_list.upper;
+
+	while (1) {
+		if (now != dev) {
+			ret = fn(now, data);
+			if (ret)
+				return ret;
+		}
+
+		next = NULL;
+		while (1) {
+			udev = netdev_next_upper_dev(now, &iter);
+			if (!udev)
+				break;
+
+			if (!next) {
+				next = udev;
+				niter = &udev->adj_list.upper;
+			} else {
+				dev_stack[cur] = udev;
+				iter_stack[cur++] = &udev->adj_list.upper;
+				break;
+			}
+		}
+
+		if (!next) {
+			if (!cur)
+				return 0;
+			next = dev_stack[--cur];
+			niter = iter_stack[cur];
+		}
+
+		now = next;
+		iter = niter;
+	}
+
+	return 0;
+}
+
 int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
 				  int (*fn)(struct net_device *dev,
 					    void *data),
 				  void *data)
 {
-	struct net_device *udev;
-	struct list_head *iter;
-	int ret;
+	struct net_device *udev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
+	struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
+	int ret, cur = 0;
 
-	for (iter = &dev->adj_list.upper,
-	     udev = netdev_next_upper_dev_rcu(dev, &iter);
-	     udev;
-	     udev = netdev_next_upper_dev_rcu(dev, &iter)) {
-		/* first is the upper device itself */
-		ret = fn(udev, data);
-		if (ret)
-			return ret;
+	now = dev;
+	iter = &dev->adj_list.upper;
 
-		/* then look at all of its upper devices */
-		ret = netdev_walk_all_upper_dev_rcu(udev, fn, data);
-		if (ret)
-			return ret;
+	while (1) {
+		if (now != dev) {
+			ret = fn(now, data);
+			if (ret)
+				return ret;
+		}
+
+		next = NULL;
+		while (1) {
+			udev = netdev_next_upper_dev_rcu(now, &iter);
+			if (!udev)
+				break;
+
+			if (!next) {
+				next = udev;
+				niter = &udev->adj_list.upper;
+			} else {
+				dev_stack[cur] = udev;
+				iter_stack[cur++] = &udev->adj_list.upper;
+				break;
+			}
+		}
+
+		if (!next) {
+			if (!cur)
+				return 0;
+			next = dev_stack[--cur];
+			niter = iter_stack[cur];
+		}
+
+		now = next;
+		iter = niter;
 	}
 
 	return 0;
+
 }
 EXPORT_SYMBOL_GPL(netdev_walk_all_upper_dev_rcu);
 
@@ -6748,23 +6836,45 @@ int netdev_walk_all_lower_dev(struct net_device *dev,
 					void *data),
 			      void *data)
 {
-	struct net_device *ldev;
-	struct list_head *iter;
-	int ret;
+	struct net_device *ldev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
+	struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
+	int ret, cur = 0;
 
-	for (iter = &dev->adj_list.lower,
-	     ldev = netdev_next_lower_dev(dev, &iter);
-	     ldev;
-	     ldev = netdev_next_lower_dev(dev, &iter)) {
-		/* first is the lower device itself */
-		ret = fn(ldev, data);
-		if (ret)
-			return ret;
+	now = dev;
+	iter = &dev->adj_list.lower;
 
-		/* then look at all of its lower devices */
-		ret = netdev_walk_all_lower_dev(ldev, fn, data);
-		if (ret)
-			return ret;
+	while (1) {
+		if (now != dev) {
+			ret = fn(now, data);
+			if (ret)
+				return ret;
+		}
+
+		next = NULL;
+		while (1) {
+			ldev = netdev_next_lower_dev(now, &iter);
+			if (!ldev)
+				break;
+
+			if (!next) {
+				next = ldev;
+				niter = &ldev->adj_list.lower;
+			} else {
+				dev_stack[cur] = ldev;
+				iter_stack[cur++] = &ldev->adj_list.lower;
+				break;
+			}
+		}
+
+		if (!next) {
+			if (!cur)
+				return 0;
+			next = dev_stack[--cur];
+			niter = iter_stack[cur];
+		}
+
+		now = next;
+		iter = niter;
 	}
 
 	return 0;
@@ -6785,31 +6895,100 @@ static struct net_device *netdev_next_lower_dev_rcu(struct net_device *dev,
 	return lower->dev;
 }
 
-int netdev_walk_all_lower_dev_rcu(struct net_device *dev,
-				  int (*fn)(struct net_device *dev,
-					    void *data),
-				  void *data)
+static u8 __netdev_upper_depth(struct net_device *dev)
+{
+	struct net_device *udev;
+	struct list_head *iter;
+	u8 max_depth = 0;
+
+	for (iter = &dev->adj_list.upper,
+	     udev = netdev_next_upper_dev(dev, &iter);
+	     udev;
+	     udev = netdev_next_upper_dev(dev, &iter)) {
+		if (max_depth < udev->upper_level)
+			max_depth = udev->upper_level;
+	}
+
+	return max_depth;
+}
+
+static u8 __netdev_lower_depth(struct net_device *dev)
 {
 	struct net_device *ldev;
 	struct list_head *iter;
-	int ret;
+	u8 max_depth = 0;
 
 	for (iter = &dev->adj_list.lower,
-	     ldev = netdev_next_lower_dev_rcu(dev, &iter);
+	     ldev = netdev_next_lower_dev(dev, &iter);
 	     ldev;
-	     ldev = netdev_next_lower_dev_rcu(dev, &iter)) {
-		/* first is the lower device itself */
-		ret = fn(ldev, data);
-		if (ret)
-			return ret;
+	     ldev = netdev_next_lower_dev(dev, &iter)) {
+		if (max_depth < ldev->lower_level)
+			max_depth = ldev->lower_level;
+	}
 
-		/* then look at all of its lower devices */
-		ret = netdev_walk_all_lower_dev_rcu(ldev, fn, data);
-		if (ret)
-			return ret;
+	return max_depth;
+}
+
+static int __netdev_update_upper_level(struct net_device *dev, void *data)
+{
+	dev->upper_level = __netdev_upper_depth(dev) + 1;
+	return 0;
+}
+
+static int __netdev_update_lower_level(struct net_device *dev, void *data)
+{
+	dev->lower_level = __netdev_lower_depth(dev) + 1;
+	return 0;
+}
+
+int netdev_walk_all_lower_dev_rcu(struct net_device *dev,
+				  int (*fn)(struct net_device *dev,
+					    void *data),
+				  void *data)
+{
+	struct net_device *ldev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
+	struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
+	int ret, cur = 0;
+
+	now = dev;
+	iter = &dev->adj_list.lower;
+
+	while (1) {
+		if (now != dev) {
+			ret = fn(now, data);
+			if (ret)
+				return ret;
+		}
+
+		next = NULL;
+		while (1) {
+			ldev = netdev_next_lower_dev_rcu(now, &iter);
+			if (!ldev)
+				break;
+
+			if (!next) {
+				next = ldev;
+				niter = &ldev->adj_list.lower;
+			} else {
+				dev_stack[cur] = ldev;
+				iter_stack[cur++] = &ldev->adj_list.lower;
+				break;
+			}
+		}
+
+		if (!next) {
+			if (!cur)
+				return 0;
+			next = dev_stack[--cur];
+			niter = iter_stack[cur];
+		}
+
+		now = next;
+		iter = niter;
 	}
 
 	return 0;
+
 }
 EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev_rcu);
 
@@ -7063,6 +7242,9 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 	if (netdev_has_upper_dev(upper_dev, dev))
 		return -EBUSY;
 
+	if ((dev->lower_level + upper_dev->upper_level) > MAX_NEST_DEV)
+		return -EMLINK;
+
 	if (!master) {
 		if (netdev_has_upper_dev(dev, upper_dev))
 			return -EEXIST;
@@ -7089,6 +7271,12 @@ static int __netdev_upper_dev_link(struct net_device *dev,
 	if (ret)
 		goto rollback;
 
+	__netdev_update_upper_level(dev, NULL);
+	netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
+
+	__netdev_update_lower_level(upper_dev, NULL);
+	netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level, NULL);
+
 	return 0;
 
 rollback:
@@ -7171,6 +7359,12 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 
 	call_netdevice_notifiers_info(NETDEV_CHANGEUPPER,
 				      &changeupper_info.info);
+
+	__netdev_update_upper_level(dev, NULL);
+	netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
+
+	__netdev_update_lower_level(upper_dev, NULL);
+	netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level, NULL);
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
@@ -9159,6 +9353,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->gso_max_size = GSO_MAX_SIZE;
 	dev->gso_max_segs = GSO_MAX_SEGS;
+	dev->upper_level = 1;
+	dev->lower_level = 1;
 
 	INIT_LIST_HEAD(&dev->napi_list);
 	INIT_LIST_HEAD(&dev->unreg_list);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net v3 02/11] vlan: use dynamic lockdep key instead of subclass
From: Taehee Yoo @ 2019-09-16 13:47 UTC (permalink / raw)
  To: davem, netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
  Cc: ap420073
In-Reply-To: <20190916134802.8252-1-ap420073@gmail.com>

All VLAN device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes VLAN use dynamic lockdep key instead of the subclass.

Test commands:
   ip link add dummy0 type dummy
   ip link set dummy0 up
   ip link add bond0 type bond

   ip link add vlan_dummy1 link dummy0 type vlan id 1
   ip link add vlan_bond1 link bond0 type vlan id 2
   ip link set vlan_dummy1 master bond0

   ip link set bond0 up
   ip link set vlan_dummy1 up
   ip link set vlan_bond1 up

Both vlan_dummy1 and vlan_bond1 have the same subclass and it makes
unnecessary deadlock warning message.

Splat looks like:
[   66.163164] WARNING: possible recursive locking detected
[   66.163858] 5.3.0-rc8+ #179 Not tainted
[   66.165520] --------------------------------------------
[   66.166110] ip/983 is trying to acquire lock:
[   66.166603] 000000009b85ba3e (&vlan_netdev_addr_lock_key/1){+...}, at: dev_uc_sync_multiple+0xfa/0x1a0
[   66.194006]
[   66.194006] but task is already holding lock:
[   66.194636] 00000000cc752363 (&vlan_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[   66.205191]
[   66.205191] other info that might help us debug this:
[   66.205903]  Possible unsafe locking scenario:
[   66.205903]
[   66.206504]        CPU0
[   66.206781]        ----
[   66.208737]   lock(&vlan_netdev_addr_lock_key/1);
[   66.257676]   lock(&vlan_netdev_addr_lock_key/1);
[   66.282069]
[   66.282069]  *** DEADLOCK ***
[   66.282069]
[   66.283708]  May be due to missing lock nesting notation
[   66.283708]
[   66.284588] 4 locks held by ip/983:
[   66.285035]  #0: 000000002989e16e (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[   66.286051]  #1: 00000000cc752363 (&vlan_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[   66.287217]  #2: 00000000eddac627 (&dev_addr_list_lock_key/3){+...}, at: dev_mc_sync+0xfa/0x1a0
[   66.288327]  #3: 000000001a459ff7 (rcu_read_lock){....}, at: bond_set_rx_mode+0x5/0x3c0 [bonding]
[   66.289453]
[   66.289453] stack backtrace:
[   66.290019] CPU: 1 PID: 983 Comm: ip Not tainted 5.3.0-rc8+ #179
[   66.290802] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[   66.297639] Call Trace:
[   66.298009]  dump_stack+0x7c/0xbb
[   66.298447]  __lock_acquire+0x26a9/0x3de0
[   66.298965]  ? register_lock_class+0x14d0/0x14d0
[   66.299547]  ? register_lock_class+0x14d0/0x14d0
[   66.300151]  lock_acquire+0x164/0x3b0
[   66.300621]  ? dev_uc_sync_multiple+0xfa/0x1a0
[   66.301198]  _raw_spin_lock_nested+0x2e/0x60
[   66.301743]  ? dev_uc_sync_multiple+0xfa/0x1a0
[   66.302311]  dev_uc_sync_multiple+0xfa/0x1a0
[   66.307650]  bond_set_rx_mode+0x269/0x3c0 [bonding]
[   66.308175]  ? bond_init+0x6f0/0x6f0 [bonding]
[   66.308585]  dev_mc_sync+0x15a/0x1a0
[   66.308927]  vlan_dev_set_rx_mode+0x37/0x80 [8021q]
[   66.309375]  dev_set_rx_mode+0x21/0x30
[   66.309727]  __dev_open+0x202/0x310
[   66.310100]  ? dev_set_rx_mode+0x30/0x30
[   66.310513]  ? mark_held_locks+0xa5/0xe0
[   66.310934]  ? __local_bh_enable_ip+0xe9/0x1b0
[   66.311387]  __dev_change_flags+0x3c3/0x500
[   66.311839]  ? dev_set_allmulti+0x10/0x10
[   66.312248]  ? kmem_cache_alloc_trace+0x12c/0x320
[   66.312746]  dev_change_flags+0x7a/0x160
[   66.313161]  vlan_device_event+0x846/0x20d0 [8021q]
[ ... ]

Fixes: 0fe1e567d0b4 ("[VLAN]: nested VLAN: fix lockdep's recursive locking warning")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

v2 -> v3 :
 - This patch is not changed
v1 -> v2 :
 - This patch is not changed

 include/linux/if_vlan.h |  3 +++
 net/8021q/vlan_dev.c    | 28 +++++++++++++++-------------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 244278d5c222..1aed9f613e90 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -183,6 +183,9 @@ struct vlan_dev_priv {
 	struct netpoll				*netpoll;
 #endif
 	unsigned int				nest_level;
+
+	struct lock_class_key			xmit_lock_key;
+	struct lock_class_key			addr_lock_key;
 };
 
 static inline struct vlan_dev_priv *vlan_dev_priv(const struct net_device *dev)
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 93eadf179123..12bc80650087 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -494,24 +494,24 @@ static void vlan_dev_set_rx_mode(struct net_device *vlan_dev)
  * "super class" of normal network devices; split their locks off into a
  * separate class since they always nest.
  */
-static struct lock_class_key vlan_netdev_xmit_lock_key;
-static struct lock_class_key vlan_netdev_addr_lock_key;
-
 static void vlan_dev_set_lockdep_one(struct net_device *dev,
 				     struct netdev_queue *txq,
-				     void *_subclass)
+				     void *_unused)
 {
-	lockdep_set_class_and_subclass(&txq->_xmit_lock,
-				       &vlan_netdev_xmit_lock_key,
-				       *(int *)_subclass);
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+
+	lockdep_set_class(&txq->_xmit_lock, &vlan->xmit_lock_key);
 }
 
-static void vlan_dev_set_lockdep_class(struct net_device *dev, int subclass)
+static void vlan_dev_set_lockdep_class(struct net_device *dev)
 {
-	lockdep_set_class_and_subclass(&dev->addr_list_lock,
-				       &vlan_netdev_addr_lock_key,
-				       subclass);
-	netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, &subclass);
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+
+	lockdep_register_key(&vlan->addr_lock_key);
+	lockdep_set_class(&dev->addr_list_lock, &vlan->addr_lock_key);
+
+	lockdep_register_key(&vlan->xmit_lock_key);
+	netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, NULL);
 }
 
 static int vlan_dev_get_lock_subclass(struct net_device *dev)
@@ -609,7 +609,7 @@ static int vlan_dev_init(struct net_device *dev)
 
 	SET_NETDEV_DEVTYPE(dev, &vlan_type);
 
-	vlan_dev_set_lockdep_class(dev, vlan_dev_get_lock_subclass(dev));
+	vlan_dev_set_lockdep_class(dev);
 
 	vlan->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
 	if (!vlan->vlan_pcpu_stats)
@@ -630,6 +630,8 @@ static void vlan_dev_uninit(struct net_device *dev)
 			kfree(pm);
 		}
 	}
+	lockdep_unregister_key(&vlan->addr_lock_key);
+	lockdep_unregister_key(&vlan->xmit_lock_key);
 }
 
 static netdev_features_t vlan_dev_fix_features(struct net_device *dev,
-- 
2.17.1


^ permalink raw reply related

* [PATCH net v3 03/11] bonding: fix unexpected IFF_BONDING bit unset
From: Taehee Yoo @ 2019-09-16 13:47 UTC (permalink / raw)
  To: davem, netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
  Cc: ap420073
In-Reply-To: <20190916134802.8252-1-ap420073@gmail.com>

The IFF_BONDING means bonding master or bonding slave device.
->ndo_add_slave() sets IFF_BONDING flag and ->ndo_del_slave() unsets
IFF_BONDING flag.

bond0<--bond1

Both bond0 and bond1 are bonding device and these should keep having
IFF_BONDING flag until they are removed.
But bond1 would lose IFF_BONDING at ->ndo_del_slave() because that routine
do not check whether the slave device is the bonding type or not.
This patch adds the interface type check routine before removing
IFF_BONDING flag.

Test commands:
    ip link add bond0 type bond
    ip link add bond1 type bond
    ip link set bond1 master bond0
    ip link set bond1 nomaster
    ip link del bond1 type bond
    ip link add bond1 type bond

Splat looks like:
[   58.210981] proc_dir_entry 'bonding/bond1' already registered
[   58.463875] WARNING: CPU: 0 PID: 955 at fs/proc/generic.c:361 proc_register+0x2a9/0x3e0
[   58.466423] Modules linked in: bonding veth openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nfs
[   58.483855] CPU: 0 PID: 955 Comm: ip Not tainted 5.3.0-rc8+ #179
[   58.484657] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[   58.485779] RIP: 0010:proc_register+0x2a9/0x3e0
[   58.486377] Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 39 01 00 00 48 8b 04 24 48 89 ea 48 c7 c7 60 0f 14 bd 480
[   58.489003] RSP: 0018:ffff8880cc007078 EFLAGS: 00010282
[   58.553743] RAX: dffffc0000000008 RBX: ffff8880ce23c0d0 RCX: ffffffffbbd021e2
[   58.584076] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff8880da5f6b8c
[   58.584901] RBP: ffff8880ce23c353 R08: ffffed101b4bff91 R09: ffffed101b4bff91
[   58.585724] R10: 0000000000000001 R11: ffffed101b4bff90 R12: ffff8880ce23c268
[   58.586508] R13: ffff8880ce23c352 R14: dffffc0000000000 R15: ffffed1019c4786a
[   58.587296] FS:  00007f52d53b60c0(0000) GS:ffff8880da400000(0000) knlGS:0000000000000000
[   58.588247] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   58.653694] CR2: 00007f31a9df9320 CR3: 00000000cd4ea006 CR4: 00000000000606f0
[   58.654591] Call Trace:
[   58.654895]  proc_create_seq_private+0xb3/0xf0
[   58.655400]  bond_create_proc_entry+0x1b3/0x3f0 [bonding]
[   58.655985]  bond_netdev_event+0x433/0x970 [bonding]
[   58.656545]  ? __module_text_address+0x13/0x140
[   58.657038]  notifier_call_chain+0x90/0x160
[   58.657541]  register_netdevice+0x9b3/0xd80
[   58.657999]  ? alloc_netdev_mqs+0x854/0xc10
[   58.658476]  ? netdev_change_features+0xa0/0xa0
[   58.663592]  ? rtnl_create_link+0x2ed/0xad0
[   58.664049]  bond_newlink+0x2a/0x60 [bonding]
[   58.664529]  __rtnl_newlink+0xb9f/0x11b0
[   58.665014]  ? rtnl_link_unregister+0x230/0x230
[ ... ]

Fixes: 0b680e753724 ("[PATCH] bonding: Add priv_flag to avoid event mishandling")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

v2 -> v3 :
 - This patch is not changed
v1 -> v2 :
 - Do not add a new priv_flag.

 drivers/net/bonding/bond_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 931d9d935686..0db12fcfc953 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1816,7 +1816,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	slave_disable_netpoll(new_slave);
 
 err_close:
-	slave_dev->priv_flags &= ~IFF_BONDING;
+	if (!netif_is_bond_master(slave_dev))
+		slave_dev->priv_flags &= ~IFF_BONDING;
 	dev_close(slave_dev);
 
 err_restore_mac:
@@ -2017,7 +2018,8 @@ static int __bond_release_one(struct net_device *bond_dev,
 	else
 		dev_set_mtu(slave_dev, slave->original_mtu);
 
-	slave_dev->priv_flags &= ~IFF_BONDING;
+	if (!netif_is_bond_master(slave_dev))
+		slave_dev->priv_flags &= ~IFF_BONDING;
 
 	bond_free_slave(slave);
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net v3 04/11] bonding: use dynamic lockdep key instead of subclass
From: Taehee Yoo @ 2019-09-16 13:47 UTC (permalink / raw)
  To: davem, netdev, j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm,
	manishc, rahulv, kys, haiyangz, stephen, sashal, hare, varun,
	ubraun, kgraul, jay.vosburgh
  Cc: ap420073
In-Reply-To: <20190916134802.8252-1-ap420073@gmail.com>

All bonding device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes bonding use dynamic lockdep key instead of the
subclass.

Test commands:
    ip link add bond0 type bond

    for i in {1..5}
    do
	    let A=$i-1
	    ip link add bond$i type bond
	    ip link set bond$i master bond$A
    done
    ip link set bond5 master bond0

Splat looks like:
[   53.930344] WARNING: possible recursive locking detected
[   53.931041] 5.3.0-rc8+ #179 Not tainted
[   53.931554] --------------------------------------------
[   53.932285] ip/984 is trying to acquire lock:
[   53.932854] 00000000e313b280 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[   53.934144]
[   53.934144] but task is already holding lock:
[   53.934907] 00000000f5a0c2e3 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[   53.936268]
[   53.936268] other info that might help us debug this:
[   53.937135]  Possible unsafe locking scenario:
[   53.937135]
[   53.937910]        CPU0
[   53.938247]        ----
[   53.938577]   lock(&(&bond->stats_lock)->rlock#2/2);
[   53.939234]   lock(&(&bond->stats_lock)->rlock#2/2);
[   53.939903]
[   53.939903]  *** DEADLOCK ***
[   53.939903]
[   53.940745]  May be due to missing lock nesting notation
[   53.940745]
[   53.941626] 3 locks held by ip/984:
[   53.942005]  #0: 000000009e3df2a0 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[   53.942942]  #1: 00000000f5a0c2e3 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bond]
[   53.944194]  #2: 000000005b301abc (rcu_read_lock){....}, at: bond_get_stats+0x9f/0x500 [bonding]
[   53.945168]
[   53.945168] stack backtrace:
[   53.945672] CPU: 0 PID: 984 Comm: ip Not tainted 5.3.0-rc8+ #179
[   53.946606] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[   53.947529] Call Trace:
[   53.947795]  dump_stack+0x7c/0xbb
[   53.948213]  __lock_acquire+0x26a9/0x3de0
[   53.948666]  ? register_lock_class+0x14d0/0x14d0
[   53.949206]  lock_acquire+0x164/0x3b0
[   53.949647]  ? bond_get_stats+0xb8/0x500 [bonding]
[   53.950169]  _raw_spin_lock_nested+0x2e/0x60
[   53.950630]  ? bond_get_stats+0xb8/0x500 [bonding]
[   53.951132]  bond_get_stats+0xb8/0x500 [bonding]
[   53.951655]  ? bond_arp_rcv+0xf10/0xf10 [bonding]
[   53.952220]  ? register_lock_class+0x14d0/0x14d0
[   53.952751]  ? bond_get_stats+0xb8/0x500 [bonding]
[   53.953277]  dev_get_stats+0x1ec/0x270
[   53.984289]  bond_get_stats+0x1d1/0x500 [bonding]
[   53.984803]  ? bond_arp_rcv+0xf10/0xf10 [bonding]
[   53.985323]  ? dev_get_alias+0xe2/0x190
[   53.985748]  ? nla_put_ifalias+0x71/0x100
[   53.986213]  ? rtnl_phys_switch_id_fill+0x91/0x100
[   53.986720]  dev_get_stats+0x1ec/0x270
[   53.987100]  rtnl_fill_stats+0x44/0xbe0
[   53.987494]  ? nla_put+0xc2/0x140
[   53.987828]  rtnl_fill_ifinfo+0xec7/0x35b0
[ ... ]

Fixes: d3fff6c443fe ("net: add netdev_lockdep_set_classes() helper")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---

v2 -> v3 :
 - This patch is not changed
v1 -> v2 :
 - This patch is not changed

 drivers/net/bonding/bond_main.c | 61 ++++++++++++++++++++++++++++++---
 include/net/bonding.h           |  3 ++
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0db12fcfc953..7f574e74ed78 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1857,6 +1857,32 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	return res;
 }
 
+static void bond_dev_set_lockdep_one(struct net_device *dev,
+				     struct netdev_queue *txq,
+				     void *_unused)
+{
+	struct bonding *bond = netdev_priv(dev);
+
+	lockdep_set_class(&txq->_xmit_lock, &bond->xmit_lock_key);
+}
+
+static void bond_update_lock_key(struct net_device *dev)
+{
+	struct bonding *bond = netdev_priv(dev);
+
+	lockdep_unregister_key(&bond->stats_lock_key);
+	lockdep_unregister_key(&bond->addr_lock_key);
+	lockdep_unregister_key(&bond->xmit_lock_key);
+
+	lockdep_register_key(&bond->stats_lock_key);
+	lockdep_register_key(&bond->addr_lock_key);
+	lockdep_register_key(&bond->xmit_lock_key);
+
+	lockdep_set_class(&bond->stats_lock, &bond->stats_lock_key);
+	lockdep_set_class(&dev->addr_list_lock, &bond->addr_lock_key);
+	netdev_for_each_tx_queue(dev, bond_dev_set_lockdep_one, NULL);
+}
+
 /* Try to release the slave device <slave> from the bond device <master>
  * It is legal to access curr_active_slave without a lock because all the function
  * is RTNL-locked. If "all" is true it means that the function is being called
@@ -2022,6 +2048,8 @@ static int __bond_release_one(struct net_device *bond_dev,
 		slave_dev->priv_flags &= ~IFF_BONDING;
 
 	bond_free_slave(slave);
+	if (netif_is_bond_master(slave_dev))
+		bond_update_lock_key(slave_dev);
 
 	return 0;
 }
@@ -3459,7 +3487,7 @@ static void bond_get_stats(struct net_device *bond_dev,
 	struct list_head *iter;
 	struct slave *slave;
 
-	spin_lock_nested(&bond->stats_lock, bond_get_nest_level(bond_dev));
+	spin_lock(&bond->stats_lock);
 	memcpy(stats, &bond->bond_stats, sizeof(*stats));
 
 	rcu_read_lock();
@@ -4297,8 +4325,6 @@ void bond_setup(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 
-	spin_lock_init(&bond->mode_lock);
-	spin_lock_init(&bond->stats_lock);
 	bond->params = bonding_defaults;
 
 	/* Initialize pointers */
@@ -4367,6 +4393,9 @@ static void bond_uninit(struct net_device *bond_dev)
 
 	list_del(&bond->bond_list);
 
+	lockdep_unregister_key(&bond->stats_lock_key);
+	lockdep_unregister_key(&bond->addr_lock_key);
+	lockdep_unregister_key(&bond->xmit_lock_key);
 	bond_debug_unregister(bond);
 }
 
@@ -4758,6 +4787,29 @@ static int bond_check_params(struct bond_params *params)
 	return 0;
 }
 
+static struct lock_class_key qdisc_tx_busylock_key;
+static struct lock_class_key qdisc_running_key;
+
+static void bond_dev_set_lockdep_class(struct net_device *dev)
+{
+	struct bonding *bond = netdev_priv(dev);
+
+	dev->qdisc_tx_busylock = &qdisc_tx_busylock_key;
+	dev->qdisc_running_key = &qdisc_running_key;
+
+	spin_lock_init(&bond->mode_lock);
+
+	spin_lock_init(&bond->stats_lock);
+	lockdep_register_key(&bond->stats_lock_key);
+	lockdep_set_class(&bond->stats_lock, &bond->stats_lock_key);
+
+	lockdep_register_key(&bond->addr_lock_key);
+	lockdep_set_class(&dev->addr_list_lock, &bond->addr_lock_key);
+
+	lockdep_register_key(&bond->xmit_lock_key);
+	netdev_for_each_tx_queue(dev, bond_dev_set_lockdep_one, NULL);
+}
+
 /* Called from registration process */
 static int bond_init(struct net_device *bond_dev)
 {
@@ -4771,8 +4823,7 @@ static int bond_init(struct net_device *bond_dev)
 		return -ENOMEM;
 
 	bond->nest_level = SINGLE_DEPTH_NESTING;
-	netdev_lockdep_set_classes(bond_dev);
-
+	bond_dev_set_lockdep_class(bond_dev);
 	list_add_tail(&bond->bond_list, &bn->dev_list);
 
 	bond_prepare_sysfs_group(bond);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index f7fe45689142..c39ac7061e41 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -239,6 +239,9 @@ struct bonding {
 	struct	 dentry *debug_dir;
 #endif /* CONFIG_DEBUG_FS */
 	struct rtnl_link_stats64 bond_stats;
+	struct lock_class_key stats_lock_key;
+	struct lock_class_key xmit_lock_key;
+	struct lock_class_key addr_lock_key;
 };
 
 #define bond_slave_get_rcu(dev) \
-- 
2.17.1


^ permalink raw reply related


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