[PATCH net-next 0/4] tsnep: Throttle irq, rotten pkts, RX buffer alloc and ethtool_get_channels()

[PATCH net-next 0/4] tsnep: Throttle irq, rotten pkts, RX buffer alloc and ethtool_get_channels()

[Gerhard Engleder]

Collection of improvements found during development of XDP support.
Hopefully the last patch series before the XDP support.

Fix of rotten packets increased CPU load and caused slight drop of iperf
performance, because CPU load was already at 100% before. This performance
drop is compensated with interrupt throttling, which makes sense anyway.

ethtool_get_channels() is needed for automatic TAPRIO configuration in
combination with multiple queues.

Rework of RX buffer allocation is prework of XDP. It ensures that packets
are only dropped if RX queue would otherwise run empty because of
failed allocations. So it should reduce the number of dropped packets
under low memory conditions.

Gerhard Engleder (4):
  tsnep: Throttle interrupts
  tsnep: Fix rotten packets
  tsnep: Add ethtool get_channels support
  tsnep: Rework RX buffer allocation

 drivers/net/ethernet/engleder/tsnep.h         |   4 +
 drivers/net/ethernet/engleder/tsnep_ethtool.c |  19 ++
 drivers/net/ethernet/engleder/tsnep_hw.h      |   7 +
 drivers/net/ethernet/engleder/tsnep_main.c    | 252 +++++++++++++-----
 4 files changed, 214 insertions(+), 68 deletions(-)

-- 
2.30.2

[PATCH net-next 1/4] tsnep: Throttle interrupts

[Gerhard Engleder]

Without interrupt throttling, iperf server mode generates a CPU load of
100% (A53 1.2GHz). Also the throughput suffers with less than 900Mbit/s
on a 1Gbit/s link. The reason is a high interrupt load with interrupts
every ~20us.

Reduce interrupt load by throttling of interrupts. Interrupt delay is
configured statically to 64us. For iperf server mode the CPU load is
significantly reduced to ~20% and the throughput reaches the maximum of
941MBit/s. Interrupts are generated every ~140us.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_hw.h   | 7 +++++++
 drivers/net/ethernet/engleder/tsnep_main.c | 6 ++++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/ethernet/engleder/tsnep_hw.h b/drivers/net/ethernet/engleder/tsnep_hw.h
index 315dada75323..55e1caf193a6 100644
--- a/drivers/net/ethernet/engleder/tsnep_hw.h
+++ b/drivers/net/ethernet/engleder/tsnep_hw.h
@@ -48,6 +48,13 @@
 #define ECM_COUNTER_LOW 0x0028
 #define ECM_COUNTER_HIGH 0x002C
 
+/* interrupt delay */
+#define ECM_INT_DELAY 0x0030
+#define ECM_INT_DELAY_MASK 0xF0
+#define ECM_INT_DELAY_SHIFT 4
+#define ECM_INT_DELAY_BASE_US 16
+#define ECM_INT_DELAY_OFFSET 1
+
 /* control and status */
 #define ECM_STATUS 0x0080
 #define ECM_LINK_MODE_OFF 0x01000000
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 48fb391951dd..a99320e03279 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -39,6 +39,8 @@
 #endif
 #define DMA_ADDR_LOW(dma_addr) ((u32)((dma_addr) & 0xFFFFFFFF))
 
+#define INT_DELAY (64 / ECM_INT_DELAY_BASE_US)
+
 static void tsnep_enable_irq(struct tsnep_adapter *adapter, u32 mask)
 {
 	iowrite32(mask, adapter->addr + ECM_INT_ENABLE);
@@ -1298,6 +1300,7 @@ static int tsnep_phy_init(struct tsnep_adapter *adapter)
 static int tsnep_queue_init(struct tsnep_adapter *adapter, int queue_count)
 {
 	u32 irq_mask = ECM_INT_TX_0 | ECM_INT_RX_0;
+	u8 irq_delay = (INT_DELAY << ECM_INT_DELAY_SHIFT) & ECM_INT_DELAY_MASK;
 	char name[8];
 	int i;
 	int retval;
@@ -1316,6 +1319,7 @@ static int tsnep_queue_init(struct tsnep_adapter *adapter, int queue_count)
 	adapter->queue[0].tx = &adapter->tx[0];
 	adapter->queue[0].rx = &adapter->rx[0];
 	adapter->queue[0].irq_mask = irq_mask;
+	iowrite8(irq_delay, adapter->addr + ECM_INT_DELAY);
 
 	adapter->netdev->irq = adapter->queue[0].irq;
 
@@ -1336,6 +1340,8 @@ static int tsnep_queue_init(struct tsnep_adapter *adapter, int queue_count)
 		adapter->queue[i].rx = &adapter->rx[i];
 		adapter->queue[i].irq_mask =
 			irq_mask << (ECM_INT_TXRX_SHIFT * i);
+		iowrite8(irq_delay, adapter->addr + ECM_INT_DELAY +
+				    ECM_INT_DELAY_OFFSET * i);
 	}
 
 	return 0;
-- 
2.30.2

[PATCH net-next 2/4] tsnep: Fix rotten packets

[Gerhard Engleder]

If PTP synchronisation is done every second, then sporadic the interval
is higher than one second:

ptp4l[696.582]: master offset        -17 s2 freq   -1891 path delay 573
ptp4l[697.582]: master offset        -22 s2 freq   -1901 path delay 573
ptp4l[699.368]: master offset         -1 s2 freq   -1887 path delay 573
      ^^^^^^^ Should be 698.582!

This problem is caused by rotten packets, which are received after
polling but before interrupts are enabled again. This can be fixed by
checking for pending work and rescheduling if necessary after interrupts
has been enabled again.

Fixes: 403f69bbdbad ("tsnep: Add TSN endpoint Ethernet MAC driver")
Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_main.c | 59 +++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index a99320e03279..0aca2ba97757 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -544,6 +544,27 @@ static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget)
 	return (budget != 0);
 }
 
+static bool tsnep_tx_pending(struct tsnep_tx *tx)
+{
+	unsigned long flags;
+	struct tsnep_tx_entry *entry;
+	bool pending = false;
+
+	spin_lock_irqsave(&tx->lock, flags);
+
+	if (tx->read != tx->write) {
+		entry = &tx->entry[tx->read];
+		if ((__le32_to_cpu(entry->desc_wb->properties) &
+		     TSNEP_TX_DESC_OWNER_MASK) ==
+		    (entry->properties & TSNEP_TX_DESC_OWNER_MASK))
+			pending = true;
+	}
+
+	spin_unlock_irqrestore(&tx->lock, flags);
+
+	return pending;
+}
+
 static int tsnep_tx_open(struct tsnep_adapter *adapter, void __iomem *addr,
 			 int queue_index, struct tsnep_tx *tx)
 {
@@ -823,6 +844,21 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
 	return done;
 }
 
+static bool tsnep_rx_pending(struct tsnep_rx *rx)
+{
+	struct tsnep_rx_entry *entry;
+
+	if (rx->read != rx->write) {
+		entry = &rx->entry[rx->read];
+		if ((__le32_to_cpu(entry->desc_wb->properties) &
+		     TSNEP_DESC_OWNER_COUNTER_MASK) ==
+		    (entry->properties & TSNEP_DESC_OWNER_COUNTER_MASK))
+			return true;
+	}
+
+	return false;
+}
+
 static int tsnep_rx_open(struct tsnep_adapter *adapter, void __iomem *addr,
 			 int queue_index, struct tsnep_rx *rx)
 {
@@ -868,6 +904,17 @@ static void tsnep_rx_close(struct tsnep_rx *rx)
 	tsnep_rx_ring_cleanup(rx);
 }
 
+static bool tsnep_pending(struct tsnep_queue *queue)
+{
+	if (queue->tx && tsnep_tx_pending(queue->tx))
+		return true;
+
+	if (queue->rx && tsnep_rx_pending(queue->rx))
+		return true;
+
+	return false;
+}
+
 static int tsnep_poll(struct napi_struct *napi, int budget)
 {
 	struct tsnep_queue *queue = container_of(napi, struct tsnep_queue,
@@ -888,9 +935,19 @@ static int tsnep_poll(struct napi_struct *napi, int budget)
 	if (!complete)
 		return budget;
 
-	if (likely(napi_complete_done(napi, done)))
+	if (likely(napi_complete_done(napi, done))) {
 		tsnep_enable_irq(queue->adapter, queue->irq_mask);
 
+		/* reschedule if work is already pending, prevent rotten packets
+		 * which are transmitted or received after polling but before
+		 * interrupt enable
+		 */
+		if (tsnep_pending(queue)) {
+			tsnep_disable_irq(queue->adapter, queue->irq_mask);
+			napi_schedule(napi);
+		}
+	}
+
 	return min(done, budget - 1);
 }
 
-- 
2.30.2

[PATCH net-next 3/4] tsnep: Add ethtool get_channels support

[Gerhard Engleder]

Allow user space to read number of TX and RX queue. This is useful for
device dependent qdisc configurations like TAPRIO with hardware offload.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep_ethtool.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/engleder/tsnep_ethtool.c b/drivers/net/ethernet/engleder/tsnep_ethtool.c
index a713a126b227..3da5cb75aa55 100644
--- a/drivers/net/ethernet/engleder/tsnep_ethtool.c
+++ b/drivers/net/ethernet/engleder/tsnep_ethtool.c
@@ -288,6 +288,17 @@ static int tsnep_ethtool_set_rxnfc(struct net_device *dev,
 	}
 }
 
+static void tsnep_ethtool_get_channels(struct net_device *dev,
+				       struct ethtool_channels *ch)
+{
+	struct tsnep_adapter *adapter = netdev_priv(dev);
+
+	ch->max_rx = adapter->num_rx_queues;
+	ch->max_tx = adapter->num_tx_queues;
+	ch->rx_count = adapter->num_rx_queues;
+	ch->tx_count = adapter->num_tx_queues;
+}
+
 static int tsnep_ethtool_get_ts_info(struct net_device *dev,
 				     struct ethtool_ts_info *info)
 {
@@ -327,6 +338,7 @@ const struct ethtool_ops tsnep_ethtool_ops = {
 	.get_sset_count = tsnep_ethtool_get_sset_count,
 	.get_rxnfc = tsnep_ethtool_get_rxnfc,
 	.set_rxnfc = tsnep_ethtool_set_rxnfc,
+	.get_channels = tsnep_ethtool_get_channels,
 	.get_ts_info = tsnep_ethtool_get_ts_info,
 	.get_link_ksettings = phy_ethtool_get_link_ksettings,
 	.set_link_ksettings = phy_ethtool_set_link_ksettings,
-- 
2.30.2

[PATCH net-next 4/4] tsnep: Rework RX buffer allocation

[Gerhard Engleder]

Refill RX queue in batches of descriptors to improve performance. Refill
is allowed to fail as long as a minimum number of descriptors is active.
Thus, a limited number of failed RX buffer allocations is now allowed
for normal operation. Previously every failed allocation resulted in a
dropped frame.

If the minimum number of active descriptors is reached, then RX buffers
are still reused and frames are dropped. This ensures that the RX queue
never runs empty and always continues to operate.

Prework for future XDP support.

Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
---
 drivers/net/ethernet/engleder/tsnep.h         |   4 +
 drivers/net/ethernet/engleder/tsnep_ethtool.c |   7 +
 drivers/net/ethernet/engleder/tsnep_main.c    | 191 +++++++++++-------
 3 files changed, 133 insertions(+), 69 deletions(-)

diff --git a/drivers/net/ethernet/engleder/tsnep.h b/drivers/net/ethernet/engleder/tsnep.h
index 09a723b827c7..9a840d145727 100644
--- a/drivers/net/ethernet/engleder/tsnep.h
+++ b/drivers/net/ethernet/engleder/tsnep.h
@@ -18,6 +18,8 @@
 #define TSNEP "tsnep"
 
 #define TSNEP_RING_SIZE 256
+#define TSNEP_RING_RX_REFILL 16
+#define TSNEP_RING_RX_REUSE (TSNEP_RING_SIZE - TSNEP_RING_SIZE / 4)
 #define TSNEP_RING_ENTRIES_PER_PAGE (PAGE_SIZE / TSNEP_DESC_SIZE)
 #define TSNEP_RING_PAGE_COUNT (TSNEP_RING_SIZE / TSNEP_RING_ENTRIES_PER_PAGE)
 
@@ -110,6 +112,7 @@ struct tsnep_rx {
 	dma_addr_t page_dma[TSNEP_RING_PAGE_COUNT];
 
 	struct tsnep_rx_entry entry[TSNEP_RING_SIZE];
+	int write;
 	int read;
 	u32 owner_counter;
 	int increment_owner_counter;
@@ -119,6 +122,7 @@ struct tsnep_rx {
 	u32 bytes;
 	u32 dropped;
 	u32 multicast;
+	u32 alloc_failed;
 };
 
 struct tsnep_queue {
diff --git a/drivers/net/ethernet/engleder/tsnep_ethtool.c b/drivers/net/ethernet/engleder/tsnep_ethtool.c
index 3da5cb75aa55..b74eba512755 100644
--- a/drivers/net/ethernet/engleder/tsnep_ethtool.c
+++ b/drivers/net/ethernet/engleder/tsnep_ethtool.c
@@ -8,6 +8,7 @@ static const char tsnep_stats_strings[][ETH_GSTRING_LEN] = {
 	"rx_bytes",
 	"rx_dropped",
 	"rx_multicast",
+	"rx_alloc_failed",
 	"rx_phy_errors",
 	"rx_forwarded_phy_errors",
 	"rx_invalid_frame_errors",
@@ -21,6 +22,7 @@ struct tsnep_stats {
 	u64 rx_bytes;
 	u64 rx_dropped;
 	u64 rx_multicast;
+	u64 rx_alloc_failed;
 	u64 rx_phy_errors;
 	u64 rx_forwarded_phy_errors;
 	u64 rx_invalid_frame_errors;
@@ -36,6 +38,7 @@ static const char tsnep_rx_queue_stats_strings[][ETH_GSTRING_LEN] = {
 	"rx_%d_bytes",
 	"rx_%d_dropped",
 	"rx_%d_multicast",
+	"rx_%d_alloc_failed",
 	"rx_%d_no_descriptor_errors",
 	"rx_%d_buffer_too_small_errors",
 	"rx_%d_fifo_overflow_errors",
@@ -47,6 +50,7 @@ struct tsnep_rx_queue_stats {
 	u64 rx_bytes;
 	u64 rx_dropped;
 	u64 rx_multicast;
+	u64 rx_alloc_failed;
 	u64 rx_no_descriptor_errors;
 	u64 rx_buffer_too_small_errors;
 	u64 rx_fifo_overflow_errors;
@@ -178,6 +182,7 @@ static void tsnep_ethtool_get_ethtool_stats(struct net_device *netdev,
 		tsnep_stats.rx_bytes += adapter->rx[i].bytes;
 		tsnep_stats.rx_dropped += adapter->rx[i].dropped;
 		tsnep_stats.rx_multicast += adapter->rx[i].multicast;
+		tsnep_stats.rx_alloc_failed += adapter->rx[i].alloc_failed;
 	}
 	reg = ioread32(adapter->addr + ECM_STAT);
 	tsnep_stats.rx_phy_errors =
@@ -200,6 +205,8 @@ static void tsnep_ethtool_get_ethtool_stats(struct net_device *netdev,
 		tsnep_rx_queue_stats.rx_bytes = adapter->rx[i].bytes;
 		tsnep_rx_queue_stats.rx_dropped = adapter->rx[i].dropped;
 		tsnep_rx_queue_stats.rx_multicast = adapter->rx[i].multicast;
+		tsnep_rx_queue_stats.rx_alloc_failed =
+			adapter->rx[i].alloc_failed;
 		reg = ioread32(adapter->addr + TSNEP_QUEUE(i) +
 			       TSNEP_RX_STATISTIC);
 		tsnep_rx_queue_stats.rx_no_descriptor_errors =
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c
index 0aca2ba97757..3ce5a37547cf 100644
--- a/drivers/net/ethernet/engleder/tsnep_main.c
+++ b/drivers/net/ethernet/engleder/tsnep_main.c
@@ -631,23 +631,6 @@ static void tsnep_rx_ring_cleanup(struct tsnep_rx *rx)
 	}
 }
 
-static int tsnep_rx_alloc_buffer(struct tsnep_rx *rx,
-				 struct tsnep_rx_entry *entry)
-{
-	struct page *page;
-
-	page = page_pool_dev_alloc_pages(rx->page_pool);
-	if (unlikely(!page))
-		return -ENOMEM;
-
-	entry->page = page;
-	entry->len = TSNEP_MAX_RX_BUF_SIZE;
-	entry->dma = page_pool_get_dma_addr(entry->page);
-	entry->desc->rx = __cpu_to_le64(entry->dma + TSNEP_SKB_PAD);
-
-	return 0;
-}
-
 static int tsnep_rx_ring_init(struct tsnep_rx *rx)
 {
 	struct device *dmadev = rx->adapter->dmadev;
@@ -694,10 +677,6 @@ static int tsnep_rx_ring_init(struct tsnep_rx *rx)
 		entry = &rx->entry[i];
 		next_entry = &rx->entry[(i + 1) % TSNEP_RING_SIZE];
 		entry->desc->next = __cpu_to_le64(next_entry->desc_dma);
-
-		retval = tsnep_rx_alloc_buffer(rx, entry);
-		if (retval)
-			goto failed;
 	}
 
 	return 0;
@@ -707,6 +686,45 @@ static int tsnep_rx_ring_init(struct tsnep_rx *rx)
 	return retval;
 }
 
+static int tsnep_rx_desc_available(struct tsnep_rx *rx)
+{
+	if (rx->read <= rx->write)
+		return TSNEP_RING_SIZE - rx->write + rx->read - 1;
+	else
+		return rx->read - rx->write - 1;
+}
+
+static void tsnep_rx_set_page(struct tsnep_rx *rx, struct tsnep_rx_entry *entry,
+			      struct page *page)
+{
+	entry->page = page;
+	entry->len = TSNEP_MAX_RX_BUF_SIZE;
+	entry->dma = page_pool_get_dma_addr(entry->page);
+	entry->desc->rx = __cpu_to_le64(entry->dma + TSNEP_SKB_PAD);
+}
+
+static int tsnep_rx_alloc_buffer(struct tsnep_rx *rx, int index)
+{
+	struct tsnep_rx_entry *entry = &rx->entry[index];
+	struct page *page;
+
+	page = page_pool_dev_alloc_pages(rx->page_pool);
+	if (unlikely(!page))
+		return -ENOMEM;
+	tsnep_rx_set_page(rx, entry, page);
+
+	return 0;
+}
+
+static void tsnep_rx_reuse_buffer(struct tsnep_rx *rx, int index)
+{
+	struct tsnep_rx_entry *entry = &rx->entry[index];
+	struct tsnep_rx_entry *read = &rx->entry[rx->read];
+
+	tsnep_rx_set_page(rx, entry, read->page);
+	read->page = NULL;
+}
+
 static void tsnep_rx_activate(struct tsnep_rx *rx, int index)
 {
 	struct tsnep_rx_entry *entry = &rx->entry[index];
@@ -734,6 +752,48 @@ static void tsnep_rx_activate(struct tsnep_rx *rx, int index)
 	entry->desc->properties = __cpu_to_le32(entry->properties);
 }
 
+static int tsnep_rx_refill(struct tsnep_rx *rx, int count, bool reuse)
+{
+	int index;
+	bool alloc_failed = false;
+	bool enable = false;
+	int i;
+	int retval;
+
+	for (i = 0; i < count && !alloc_failed; i++) {
+		index = (rx->write + i) % TSNEP_RING_SIZE;
+
+		retval = tsnep_rx_alloc_buffer(rx, index);
+		if (unlikely(retval)) {
+			rx->alloc_failed++;
+			alloc_failed = true;
+
+			/* reuse only if no other allocation was successful */
+			if (i == 0 && reuse)
+				tsnep_rx_reuse_buffer(rx, index);
+			else
+				break;
+		}
+
+		tsnep_rx_activate(rx, index);
+
+		enable = true;
+	}
+
+	if (enable) {
+		rx->write = (rx->write + i) % TSNEP_RING_SIZE;
+
+		/* descriptor properties shall be valid before hardware is
+		 * notified
+		 */
+		dma_wmb();
+
+		iowrite32(TSNEP_CONTROL_RX_ENABLE, rx->addr + TSNEP_CONTROL);
+	}
+
+	return i;
+}
+
 static struct sk_buff *tsnep_build_skb(struct tsnep_rx *rx, struct page *page,
 				       int length)
 {
@@ -769,23 +829,42 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
 			 int budget)
 {
 	struct device *dmadev = rx->adapter->dmadev;
+	int desc_available;
 	int done = 0;
 	enum dma_data_direction dma_dir;
 	struct tsnep_rx_entry *entry;
-	struct page *page;
 	struct sk_buff *skb;
 	int length;
-	bool enable = false;
-	int retval;
 
+	desc_available = tsnep_rx_desc_available(rx);
 	dma_dir = page_pool_get_dma_dir(rx->page_pool);
 
-	while (likely(done < budget)) {
+	while (likely(done < budget) && (rx->read != rx->write)) {
 		entry = &rx->entry[rx->read];
 		if ((__le32_to_cpu(entry->desc_wb->properties) &
 		     TSNEP_DESC_OWNER_COUNTER_MASK) !=
 		    (entry->properties & TSNEP_DESC_OWNER_COUNTER_MASK))
 			break;
+		done++;
+
+		if (desc_available >= TSNEP_RING_RX_REFILL) {
+			bool reuse = desc_available >= TSNEP_RING_RX_REUSE;
+
+			desc_available -= tsnep_rx_refill(rx, desc_available,
+							  reuse);
+			if (!entry->page) {
+				/* buffer has been reused for refill to prevent
+				 * empty RX ring, thus buffer cannot be used for
+				 * RX processing
+				 */
+				rx->read = (rx->read + 1) % TSNEP_RING_SIZE;
+				desc_available++;
+
+				rx->dropped++;
+
+				continue;
+			}
+		}
 
 		/* descriptor properties shall be read first, because valid data
 		 * is signaled there
@@ -797,49 +876,30 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi,
 			 TSNEP_DESC_LENGTH_MASK;
 		dma_sync_single_range_for_cpu(dmadev, entry->dma, TSNEP_SKB_PAD,
 					      length, dma_dir);
-		page = entry->page;
 
-		/* forward skb only if allocation is successful, otherwise
-		 * page is reused and frame dropped
-		 */
-		retval = tsnep_rx_alloc_buffer(rx, entry);
-		if (!retval) {
-			skb = tsnep_build_skb(rx, page, length);
-			if (skb) {
-				page_pool_release_page(rx->page_pool, page);
-
-				rx->packets++;
-				rx->bytes += length -
-					     TSNEP_RX_INLINE_METADATA_SIZE;
-				if (skb->pkt_type == PACKET_MULTICAST)
-					rx->multicast++;
-
-				napi_gro_receive(napi, skb);
-			} else {
-				page_pool_recycle_direct(rx->page_pool, page);
+		rx->read = (rx->read + 1) % TSNEP_RING_SIZE;
+		desc_available++;
 
-				rx->dropped++;
-			}
-			done++;
-		} else {
-			rx->dropped++;
-		}
+		skb = tsnep_build_skb(rx, entry->page, length);
+		if (skb) {
+			page_pool_release_page(rx->page_pool, entry->page);
 
-		tsnep_rx_activate(rx, rx->read);
+			rx->packets++;
+			rx->bytes += length - TSNEP_RX_INLINE_METADATA_SIZE;
+			if (skb->pkt_type == PACKET_MULTICAST)
+				rx->multicast++;
 
-		enable = true;
+			napi_gro_receive(napi, skb);
+		} else {
+			page_pool_recycle_direct(rx->page_pool, entry->page);
 
-		rx->read = (rx->read + 1) % TSNEP_RING_SIZE;
+			rx->dropped++;
+		}
+		entry->page = NULL;
 	}
 
-	if (enable) {
-		/* descriptor properties shall be valid before hardware is
-		 * notified
-		 */
-		dma_wmb();
-
-		iowrite32(TSNEP_CONTROL_RX_ENABLE, rx->addr + TSNEP_CONTROL);
-	}
+	if (desc_available)
+		tsnep_rx_refill(rx, desc_available, false);
 
 	return done;
 }
@@ -863,7 +923,6 @@ static int tsnep_rx_open(struct tsnep_adapter *adapter, void __iomem *addr,
 			 int queue_index, struct tsnep_rx *rx)
 {
 	dma_addr_t dma;
-	int i;
 	int retval;
 
 	memset(rx, 0, sizeof(*rx));
@@ -881,13 +940,7 @@ static int tsnep_rx_open(struct tsnep_adapter *adapter, void __iomem *addr,
 	rx->owner_counter = 1;
 	rx->increment_owner_counter = TSNEP_RING_SIZE - 1;
 
-	for (i = 0; i < TSNEP_RING_SIZE; i++)
-		tsnep_rx_activate(rx, i);
-
-	/* descriptor properties shall be valid before hardware is notified */
-	dma_wmb();
-
-	iowrite32(TSNEP_CONTROL_RX_ENABLE, rx->addr + TSNEP_CONTROL);
+	tsnep_rx_refill(rx, tsnep_rx_desc_available(rx), false);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH net-next 1/4] tsnep: Throttle interrupts

[Andrew Lunn]

On Thu, Nov 17, 2022 at 09:14:37PM +0100, Gerhard Engleder wrote:
> Without interrupt throttling, iperf server mode generates a CPU load of
> 100% (A53 1.2GHz). Also the throughput suffers with less than 900Mbit/s
> on a 1Gbit/s link. The reason is a high interrupt load with interrupts
> every ~20us.

Not my area of expertise, but is NAPI working correctly? It should be
that you disable interrupts while NAPI is polling, and only re-enable
interrupts when polling has stopped. If you are receiving at near line
rate at 100% load, i would of thought that NAPI would be polling most
of the time and interrupts would be mostly disabled?

Interrupt coalescence makes a lot of sense thought, so the patch
itself is useful.

       Andrew

Re: [PATCH net-next 2/4] tsnep: Fix rotten packets

[Andrew Lunn]

On Thu, Nov 17, 2022 at 09:14:38PM +0100, Gerhard Engleder wrote:
> If PTP synchronisation is done every second, then sporadic the interval
> is higher than one second:
> 
> ptp4l[696.582]: master offset        -17 s2 freq   -1891 path delay 573
> ptp4l[697.582]: master offset        -22 s2 freq   -1901 path delay 573
> ptp4l[699.368]: master offset         -1 s2 freq   -1887 path delay 573
>       ^^^^^^^ Should be 698.582!
> 
> This problem is caused by rotten packets, which are received after
> polling but before interrupts are enabled again.

Is this a hardware bug? At the end of the interrupt coalescence
period, should it not check the queue and fire an interrupt?

	Andrew

Re: [PATCH net-next 1/4] tsnep: Throttle interrupts

[Gerhard Engleder]

On 17.11.22 21:33, Andrew Lunn wrote:
> On Thu, Nov 17, 2022 at 09:14:37PM +0100, Gerhard Engleder wrote:
>> Without interrupt throttling, iperf server mode generates a CPU load of
>> 100% (A53 1.2GHz). Also the throughput suffers with less than 900Mbit/s
>> on a 1Gbit/s link. The reason is a high interrupt load with interrupts
>> every ~20us.
> 
> Not my area of expertise, but is NAPI working correctly? It should be
> that you disable interrupts while NAPI is polling, and only re-enable
> interrupts when polling has stopped. If you are receiving at near line
> rate at 100% load, i would of thought that NAPI would be polling most
> of the time and interrupts would be mostly disabled?

I disable and re-enable interrupts in the driver, but I don't know
exactly under which conditions interrupts will be disabled mostly. At
least during my XDP tests with minimum size packets the interrupt rate
dropped significantly. Here the CPU had also 100% load but packet rate
was much higher with ~880000 pkt/s.

I compared the performance and the interrupt rate with the macb driver
on the same CPU and the results were similar.

> Interrupt coalescence makes a lot of sense thought, so the patch
> itself is useful.

Thanks!

Gerhard

Re: [PATCH net-next 2/4] tsnep: Fix rotten packets

[Gerhard Engleder]

On 17.11.22 21:39, Andrew Lunn wrote:
> On Thu, Nov 17, 2022 at 09:14:38PM +0100, Gerhard Engleder wrote:
>> If PTP synchronisation is done every second, then sporadic the interval
>> is higher than one second:
>>
>> ptp4l[696.582]: master offset        -17 s2 freq   -1891 path delay 573
>> ptp4l[697.582]: master offset        -22 s2 freq   -1901 path delay 573
>> ptp4l[699.368]: master offset         -1 s2 freq   -1887 path delay 573
>>        ^^^^^^^ Should be 698.582!
>>
>> This problem is caused by rotten packets, which are received after
>> polling but before interrupts are enabled again.
> 
> Is this a hardware bug? At the end of the interrupt coalescence
> period, should it not check the queue and fire an interrupt?

In my case, the hardware is not signaled if a descriptor is processed by
the software. The hardware is only signaled if it gets new descriptors
assigned. So the hardware does not know if there are still descriptors
in the RX queue which need to be processed by the software. As a result,
it would only be possible to trigger an interrupt for descriptors which
may has been processed already anyway.

In the end I made the hardware stupid. If interrupts are disabled for
NAPI polling, then interrupts events in the hardware are ignored. If
interrupts are enabled again, then only new interrupt events will
trigger the interrupt. I was afraid that too intelligent hardware will
lead to hardware bugs in this case.

Gerhard

Re: [PATCH net-next 1/4] tsnep: Throttle interrupts

[Jakub Kicinski]

On Thu, 17 Nov 2022 21:14:37 +0100 Gerhard Engleder wrote:
> Without interrupt throttling, iperf server mode generates a CPU load of
> 100% (A53 1.2GHz). Also the throughput suffers with less than 900Mbit/s
> on a 1Gbit/s link. The reason is a high interrupt load with interrupts
> every ~20us.
> 
> Reduce interrupt load by throttling of interrupts. Interrupt delay is
> configured statically to 64us. For iperf server mode the CPU load is
> significantly reduced to ~20% and the throughput reaches the maximum of
> 941MBit/s. Interrupts are generated every ~140us.

User should be able to control these settings, please implement
ethtool::get_coalesce / set_coalesce.

Re: [PATCH net-next 2/4] tsnep: Fix rotten packets

[Jakub Kicinski]

On Thu, 17 Nov 2022 21:14:38 +0100 Gerhard Engleder wrote:
> If PTP synchronisation is done every second, then sporadic the interval
> is higher than one second:
> 
> ptp4l[696.582]: master offset        -17 s2 freq   -1891 path delay 573
> ptp4l[697.582]: master offset        -22 s2 freq   -1901 path delay 573
> ptp4l[699.368]: master offset         -1 s2 freq   -1887 path delay 573
>       ^^^^^^^ Should be 698.582!
> 
> This problem is caused by rotten packets, which are received after
> polling but before interrupts are enabled again. This can be fixed by
> checking for pending work and rescheduling if necessary after interrupts
> has been enabled again.
> 
> Fixes: 403f69bbdbad ("tsnep: Add TSN endpoint Ethernet MAC driver")
> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>

This patch needs to go to net separately :(
Packets getting stuck in a queue can cause real issues to users.

Re: [PATCH net-next 1/4] tsnep: Throttle interrupts

[Gerhard Engleder]

On 19.11.22 02:24, Jakub Kicinski wrote:
> On Thu, 17 Nov 2022 21:14:37 +0100 Gerhard Engleder wrote:
>> Without interrupt throttling, iperf server mode generates a CPU load of
>> 100% (A53 1.2GHz). Also the throughput suffers with less than 900Mbit/s
>> on a 1Gbit/s link. The reason is a high interrupt load with interrupts
>> every ~20us.
>>
>> Reduce interrupt load by throttling of interrupts. Interrupt delay is
>> configured statically to 64us. For iperf server mode the CPU load is
>> significantly reduced to ~20% and the throughput reaches the maximum of
>> 941MBit/s. Interrupts are generated every ~140us.
> 
> User should be able to control these settings, please implement
> ethtool::get_coalesce / set_coalesce.

I will implement it.

Re: [PATCH net-next 2/4] tsnep: Fix rotten packets

[Gerhard Engleder]

On 19.11.22 02:26, Jakub Kicinski wrote:
> On Thu, 17 Nov 2022 21:14:38 +0100 Gerhard Engleder wrote:
>> If PTP synchronisation is done every second, then sporadic the interval
>> is higher than one second:
>>
>> ptp4l[696.582]: master offset        -17 s2 freq   -1891 path delay 573
>> ptp4l[697.582]: master offset        -22 s2 freq   -1901 path delay 573
>> ptp4l[699.368]: master offset         -1 s2 freq   -1887 path delay 573
>>        ^^^^^^^ Should be 698.582!
>>
>> This problem is caused by rotten packets, which are received after
>> polling but before interrupts are enabled again. This can be fixed by
>> checking for pending work and rescheduling if necessary after interrupts
>> has been enabled again.
>>
>> Fixes: 403f69bbdbad ("tsnep: Add TSN endpoint Ethernet MAC driver")
>> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com>
> 
> This patch needs to go to net separately :(
> Packets getting stuck in a queue can cause real issues to users.

I will post it separately.