[PATCH net-next v14 0/7] r8169: add RSS support for RTL8127

[PATCH net-next v14 0/7] r8169: add RSS support for RTL8127

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch series adds RSS (Receive Side Scaling) support for the r8169
ethernet driver, specifically for RTL8127 (RTL_GIGA_MAC_VER_80).

RSS enables packet distribution across multiple receive queues, which can
significantly improve network throughput on multi-core systems by allowing
parallel processing of incoming packets.

Key features:
- Multi-queue RX support (up to 8 queues)
- MSI-X interrupt with vector mapping
- Dynamic queue configuration via ethtool (-L)
- RSS hash computation for flow classification

Experiments:
Platform: AMD Ryzen Embedded R2514 with Radeon Graphics(4 Cores/8 Threads)
Arch: x86_64
Test command: 
  Server: iperf3 -s
  Client: iperf3 -c 192.168.2.1 -P 20 -t 3600
Monitor: mpstat -P ALL 1

Before this patch (Without RSS):
  Throughput: Unstable, fluctuating between 3.76 Gbits/sec and
  8.2 Gbits/sec.
  CPU Usage: A single CPU core is fully occupied with softirq reaching 
  up to 96%.

After this patch (With RSS enabled):
  Throughput: Stable at 9.42 Gbits/sec.
  CPU Usage: The traffic load is evenly distributed across multiple CPU
  cores. The maximum softirq on a single core dropped to 63%.
  
Other Experiments:
Link: https://lore.kernel.org/netdev/0A5279953D81BB9C+f50c9b49-3e5d-467f-b69a-7e49ed223383@radxa.com/

Javen Xu (7):
  r8169: prepare for multi irqs and napi
  r8169: refactor RX path to prepare for multi-queue
  r8169: add support for new interrupt mapping
  r8169: enable new interrupt mapping
  r8169: add support and enable rss
  r8169: move struct ethtool_ops
  r8169: add get_channel support for ethtool

 drivers/net/ethernet/realtek/r8169_main.c | 1099 ++++++++++++++++++---
 1 file changed, 950 insertions(+), 149 deletions(-)

-- 
2.43.0

[PATCH net-next v14 1/7] r8169: prepare for multi irqs and napi

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch converts the single struct napi_struct into a dynamically
allocated array indexed by IRQ. This is a preparation step for adding
RSS support. Currently, irq_nvecs and num_rx_rings are still hard-coded
to 1, so there is no functional change.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - remove some unused definitions, such as index, name in rtl8169_irq
 - remove array imr and isr
 - remove min_irq_nvecs and max_irq_nvecs, replaced with help function
   get_min_irq_nvecs and get_max_irq_nvecs
 - alloc irq by flags, instead of PCI_IRQ_ALL_TYPES

Changes in v3:
 - add enum rtl_isr_version to replace macro definition
 - remove struct rtl8169_napi, use napi_struct array instead and alloc
   memory for this array dynamically
 - remove struct rtl8169_irq

Changes in v4:
 - change retval to ret in rtl8169_set_real_num_queue()
 - reverse xmas tree in rtl8169_poll() and rtl8169_interrupt()
 - remove tp->hw_supp_isr_ver

Changes in v5:
 - rtl8169_request_irq(), when failed, only free irqs which are
   allocated
 - remove rss_support, simplied napi init, call r8169_init_napi()
   directly
 - remove rtl_isr_version, INTR_VEC_MAP_MASK, INTR_VEC_MAP_STATUS,
   R8169_MAX_MSIX_VEC, rss_enable, recheck_desc_ownbit
 - rtl_software_parameter_initialize() this function will be expanded in
   next patch, so i want to remain it here.

Changes in v6:
 - Fix netpoll crash
 - Fix use-after-free during driver unload by registering a devm action
   for netif_napi_del()
 - remove tp->irq

Changes in v7:
 - pass NAPI as arg to rtl_rx()
 - use netif_set_real_num_queues to replace rtl8169_set_real_num_queues
 - replace rtl_software_parameter_initialize with rtl_setup_rx_params

Changes in v8:
 - no changes

Changes in v9:
 - no changes

Changes in v10:
 - no changes

Changes in v11:
 - no changes

Changes in v12:
 - no changes

Changes in v13:
 - Fix sleep-in-atomic bug in netpoll by passing 0 to
   rtl8169_interrupt()
 - call netif_napi_set_irq() to properly associate NAPI with IRQ vector
 - Fix print format for unsigned int tp->irq_nvecs from %d to %u

Changes in v14:
 - modify commit message and patch title
 - fall back to request_irq() + pci_irq_vector() instead of
   pci_request_irq() to avoid dropping IRQF_NO_THREAD flag for MSI/MSI-X
 - add phylink_destory() in NAPI kcalloc failure path
---
 drivers/net/ethernet/realtek/r8169_main.c | 165 ++++++++++++++++++----
 1 file changed, 136 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 5415ff62a286..a8214c48a775 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -761,7 +761,6 @@ struct rtl8169_private {
 	struct pci_dev *pci_dev;
 	struct net_device *dev;
 	struct phy_device *phydev;
-	struct napi_struct napi;
 	enum mac_version mac_version;
 	enum rtl_dash_type dash_type;
 	enum rtl_sfp_mode sfp_mode;
@@ -774,10 +773,12 @@ struct rtl8169_private {
 	dma_addr_t RxPhyAddr;
 	struct page *Rx_databuff[NUM_RX_DESC];	/* Rx data buffers */
 	struct ring_info tx_skb[NUM_TX_DESC];	/* Tx data buffers */
+	struct napi_struct *rtl8169_napi;
+	unsigned int num_rx_rings;
 	u16 cp_cmd;
 	u16 tx_lpi_timer;
 	u32 irq_mask;
-	int irq;
+	unsigned int irq_nvecs;
 	struct clk *clk;
 	int speed;
 
@@ -2769,6 +2770,11 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
 	rtl_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
 }
 
+static void rtl_setup_rx_params(struct rtl8169_private *tp)
+{
+	tp->num_rx_rings = 1;
+}
+
 static void rtl_request_firmware(struct rtl8169_private *tp)
 {
 	struct rtl_fw *rtl_fw;
@@ -4431,9 +4437,21 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 	netdev_reset_queue(tp->dev);
 }
 
+static void rtl8169_napi_disable(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->irq_nvecs; i++)
+		napi_disable(&tp->rtl8169_napi[i]);
+}
+
+static void rtl8169_napi_enable(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->irq_nvecs; i++)
+		napi_enable(&tp->rtl8169_napi[i]);
+}
+
 static void rtl8169_cleanup(struct rtl8169_private *tp)
 {
-	napi_disable(&tp->napi);
+	rtl8169_napi_disable(tp);
 
 	/* Give a racing hard_start_xmit a few cycles to complete. */
 	synchronize_net();
@@ -4479,7 +4497,7 @@ static void rtl_reset_work(struct rtl8169_private *tp)
 	for (i = 0; i < NUM_RX_DESC; i++)
 		rtl8169_mark_to_asic(tp->RxDescArray + i);
 
-	napi_enable(&tp->napi);
+	rtl8169_napi_enable(tp);
 	rtl_hw_start(tp);
 }
 
@@ -4933,7 +4951,8 @@ static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
 		skb_checksum_none_assert(skb);
 }
 
-static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, int budget)
+static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
+		  int budget, struct napi_struct *napi)
 {
 	struct device *d = tp_to_dev(tp);
 	int count;
@@ -4985,7 +5004,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, int budget
 			goto release_descriptor;
 		}
 
-		skb = napi_alloc_skb(&tp->napi, pkt_size);
+		skb = napi_alloc_skb(napi, pkt_size);
 		if (unlikely(!skb)) {
 			dev->stats.rx_dropped++;
 			goto release_descriptor;
@@ -5009,7 +5028,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, int budget
 		if (skb->pkt_type == PACKET_MULTICAST)
 			dev->stats.multicast++;
 
-		napi_gro_receive(&tp->napi, skb);
+		napi_gro_receive(napi, skb);
 
 		dev_sw_netstats_rx_add(dev, pkt_size);
 release_descriptor:
@@ -5021,8 +5040,12 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, int budget
 
 static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 {
-	struct rtl8169_private *tp = dev_instance;
-	u32 status = rtl_get_events(tp);
+	struct napi_struct *napi = dev_instance;
+	struct rtl8169_private *tp;
+	u32 status;
+
+	tp = netdev_priv(napi->dev);
+	status = rtl_get_events(tp);
 
 	if ((status & 0xffff) == 0xffff || !(status & tp->irq_mask))
 		return IRQ_NONE;
@@ -5043,13 +5066,55 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	}
 
 	rtl_irq_disable(tp);
-	napi_schedule(&tp->napi);
+	napi_schedule(napi);
 out:
 	rtl_ack_events(tp, status);
 
 	return IRQ_HANDLED;
 }
 
+static void rtl8169_free_irq(struct rtl8169_private *tp)
+{
+	struct pci_dev *pdev = tp->pci_dev;
+	int i, irq;
+
+	for (i = 0; i < tp->irq_nvecs; i++) {
+		struct napi_struct *napi = &tp->rtl8169_napi[i];
+
+		irq = pci_irq_vector(pdev, i);
+		free_irq(irq, napi);
+	}
+}
+
+static int rtl8169_request_irq(struct rtl8169_private *tp)
+{
+	struct pci_dev *pdev = tp->pci_dev;
+	struct net_device *dev = tp->dev;
+	struct napi_struct *napi;
+	unsigned long irqflags;
+	int i, rc, irq;
+
+	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_NO_THREAD : IRQF_SHARED;
+
+	for (i = 0; i < tp->irq_nvecs; i++) {
+		napi = &tp->rtl8169_napi[i];
+		irq = pci_irq_vector(pdev, i);
+
+		rc = request_irq(irq, rtl8169_interrupt, irqflags,
+				 dev->name, napi);
+		if (rc)
+			goto free_irq;
+	}
+	return 0;
+
+free_irq:
+	while (--i >= 0) {
+		irq = pci_irq_vector(pdev, i);
+		free_irq(irq, &tp->rtl8169_napi[i]);
+	}
+	return rc;
+}
+
 static void rtl_task(struct work_struct *work)
 {
 	struct rtl8169_private *tp =
@@ -5084,13 +5149,13 @@ static void rtl_task(struct work_struct *work)
 
 static int rtl8169_poll(struct napi_struct *napi, int budget)
 {
-	struct rtl8169_private *tp = container_of(napi, struct rtl8169_private, napi);
-	struct net_device *dev = tp->dev;
-	int work_done;
+	struct rtl8169_private *tp = netdev_priv(napi->dev);
+	struct net_device *dev = napi->dev;
+	int work_done = 0;
 
 	rtl_tx(dev, tp, budget);
 
-	work_done = rtl_rx(dev, tp, budget);
+	work_done = rtl_rx(dev, tp, budget, napi);
 
 	if (work_done < budget && napi_complete_done(napi, work_done))
 		rtl_irq_enable(tp);
@@ -5175,7 +5240,7 @@ static void rtl8169_up(struct rtl8169_private *tp)
 	if (tp->phydev)
 		rtl8169_init_phy(tp);
 
-	napi_enable(&tp->napi);
+	rtl8169_napi_enable(tp);
 	enable_work(&tp->wk.work);
 	rtl_reset_work(tp);
 }
@@ -5192,7 +5257,7 @@ static int rtl8169_close(struct net_device *dev)
 	rtl8169_down(tp);
 	rtl8169_rx_clear(tp);
 
-	free_irq(tp->irq, tp);
+	rtl8169_free_irq(tp);
 
 	phylink_disconnect_phy(tp->phylink);
 
@@ -5213,7 +5278,8 @@ static void rtl8169_netpoll(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
 
-	rtl8169_interrupt(tp->irq, tp);
+	for (int i = 0; i < tp->irq_nvecs; i++)
+		rtl8169_interrupt(0, &tp->rtl8169_napi[i]);
 }
 #endif
 
@@ -5221,7 +5287,6 @@ static int rtl_open(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
 	struct pci_dev *pdev = tp->pci_dev;
-	unsigned long irqflags;
 	int retval = -ENOMEM;
 
 	pm_runtime_get_sync(&pdev->dev);
@@ -5246,8 +5311,7 @@ static int rtl_open(struct net_device *dev)
 
 	rtl_request_firmware(tp);
 
-	irqflags = pci_dev_msi_enabled(pdev) ? IRQF_NO_THREAD : IRQF_SHARED;
-	retval = request_irq(tp->irq, rtl8169_interrupt, irqflags, dev->name, tp);
+	retval = rtl8169_request_irq(tp);
 	if (retval < 0)
 		goto err_release_fw_2;
 
@@ -5266,7 +5330,7 @@ static int rtl_open(struct net_device *dev)
 	return retval;
 
 err_free_irq:
-	free_irq(tp->irq, tp);
+	rtl8169_free_irq(tp);
 err_release_fw_2:
 	rtl_release_firmware(tp);
 	rtl8169_rx_clear(tp);
@@ -5419,6 +5483,14 @@ static void rtl_shutdown(struct pci_dev *pdev)
 		pci_prepare_to_sleep(pdev);
 }
 
+static void r8169_free_napi(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->irq_nvecs; i++)
+		netif_napi_del(&tp->rtl8169_napi[i]);
+
+	kfree(tp->rtl8169_napi);
+}
+
 static void rtl_remove_one(struct pci_dev *pdev)
 {
 	struct rtl8169_private *tp = pci_get_drvdata(pdev);
@@ -5432,6 +5504,7 @@ static void rtl_remove_one(struct pci_dev *pdev)
 		r8169_remove_leds(tp->leds);
 
 	unregister_netdev(tp->dev);
+	r8169_free_napi(tp);
 	phylink_destroy(tp->phylink);
 
 	if (tp->dash_type != RTL_DASH_NONE)
@@ -5473,7 +5546,9 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
 
 static int rtl_alloc_irq(struct rtl8169_private *tp)
 {
+	struct pci_dev *pdev = tp->pci_dev;
 	unsigned int flags;
+	int nvecs;
 
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_02 ... RTL_GIGA_MAC_VER_06:
@@ -5489,7 +5564,14 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
 		break;
 	}
 
-	return pci_alloc_irq_vectors(tp->pci_dev, 1, 1, flags);
+	nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);
+
+	if (nvecs < 0)
+		return nvecs;
+
+	tp->irq_nvecs = nvecs;
+
+	return 0;
 }
 
 static void rtl_read_mac_address(struct rtl8169_private *tp,
@@ -6005,6 +6087,15 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
 	return 0;
 }
 
+static void r8169_init_napi(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->irq_nvecs; i++) {
+		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], rtl8169_poll);
+		netif_napi_set_irq(&tp->rtl8169_napi[i],
+				   pci_irq_vector(tp->pci_dev, i));
+	}
+}
+
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	const struct rtl_chip_info *chip;
@@ -6105,12 +6196,12 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	rtl_hw_reset(tp);
 
+	rtl_setup_rx_params(tp);
+
 	rc = rtl_alloc_irq(tp);
 	if (rc < 0)
 		return dev_err_probe(&pdev->dev, rc, "Can't allocate interrupt\n");
 
-	tp->irq = pci_irq_vector(pdev, 0);
-
 	INIT_WORK(&tp->wk.work, rtl_task);
 	disable_work(&tp->wk.work);
 
@@ -6118,8 +6209,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	dev->ethtool_ops = &rtl8169_ethtool_ops;
 
-	netif_napi_add(dev, &tp->napi, rtl8169_poll);
-
 	dev->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
 			   NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
@@ -6180,6 +6269,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (jumbo_max)
 		dev->max_mtu = jumbo_max;
 
+	rc = netif_set_real_num_queues(tp->dev, 1, tp->num_rx_rings);
+	if (rc < 0)
+		return dev_err_probe(&pdev->dev, rc, "set tx/rx num failure\n");
+
 	rtl_set_irq_mask(tp);
 
 	tp->counters = dmam_alloc_coherent (&pdev->dev, sizeof(*tp->counters),
@@ -6202,10 +6295,19 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
+	tp->rtl8169_napi = kcalloc(tp->irq_nvecs, sizeof(struct napi_struct),
+				   GFP_KERNEL);
+	if (!tp->rtl8169_napi) {
+		phylink_destroy(tp->phylink);
+		return -ENOMEM;
+	}
+
+	r8169_init_napi(tp);
+
 	rc = register_netdev(dev);
 	if (rc) {
 		phylink_destroy(tp->phylink);
-		return rc;
+		goto err_free_napi;
 	}
 
 	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
@@ -6215,8 +6317,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 			tp->leds = rtl8168_init_leds(dev);
 	}
 
-	netdev_info(dev, "%s, %pM, %sXID %x, IRQ %d\n",
-		    chip->name, dev->dev_addr, ext_xid_str, xid, tp->irq);
+	netdev_info(dev, "%s, %pM, %sXID %x, IRQ %d (%u total)\n",
+		    chip->name, dev->dev_addr, ext_xid_str, xid,
+		    pci_irq_vector(pdev, 0), tp->irq_nvecs);
 
 	if (jumbo_max)
 		netdev_info(dev, "jumbo features [frames: %d bytes, tx checksumming: %s]\n",
@@ -6236,6 +6339,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		pm_runtime_put_sync(&pdev->dev);
 
 	return 0;
+
+err_free_napi:
+	r8169_free_napi(tp);
+	return rc;
 }
 
 static struct pci_driver rtl8169_pci_driver = {
-- 
2.43.0

[PATCH net-next v14 2/7] r8169: refactor RX path to prepare for multi-queue

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch is a preparatory refactoring of the RX path. It introduces
struct rtl8169_rx_ring and turns the previously embedded RX state in
rtl8169_private into a per-queue array.
While the netdev allocation is changed to devm_alloc_etherdev_mqs()
with up to 8 RX queues, the actual number of active RX rings
(num_rx_rings) is currently kept at 1. The actual multi-queue operation
and RSS enablement will be introduced in subsequent patches.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - sort some registers by its number
 - remove some unused definitions, like RX_DESC_RING_TYPE_MAX
 - change recheck_desc_ownbit type
 - remove rdsar_reg in rx_ring struct
 - opts1 are different in rx_desc and rx_desc_rss, move the judgement
   to Patch 5/7

Changes in v3:
 - remove ring->rx_desc_alloc_size, use constant instead

Changes in v4:
 - change rdsar_reg type to unsigned int
 - follow reverse xmas tree, in rtl_set_rx_tx_desc_registers(),
   rtl8169_alloc_rx_data(), rtl8169_alloc_rx_desc(),
   rtl8169_free_rx_desc()
 - add comments on LED_CTRL, remove helper function

Changes in v5:
 - modify rtl8169_init_ring(), do rx clear when failed
 - add definition R8169_MAX_TX_QUEUES 1

Changes in v6:
 - Restore the secondary Rx error filter when NETIF_F_RXFALL is enabled
   in rtl_rx()

Changes in v7:
 - remove code associated with recheck_desc_ownbit

Changes in v8:
 - remove le64_to_cpu() for addr, rx get addr from rx_desc_phy_addr

Changes in v9:
 - remove R8127_MAX_RX_QUEUES
 - remvoe rx_desc_ring_type to the following patch
 - Fix loop bound in init_ring_indexes
 - Restore checksum API

Changes in v10:
 - alloc rtl8169_rx_ring struct according to the num_rx_ring dynamically

Changes in v11:
 - leak rx_ring array on driver removal

Changes in v12:
 - no changes

Changes in v13:
 - remove unused dirty_rx

Changes in v14:
 - REvert R8169_RX_RING_BYTES padding changes. It will be split into a
   bugfix patch.
 - Fix phylink memory in rtl_init_one() by implementing a proper goto
   unwind laber (err_destory_phylink) for memory allocation failures.
---
 drivers/net/ethernet/realtek/r8169_main.c | 244 +++++++++++++++++-----
 1 file changed, 186 insertions(+), 58 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index a8214c48a775..7a2e7ce56e48 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -74,9 +74,13 @@
 #define NUM_TX_DESC	256	/* Number of Tx descriptor registers */
 #define NUM_RX_DESC	256	/* Number of Rx descriptor registers */
 #define R8169_TX_RING_BYTES	(NUM_TX_DESC * sizeof(struct TxDesc))
+
 #define R8169_RX_RING_BYTES	(NUM_RX_DESC * sizeof(struct RxDesc))
 #define R8169_TX_STOP_THRS	(MAX_SKB_FRAGS + 1)
 #define R8169_TX_START_THRS	(2 * R8169_TX_STOP_THRS)
+#define R8169_MAX_RX_QUEUES	8
+#define R8169_DEFAULT_RX_QUEUES	1
+#define R8169_MAX_TX_QUEUES	1
 
 #define OCP_STD_PHY_BASE	0xa400
 
@@ -463,6 +467,7 @@ enum rtl8125_registers {
 	TxPoll_8125		= 0x90,
 	LEDSEL3			= 0x96,
 	MAC0_BKP		= 0x19e0,
+	RDSAR_Q1_LOW		= 0x4000,
 	RSS_CTRL_8125		= 0x4500,
 	Q_NUM_CTRL_8125		= 0x4800,
 	EEE_TXIDLE_TIMER_8125	= 0x6048,
@@ -750,6 +755,14 @@ enum rtl_dash_type {
 	RTL_DASH_25_BP,
 };
 
+struct rtl8169_rx_ring {
+	u32 cur_rx;
+	struct RxDesc *rx_desc_array;
+	dma_addr_t rx_desc_phy_addr[NUM_RX_DESC];
+	dma_addr_t rx_phy_addr;
+	struct page *rx_databuff[NUM_RX_DESC];
+};
+
 enum rtl_sfp_mode {
 	RTL_SFP_NONE,
 	RTL_SFP_8168_AF,
@@ -764,20 +777,18 @@ struct rtl8169_private {
 	enum mac_version mac_version;
 	enum rtl_dash_type dash_type;
 	enum rtl_sfp_mode sfp_mode;
-	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
 	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
 	u32 dirty_tx;
 	struct TxDesc *TxDescArray;	/* 256-aligned Tx descriptor ring */
-	struct RxDesc *RxDescArray;	/* 256-aligned Rx descriptor ring */
 	dma_addr_t TxPhyAddr;
-	dma_addr_t RxPhyAddr;
-	struct page *Rx_databuff[NUM_RX_DESC];	/* Rx data buffers */
 	struct ring_info tx_skb[NUM_TX_DESC];	/* Tx data buffers */
 	struct napi_struct *rtl8169_napi;
+	struct rtl8169_rx_ring *rx_ring;
 	unsigned int num_rx_rings;
 	u16 cp_cmd;
 	u16 tx_lpi_timer;
 	u32 irq_mask;
+	unsigned int hw_supp_num_rx_queues;
 	unsigned int irq_nvecs;
 	struct clk *clk;
 	int speed;
@@ -2718,9 +2729,25 @@ static void rtl_init_rxcfg(struct rtl8169_private *tp)
 	}
 }
 
+static void rtl8169_rx_desc_init(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+
+		memset(ring->rx_desc_array, 0x0, R8169_RX_RING_BYTES);
+	}
+}
+
 static void rtl8169_init_ring_indexes(struct rtl8169_private *tp)
 {
-	tp->dirty_tx = tp->cur_tx = tp->cur_rx = 0;
+	tp->dirty_tx = 0;
+	tp->cur_tx = 0;
+
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+
+		ring->cur_rx = 0;
+	}
 }
 
 static void rtl_jumbo_config(struct rtl8169_private *tp)
@@ -2773,6 +2800,14 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
 static void rtl_setup_rx_params(struct rtl8169_private *tp)
 {
 	tp->num_rx_rings = 1;
+	switch (tp->mac_version) {
+	case RTL_GIGA_MAC_VER_80:
+		tp->hw_supp_num_rx_queues = R8169_MAX_RX_QUEUES;
+		break;
+	default:
+		tp->hw_supp_num_rx_queues = R8169_DEFAULT_RX_QUEUES;
+		break;
+	}
 }
 
 static void rtl_request_firmware(struct rtl8169_private *tp)
@@ -2899,6 +2934,8 @@ static void rtl_set_rx_max_size(struct rtl8169_private *tp)
 
 static void rtl_set_rx_tx_desc_registers(struct rtl8169_private *tp)
 {
+	struct rtl8169_rx_ring *ring = &tp->rx_ring[0];
+
 	/*
 	 * Magic spell: some iop3xx ARM board needs the TxDescAddrHigh
 	 * register to be written before TxDescAddrLow to work.
@@ -2906,8 +2943,18 @@ static void rtl_set_rx_tx_desc_registers(struct rtl8169_private *tp)
 	 */
 	RTL_W32(tp, TxDescStartAddrHigh, ((u64) tp->TxPhyAddr) >> 32);
 	RTL_W32(tp, TxDescStartAddrLow, ((u64) tp->TxPhyAddr) & DMA_BIT_MASK(32));
-	RTL_W32(tp, RxDescAddrHigh, ((u64) tp->RxPhyAddr) >> 32);
-	RTL_W32(tp, RxDescAddrLow, ((u64) tp->RxPhyAddr) & DMA_BIT_MASK(32));
+	RTL_W32(tp, RxDescAddrHigh, ((u64)ring->rx_phy_addr) >> 32);
+	RTL_W32(tp, RxDescAddrLow,
+		((u64)ring->rx_phy_addr) & DMA_BIT_MASK(32));
+
+	for (int i = 1; i < tp->num_rx_rings; i++) {
+		unsigned int rdsar_reg = RDSAR_Q1_LOW + (i - 1) * 8;
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+
+		RTL_W32(tp, rdsar_reg + 4, ((u64)ring->rx_phy_addr >> 32));
+		RTL_W32(tp, rdsar_reg,
+			((u64)ring->rx_phy_addr) & DMA_BIT_MASK(32));
+	}
 }
 
 static void rtl8169_set_magic_reg(struct rtl8169_private *tp)
@@ -4330,8 +4377,9 @@ static void rtl8169_mark_to_asic(struct RxDesc *desc)
 }
 
 static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
-					  struct RxDesc *desc)
+					  struct rtl8169_rx_ring *ring, unsigned int index)
 {
+	struct RxDesc *desc = ring->rx_desc_array + index;
 	struct device *d = tp_to_dev(tp);
 	int node = dev_to_node(d);
 	dma_addr_t mapping;
@@ -4349,55 +4397,107 @@ static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
 	}
 
 	desc->addr = cpu_to_le64(mapping);
+	ring->rx_desc_phy_addr[index] = mapping;
 	rtl8169_mark_to_asic(desc);
 
 	return data;
 }
 
-static void rtl8169_rx_clear(struct rtl8169_private *tp)
+static void rtl8169_rx_clear(struct rtl8169_private *tp,
+			     struct rtl8169_rx_ring *ring)
 {
 	int i;
 
-	for (i = 0; i < NUM_RX_DESC && tp->Rx_databuff[i]; i++) {
+	for (i = 0; i < NUM_RX_DESC && ring->rx_databuff[i]; i++) {
 		dma_unmap_page(tp_to_dev(tp),
-			       le64_to_cpu(tp->RxDescArray[i].addr),
+			       ring->rx_desc_phy_addr[i],
 			       R8169_RX_BUF_SIZE, DMA_FROM_DEVICE);
-		__free_pages(tp->Rx_databuff[i], get_order(R8169_RX_BUF_SIZE));
-		tp->Rx_databuff[i] = NULL;
-		tp->RxDescArray[i].addr = 0;
-		tp->RxDescArray[i].opts1 = 0;
+		__free_pages(ring->rx_databuff[i], get_order(R8169_RX_BUF_SIZE));
+		ring->rx_databuff[i] = NULL;
+		ring->rx_desc_phy_addr[i] = 0;
+		ring->rx_desc_array[i].addr = 0;
+		ring->rx_desc_array[i].opts1 = 0;
 	}
 }
 
-static int rtl8169_rx_fill(struct rtl8169_private *tp)
+static int rtl8169_rx_fill(struct rtl8169_private *tp, struct rtl8169_rx_ring *ring)
 {
 	int i;
 
 	for (i = 0; i < NUM_RX_DESC; i++) {
 		struct page *data;
 
-		data = rtl8169_alloc_rx_data(tp, tp->RxDescArray + i);
+		data = rtl8169_alloc_rx_data(tp, ring, i);
 		if (!data) {
-			rtl8169_rx_clear(tp);
+			rtl8169_rx_clear(tp, ring);
 			return -ENOMEM;
 		}
-		tp->Rx_databuff[i] = data;
+		ring->rx_databuff[i] = data;
 	}
 
 	/* mark as last descriptor in the ring */
-	tp->RxDescArray[NUM_RX_DESC - 1].opts1 |= cpu_to_le32(RingEnd);
+	ring->rx_desc_array[NUM_RX_DESC - 1].opts1 |= cpu_to_le32(RingEnd);
 
 	return 0;
 }
 
+static int rtl8169_alloc_rx_desc(struct rtl8169_private *tp)
+{
+	struct pci_dev *pdev = tp->pci_dev;
+	struct rtl8169_rx_ring *ring;
+
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		ring = &tp->rx_ring[i];
+		ring->rx_desc_array = dma_alloc_coherent(&pdev->dev,
+							 R8169_RX_RING_BYTES,
+							 &ring->rx_phy_addr,
+							 GFP_KERNEL);
+		if (!ring->rx_desc_array)
+			return -ENOMEM;
+	}
+	return 0;
+}
+
+static void rtl8169_free_rx_desc(struct rtl8169_private *tp)
+{
+	struct pci_dev *pdev = tp->pci_dev;
+	struct rtl8169_rx_ring *ring;
+
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		ring = &tp->rx_ring[i];
+		if (ring->rx_desc_array) {
+			dma_free_coherent(&pdev->dev,
+					  R8169_RX_RING_BYTES,
+					  ring->rx_desc_array,
+					  ring->rx_phy_addr);
+			ring->rx_desc_array = NULL;
+		}
+	}
+}
+
 static int rtl8169_init_ring(struct rtl8169_private *tp)
 {
+	int i, ret;
+
 	rtl8169_init_ring_indexes(tp);
+	rtl8169_rx_desc_init(tp);
 
 	memset(tp->tx_skb, 0, sizeof(tp->tx_skb));
-	memset(tp->Rx_databuff, 0, sizeof(tp->Rx_databuff));
 
-	return rtl8169_rx_fill(tp);
+	for (i = 0; i < tp->num_rx_rings; i++) {
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+
+		memset(ring->rx_databuff, 0, sizeof(ring->rx_databuff));
+		ret = rtl8169_rx_fill(tp, ring);
+		if (ret < 0)
+			goto err_clear;
+	}
+	return 0;
+
+err_clear:
+	while (--i >= 0)
+		rtl8169_rx_clear(tp, &tp->rx_ring[i]);
+	return ret;
 }
 
 static void rtl8169_unmap_tx_skb(struct rtl8169_private *tp, unsigned int entry)
@@ -4486,16 +4586,23 @@ static void rtl8169_cleanup(struct rtl8169_private *tp)
 	rtl8169_init_ring_indexes(tp);
 }
 
-static void rtl_reset_work(struct rtl8169_private *tp)
+static void rtl8169_rx_desc_reset(struct rtl8169_private *tp)
 {
-	int i;
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+
+		for (int j = 0; j < NUM_RX_DESC; j++)
+			rtl8169_mark_to_asic(ring->rx_desc_array + j);
+	}
+}
 
+static void rtl_reset_work(struct rtl8169_private *tp)
+{
 	netif_stop_queue(tp->dev);
 
 	rtl8169_cleanup(tp);
 
-	for (i = 0; i < NUM_RX_DESC; i++)
-		rtl8169_mark_to_asic(tp->RxDescArray + i);
+	rtl8169_rx_desc_reset(tp);
 
 	rtl8169_napi_enable(tp);
 	rtl_hw_start(tp);
@@ -4941,7 +5048,8 @@ static inline int rtl8169_fragmented_frame(u32 status)
 	return (status & (FirstFrag | LastFrag)) != (FirstFrag | LastFrag);
 }
 
-static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
+static inline void rtl8169_rx_csum(struct sk_buff *skb,
+				   u32 opts1)
 {
 	u32 status = opts1 & (RxProtoMask | RxCSFailMask);
 
@@ -4951,15 +5059,30 @@ static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
 		skb_checksum_none_assert(skb);
 }
 
+static bool rtl8169_check_rx_desc_error(struct net_device *dev,
+					struct rtl8169_private *tp,
+					u32 status)
+{
+	if (unlikely(status & RxRES)) {
+		if (status & (RxRWT | RxRUNT))
+			dev->stats.rx_length_errors++;
+		if (status & RxCRC)
+			dev->stats.rx_crc_errors++;
+		return true;
+	}
+	return false;
+}
+
 static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
-		  int budget, struct napi_struct *napi)
+		  struct rtl8169_rx_ring *ring, int budget,
+		  struct napi_struct *napi)
 {
 	struct device *d = tp_to_dev(tp);
 	int count;
 
-	for (count = 0; count < budget; count++, tp->cur_rx++) {
-		unsigned int pkt_size, entry = tp->cur_rx % NUM_RX_DESC;
-		struct RxDesc *desc = tp->RxDescArray + entry;
+	for (count = 0; count < budget; count++, ring->cur_rx++) {
+		unsigned int pkt_size, entry = ring->cur_rx % NUM_RX_DESC;
+		struct RxDesc *desc = ring->rx_desc_array + entry;
 		struct sk_buff *skb;
 		const void *rx_buf;
 		dma_addr_t addr;
@@ -4975,15 +5098,11 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		 */
 		dma_rmb();
 
-		if (unlikely(status & RxRES)) {
+		if (rtl8169_check_rx_desc_error(dev, tp, status)) {
 			if (net_ratelimit())
 				netdev_warn(dev, "Rx ERROR. status = %08x\n",
 					    status);
 			dev->stats.rx_errors++;
-			if (status & (RxRWT | RxRUNT))
-				dev->stats.rx_length_errors++;
-			if (status & RxCRC)
-				dev->stats.rx_crc_errors++;
 
 			if (!(dev->features & NETIF_F_RXALL))
 				goto release_descriptor;
@@ -5010,8 +5129,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 			goto release_descriptor;
 		}
 
-		addr = le64_to_cpu(desc->addr);
-		rx_buf = page_address(tp->Rx_databuff[entry]);
+		addr = ring->rx_desc_phy_addr[entry];
+		rx_buf = page_address(ring->rx_databuff[entry]);
 
 		dma_sync_single_for_cpu(d, addr, pkt_size, DMA_FROM_DEVICE);
 		prefetch(rx_buf);
@@ -5155,7 +5274,8 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
 
 	rtl_tx(dev, tp, budget);
 
-	work_done = rtl_rx(dev, tp, budget, napi);
+	/* rtl8169_poll() is used only when there is a single RX ring. */
+	work_done = rtl_rx(dev, tp, &tp->rx_ring[0], budget, napi);
 
 	if (work_done < budget && napi_complete_done(napi, work_done))
 		rtl_irq_enable(tp);
@@ -5255,18 +5375,17 @@ static int rtl8169_close(struct net_device *dev)
 	phylink_stop(tp->phylink);
 	netif_stop_queue(dev);
 	rtl8169_down(tp);
-	rtl8169_rx_clear(tp);
+	for (int i = 0; i < tp->num_rx_rings; i++)
+		rtl8169_rx_clear(tp, &tp->rx_ring[i]);
 
 	rtl8169_free_irq(tp);
 
 	phylink_disconnect_phy(tp->phylink);
 
-	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
-			  tp->RxPhyAddr);
 	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
 			  tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
-	tp->RxDescArray = NULL;
+	rtl8169_free_rx_desc(tp);
 
 	pm_runtime_put_sync(&pdev->dev);
 
@@ -5300,10 +5419,8 @@ static int rtl_open(struct net_device *dev)
 	if (!tp->TxDescArray)
 		goto out;
 
-	tp->RxDescArray = dma_alloc_coherent(&pdev->dev, R8169_RX_RING_BYTES,
-					     &tp->RxPhyAddr, GFP_KERNEL);
-	if (!tp->RxDescArray)
-		goto err_free_tx_0;
+	if (rtl8169_alloc_rx_desc(tp) < 0)
+		goto err_free_rx_1;
 
 	retval = rtl8169_init_ring(tp);
 	if (retval < 0)
@@ -5333,12 +5450,10 @@ static int rtl_open(struct net_device *dev)
 	rtl8169_free_irq(tp);
 err_release_fw_2:
 	rtl_release_firmware(tp);
-	rtl8169_rx_clear(tp);
+	for (int i = 0; i < tp->num_rx_rings; i++)
+		rtl8169_rx_clear(tp, &tp->rx_ring[i]);
 err_free_rx_1:
-	dma_free_coherent(&pdev->dev, R8169_RX_RING_BYTES, tp->RxDescArray,
-			  tp->RxPhyAddr);
-	tp->RxDescArray = NULL;
-err_free_tx_0:
+	rtl8169_free_rx_desc(tp);
 	dma_free_coherent(&pdev->dev, R8169_TX_RING_BYTES, tp->TxDescArray,
 			  tp->TxPhyAddr);
 	tp->TxDescArray = NULL;
@@ -5505,6 +5620,7 @@ static void rtl_remove_one(struct pci_dev *pdev)
 
 	unregister_netdev(tp->dev);
 	r8169_free_napi(tp);
+	kfree(tp->rx_ring);
 	phylink_destroy(tp->phylink);
 
 	if (tp->dash_type != RTL_DASH_NONE)
@@ -6106,7 +6222,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	u32 txconfig;
 	u32 xid;
 
-	dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
+	dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*tp),
+				      R8169_MAX_TX_QUEUES,
+				      R8169_MAX_RX_QUEUES);
+
 	if (!dev)
 		return -ENOMEM;
 
@@ -6295,20 +6414,25 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
+	tp->rx_ring = kcalloc(tp->num_rx_rings, sizeof(struct rtl8169_rx_ring),
+			      GFP_KERNEL);
+	if (!tp->rx_ring) {
+		rc = -ENOMEM;
+		goto err_destory_phylink;
+	}
+
 	tp->rtl8169_napi = kcalloc(tp->irq_nvecs, sizeof(struct napi_struct),
 				   GFP_KERNEL);
 	if (!tp->rtl8169_napi) {
-		phylink_destroy(tp->phylink);
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto err_free_rx_ring;
 	}
 
 	r8169_init_napi(tp);
 
 	rc = register_netdev(dev);
-	if (rc) {
-		phylink_destroy(tp->phylink);
+	if (rc)
 		goto err_free_napi;
-	}
 
 	if (IS_ENABLED(CONFIG_R8169_LEDS)) {
 		if (rtl_is_8125(tp))
@@ -6342,6 +6466,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 err_free_napi:
 	r8169_free_napi(tp);
+err_free_rx_ring:
+	kfree(tp->rx_ring);
+err_destory_phylink:
+	phylink_destroy(tp->phylink);
 	return rc;
 }
 
-- 
2.43.0

[PATCH net-next v14 3/7] r8169: add support for new interrupt mapping

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

To support RSS, the number of hardware interrupt bits should match the
interrupt of software. So we add support for new interrupt mapping here.
ISR_VEC_MAP_REG is the hardware register to indicate interrupt status.
IMR_SET_VEC_MAP_REG is interrupt mask which is set to enable irq.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - no changes

Changes in v3:
 - init index in napi_struct and get message_id from index
 - move rtl8169_disable_hw_interrupt_msix directly before the call to
   napi_schedule()
 - change the condition in rtl8169_request_irq when RTL_VEC_MAP_ENABLE
   enabled, use rtl8169_interrupt_msix

Changes in v4:
 - remove flag tp->feature, replace tp->features & RTL_VEC_MAP_ENABLE
   with tp->irq_nvecs > 1, they are equivalent.
 - follow reverse xmas tree, in rtl8169_interrupt_msix(),
   rtl8169_poll_msix_rx(), rtl8169_poll_msix_tx(),
   rtl8169_poll_msix_other()
 - use napi->index in rtl8169_poll_msix_other()
 - add a comment to describe RTL8127 MSI-X vector layout
 - simplify r8169_init_napi()

Changes in v5:
 - replace magic number in rtl8169_poll_msix_tx()

Changes in v6:
 - when irq_nvecs <= 1, use register IntrMask_8125, else using vec map
 - fix irq sequence in rtl8169_interrupt_msix(), disable interrupts
   before clean it
 - remove dead code in rtl8169_poll_msix_tx()

Changes in v7:
 - remove recheck_desc_ownbit
 - change return value of rtl_tx
 - remove message_id which only used once

Changes in v8:
 - fix rtl8169_netpoll()
 - remove tx_done

Changes in v9:
 - change the way of getting message_id of napi

Changes in v10:
 - no changes

Changes in v11:
 - add comment on rtl8169_poll_msix_tx, only use 1 tx
 - remove napi for other. Separate napi only for datapath, control path
   like linkchg is handled in interrupt function, which will not call
   napi any more.

Changes in v12:
 - no changes

Changes in v13:
 - only request IRQs and napi for active vectors(0~7 rx, 8 tx)

Changes in v14:
 - fix tp->phydev null pointer issue when the nic is fiber mode
---
 drivers/net/ethernet/realtek/r8169_main.c | 223 +++++++++++++++++++---
 1 file changed, 193 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 7a2e7ce56e48..c647b4327ff7 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -471,8 +471,12 @@ enum rtl8125_registers {
 	RSS_CTRL_8125		= 0x4500,
 	Q_NUM_CTRL_8125		= 0x4800,
 	EEE_TXIDLE_TIMER_8125	= 0x6048,
+	IMR_CLEAR_VEC_MAP_REG	= 0x0d00,
+	ISR_VEC_MAP_REG		= 0x0d04,
+	IMR_SET_VEC_MAP_REG	= 0x0d0c,
 };
 
+#define MSIX_ID_VEC_MAP_LINKCHG	29
 #define LEDSEL_MASK_8125	0x23f
 
 #define RX_VLAN_INNER_8125	BIT(22)
@@ -603,6 +607,9 @@ enum rtl_register_content {
 
 	/* magic enable v2 */
 	MagicPacket_v2	= (1 << 16),	/* Wake up when receives a Magic Packet */
+#define	ISRIMR_LINKCHG	BIT(29)
+#define	ISRIMR_TOK_Q0	BIT(8)
+#define	ISRIMR_ROK_Q0	BIT(0)
 };
 
 enum rtl_desc_bit {
@@ -1770,26 +1777,38 @@ static u32 rtl_get_events(struct rtl8169_private *tp)
 
 static void rtl_ack_events(struct rtl8169_private *tp, u32 bits)
 {
-	if (rtl_is_8125(tp))
-		RTL_W32(tp, IntrStatus_8125, bits);
-	else
+	if (rtl_is_8125(tp)) {
+		if (tp->irq_nvecs > 1)
+			RTL_W32(tp, ISR_VEC_MAP_REG, bits);
+		else
+			RTL_W32(tp, IntrStatus_8125, bits);
+	} else {
 		RTL_W16(tp, IntrStatus, bits);
+	}
 }
 
 static void rtl_irq_disable(struct rtl8169_private *tp)
 {
-	if (rtl_is_8125(tp))
-		RTL_W32(tp, IntrMask_8125, 0);
-	else
+	if (rtl_is_8125(tp)) {
+		if (tp->irq_nvecs > 1)
+			RTL_W32(tp, IMR_CLEAR_VEC_MAP_REG, 0xffffffff);
+		else
+			RTL_W32(tp, IntrMask_8125, 0);
+	} else {
 		RTL_W16(tp, IntrMask, 0);
+	}
 }
 
 static void rtl_irq_enable(struct rtl8169_private *tp)
 {
-	if (rtl_is_8125(tp))
-		RTL_W32(tp, IntrMask_8125, tp->irq_mask);
-	else
+	if (rtl_is_8125(tp)) {
+		if (tp->irq_nvecs > 1)
+			RTL_W32(tp, IMR_SET_VEC_MAP_REG, tp->irq_mask);
+		else
+			RTL_W32(tp, IntrMask_8125, tp->irq_mask);
+	} else {
 		RTL_W16(tp, IntrMask, tp->irq_mask);
+	}
 }
 
 static void rtl8169_irq_mask_and_ack(struct rtl8169_private *tp)
@@ -4539,13 +4558,17 @@ static void rtl8169_tx_clear(struct rtl8169_private *tp)
 
 static void rtl8169_napi_disable(struct rtl8169_private *tp)
 {
-	for (int i = 0; i < tp->irq_nvecs; i++)
+	int napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
+
+	for (int i = 0; i < napi_num; i++)
 		napi_disable(&tp->rtl8169_napi[i]);
 }
 
 static void rtl8169_napi_enable(struct rtl8169_private *tp)
 {
-	for (int i = 0; i < tp->irq_nvecs; i++)
+	int napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
+
+	for (int i = 0; i < napi_num; i++)
 		napi_enable(&tp->rtl8169_napi[i]);
 }
 
@@ -5192,19 +5215,81 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	return IRQ_HANDLED;
 }
 
-static void rtl8169_free_irq(struct rtl8169_private *tp)
+static void rtl8169_free_one_irq(struct rtl8169_private *tp, int i)
 {
 	struct pci_dev *pdev = tp->pci_dev;
-	int i, irq;
+	struct napi_struct *napi;
+	int irq;
 
-	for (i = 0; i < tp->irq_nvecs; i++) {
-		struct napi_struct *napi = &tp->rtl8169_napi[i];
+	irq = pci_irq_vector(pdev, i);
+	napi = &tp->rtl8169_napi[i];
 
-		irq = pci_irq_vector(pdev, i);
+	if (tp->irq_nvecs > 1) {
+		if (i <= R8169_MAX_RX_QUEUES)
+			free_irq(irq, napi);
+		else if (i == MSIX_ID_VEC_MAP_LINKCHG)
+			free_irq(irq, tp);
+	} else {
 		free_irq(irq, napi);
 	}
 }
 
+static void rtl8169_free_irq(struct rtl8169_private *tp)
+{
+	int i;
+
+	for (i = 0; i < tp->irq_nvecs; i++)
+		rtl8169_free_one_irq(tp, i);
+}
+
+static void rtl8169_disable_hw_interrupt_msix(struct rtl8169_private *tp,
+					      int message_id)
+{
+	RTL_W32(tp, IMR_CLEAR_VEC_MAP_REG, BIT(message_id));
+}
+
+static void rtl8169_clear_hw_isr(struct rtl8169_private *tp, int message_id)
+{
+	RTL_W32(tp, ISR_VEC_MAP_REG, BIT(message_id));
+}
+
+static void rtl8169_enable_hw_interrupt_msix(struct rtl8169_private *tp,
+					     int message_id)
+{
+	RTL_W32(tp, IMR_SET_VEC_MAP_REG, BIT(message_id));
+}
+
+static irqreturn_t rtl8169_interrupt_msix(int irq, void *dev_instance)
+{
+	struct napi_struct *napi = dev_instance;
+	struct net_device *dev = napi->dev;
+	struct rtl8169_private *tp;
+	int message_id;
+
+	tp = netdev_priv(dev);
+	message_id = napi - tp->rtl8169_napi;
+
+	rtl8169_disable_hw_interrupt_msix(tp, message_id);
+	rtl8169_clear_hw_isr(tp, message_id);
+
+	napi_schedule(napi);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t rtl8169_interrupt_other(int irq, void *dev_instance)
+{
+	struct rtl8169_private *tp = dev_instance;
+
+	rtl8169_clear_hw_isr(tp, MSIX_ID_VEC_MAP_LINKCHG);
+	if (tp->phydev)
+		phy_mac_interrupt(tp->phydev);
+	else if (tp->sfp_mode)
+		phylink_mac_change(tp->phylink,
+				   !!(RTL_R8(tp, PHYstatus) & LinkStatus));
+	return IRQ_HANDLED;
+}
+
 static int rtl8169_request_irq(struct rtl8169_private *tp)
 {
 	struct pci_dev *pdev = tp->pci_dev;
@@ -5219,18 +5304,28 @@ static int rtl8169_request_irq(struct rtl8169_private *tp)
 		napi = &tp->rtl8169_napi[i];
 		irq = pci_irq_vector(pdev, i);
 
-		rc = request_irq(irq, rtl8169_interrupt, irqflags,
-				 dev->name, napi);
+		if (tp->irq_nvecs > 1) {
+			/* RX: 0~7, TX: 8*/
+			if (i <= R8169_MAX_RX_QUEUES)
+				rc = request_irq(irq, rtl8169_interrupt_msix,
+						 irqflags, dev->name, napi);
+			else if (i == MSIX_ID_VEC_MAP_LINKCHG)
+				rc = request_irq(irq, rtl8169_interrupt_other,
+						 irqflags, dev->name, tp);
+			else
+				continue;
+		} else {
+			rc = request_irq(irq, rtl8169_interrupt,
+					 irqflags, dev->name, napi);
+		}
 		if (rc)
 			goto free_irq;
 	}
 	return 0;
 
 free_irq:
-	while (--i >= 0) {
-		irq = pci_irq_vector(pdev, i);
-		free_irq(irq, &tp->rtl8169_napi[i]);
-	}
+	while (--i >= 0)
+		rtl8169_free_one_irq(tp, i);
 	return rc;
 }
 
@@ -5396,9 +5491,16 @@ static int rtl8169_close(struct net_device *dev)
 static void rtl8169_netpoll(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
+	int napi_num;
+
+	napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
 
-	for (int i = 0; i < tp->irq_nvecs; i++)
-		rtl8169_interrupt(0, &tp->rtl8169_napi[i]);
+	for (int i = 0; i < napi_num; i++) {
+		if (tp->irq_nvecs > 1)
+			rtl8169_interrupt_msix(0, &tp->rtl8169_napi[i]);
+		else
+			rtl8169_interrupt(0, &tp->rtl8169_napi[i]);
+	}
 }
 #endif
 
@@ -5600,7 +5702,9 @@ static void rtl_shutdown(struct pci_dev *pdev)
 
 static void r8169_free_napi(struct rtl8169_private *tp)
 {
-	for (int i = 0; i < tp->irq_nvecs; i++)
+	int napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
+
+	for (int i = 0; i < napi_num; i++)
 		netif_napi_del(&tp->rtl8169_napi[i]);
 
 	kfree(tp->rtl8169_napi);
@@ -5654,10 +5758,16 @@ static const struct net_device_ops rtl_netdev_ops = {
 
 static void rtl_set_irq_mask(struct rtl8169_private *tp)
 {
-	tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
+	if (tp->irq_nvecs > 1) {
+		tp->irq_mask = ISRIMR_LINKCHG | ISRIMR_TOK_Q0;
+		for (int i = 0; i < tp->num_rx_rings; i++)
+			tp->irq_mask |= ISRIMR_ROK_Q0 << i;
+	} else {
+		tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
 
-	if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
-		tp->irq_mask |= SYSErr | RxFIFOOver;
+		if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
+			tp->irq_mask |= SYSErr | RxFIFOOver;
+	}
 }
 
 static int rtl_alloc_irq(struct rtl8169_private *tp)
@@ -6203,10 +6313,63 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
 	return 0;
 }
 
+static int rtl8169_poll_msix_rx(struct napi_struct *napi, int budget)
+{
+	struct net_device *dev = napi->dev;
+	struct rtl8169_private *tp;
+	int work_done = 0;
+	int message_id;
+
+	tp = netdev_priv(dev);
+	message_id = napi - tp->rtl8169_napi;
+
+	if (message_id < tp->num_rx_rings)
+		work_done += rtl_rx(dev, tp, &tp->rx_ring[message_id],
+				    budget, napi);
+
+	if (work_done < budget && napi_complete_done(napi, work_done))
+		rtl8169_enable_hw_interrupt_msix(tp, message_id);
+
+	return work_done;
+}
+
+static int rtl8169_poll_msix_tx(struct napi_struct *napi, int budget)
+{
+	struct net_device *dev = napi->dev;
+	struct rtl8169_private *tp;
+
+	tp = netdev_priv(dev);
+
+	/* Currently r8169 only supports a single Tx ring.
+	 * Therefore, we don't need a per-ring Tx processing loop here.
+	 */
+	rtl_tx(dev, tp, budget);
+
+	if (napi_complete_done(napi, 0))
+		rtl8169_enable_hw_interrupt_msix(tp, (int)(napi - tp->rtl8169_napi));
+
+	return 0;
+}
+
+/* RTL8127 MSI-X vector layout:
+ * Vectors 0 .. (MAX_RXQ - 1)			: Rx Queues
+ * Vectors MAX_RXQ .. (MAX_RXQ + MAX_TXQ - 1)	: Tx Queues
+ * NAPI is only allocated for data path
+ */
 static void r8169_init_napi(struct rtl8169_private *tp)
 {
-	for (int i = 0; i < tp->irq_nvecs; i++) {
-		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], rtl8169_poll);
+	int napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
+
+	for (int i = 0; i < napi_num; i++) {
+		int (*poll_fn)(struct napi_struct *, int) = rtl8169_poll;
+
+		if (tp->irq_nvecs > 1) {
+			if (i < R8169_MAX_RX_QUEUES)
+				poll_fn = rtl8169_poll_msix_rx;
+			else
+				poll_fn = rtl8169_poll_msix_tx;
+		}
+		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], poll_fn);
 		netif_napi_set_irq(&tp->rtl8169_napi[i],
 				   pci_irq_vector(tp->pci_dev, i));
 	}
-- 
2.43.0

[PATCH net-next v14 4/7] r8169: enable new interrupt mapping

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch enables new interrupt mapping for RTL8127 and add error pkts
counter per ring.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - no changes

Changes in v3:
 - no changes

Changes in v4:
 - no changes

Changes in v5:
 - no changes

Changes in v6:
 - no changes

Changes in v7:
 - no changes

Changes in v8:
 - no changes

Changes in v9:
 - no changes

Changes in v10:
 - no changes

Changes in v11:
 - add error pkts counter per ring

Changes in v12:
 - drop unrelated change in rtl8169_init_ring()

Changes in v13:
 - no changes

Changes in v14:
 - no changes
---
 drivers/net/ethernet/realtek/r8169_main.c | 78 +++++++++++++++++++----
 1 file changed, 67 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index c647b4327ff7..ec3643892fe2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -30,6 +30,7 @@
 #include <linux/prefetch.h>
 #include <linux/ipv6.h>
 #include <linux/unaligned.h>
+#include <linux/u64_stats_sync.h>
 #include <net/ip6_checksum.h>
 #include <net/netdev_queues.h>
 
@@ -768,6 +769,15 @@ struct rtl8169_rx_ring {
 	dma_addr_t rx_desc_phy_addr[NUM_RX_DESC];
 	dma_addr_t rx_phy_addr;
 	struct page *rx_databuff[NUM_RX_DESC];
+
+	struct {
+		u64 rx_errors;
+		u64 rx_dropped;
+		u64 rx_length_errors;
+		u64 rx_crc_errors;
+		u64 multicast;
+		struct u64_stats_sync syncp;
+	} stats;
 };
 
 enum rtl_sfp_mode {
@@ -4084,6 +4094,15 @@ DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)
 	return r8168_mac_ocp_read(tp, 0xe00e) & BIT(13);
 }
 
+static void rtl8169_hw_enable_vec_mapping(struct rtl8169_private *tp)
+{
+	u8 tmp;
+
+	tmp = RTL_R8(tp, INT_CFG0_8125);
+	tmp |= INT_CFG0_ENABLE_8125;
+	RTL_W8(tp, INT_CFG0_8125, tmp);
+}
+
 static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
 {
 	rtl_pcie_state_l2l3_disable(tp);
@@ -4092,6 +4111,9 @@ static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
 	RTL_W32(tp, RSS_CTRL_8125, 0);
 	RTL_W16(tp, Q_NUM_CTRL_8125, 0);
 
+	if (tp->irq_nvecs > 1)
+		rtl8169_hw_enable_vec_mapping(tp);
+
 	/* disable UPS */
 	r8168_mac_ocp_modify(tp, 0xd40a, 0x0010, 0x0000);
 
@@ -5082,15 +5104,16 @@ static inline void rtl8169_rx_csum(struct sk_buff *skb,
 		skb_checksum_none_assert(skb);
 }
 
-static bool rtl8169_check_rx_desc_error(struct net_device *dev,
-					struct rtl8169_private *tp,
+static bool rtl8169_check_rx_desc_error(struct rtl8169_rx_ring *ring,
 					u32 status)
 {
 	if (unlikely(status & RxRES)) {
+		u64_stats_update_begin(&ring->stats.syncp);
 		if (status & (RxRWT | RxRUNT))
-			dev->stats.rx_length_errors++;
+			ring->stats.rx_length_errors++;
 		if (status & RxCRC)
-			dev->stats.rx_crc_errors++;
+			ring->stats.rx_crc_errors++;
+		u64_stats_update_end(&ring->stats.syncp);
 		return true;
 	}
 	return false;
@@ -5121,11 +5144,13 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		 */
 		dma_rmb();
 
-		if (rtl8169_check_rx_desc_error(dev, tp, status)) {
+		if (rtl8169_check_rx_desc_error(ring, status)) {
 			if (net_ratelimit())
 				netdev_warn(dev, "Rx ERROR. status = %08x\n",
 					    status);
-			dev->stats.rx_errors++;
+			u64_stats_update_begin(&ring->stats.syncp);
+			ring->stats.rx_errors++;
+			u64_stats_update_end(&ring->stats.syncp);
 
 			if (!(dev->features & NETIF_F_RXALL))
 				goto release_descriptor;
@@ -5141,14 +5166,18 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		 * They are seen as a symptom of over-mtu sized frames.
 		 */
 		if (unlikely(rtl8169_fragmented_frame(status))) {
-			dev->stats.rx_dropped++;
-			dev->stats.rx_length_errors++;
+			u64_stats_update_begin(&ring->stats.syncp);
+			ring->stats.rx_dropped++;
+			ring->stats.rx_length_errors++;
+			u64_stats_update_end(&ring->stats.syncp);
 			goto release_descriptor;
 		}
 
 		skb = napi_alloc_skb(napi, pkt_size);
 		if (unlikely(!skb)) {
-			dev->stats.rx_dropped++;
+			u64_stats_update_begin(&ring->stats.syncp);
+			ring->stats.rx_dropped++;
+			u64_stats_update_end(&ring->stats.syncp);
 			goto release_descriptor;
 		}
 
@@ -5167,8 +5196,11 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 
 		rtl8169_rx_vlan_tag(desc, skb);
 
-		if (skb->pkt_type == PACKET_MULTICAST)
-			dev->stats.multicast++;
+		if (skb->pkt_type == PACKET_MULTICAST) {
+			u64_stats_update_begin(&ring->stats.syncp);
+			ring->stats.multicast++;
+			u64_stats_update_end(&ring->stats.syncp);
+		}
 
 		napi_gro_receive(napi, skb);
 
@@ -5574,6 +5606,27 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	netdev_stats_to_stats64(stats, &dev->stats);
 	dev_fetch_sw_netstats(stats, dev->tstats);
 
+	for (int i = 0; i < tp->num_rx_rings; i++) {
+		u64 errors, dropped, length_errors, crc_errors, multicast;
+		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
+		unsigned int start;
+
+		do {
+			start = u64_stats_fetch_begin(&ring->stats.syncp);
+			errors = ring->stats.rx_errors;
+			dropped = ring->stats.rx_dropped;
+			length_errors = ring->stats.rx_length_errors;
+			crc_errors = ring->stats.rx_crc_errors;
+			multicast = ring->stats.multicast;
+		} while (u64_stats_fetch_retry(&ring->stats.syncp, start));
+
+		stats->rx_errors += errors;
+		stats->rx_dropped += dropped;
+		stats->rx_length_errors += length_errors;
+		stats->rx_crc_errors += crc_errors;
+		stats->multicast += multicast;
+	}
+
 	/*
 	 * Fetch additional counter values missing in stats collected by driver
 	 * from tally counters.
@@ -6584,6 +6637,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_destory_phylink;
 	}
 
+	for (int i = 0; i < tp->num_rx_rings; i++)
+		u64_stats_init(&tp->rx_ring[i].stats.syncp);
+
 	tp->rtl8169_napi = kcalloc(tp->irq_nvecs, sizeof(struct napi_struct),
 				   GFP_KERNEL);
 	if (!tp->rtl8169_napi) {
-- 
2.43.0

[PATCH net-next v14 5/7] r8169: add support and enable rss

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

This patch adds support and enable rss for RTL8127.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - some changes moved from Patch 2/7

Changes in v3:
 - add struct rtl8169_rss_data. Allocate it dynamically when needed.
 - define rss_key as an u32 array
 - replace some magic bit numbers in rtl8169_set_rss_hash_opt() and
   rtl8125_set_rx_q_num()
 - use union to combine different rx descriptor, refactor struct RxDesc
 - remove dead code from rtl8169_double_check_rss_support()

Changes in v4:
 - rename macro definition, e.g R8127_MAX_IRQ to R8127_MAX_NUM_IRQVEC
 - change hw_supp_indir_tbl_entries type to unsigned int
 - change init_rx_desc_type type to enum
 - remove rtl_check_rss_support(), add helper function
   rtl_hw_support_rss()
 - remove hw_curr_isr_ver, use irq_nvecs to judge whether we should
   enable vector interrupt mapping, use tp->num_rx_ring to judge whether
   we should enable rss
 - remove function rtl8169_double_check_rss_support(), use
   rtl8169_set_rx_ring_num() to set num_rx_ring according to tp->irq_nvecs

Changes in v5:
 - no changes

Changes in v6:
 - change rss_queue_num type from u8 to unsigned int
 - fix rx desc clear in rtl8169_rx_clear() for different desc type
 - clamping num_rx_ring with rounddown_pow_of_two()

Changes in v7:
 - remove unused macro
 - change unfixed type in rtl8169_store_reta

Changes in v8:
 - refill desc->addr when rx_desc reset
 - rtl8169_set_channels fixed in patch 7/7

Changes in v9:
 - remove rtl8169_set_desc_dma_addr, only set desc dma addr for
   RX_DESC_TYPE_RSS desc

Changes in v10:
 - Change rss_key to u8 array and write rss_key_reg as u32 values.
   Use get_unaligned_le32() to keep behavior consistent on big-endian
   and little-endian

Changes in v11:
 - fix compilation error by adding block in switch default case
 - fix concurrency bug on updating global dev->stats by using per-queue
   stat
 - fix packet drop logic to properlly handle fatal errors when rss is
   enable
 - use get_unaligned_le32() uniformly in rtl8169_store_reta()
 - fix coding style issues
 - add comment on pci_alloc_irq_vectors() call

Changes in v12:
 - add support for UDP rss

Changes in v13:
 - no changes

Changes in v14:
 - fix an integer underflow in rtl_rx() when handling extremely small
   RUNT packets with RXALL enabled.
 - remove redundant macro definition such as RX_RUNT_RSS/RX_CRC_RSS
---
 drivers/net/ethernet/realtek/r8169_main.c | 406 ++++++++++++++++++++--
 1 file changed, 368 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ec3643892fe2..ad360f145076 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -82,6 +82,19 @@
 #define R8169_MAX_RX_QUEUES	8
 #define R8169_DEFAULT_RX_QUEUES	1
 #define R8169_MAX_TX_QUEUES	1
+#define R8127_MAX_NUM_IRQVEC	32
+#define R8127_MIN_NUM_IRQVEC	30
+#define R8169_IRQ_DEFAULT	1
+#define RTL_RSS_KEY_SIZE	40
+#define RSS_CPU_NUM_MASK	GENMASK(18, 16)
+#define RSS_HASH_MASK		GENMASK(10, 8)
+#define RTL_MAX_INDIRECTION_TABLE_ENTRIES 128
+#define RXS_RSS_UDP		BIT(27)
+#define RXS_RSS_IPV4		BIT(28)
+#define RXS_RSS_IPV6		BIT(29)
+#define RXS_RSS_TCP		BIT(30)
+#define RXS_RSS_L3_TYPE_MASK	(RXS_RSS_IPV4 | RXS_RSS_IPV6)
+#define RXS_RSS_L4_TYPE_MASK	(RXS_RSS_TCP | RXS_RSS_UDP)
 
 #define OCP_STD_PHY_BASE	0xa400
 
@@ -506,6 +519,9 @@ enum rtl_register_content {
 	RxRUNT	= (1 << 20),
 	RxCRC	= (1 << 19),
 
+	RXRUNT_RSS	= (1 << 21),
+	RXCRC_RSS	= (1 << 20),
+
 	/* ChipCmdBits */
 	StopReq		= 0x80,
 	CmdReset	= 0x10,
@@ -611,6 +627,20 @@ enum rtl_register_content {
 #define	ISRIMR_LINKCHG	BIT(29)
 #define	ISRIMR_TOK_Q0	BIT(8)
 #define	ISRIMR_ROK_Q0	BIT(0)
+#define RTL_DESC_TYPE_CTRL		0xd8
+#define RSS_KEY_REG			0x4600
+#define RSS_INDIRECTION_TBL_REG		0x4700
+#define RSS_CTRL_TCP_IPV4_SUPP		BIT(0)
+#define RTL_DESC_TYPE_RSS		BIT(1)
+#define RSS_CTRL_IPV4_SUPP		BIT(1)
+#define RSS_CTRL_TCP_IPV6_SUPP		BIT(2)
+#define RSS_CTRL_IPV6_SUPP		BIT(3)
+#define RSS_CTRL_IPV6_EXT_SUPP		BIT(4)
+#define RSS_CTRL_TCP_IPV6_EXT_SUPP	BIT(5)
+#define RSS_CTRL_UDP_IPV4_SUPP		BIT(11)
+#define RSS_CTRL_UDP_IPV6_SUPP		BIT(12)
+#define	RX_RES_RSS			BIT(22)
+#define RTL_RX_Q_NUM_MASK		GENMASK(4, 2)
 };
 
 enum rtl_desc_bit {
@@ -668,6 +698,11 @@ enum rtl_rx_desc_bit {
 #define RxProtoIP	(PID1 | PID0)
 #define RxProtoMask	RxProtoIP
 
+#define	RX_UDPT_DESC_RSS	BIT(19)
+#define	RX_TCPT_DESC_RSS	BIT(18)
+#define	RX_UDPF_DESC_RSS	BIT(16) /* UDP/IP checksum failed */
+#define	RX_TCPF_DESC_RSS	BIT(15) /* TCP/IP checksum failed */
+
 	IPFail		= (1 << 16), /* IP checksum failed */
 	UDPFail		= (1 << 15), /* UDP/IP checksum failed */
 	TCPFail		= (1 << 14), /* TCP/IP checksum failed */
@@ -689,9 +724,27 @@ struct TxDesc {
 };
 
 struct RxDesc {
-	__le32 opts1;
-	__le32 opts2;
-	__le64 addr;
+	union {
+		/* RX_DESC_TYPE_DEFAULT */
+		struct {
+			__le32 opts1;
+			__le32 opts2;
+			__le64 addr;
+		};
+
+		/* RX_DESC_TYPE_RSS */
+		struct {
+			union {
+				__le64 rss_addr;
+				struct {
+					__le32 rss_info;
+					__le32 rss_result;
+				} rss_dword;
+			};
+			__le32 rss_opts2;
+			__le32 rss_opts1;
+		};
+	};
 };
 
 struct ring_info {
@@ -763,6 +816,11 @@ enum rtl_dash_type {
 	RTL_DASH_25_BP,
 };
 
+enum rx_desc_type {
+	RX_DESC_TYPE_DEFAULT,
+	RX_DESC_TYPE_RSS,
+};
+
 struct rtl8169_rx_ring {
 	u32 cur_rx;
 	struct RxDesc *rx_desc_array;
@@ -786,6 +844,12 @@ enum rtl_sfp_mode {
 	RTL_SFP_8127_ATF,
 };
 
+struct rtl8169_rss_data {
+	u8 rss_key[RTL_RSS_KEY_SIZE];
+	u8 rss_indir_tbl[RTL_MAX_INDIRECTION_TABLE_ENTRIES];
+	unsigned int hw_supp_indir_tbl_entries;
+};
+
 struct rtl8169_private {
 	void __iomem *mmio_addr;	/* memory map physical address */
 	struct pci_dev *pci_dev;
@@ -806,7 +870,9 @@ struct rtl8169_private {
 	u16 tx_lpi_timer;
 	u32 irq_mask;
 	unsigned int hw_supp_num_rx_queues;
+	struct rtl8169_rss_data *rss_data;
 	unsigned int irq_nvecs;
+	enum rx_desc_type init_rx_desc_type;
 	struct clk *clk;
 	int speed;
 
@@ -1721,6 +1787,11 @@ static enum rtl_sfp_mode rtl_get_sfp_mode(struct rtl8169_private *tp)
 	return RTL_SFP_NONE;
 }
 
+static bool rtl_hw_support_rss(struct rtl8169_private *tp)
+{
+	return tp->mac_version == RTL_GIGA_MAC_VER_80;
+}
+
 static enum rtl_dash_type rtl_get_dash_type(struct rtl8169_private *tp)
 {
 	switch (tp->mac_version) {
@@ -2020,9 +2091,20 @@ static inline u32 rtl8169_tx_vlan_tag(struct sk_buff *skb)
 		TxVlanTag | swab16(skb_vlan_tag_get(skb)) : 0x00;
 }
 
-static void rtl8169_rx_vlan_tag(struct RxDesc *desc, struct sk_buff *skb)
+static void rtl8169_rx_vlan_tag(struct rtl8169_private *tp,
+				struct RxDesc *desc,
+				struct sk_buff *skb)
 {
-	u32 opts2 = le32_to_cpu(desc->opts2);
+	u32 opts2;
+
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		opts2 = le32_to_cpu(desc->rss_opts2);
+		break;
+	default:
+		opts2 = le32_to_cpu(desc->opts2);
+		break;
+	}
 
 	if (opts2 & RxVlanTag)
 		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), swab16(opts2 & 0xffff));
@@ -2826,17 +2908,27 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
 	rtl_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100);
 }
 
+static void rtl8169_init_rss(struct rtl8169_private *tp)
+{
+	for (int i = 0; i < tp->rss_data->hw_supp_indir_tbl_entries; i++)
+		tp->rss_data->rss_indir_tbl[i] = ethtool_rxfh_indir_default(i, tp->num_rx_rings);
+
+	netdev_rss_key_fill(tp->rss_data->rss_key, RTL_RSS_KEY_SIZE);
+}
+
 static void rtl_setup_rx_params(struct rtl8169_private *tp)
 {
 	tp->num_rx_rings = 1;
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_80:
 		tp->hw_supp_num_rx_queues = R8169_MAX_RX_QUEUES;
+		tp->rss_data->hw_supp_indir_tbl_entries = RTL_MAX_INDIRECTION_TABLE_ENTRIES;
 		break;
 	default:
 		tp->hw_supp_num_rx_queues = R8169_DEFAULT_RX_QUEUES;
 		break;
 	}
+	tp->init_rx_desc_type = RX_DESC_TYPE_DEFAULT;
 }
 
 static void rtl_request_firmware(struct rtl8169_private *tp)
@@ -2961,6 +3053,58 @@ static void rtl_set_rx_max_size(struct rtl8169_private *tp)
 	RTL_W16(tp, RxMaxSize, R8169_RX_BUF_SIZE + 1);
 }
 
+static void rtl8169_store_rss_key(struct rtl8169_private *tp)
+{
+	u8 *rss_key = tp->rss_data->rss_key;
+	const u16 rss_key_reg = RSS_KEY_REG;
+
+	/* Write RSS key to HW */
+	for (int i = 0; i < RTL_RSS_KEY_SIZE; i += sizeof(u32))
+		RTL_W32(tp, rss_key_reg + i, get_unaligned_le32(rss_key + i));
+}
+
+static void rtl8169_store_reta(struct rtl8169_private *tp)
+{
+	u8 *indir_tbl = tp->rss_data->rss_indir_tbl;
+	unsigned int i;
+
+	/* Write redirection table to HW */
+	for (i = 0; i < tp->rss_data->hw_supp_indir_tbl_entries; i += 4) {
+		u32 reta = get_unaligned_le32(&indir_tbl[i]);
+
+		RTL_W32(tp, RSS_INDIRECTION_TBL_REG + i, reta);
+	}
+}
+
+static void rtl8169_set_rss_hash_opt(struct rtl8169_private *tp)
+{
+	u32 rss_ctrl;
+
+	rss_ctrl = FIELD_PREP(RSS_CPU_NUM_MASK, ilog2(tp->num_rx_rings));
+
+	/* Perform hash on these packet types */
+	rss_ctrl |= RSS_CTRL_TCP_IPV4_SUPP |
+		    RSS_CTRL_IPV4_SUPP |
+		    RSS_CTRL_IPV6_SUPP |
+		    RSS_CTRL_IPV6_EXT_SUPP |
+		    RSS_CTRL_TCP_IPV6_SUPP |
+		    RSS_CTRL_TCP_IPV6_EXT_SUPP |
+		    RSS_CTRL_UDP_IPV4_SUPP |
+		    RSS_CTRL_UDP_IPV6_SUPP;
+
+	rss_ctrl |= FIELD_PREP(RSS_HASH_MASK,
+			       ilog2(tp->rss_data->hw_supp_indir_tbl_entries));
+
+	RTL_W32(tp, RSS_CTRL_8125, rss_ctrl);
+}
+
+static void rtl_set_rss_config(struct rtl8169_private *tp)
+{
+	rtl8169_set_rss_hash_opt(tp);
+	rtl8169_store_reta(tp);
+	rtl8169_store_rss_key(tp);
+}
+
 static void rtl_set_rx_tx_desc_registers(struct rtl8169_private *tp)
 {
 	struct rtl8169_rx_ring *ring = &tp->rx_ring[0];
@@ -4094,6 +4238,18 @@ DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)
 	return r8168_mac_ocp_read(tp, 0xe00e) & BIT(13);
 }
 
+static void rtl8125_set_rx_q_num(struct rtl8169_private *tp)
+{
+	u16 rx_q_num;
+	u16 q_ctrl;
+
+	rx_q_num = ilog2(tp->num_rx_rings);
+	q_ctrl = RTL_R16(tp, Q_NUM_CTRL_8125);
+	q_ctrl &= ~RTL_RX_Q_NUM_MASK;
+	q_ctrl |= FIELD_PREP(RTL_RX_Q_NUM_MASK, rx_q_num);
+	RTL_W16(tp, Q_NUM_CTRL_8125, q_ctrl);
+}
+
 static void rtl8169_hw_enable_vec_mapping(struct rtl8169_private *tp)
 {
 	u8 tmp;
@@ -4133,6 +4289,13 @@ static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
 	    tp->mac_version == RTL_GIGA_MAC_VER_80)
 		RTL_W8(tp, 0xD8, RTL_R8(tp, 0xD8) & ~0x02);
 
+	/* enable rx descriptor type v4 and set queue num for rss */
+	if (tp->num_rx_rings > 1) {
+		rtl8125_set_rx_q_num(tp);
+		RTL_W8(tp, RTL_DESC_TYPE_CTRL,
+		       RTL_R8(tp, RTL_DESC_TYPE_CTRL) | RTL_DESC_TYPE_RSS);
+	}
+
 	if (tp->mac_version == RTL_GIGA_MAC_VER_80)
 		r8168_mac_ocp_modify(tp, 0xe614, 0x0f00, 0x0f00);
 	else if (tp->mac_version == RTL_GIGA_MAC_VER_70)
@@ -4369,6 +4532,12 @@ static void rtl_hw_start(struct  rtl8169_private *tp)
 	rtl_hw_aspm_clkreq_enable(tp, true);
 	rtl_set_rx_max_size(tp);
 	rtl_set_rx_tx_desc_registers(tp);
+	if (rtl_is_8125(tp)) {
+		if (tp->num_rx_rings > 1)
+			rtl_set_rss_config(tp);
+		else
+			RTL_W32(tp, RSS_CTRL_8125, 0x00);
+	}
 	rtl_lock_config_regs(tp);
 
 	rtl_jumbo_config(tp);
@@ -4407,14 +4576,26 @@ static int rtl8169_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
-static void rtl8169_mark_to_asic(struct RxDesc *desc)
+static void rtl8169_mark_to_asic(struct rtl8169_private *tp, struct RxDesc *desc)
 {
-	u32 eor = le32_to_cpu(desc->opts1) & RingEnd;
+	u32 eor;
 
-	desc->opts2 = 0;
-	/* Force memory writes to complete before releasing descriptor */
-	dma_wmb();
-	WRITE_ONCE(desc->opts1, cpu_to_le32(DescOwn | eor | R8169_RX_BUF_SIZE));
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		eor = le32_to_cpu(desc->rss_opts1) & RingEnd;
+		desc->rss_opts2 = cpu_to_le32(0);
+		/* Force memory writes to complete before releasing descriptor */
+		dma_wmb();
+		WRITE_ONCE(desc->rss_opts1, cpu_to_le32(DescOwn | eor | R8169_RX_BUF_SIZE));
+		break;
+	default:
+		eor = le32_to_cpu(desc->opts1) & RingEnd;
+		desc->opts2 = cpu_to_le32(0);
+		/* Force memory writes to complete before releasing descriptor */
+		dma_wmb();
+		WRITE_ONCE(desc->opts1, cpu_to_le32(DescOwn | eor | R8169_RX_BUF_SIZE));
+		break;
+	}
 }
 
 static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
@@ -4437,9 +4618,12 @@ static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
 		return NULL;
 	}
 
-	desc->addr = cpu_to_le64(mapping);
 	ring->rx_desc_phy_addr[index] = mapping;
-	rtl8169_mark_to_asic(desc);
+	if (tp->init_rx_desc_type == RX_DESC_TYPE_RSS)
+		desc->rss_addr = cpu_to_le64(mapping);
+	else
+		desc->addr = cpu_to_le64(mapping);
+	rtl8169_mark_to_asic(tp, desc);
 
 	return data;
 }
@@ -4456,8 +4640,25 @@ static void rtl8169_rx_clear(struct rtl8169_private *tp,
 		__free_pages(ring->rx_databuff[i], get_order(R8169_RX_BUF_SIZE));
 		ring->rx_databuff[i] = NULL;
 		ring->rx_desc_phy_addr[i] = 0;
-		ring->rx_desc_array[i].addr = 0;
-		ring->rx_desc_array[i].opts1 = 0;
+		if (tp->init_rx_desc_type == RX_DESC_TYPE_RSS) {
+			ring->rx_desc_array[i].rss_addr = 0;
+			ring->rx_desc_array[i].rss_opts1 = 0;
+		} else {
+			ring->rx_desc_array[i].addr = 0;
+			ring->rx_desc_array[i].opts1 = 0;
+		}
+	}
+}
+
+static void rtl8169_mark_as_last_descriptor(struct rtl8169_private *tp, struct RxDesc *desc)
+{
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		desc->rss_opts1 |= cpu_to_le32(RingEnd);
+		break;
+	default:
+		desc->opts1 |= cpu_to_le32(RingEnd);
+		break;
 	}
 }
 
@@ -4477,7 +4678,7 @@ static int rtl8169_rx_fill(struct rtl8169_private *tp, struct rtl8169_rx_ring *r
 	}
 
 	/* mark as last descriptor in the ring */
-	ring->rx_desc_array[NUM_RX_DESC - 1].opts1 |= cpu_to_le32(RingEnd);
+	rtl8169_mark_as_last_descriptor(tp, &ring->rx_desc_array[NUM_RX_DESC - 1]);
 
 	return 0;
 }
@@ -4636,8 +4837,13 @@ static void rtl8169_rx_desc_reset(struct rtl8169_private *tp)
 	for (int i = 0; i < tp->num_rx_rings; i++) {
 		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
 
-		for (int j = 0; j < NUM_RX_DESC; j++)
-			rtl8169_mark_to_asic(ring->rx_desc_array + j);
+		for (int j = 0; j < NUM_RX_DESC; j++) {
+			dma_addr_t phy_addr = ring->rx_desc_phy_addr[j];
+
+			if (tp->init_rx_desc_type == RX_DESC_TYPE_RSS)
+				ring->rx_desc_array[j].rss_addr = cpu_to_le64(phy_addr);
+			rtl8169_mark_to_asic(tp, ring->rx_desc_array + j);
+		}
 	}
 }
 
@@ -5093,28 +5299,91 @@ static inline int rtl8169_fragmented_frame(u32 status)
 	return (status & (FirstFrag | LastFrag)) != (FirstFrag | LastFrag);
 }
 
-static inline void rtl8169_rx_csum(struct sk_buff *skb,
-				   u32 opts1)
+static void rtl8169_rx_hash(struct rtl8169_private *tp,
+			    struct RxDesc *desc,
+			    struct sk_buff *skb)
 {
-	u32 status = opts1 & (RxProtoMask | RxCSFailMask);
+	u32 rss_header_info;
+	u32 hash_val;
+
+	if (!(tp->dev->features & NETIF_F_RXHASH))
+		return;
+
+	rss_header_info = le32_to_cpu(desc->rss_dword.rss_info);
+
+	if (!(rss_header_info & RXS_RSS_L3_TYPE_MASK))
+		return;
 
-	if (status == RxProtoTCP || status == RxProtoUDP)
+	hash_val = le32_to_cpu(desc->rss_dword.rss_result);
+
+	skb_set_hash(skb, hash_val,
+		     (RXS_RSS_L4_TYPE_MASK & rss_header_info) ?
+		     PKT_HASH_TYPE_L4 : PKT_HASH_TYPE_L3);
+}
+
+static void rtl8169_rx_csum(struct rtl8169_private *tp,
+			    struct sk_buff *skb,
+			    u32 opts1)
+{
+	bool csum_ok = false;
+
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		if (((opts1 & RX_TCPT_DESC_RSS) && !(opts1 & RX_TCPF_DESC_RSS)) ||
+		    ((opts1 & RX_UDPT_DESC_RSS) && !(opts1 & RX_UDPF_DESC_RSS)))
+			csum_ok = true;
+		break;
+	default: {
+		u32 status = opts1 & (RxProtoMask | RxCSFailMask);
+
+		if (status == RxProtoTCP || status == RxProtoUDP)
+			csum_ok = true;
+		break;
+	}
+	}
+
+	if (csum_ok)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 	else
 		skb_checksum_none_assert(skb);
 }
 
+static __le32 rtl8169_rx_desc_opts1(struct rtl8169_private *tp, struct RxDesc *desc)
+{
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		return READ_ONCE(desc->rss_opts1);
+	default:
+		return READ_ONCE(desc->opts1);
+	}
+}
+
 static bool rtl8169_check_rx_desc_error(struct rtl8169_rx_ring *ring,
+					struct rtl8169_private *tp,
 					u32 status)
 {
-	if (unlikely(status & RxRES)) {
-		u64_stats_update_begin(&ring->stats.syncp);
-		if (status & (RxRWT | RxRUNT))
-			ring->stats.rx_length_errors++;
-		if (status & RxCRC)
-			ring->stats.rx_crc_errors++;
-		u64_stats_update_end(&ring->stats.syncp);
-		return true;
+	switch (tp->init_rx_desc_type) {
+	case RX_DESC_TYPE_RSS:
+		if (unlikely(status & RX_RES_RSS)) {
+			u64_stats_update_begin(&ring->stats.syncp);
+			if (status & RXRUNT_RSS)
+				ring->stats.rx_length_errors++;
+			if (status & RXCRC_RSS)
+				ring->stats.rx_crc_errors++;
+			u64_stats_update_end(&ring->stats.syncp);
+			return true;
+		}
+		break;
+	default:
+		if (unlikely(status & RxRES)) {
+			u64_stats_update_begin(&ring->stats.syncp);
+			if (status & (RxRWT | RxRUNT))
+				ring->stats.rx_length_errors++;
+			if (status & RxCRC)
+				ring->stats.rx_crc_errors++;
+			u64_stats_update_end(&ring->stats.syncp);
+			return true;
+		}
 	}
 	return false;
 }
@@ -5134,7 +5403,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		dma_addr_t addr;
 		u32 status;
 
-		status = le32_to_cpu(READ_ONCE(desc->opts1));
+		status = le32_to_cpu(rtl8169_rx_desc_opts1(tp, desc));
 		if (status & DescOwn)
 			break;
 
@@ -5144,7 +5413,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		 */
 		dma_rmb();
 
-		if (rtl8169_check_rx_desc_error(ring, status)) {
+		if (rtl8169_check_rx_desc_error(ring, tp, status)) {
 			if (net_ratelimit())
 				netdev_warn(dev, "Rx ERROR. status = %08x\n",
 					    status);
@@ -5154,11 +5423,21 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 
 			if (!(dev->features & NETIF_F_RXALL))
 				goto release_descriptor;
-			else if (status & RxRWT || !(status & (RxRUNT | RxCRC)))
-				goto release_descriptor;
+
+			if (tp->init_rx_desc_type == RX_DESC_TYPE_DEFAULT) {
+				if (status & RxRWT || !(status & (RxRUNT | RxCRC)))
+					goto release_descriptor;
+			} else {
+				if (!(status & (RXRUNT_RSS | RXCRC_RSS)))
+					goto release_descriptor;
+			}
 		}
 
 		pkt_size = status & GENMASK(13, 0);
+
+		if (unlikely(pkt_size < ETH_FCS_LEN))
+			goto release_descriptor;
+
 		if (likely(!(dev->features & NETIF_F_RXFCS)))
 			pkt_size -= ETH_FCS_LEN;
 
@@ -5191,10 +5470,12 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 		skb->len = pkt_size;
 		dma_sync_single_for_device(d, addr, pkt_size, DMA_FROM_DEVICE);
 
-		rtl8169_rx_csum(skb, status);
+		if (tp->num_rx_rings > 1)
+			rtl8169_rx_hash(tp, desc, skb);
+		rtl8169_rx_csum(tp, skb, status);
 		skb->protocol = eth_type_trans(skb, dev);
 
-		rtl8169_rx_vlan_tag(desc, skb);
+		rtl8169_rx_vlan_tag(tp, desc, skb);
 
 		if (skb->pkt_type == PACKET_MULTICAST) {
 			u64_stats_update_begin(&ring->stats.syncp);
@@ -5206,7 +5487,9 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
 
 		dev_sw_netstats_rx_add(dev, pkt_size);
 release_descriptor:
-		rtl8169_mark_to_asic(desc);
+		if (tp->init_rx_desc_type == RX_DESC_TYPE_RSS)
+			desc->rss_addr = cpu_to_le64(ring->rx_desc_phy_addr[entry]);
+		rtl8169_mark_to_asic(tp, desc);
 	}
 
 	return count;
@@ -5823,6 +6106,32 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
 	}
 }
 
+static int get_max_irq_nvecs(struct rtl8169_private *tp)
+{
+	if (tp->mac_version == RTL_GIGA_MAC_VER_80)
+		return R8127_MAX_NUM_IRQVEC;
+	return R8169_IRQ_DEFAULT;
+}
+
+static int get_min_irq_nvecs(struct rtl8169_private *tp)
+{
+	if (tp->mac_version == RTL_GIGA_MAC_VER_80)
+		return R8127_MIN_NUM_IRQVEC;
+	return R8169_IRQ_DEFAULT;
+}
+
+static void rtl8169_set_rx_ring_num(struct rtl8169_private *tp)
+{
+	if (tp->irq_nvecs >= get_min_irq_nvecs(tp)) {
+		unsigned int rss_queue_num = netif_get_num_default_rss_queues();
+
+		tp->num_rx_rings = rounddown_pow_of_two(min(rss_queue_num,
+							    tp->hw_supp_num_rx_queues));
+		if (tp->num_rx_rings >= 2)
+			tp->init_rx_desc_type = RX_DESC_TYPE_RSS;
+	}
+}
+
 static int rtl_alloc_irq(struct rtl8169_private *tp)
 {
 	struct pci_dev *pdev = tp->pci_dev;
@@ -5843,7 +6152,11 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
 		break;
 	}
 
-	nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);
+	nvecs = pci_alloc_irq_vectors(pdev, get_min_irq_nvecs(tp),
+				      get_max_irq_nvecs(tp), flags);
+
+	if (nvecs < 0)
+		nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);
 
 	if (nvecs < 0)
 		return nvecs;
@@ -6517,6 +6830,13 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->dash_type = rtl_get_dash_type(tp);
 	tp->dash_enabled = rtl_dash_is_enabled(tp);
 
+	if (rtl_hw_support_rss(tp)) {
+		tp->rss_data = devm_kzalloc(&pdev->dev, sizeof(*tp->rss_data),
+					    GFP_KERNEL);
+		if (!tp->rss_data)
+			return -ENOMEM;
+	}
+
 	tp->cp_cmd = RTL_R16(tp, CPlusCmd) & CPCMD_MASK;
 
 	if (sizeof(dma_addr_t) > 4 && tp->mac_version >= RTL_GIGA_MAC_VER_18 &&
@@ -6537,6 +6857,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (rc < 0)
 		return dev_err_probe(&pdev->dev, rc, "Can't allocate interrupt\n");
 
+	rtl8169_set_rx_ring_num(tp);
+
+	if (rtl_hw_support_rss(tp))
+		rtl8169_init_rss(tp);
+
 	INIT_WORK(&tp->wk.work, rtl_task);
 	disable_work(&tp->wk.work);
 
@@ -6549,6 +6874,11 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 
+	if (rtl_hw_support_rss(tp) && tp->num_rx_rings > 1) {
+		dev->hw_features |= NETIF_F_RXHASH;
+		dev->features |= NETIF_F_RXHASH;
+	}
+
 	/*
 	 * Pretend we are using VLANs; This bypasses a nasty bug where
 	 * Interrupts stop flowing on high load on 8110SCd controllers.
-- 
2.43.0

[PATCH net-next v14 6/7] r8169: move struct ethtool_ops

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

The patch moves the rtl8169_ethtool_ops definition further down in
r8169_main.c so that subsequent additions of rtl8169_get_channels and
rtl8169_set_channels can be referenced from the ops struct without
needing forward declarations.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v2:
 - no changes

Changes in v3:
 - no changes

Changes in v4:
 - no changes

Changes in v5:
 - no changes

Changes in v6:
 - modify commit message

Changes in v7:
 - no changes

Changes in v8:
 - no changes

Changes in v9:
 - no changes

Changes in v10:
 - no changes

Changes in v11:
 - no changes

Changes in v12:
 - no changes

Changes in v13:
 - no changes

Changes in v14:
 - no changes
---
 drivers/net/ethernet/realtek/r8169_main.c | 56 +++++++++++------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index ad360f145076..3be6f6e708f2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2637,34 +2637,6 @@ static int rtl8169_nway_reset(struct net_device *dev)
 	return phylink_ethtool_nway_reset(tp->phylink);
 }
 
-static const struct ethtool_ops rtl8169_ethtool_ops = {
-	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
-				     ETHTOOL_COALESCE_MAX_FRAMES,
-	.get_drvinfo		= rtl8169_get_drvinfo,
-	.get_regs_len		= rtl8169_get_regs_len,
-	.get_link		= ethtool_op_get_link,
-	.get_coalesce		= rtl_get_coalesce,
-	.set_coalesce		= rtl_set_coalesce,
-	.get_regs		= rtl8169_get_regs,
-	.get_wol		= rtl8169_get_wol,
-	.set_wol		= rtl8169_set_wol,
-	.get_strings		= rtl8169_get_strings,
-	.get_sset_count		= rtl8169_get_sset_count,
-	.get_ethtool_stats	= rtl8169_get_ethtool_stats,
-	.get_ts_info		= ethtool_op_get_ts_info,
-	.nway_reset		= rtl8169_nway_reset,
-	.get_eee		= rtl8169_get_eee,
-	.set_eee		= rtl8169_set_eee,
-	.get_link_ksettings	= rtl8169_get_link_ksettings,
-	.set_link_ksettings	= rtl8169_set_link_ksettings,
-	.get_ringparam		= rtl8169_get_ringparam,
-	.get_pause_stats	= rtl8169_get_pause_stats,
-	.get_pauseparam		= rtl8169_get_pauseparam,
-	.set_pauseparam		= rtl8169_set_pauseparam,
-	.get_eth_mac_stats	= rtl8169_get_eth_mac_stats,
-	.get_eth_ctrl_stats	= rtl8169_get_eth_ctrl_stats,
-};
-
 static const struct rtl_chip_info *rtl8169_get_chip_version(u32 xid, bool gmii)
 {
 	/* Chips combining a 1Gbps MAC with a 100Mbps PHY */
@@ -6741,6 +6713,34 @@ static void r8169_init_napi(struct rtl8169_private *tp)
 	}
 }
 
+static const struct ethtool_ops rtl8169_ethtool_ops = {
+	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
+				     ETHTOOL_COALESCE_MAX_FRAMES,
+	.get_drvinfo		= rtl8169_get_drvinfo,
+	.get_regs_len		= rtl8169_get_regs_len,
+	.get_link		= ethtool_op_get_link,
+	.get_coalesce		= rtl_get_coalesce,
+	.set_coalesce		= rtl_set_coalesce,
+	.get_regs		= rtl8169_get_regs,
+	.get_wol		= rtl8169_get_wol,
+	.set_wol		= rtl8169_set_wol,
+	.get_strings		= rtl8169_get_strings,
+	.get_sset_count		= rtl8169_get_sset_count,
+	.get_ethtool_stats	= rtl8169_get_ethtool_stats,
+	.get_ts_info		= ethtool_op_get_ts_info,
+	.nway_reset		= rtl8169_nway_reset,
+	.get_eee		= rtl8169_get_eee,
+	.set_eee		= rtl8169_set_eee,
+	.get_link_ksettings	= rtl8169_get_link_ksettings,
+	.set_link_ksettings	= rtl8169_set_link_ksettings,
+	.get_ringparam		= rtl8169_get_ringparam,
+	.get_pause_stats	= rtl8169_get_pause_stats,
+	.get_pauseparam		= rtl8169_get_pauseparam,
+	.set_pauseparam		= rtl8169_set_pauseparam,
+	.get_eth_mac_stats	= rtl8169_get_eth_mac_stats,
+	.get_eth_ctrl_stats	= rtl8169_get_eth_ctrl_stats,
+};
+
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	const struct rtl_chip_info *chip;
-- 
2.43.0

[PATCH net-next v14 7/7] r8169: add get_channel support for ethtool

[javen]

From: Javen Xu <javen_xu@realsil.com.cn>

Add get_channel support for ethtool.

Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>
---
Changes in v11:
 - new file, no change

Changes in v12:
 - no changes

Changes in v13:
 - no changes

Changes in v14:
 - no changes
---
 drivers/net/ethernet/realtek/r8169_main.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 3be6f6e708f2..7a61c2ecc7d1 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -6713,6 +6713,22 @@ static void r8169_init_napi(struct rtl8169_private *tp)
 	}
 }
 
+static void rtl8169_get_channels(struct net_device *dev,
+				 struct ethtool_channels *ch)
+{
+	struct rtl8169_private *tp = netdev_priv(dev);
+
+	if (tp->irq_nvecs > 1) {
+		ch->max_rx = tp->hw_supp_num_rx_queues;
+		ch->max_tx = 1;
+		ch->rx_count = tp->num_rx_rings;
+		ch->tx_count = 1;
+	} else {
+		ch->max_combined = 1;
+		ch->combined_count = 1;
+	}
+}
+
 static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
@@ -6731,6 +6747,7 @@ static const struct ethtool_ops rtl8169_ethtool_ops = {
 	.nway_reset		= rtl8169_nway_reset,
 	.get_eee		= rtl8169_get_eee,
 	.set_eee		= rtl8169_set_eee,
+	.get_channels		= rtl8169_get_channels,
 	.get_link_ksettings	= rtl8169_get_link_ksettings,
 	.set_link_ksettings	= rtl8169_set_link_ksettings,
 	.get_ringparam		= rtl8169_get_ringparam,
-- 
2.43.0

Re: [PATCH net-next v14 1/7] r8169: prepare for multi irqs and napi

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Commit message claims "there is no functional change", but the
  patch introduces two user-visible changes in…

--

r8169: prepare for multi irqs and napi

This converts the single struct napi_struct in struct rtl8169_private into a
dynamically allocated array sized by the number of IRQ vectors, and moves
IRQ request/free into helpers that iterate over that array.  irq_nvecs and
num_rx_rings stay at 1, so this is described as a preparation step for RSS.

>     Currently, irq_nvecs and num_rx_rings are still hard-coded
>     to 1, so there is no functional change.

[Severity: Low]
Is the "no functional change" statement accurate?  There look to be two
observable changes for the currently supported single-vector case.

The first is the probe banner in rtl_init_one(), which gains a field that
always reads "(1 total)" while irq_nvecs is hard-coded to 1.

The second is the new netif_napi_set_irq() call in r8169_init_napi().
netif_napi_add_weight_locked() leaves napi->irq at -1, and
netdev_nl_napi_fill_one() only emits the attribute when it is non-negative:

net/core/netdev-genl.c:netdev_nl_napi_fill_one() {
	...
	if (napi->irq >= 0 && nla_put_u32(rsp, NETDEV_A_NAPI_IRQ, napi->irq))
		goto nla_put_failure;
	...
}

So NETDEV_A_NAPI_IRQ goes from absent to present in NETDEV_CMD_NAPI_GET
replies for this driver.

Could the commit message mention both, or could the banner change be moved
to the patch that actually enables more than one vector?

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 5415ff62a286..a8214c48a775 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c

[ ... ]

> @@ -6005,6 +6087,15 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
>  	return 0;
>  }
>  
> +static void r8169_init_napi(struct rtl8169_private *tp)
> +{
> +	for (int i = 0; i < tp->irq_nvecs; i++) {
> +		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], rtl8169_poll);
> +		netif_napi_set_irq(&tp->rtl8169_napi[i],
> +				   pci_irq_vector(tp->pci_dev, i));
> +	}
> +}
> +

[ ... ]

> @@ -6215,8 +6317,9 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  			tp->leds = rtl8168_init_leds(dev);
>  	}
>  
> -	netdev_info(dev, "%s, %pM, %sXID %x, IRQ %d\n",
> -		    chip->name, dev->dev_addr, ext_xid_str, xid, tp->irq);
> +	netdev_info(dev, "%s, %pM, %sXID %x, IRQ %d (%u total)\n",
> +		    chip->name, dev->dev_addr, ext_xid_str, xid,
> +		    pci_irq_vector(pdev, 0), tp->irq_nvecs);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 2/7] r8169: refactor RX path to prepare for multi-queue

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Queue-count contract mismatch: rtl_init_one() allocates the
  netdev with a hard-coded 8 RX queues (devm_alloc_etherdev_mqs(...,…
- [Low] The patch adds a new 2 KB-per-ring shadow array `dma_addr_t
  rx_desc_phy_addr[NUM_RX_DESC]` to struct rtl8169_rx_ring and switches…
- [Low] Unrelated churn in a refactor-only patch: rtl8169_rx_csum()'s
  prototype is re-wrapped across two lines with byte-identical…

Pre-existing issues:
- [Low] In rtl_rx(), `pkt_size = status & GENMASK(13, 0)` followed by an
  unguarded `pkt_size -= ETH_FCS_LEN` (when NETIF_F_RXFCS is clear) can…

--

r8169: refactor RX path to prepare for multi-queue

This patch introduces struct rtl8169_rx_ring and moves the RX ring state
out of rtl8169_private into a per-queue array, switching the netdev
allocation to devm_alloc_etherdev_mqs() with room for 8 RX queues.  The
number of active RX rings stays at 1 for now.

A few questions below.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index a8214c48a775..7a2e7ce56e48 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -74,9 +74,13 @@
>  #define NUM_TX_DESC	256	/* Number of Tx descriptor registers */
>  #define NUM_RX_DESC	256	/* Number of Rx descriptor registers */
>  #define R8169_TX_RING_BYTES	(NUM_TX_DESC * sizeof(struct TxDesc))
> +
>  #define R8169_RX_RING_BYTES	(NUM_RX_DESC * sizeof(struct RxDesc))
>  #define R8169_TX_STOP_THRS	(MAX_SKB_FRAGS + 1)
>  #define R8169_TX_START_THRS	(2 * R8169_TX_STOP_THRS)
> +#define R8169_MAX_RX_QUEUES	8
> +#define R8169_DEFAULT_RX_QUEUES	1
> +#define R8169_MAX_TX_QUEUES	1
>  
>  #define OCP_STD_PHY_BASE	0xa400
>  

[ ... ]

> @@ -750,6 +755,14 @@ enum rtl_dash_type {
>  	RTL_DASH_25_BP,
>  };
>  
> +struct rtl8169_rx_ring {
> +	u32 cur_rx;
> +	struct RxDesc *rx_desc_array;
> +	dma_addr_t rx_desc_phy_addr[NUM_RX_DESC];
> +	dma_addr_t rx_phy_addr;
> +	struct page *rx_databuff[NUM_RX_DESC];
> +};
> +

[Severity: Low]
The commit message describes this as turning "the previously embedded RX
state in rtl8169_private into a per-queue array", but rx_desc_phy_addr[]
is not moved state, it is new state (2 KB per ring on 64-bit), and it
comes with a new invariant: rtl8169_alloc_rx_data() writes it next to
desc->addr, rtl8169_rx_clear() unmaps from it and then zeroes it, and
rtl_rx() now trusts it instead of reading the descriptor back.

The reason for no longer reading the address out of the descriptor
appears only in the v8 changelog below the "---" line, which is not kept
in git history.

Could the commit message gain a sentence describing the new shadow array
and why the descriptor read-back is dropped?

>  enum rtl_sfp_mode {
>  	RTL_SFP_NONE,
>  	RTL_SFP_8168_AF,
> @@ -764,20 +777,18 @@ struct rtl8169_private {
>  	enum mac_version mac_version;
>  	enum rtl_dash_type dash_type;
>  	enum rtl_sfp_mode sfp_mode;
> -	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
>  	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
>  	u32 dirty_tx;
>  	struct TxDesc *TxDescArray;	/* 256-aligned Tx descriptor ring */
> -	struct RxDesc *RxDescArray;	/* 256-aligned Rx descriptor ring */
>  	dma_addr_t TxPhyAddr;
> -	dma_addr_t RxPhyAddr;
> -	struct page *Rx_databuff[NUM_RX_DESC];	/* Rx data buffers */
>  	struct ring_info tx_skb[NUM_TX_DESC];	/* Tx data buffers */
>  	struct napi_struct *rtl8169_napi;
> +	struct rtl8169_rx_ring *rx_ring;
>  	unsigned int num_rx_rings;
>  	u16 cp_cmd;
>  	u16 tx_lpi_timer;
>  	u32 irq_mask;
> +	unsigned int hw_supp_num_rx_queues;
>  	unsigned int irq_nvecs;
>  	struct clk *clk;
>  	int speed;

[ ... ]

> @@ -2773,6 +2800,14 @@ static void rtl_hw_reset(struct rtl8169_private *tp)
>  static void rtl_setup_rx_params(struct rtl8169_private *tp)
>  {
>  	tp->num_rx_rings = 1;
> +	switch (tp->mac_version) {
> +	case RTL_GIGA_MAC_VER_80:
> +		tp->hw_supp_num_rx_queues = R8169_MAX_RX_QUEUES;
> +		break;
> +	default:
> +		tp->hw_supp_num_rx_queues = R8169_DEFAULT_RX_QUEUES;
> +		break;
> +	}
>  }

[ ... ]

> @@ -4941,7 +5048,8 @@ static inline int rtl8169_fragmented_frame(u32 status)
>  	return (status & (FirstFrag | LastFrag)) != (FirstFrag | LastFrag);
>  }
>  
> -static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
> +static inline void rtl8169_rx_csum(struct sk_buff *skb,
> +				   u32 opts1)
>  {
>  	u32 status = opts1 & (RxProtoMask | RxCSFailMask);
>  
> @@ -4951,15 +5059,30 @@ static inline void rtl8169_rx_csum(struct sk_buff *skb, u32 opts1)
>  		skb_checksum_none_assert(skb);
>  }
>  
> +static bool rtl8169_check_rx_desc_error(struct net_device *dev,
> +					struct rtl8169_private *tp,
> +					u32 status)
> +{
> +	if (unlikely(status & RxRES)) {
> +		if (status & (RxRWT | RxRUNT))
> +			dev->stats.rx_length_errors++;
> +		if (status & RxCRC)
> +			dev->stats.rx_crc_errors++;
> +		return true;
> +	}
> +	return false;
> +}
> +

[Severity: Low]
These aren't bugs, but two bits of churn here look unrelated to the
per-queue RX refactor.

The rtl8169_rx_csum() prototype is re-wrapped onto two lines with the
same two parameters, so it is a whitespace-only change.  Could it be
folded into the later patch that actually adds a parameter to it?

rtl8169_check_rx_desc_error() takes a struct rtl8169_private *tp that is
never dereferenced in its body at this commit.  Would it be cleaner to
add the tp argument in the patch that starts using it?

>  static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
> -		  int budget, struct napi_struct *napi)
> +		  struct rtl8169_rx_ring *ring, int budget,
> +		  struct napi_struct *napi)
>  {
>  	struct device *d = tp_to_dev(tp);
>  	int count;
>  
> -	for (count = 0; count < budget; count++, tp->cur_rx++) {
> -		unsigned int pkt_size, entry = tp->cur_rx % NUM_RX_DESC;
> -		struct RxDesc *desc = tp->RxDescArray + entry;
> +	for (count = 0; count < budget; count++, ring->cur_rx++) {
> +		unsigned int pkt_size, entry = ring->cur_rx % NUM_RX_DESC;
> +		struct RxDesc *desc = ring->rx_desc_array + entry;
>  		struct sk_buff *skb;
>  		const void *rx_buf;
>  		dma_addr_t addr;
> @@ -4975,15 +5098,11 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  		 */
>  		dma_rmb();
>  
> -		if (unlikely(status & RxRES)) {
> +		if (rtl8169_check_rx_desc_error(dev, tp, status)) {
>  			if (net_ratelimit())
>  				netdev_warn(dev, "Rx ERROR. status = %08x\n",
>  					    status);
>  			dev->stats.rx_errors++;
> -			if (status & (RxRWT | RxRUNT))
> -				dev->stats.rx_length_errors++;
> -			if (status & RxCRC)
> -				dev->stats.rx_crc_errors++;
>  
>  			if (!(dev->features & NETIF_F_RXALL))
>  				goto release_descriptor;

[Severity: Low]
This is a pre-existing issue and was not introduced by this patch, but
since the surrounding error filter is being touched here it may be worth
a look.  With NETIF_F_RXALL set, a short descriptor can reach the length
computation just below:

	pkt_size = status & GENMASK(13, 0);
	if (likely(!(dev->features & NETIF_F_RXFCS)))
		pkt_size -= ETH_FCS_LEN;

Can pkt_size wrap here when the descriptor reports fewer than
ETH_FCS_LEN bytes?  pkt_size is unsigned, so the subtraction would leave
a huge value that is then handed to napi_alloc_skb(),
dma_sync_single_for_cpu() and skb_copy_to_linear_data().

The same code is present in the pre-series baseline, and a guard

	if (unlikely(pkt_size < ETH_FCS_LEN))
		goto release_descriptor;

does appear later in this series in "r8169: add support and enable rss".
Would it make sense to move that guard into a separate fix ahead of the
refactor?

> @@ -5010,8 +5129,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  			goto release_descriptor;
>  		}
>  
> -		addr = le64_to_cpu(desc->addr);
> -		rx_buf = page_address(tp->Rx_databuff[entry]);
> +		addr = ring->rx_desc_phy_addr[entry];
> +		rx_buf = page_address(ring->rx_databuff[entry]);
>  
>  		dma_sync_single_for_cpu(d, addr, pkt_size, DMA_FROM_DEVICE);
>  		prefetch(rx_buf);

[ ... ]

> @@ -6106,7 +6222,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	u32 txconfig;
>  	u32 xid;
>  
> -	dev = devm_alloc_etherdev(&pdev->dev, sizeof (*tp));
> +	dev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(*tp),
> +				      R8169_MAX_TX_QUEUES,
> +				      R8169_MAX_RX_QUEUES);
> +
>  	if (!dev)
>  		return -ENOMEM;
>  

[Severity: Low]
There are three different RX queue counts in play after this patch, and
they do not agree.  Is that intentional?

The netdev is allocated with R8169_MAX_RX_QUEUES (8) struct
netdev_rx_queue slots for every supported chip, including the ones for
which rtl_setup_rx_params() then sets hw_supp_num_rx_queues to
R8169_DEFAULT_RX_QUEUES (1).

hw_supp_num_rx_queues itself has no reader in this patch; it is only
written in rtl_setup_rx_params().  Its first users
(rtl8169_set_rx_ring_num() and rtl8169_get_channels()) arrive in later
patches of the series.

The user-visible count is clamped back down later in rtl_init_one():

	rc = netif_set_real_num_queues(tp->dev, 1, tp->num_rx_rings);

so only rx-0 is exposed.  Since tp->mac_version is not known until the
MMIO BAR is mapped, allocating for the hardware maximum looks like the
only possible ordering; would a short comment at the
devm_alloc_etherdev_mqs() call saying so help?

> @@ -6295,20 +6414,25 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  		}
>  	}
>  
> +	tp->rx_ring = kcalloc(tp->num_rx_rings, sizeof(struct rtl8169_rx_ring),
> +			      GFP_KERNEL);
> +	if (!tp->rx_ring) {
> +		rc = -ENOMEM;
> +		goto err_destory_phylink;
> +	}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 3/7] r8169: add support for new interrupt mapping

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] DISCARDED (series-resolved): dead code — every new `tp->irq_nvecs
  > 1` branch added by this patch is unreachable at this commit because…
- [Low] DISCARDED (series-resolved): claim that a partial MSI-X grant
  with 2 <= irq_nvecs <= 29 arms hardware sources (TOK bit 8, LINKCHG…
- [Low] rtl8169_poll_msix_tx() calls napi_complete_done()
  unconditionally, including when the core invokes the poll callback…
- [Low] The new Tx-only NAPI instance (index R8169_MAX_RX_QUEUES, poll
  routine rtl8169_poll_msix_tx()) is registered with netif_napi_add()…
- [Low] In multi-vector mode rtl8169_netpoll() invokes
  rtl8169_interrupt_msix() for every data-path NAPI slot…

--

r8169: add support for new interrupt mapping

This patch adds the new per-vector interrupt mapping registers
(IMR_CLEAR_VEC_MAP_REG, ISR_VEC_MAP_REG, IMR_SET_VEC_MAP_REG) and new
MSI-X handlers plus per-vector Rx/Tx NAPI poll routines. The old
single-vector paths are kept for the tp->irq_nvecs <= 1 case. A few
questions below about the new poll and netpoll paths.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 7a2e7ce56e48..c647b4327ff7 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c

[ ... ]

> @@ -5396,9 +5491,16 @@ static int rtl8169_close(struct net_device *dev)
>  static void rtl8169_netpoll(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
> +	int napi_num;
> +
> +	napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
>  
> -	for (int i = 0; i < tp->irq_nvecs; i++)
> -		rtl8169_interrupt(0, &tp->rtl8169_napi[i]);
> +	for (int i = 0; i < napi_num; i++) {
> +		if (tp->irq_nvecs > 1)
> +			rtl8169_interrupt_msix(0, &tp->rtl8169_napi[i]);
> +		else
> +			rtl8169_interrupt(0, &tp->rtl8169_napi[i]);
> +	}
>  }

[Severity: Low]
Should this loop be bounded by tp->num_rx_rings rather than by
R8169_MAX_RX_QUEUES + 1?

In multi-vector mode this walks all nine data-path slots regardless of how
many Rx rings are actually configured. For a slot whose index is >=
tp->num_rx_rings, rtl8169_poll_msix_rx() skips rtl_rx() entirely and then
still re-arms the vector:

	if (message_id < tp->num_rx_rings)
		work_done += rtl_rx(dev, tp, &tp->rx_ring[message_id],
				    budget, napi);

	if (work_done < budget && napi_complete_done(napi, work_done))
		rtl8169_enable_hw_interrupt_msix(tp, message_id);

rtl_set_irq_mask() deliberately only sets the ROK bits for the active rings:

	for (int i = 0; i < tp->num_rx_rings; i++)
		tp->irq_mask |= ISRIMR_ROK_Q0 << i;

so after one netpoll pass, does the hardware mask in IMR_SET_VEC_MAP_REG end
up containing ROK bits that tp->irq_mask excludes? tp->num_rx_rings is
normally smaller than 8 (it is derived from
netif_get_num_default_rss_queues() rounded down to a power of two), while
nine NAPI instances are always created in multi-vector mode, so slots 1..8
can be inactive on a typical machine.

> @@ -5654,10 +5758,16 @@ static const struct net_device_ops rtl_netdev_ops = {
>  
>  static void rtl_set_irq_mask(struct rtl8169_private *tp)
>  {
> -	tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
> +	if (tp->irq_nvecs > 1) {
> +		tp->irq_mask = ISRIMR_LINKCHG | ISRIMR_TOK_Q0;
> +		for (int i = 0; i < tp->num_rx_rings; i++)
> +			tp->irq_mask |= ISRIMR_ROK_Q0 << i;
> +	} else {
> +		tp->irq_mask = RxOK | RxErr | TxOK | TxErr | LinkChg;
>  
> -	if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
> -		tp->irq_mask |= SYSErr | RxFIFOOver;
> +		if (tp->mac_version <= RTL_GIGA_MAC_VER_06)
> +			tp->irq_mask |= SYSErr | RxFIFOOver;
> +	}
>  }

[ ... ]

> @@ -6203,10 +6313,63 @@ static int rtl_init_phylink(struct rtl8169_private *tp)
>  	return 0;
>  }
>  
> +static int rtl8169_poll_msix_rx(struct napi_struct *napi, int budget)
> +{

[ ... ]

> +static int rtl8169_poll_msix_tx(struct napi_struct *napi, int budget)
> +{
> +	struct net_device *dev = napi->dev;
> +	struct rtl8169_private *tp;
> +
> +	tp = netdev_priv(dev);
> +
> +	/* Currently r8169 only supports a single Tx ring.
> +	 * Therefore, we don't need a per-ring Tx processing loop here.
> +	 */
> +	rtl_tx(dev, tp, budget);
> +
> +	if (napi_complete_done(napi, 0))
> +		rtl8169_enable_hw_interrupt_msix(tp, (int)(napi - tp->rtl8169_napi));
> +
> +	return 0;
> +}

[Severity: Low]
Should this call to napi_complete_done() be skipped when budget is 0?

Documentation/networking/napi.rst states:

   If the ``budget`` is 0 napi_complete_done() should never be called.

and netpoll does invoke the poll callback with a zero budget:

net/core/netpoll.c:poll_one_napi() {
	/* We explicitly pass the polling call a budget of 0 to
	 * indicate that we are clearing the Tx path only.
	 */
	work = napi->poll(napi, 0);
}

Today the effect appears to be absorbed by napi_complete_done() itself,
which returns early when NAPI_STATE_NPSVC is set (the flag poll_one_napi()
sets before the zero-budget call), so the Tx vector is not re-armed behind
the core's back. Would the documented form be preferable here:

	if (budget && napi_complete_done(napi, 0))
		rtl8169_enable_hw_interrupt_msix(tp, ...);

> +/* RTL8127 MSI-X vector layout:
> + * Vectors 0 .. (MAX_RXQ - 1)			: Rx Queues
> + * Vectors MAX_RXQ .. (MAX_RXQ + MAX_TXQ - 1)	: Tx Queues
> + * NAPI is only allocated for data path
> + */
>  static void r8169_init_napi(struct rtl8169_private *tp)
>  {
> -	for (int i = 0; i < tp->irq_nvecs; i++) {
> -		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], rtl8169_poll);
> +	int napi_num = min(tp->irq_nvecs, R8169_MAX_RX_QUEUES + 1);
> +
> +	for (int i = 0; i < napi_num; i++) {
> +		int (*poll_fn)(struct napi_struct *, int) = rtl8169_poll;
> +
> +		if (tp->irq_nvecs > 1) {
> +			if (i < R8169_MAX_RX_QUEUES)
> +				poll_fn = rtl8169_poll_msix_rx;
> +			else
> +				poll_fn = rtl8169_poll_msix_tx;
> +		}
> +		netif_napi_add(tp->dev, &tp->rtl8169_napi[i], poll_fn);

[Severity: Low]
Would netif_napi_add_tx() be the right registration helper for the slot at
index R8169_MAX_RX_QUEUES, since rtl8169_poll_msix_tx() only calls rtl_tx()
and never touches an Rx ring?

The kernel-doc for netif_napi_add_tx() in include/linux/netdevice.h says it
"should be used from drivers using NAPI to exclusively poll a TX queue", and
netif_napi_add_tx_weight() additionally does:

	set_bit(NAPI_STATE_NO_BUSY_POLL, &napi->state);

With plain netif_napi_add(), does this Tx-only instance end up in napi_hash[]
and advertised as busy-poll capable?

>  		netif_napi_set_irq(&tp->rtl8169_napi[i],
>  				   pci_irq_vector(tp->pci_dev, i));
>  	}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 4/7] r8169: enable new interrupt mapping

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] In `rtl_hw_start_8125_common()`
  (drivers/net/ethernet/realtek/r8169_main.c:4114) the patch adds `if…
- [Low] The second half of this patch converts RX error/drop/multicast
  accounting in `rtl_rx()` and `rtl8169_check_rx_desc_error()` from…

Pre-existing issues:
- [Medium] In `rtl_rx()` (drivers/net/ethernet/realtek/r8169_main.c), the
  fragmented-frame path increments the new per-ring `rx_dropped` and…

--

r8169: enable new interrupt mapping

This patch adds rtl8169_hw_enable_vec_mapping() and calls it from
rtl_hw_start_8125_common() when more than one IRQ vector is in use.  It
also moves the RX error, drop and multicast accounting from dev->stats
into new per-ring u64_stats counters aggregated in rtl8169_get_stats64().

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index c647b4327ff7..ec3643892fe2 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c

[ ... ]

> @@ -768,6 +769,15 @@ struct rtl8169_rx_ring {
>  	dma_addr_t rx_desc_phy_addr[NUM_RX_DESC];
>  	dma_addr_t rx_phy_addr;
>  	struct page *rx_databuff[NUM_RX_DESC];
> +
> +	struct {
> +		u64 rx_errors;
> +		u64 rx_dropped;
> +		u64 rx_length_errors;
> +		u64 rx_crc_errors;
> +		u64 multicast;
> +		struct u64_stats_sync syncp;
> +	} stats;
>  };

[Severity: Low]
This isn't a bug, but does the commit message match what this half of the
patch does?  The message says "add error pkts counter per ring", while
rx_dropped and multicast are not error counters.

Also, rtl_setup_rx_params() still does:

	tp->num_rx_rings = 1;

so the per-ring aspect has no effect until "r8169: add support and enable
rss" raises num_rx_rings.  Would it be clearer to split this accounting
migration into its own patch, or at least describe the full set of
counters and the reason for moving them off dev->stats?

[ ... ]

> @@ -4084,6 +4094,15 @@ DECLARE_RTL_COND(rtl_mac_ocp_e00e_cond)
>  	return r8168_mac_ocp_read(tp, 0xe00e) & BIT(13);
>  }
>  
> +static void rtl8169_hw_enable_vec_mapping(struct rtl8169_private *tp)
> +{
> +	u8 tmp;
> +
> +	tmp = RTL_R8(tp, INT_CFG0_8125);
> +	tmp |= INT_CFG0_ENABLE_8125;
> +	RTL_W8(tp, INT_CFG0_8125, tmp);
> +}
> +
>  static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
>  {
>  	rtl_pcie_state_l2l3_disable(tp);
> @@ -4092,6 +4111,9 @@ static void rtl_hw_start_8125_common(struct rtl8169_private *tp)
>  	RTL_W32(tp, RSS_CTRL_8125, 0);
>  	RTL_W16(tp, Q_NUM_CTRL_8125, 0);
>  
> +	if (tp->irq_nvecs > 1)
> +		rtl8169_hw_enable_vec_mapping(tp);
> +

[Severity: Low]
Can this branch be taken at this commit?  The only writer of
tp->irq_nvecs is rtl_alloc_irq(), which still does:

	nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);

	if (nvecs < 0)
		return nvecs;

	tp->irq_nvecs = nvecs;

With min_vecs == max_vecs == 1, tp->irq_nvecs can only be 1 here, so
INT_CFG0_ENABLE_8125 is never written and the MSI-X paths in
rtl8169_request_irq(), rtl8169_free_one_irq() and r8169_init_napi()
stay unreachable as well.

The later patch in this series, "r8169: add support and enable rss",
switches rtl_alloc_irq() to pci_alloc_irq_vectors(pdev,
get_min_irq_nvecs(tp), get_max_irq_nvecs(tp), flags), which is what
makes the gate satisfiable, so this looks like prerequisite work rather
than dead code.  Could the changelog say that the mapping is only
actually enabled once the RSS patch lands, instead of stating that this
commit "enables new interrupt mapping for RTL8127"?

[ ... ]

> @@ -5121,11 +5144,13 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  		 */
>  		dma_rmb();
>  
> -		if (rtl8169_check_rx_desc_error(dev, tp, status)) {
> +		if (rtl8169_check_rx_desc_error(ring, status)) {
>  			if (net_ratelimit())
>  				netdev_warn(dev, "Rx ERROR. status = %08x\n",
>  					    status);
> -			dev->stats.rx_errors++;
> +			u64_stats_update_begin(&ring->stats.syncp);
> +			ring->stats.rx_errors++;
> +			u64_stats_update_end(&ring->stats.syncp);
>  
>  			if (!(dev->features & NETIF_F_RXALL))
>  				goto release_descriptor;
> @@ -5141,14 +5166,18 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  		 * They are seen as a symptom of over-mtu sized frames.
>  		 */
>  		if (unlikely(rtl8169_fragmented_frame(status))) {
> -			dev->stats.rx_dropped++;
> -			dev->stats.rx_length_errors++;
> +			u64_stats_update_begin(&ring->stats.syncp);
> +			ring->stats.rx_dropped++;
> +			ring->stats.rx_length_errors++;
> +			u64_stats_update_end(&ring->stats.syncp);
>  			goto release_descriptor;
>  		}

[Severity: Medium]
This is a pre-existing imbalance and not something introduced here, but
since the counters are being reworked anyway: should rx_errors be bumped
on this path too?

rx_errors is only incremented when rtl8169_check_rx_desc_error() sees
RxRES set, while the fragmented-frame path increments rx_length_errors
alone.  The kernel-doc for struct rtnl_link_stats64 in
include/uapi/linux/if_link.h says rx_errors is the total of the detailed
receive error counters, including rx_length_errors.

If the hardware can hand up a descriptor without FirstFrag/LastFrag while
RxRES is clear, userspace would see rx_length_errors climb with no
matching rx_errors.  Can that descriptor combination occur on these
chips?

> @@ -5574,6 +5606,27 @@ rtl8169_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>  	netdev_stats_to_stats64(stats, &dev->stats);
>  	dev_fetch_sw_netstats(stats, dev->tstats);
>  
> +	for (int i = 0; i < tp->num_rx_rings; i++) {
> +		u64 errors, dropped, length_errors, crc_errors, multicast;
> +		struct rtl8169_rx_ring *ring = &tp->rx_ring[i];
> +		unsigned int start;
> +
> +		do {
> +			start = u64_stats_fetch_begin(&ring->stats.syncp);
> +			errors = ring->stats.rx_errors;
> +			dropped = ring->stats.rx_dropped;
> +			length_errors = ring->stats.rx_length_errors;
> +			crc_errors = ring->stats.rx_crc_errors;
> +			multicast = ring->stats.multicast;
> +		} while (u64_stats_fetch_retry(&ring->stats.syncp, start));
> +
> +		stats->rx_errors += errors;
> +		stats->rx_dropped += dropped;
> +		stats->rx_length_errors += length_errors;
> +		stats->rx_crc_errors += crc_errors;
> +		stats->multicast += multicast;
> +	}
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 5/7] r8169: add support and enable rss

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 5 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The patch silently bundles a fix for a pre-existing integer
  underflow in the chip-independent RX path into a feature commit.
- [Medium] The one-line commit message does not disclose the new IRQ
  allocation policy for RTL8127, which is all-or-nothing and consumes…
- [Medium] For the new RSS (v4) RX descriptor the patch re-declares the
  MAC error status bits shifted one position up relative to the legacy…
- [Medium] Packets received on the newly enabled RSS RX rings are handed
  to GRO without recording the receiving queue, so per-queue RPS/RFS…
- [Medium] The new minimum-length guard in rtl_rx() is too weak: it only
  rejects `pkt_size < ETH_FCS_LEN`, so a descriptor reporting exactly 4…

--

r8169: add support and enable rss

This patch adds RSS support for RTL8127 (RTL_GIGA_MAC_VER_80): a second RX
descriptor layout, an RSS key and indirection table, per-ring RX error
accounting and a new MSI-X vector allocation policy.  RSS is turned on by
default when enough MSI-X vectors are granted.  A few questions below.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index ec3643892fe2..ad360f145076 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -82,6 +82,19 @@
>  #define R8169_MAX_RX_QUEUES	8
>  #define R8169_DEFAULT_RX_QUEUES	1
>  #define R8169_MAX_TX_QUEUES	1
> +#define R8127_MAX_NUM_IRQVEC	32
> +#define R8127_MIN_NUM_IRQVEC	30
> +#define R8169_IRQ_DEFAULT	1
> +#define RTL_RSS_KEY_SIZE	40
> +#define RSS_CPU_NUM_MASK	GENMASK(18, 16)
> +#define RSS_HASH_MASK		GENMASK(10, 8)
> +#define RTL_MAX_INDIRECTION_TABLE_ENTRIES 128
> +#define RXS_RSS_UDP		BIT(27)
> +#define RXS_RSS_IPV4		BIT(28)
> +#define RXS_RSS_IPV6		BIT(29)
> +#define RXS_RSS_TCP		BIT(30)
> +#define RXS_RSS_L3_TYPE_MASK	(RXS_RSS_IPV4 | RXS_RSS_IPV6)
> +#define RXS_RSS_L4_TYPE_MASK	(RXS_RSS_TCP | RXS_RSS_UDP)
>  
>  #define OCP_STD_PHY_BASE	0xa400
>  
> @@ -506,6 +519,9 @@ enum rtl_register_content {
>  	RxRUNT	= (1 << 20),
>  	RxCRC	= (1 << 19),
>  
> +	RXRUNT_RSS	= (1 << 21),
> +	RXCRC_RSS	= (1 << 20),
> +
>  	/* ChipCmdBits */
>  	StopReq		= 0x80,
>  	CmdReset	= 0x10,

[ ... ]

> @@ -611,6 +627,20 @@ enum rtl_register_content {
>  #define	ISRIMR_LINKCHG	BIT(29)
>  #define	ISRIMR_TOK_Q0	BIT(8)
>  #define	ISRIMR_ROK_Q0	BIT(0)
> +#define RTL_DESC_TYPE_CTRL		0xd8
> +#define RSS_KEY_REG			0x4600
> +#define RSS_INDIRECTION_TBL_REG		0x4700
> +#define RSS_CTRL_TCP_IPV4_SUPP		BIT(0)
> +#define RTL_DESC_TYPE_RSS		BIT(1)
> +#define RSS_CTRL_IPV4_SUPP		BIT(1)
> +#define RSS_CTRL_TCP_IPV6_SUPP		BIT(2)
> +#define RSS_CTRL_IPV6_SUPP		BIT(3)
> +#define RSS_CTRL_IPV6_EXT_SUPP		BIT(4)
> +#define RSS_CTRL_TCP_IPV6_EXT_SUPP	BIT(5)
> +#define RSS_CTRL_UDP_IPV4_SUPP		BIT(11)
> +#define RSS_CTRL_UDP_IPV6_SUPP		BIT(12)
> +#define	RX_RES_RSS			BIT(22)
> +#define RTL_RX_Q_NUM_MASK		GENMASK(4, 2)
>  };

[ ... ]

> @@ -5093,28 +5299,91 @@ static inline int rtl8169_fragmented_frame(u32 status)

[ ... ]

>  static bool rtl8169_check_rx_desc_error(struct rtl8169_rx_ring *ring,
> +					struct rtl8169_private *tp,
>  					u32 status)
>  {
> -	if (unlikely(status & RxRES)) {
> -		u64_stats_update_begin(&ring->stats.syncp);
> -		if (status & (RxRWT | RxRUNT))
> -			ring->stats.rx_length_errors++;
> -		if (status & RxCRC)
> -			ring->stats.rx_crc_errors++;
> -		u64_stats_update_end(&ring->stats.syncp);
> -		return true;
> +	switch (tp->init_rx_desc_type) {
> +	case RX_DESC_TYPE_RSS:
> +		if (unlikely(status & RX_RES_RSS)) {
> +			u64_stats_update_begin(&ring->stats.syncp);
> +			if (status & RXRUNT_RSS)
> +				ring->stats.rx_length_errors++;
> +			if (status & RXCRC_RSS)
> +				ring->stats.rx_crc_errors++;
> +			u64_stats_update_end(&ring->stats.syncp);
> +			return true;
> +		}
> +		break;

[Severity: Medium]

The three v4 error bits are the legacy bits shifted up by one:

	RX_RES_RSS   = BIT(22)  vs  RxRES   = BIT(21)
	RXRUNT_RSS   = BIT(21)  vs  RxRUNT  = BIT(20)
	RXCRC_RSS    = BIT(20)  vs  RxCRC   = BIT(19)

Is there also a shifted counterpart of RxRWT (receive watchdog /
over-length, legacy BIT(22), so BIT(23) here)?  It is neither defined nor
tested anywhere in the patch, which leads to two differences on the RSS
path.

First, the RSS branch above never bumps ring->stats.rx_length_errors for a
watchdog/over-length error, while the default branch counts
status & (RxRWT | RxRUNT) as a length error, so rx_length_errors is fed by
two different bit maps depending on descriptor type.

Second, the rx-all bypass in rtl_rx() below drops an RWT frame
unconditionally on the legacy layout but not on the RSS layout:

	if (tp->init_rx_desc_type == RX_DESC_TYPE_DEFAULT) {
		if (status & RxRWT || !(status & (RxRUNT | RxCRC)))
			goto release_descriptor;
	} else {
		if (!(status & (RXRUNT_RSS | RXCRC_RSS)))
			goto release_descriptor;
	}

So a descriptor carrying the over-length bit together with a CRC or RUNT
error is delivered to the stack on RTL8127 but discarded on every other
chip.  Is that difference intended, or is the RWT equivalent simply missing
from the new bit definitions?

> @@ -5154,11 +5423,21 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  
>  			if (!(dev->features & NETIF_F_RXALL))
>  				goto release_descriptor;
> -			else if (status & RxRWT || !(status & (RxRUNT | RxCRC)))
> -				goto release_descriptor;
> +
> +			if (tp->init_rx_desc_type == RX_DESC_TYPE_DEFAULT) {
> +				if (status & RxRWT || !(status & (RxRUNT | RxCRC)))
> +					goto release_descriptor;
> +			} else {
> +				if (!(status & (RXRUNT_RSS | RXCRC_RSS)))
> +					goto release_descriptor;
> +			}
>  		}
>  
>  		pkt_size = status & GENMASK(13, 0);
> +
> +		if (unlikely(pkt_size < ETH_FCS_LEN))
> +			goto release_descriptor;
> +
>  		if (likely(!(dev->features & NETIF_F_RXFCS)))
>  			pkt_size -= ETH_FCS_LEN;

[Severity: Medium]

This guard is placed after the shared pkt_size = status & GENMASK(13, 0)
assignment, so it runs for RX_DESC_TYPE_DEFAULT as well.  At the baseline
the code was:

	pkt_size = status & GENMASK(13, 0);
	if (likely(!(dev->features & NETIF_F_RXFCS)))
		pkt_size -= ETH_FCS_LEN;

with no lower bound, so the pkt_size underflow was reachable on every
r8169/r8168/r8125 chip with rx-all enabled, not only on the new RTL8127 RSS
path.

The recorded commit message is only "This patch adds support and enable rss
for RTL8127." — the "fix an integer underflow in rtl_rx()" note lives below
the scissors line and is stripped from git history, and there is no Fixes:
tag or Cc: stable.

Would it make sense to split this hunk into its own patch with a Fixes: tag
so it can be found and backported on its own?

[Severity: Medium]

Is ETH_FCS_LEN a large enough lower bound here?  A descriptor reporting
exactly 4 gives a zero-length skb, and any reported length in 4..17 (or
4..13 with rx-fcs on) gives an skb shorter than ETH_HLEN, which is then
handed to eth_type_trans():

	skb_copy_to_linear_data(skb, rx_buf, pkt_size);
	skb->tail += pkt_size;
	skb->len = pkt_size;
	...
	skb->protocol = eth_type_trans(skb, dev);

eth_type_trans() uses eth_skb_pull_mac(), which ignores the NULL return of
skb_pull_inline() when ETH_HLEN > skb->len:

include/linux/etherdevice.h:
	static inline struct ethhdr *eth_skb_pull_mac(struct sk_buff *skb)
	{
		struct ethhdr *eth = (struct ethhdr *)skb->data;

		skb_pull_inline(skb, ETH_HLEN);
		return eth;
	}

so eth_skb_pkt_type() reads eth->h_dest and eth_type_trans() reads
eth->h_proto at offsets 0..13, while only pkt_size bytes were copied into a
head that napi_alloc_skb() does not zero.  Can skb->protocol, skb->pkt_type
and the ring->stats.multicast accounting then be derived from stale heap
bytes?

For an AF_PACKET listener those bytes also become visible, since
eth_header_parse() copies six source-MAC bytes with no length check:

net/ethernet/eth.c:
	const struct ethhdr *eth = eth_hdr(skb);

	memcpy(haddr, eth->h_source, ETH_ALEN);

and packet_recvmsg() returns them in sockaddr_ll.sll_addr.

Would testing for a complete Ethernet header be more appropriate here, for
example ETH_HLEN + ETH_FCS_LEN before the FCS adjustment?  Also, when this
goto fires no rx_dropped or rx_length_errors counter is updated, so the
frames disappear without accounting — is that intentional?

> @@ -5191,10 +5470,12 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp,
>  		skb->len = pkt_size;
>  		dma_sync_single_for_device(d, addr, pkt_size, DMA_FROM_DEVICE);
>  
> -		rtl8169_rx_csum(skb, status);
> +		if (tp->num_rx_rings > 1)
> +			rtl8169_rx_hash(tp, desc, skb);
> +		rtl8169_rx_csum(tp, skb, status);
>  		skb->protocol = eth_type_trans(skb, dev);

[Severity: Medium]

Should rtl_rx() also call skb_record_rx_queue() here?  The receiving ring is
known to the caller, rtl8169_poll_msix_rx() picks it from the MSI-X message
id and passes it to rtl_rx(), but no driver code records it before
napi_gro_receive().  git grep skb_record_rx_queue finds no hit in
r8169_main.c at the baseline, at this commit, or at the end of the series.

rtl_init_one() calls netif_set_real_num_queues(tp->dev, 1,
tp->num_rx_rings), so sysfs exposes rps_cpus and rps_flow_cnt for every
ring, but get_rps_cpu() only advances past rx-0 when the queue is recorded:

net/core/dev.c:get_rps_cpu() {
	struct netdev_rx_queue *rxqueue = dev->_rx;
	...
	if (skb_rx_queue_recorded(skb)) {
		u16 index = skb_get_rx_queue(skb);
	...
}

Does this mean per-queue rps_cpus settings on rx-1..rx-7 have no effect, and
the rx-0 setting is applied to traffic from all rings?  Setting the RSS hash
does not set skb->queue_mapping, so it doesn't substitute for the missing
call.

> @@ -5823,6 +6106,32 @@ static void rtl_set_irq_mask(struct rtl8169_private *tp)
>  	}
>  }
>  
> +static int get_max_irq_nvecs(struct rtl8169_private *tp)
> +{
> +	if (tp->mac_version == RTL_GIGA_MAC_VER_80)
> +		return R8127_MAX_NUM_IRQVEC;
> +	return R8169_IRQ_DEFAULT;
> +}
> +
> +static int get_min_irq_nvecs(struct rtl8169_private *tp)
> +{
> +	if (tp->mac_version == RTL_GIGA_MAC_VER_80)
> +		return R8127_MIN_NUM_IRQVEC;
> +	return R8169_IRQ_DEFAULT;
> +}
> +
> +static void rtl8169_set_rx_ring_num(struct rtl8169_private *tp)
> +{
> +	if (tp->irq_nvecs >= get_min_irq_nvecs(tp)) {
> +		unsigned int rss_queue_num = netif_get_num_default_rss_queues();
> +
> +		tp->num_rx_rings = rounddown_pow_of_two(min(rss_queue_num,
> +							    tp->hw_supp_num_rx_queues));
> +		if (tp->num_rx_rings >= 2)
> +			tp->init_rx_desc_type = RX_DESC_TYPE_RSS;
> +	}
> +}
> +
>  static int rtl_alloc_irq(struct rtl8169_private *tp)
>  {
>  	struct pci_dev *pdev = tp->pci_dev;
> @@ -5843,7 +6152,11 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
>  		break;
>  	}
>  
> -	nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);
> +	nvecs = pci_alloc_irq_vectors(pdev, get_min_irq_nvecs(tp),
> +				      get_max_irq_nvecs(tp), flags);
> +
> +	if (nvecs < 0)
> +		nvecs = pci_alloc_irq_vectors(pdev, 1, 1, flags);
>  
>  	if (nvecs < 0)
>  		return nvecs;

[Severity: Medium]

This makes the RTL8127 allocation all-or-nothing: 30 to 32 vectors, or a
single vector.  Are the extra reserved vectors used anywhere?
rtl8169_request_irq() only installs handlers for vectors up to
R8169_MAX_RX_QUEUES and for MSIX_ID_VEC_MAP_LINKCHG:

	for (i = 0; i < tp->irq_nvecs; i++) {
		...
			if (i <= R8169_MAX_RX_QUEUES)
				rc = request_irq(irq, rtl8169_interrupt_msix, ...);
			else if (i == MSIX_ID_VEC_MAP_LINKCHG)
				rc = request_irq(irq, rtl8169_interrupt_other, ...);
			else
				continue;

so 21 to 23 of the reserved vectors end up with no handler, and the
kcalloc() of napi structures is sized by tp->irq_nvecs as well.

Both downgrades are also silent.  If the platform cannot supply 30 vectors
the fallback to (1, 1) leaves num_rx_rings at 1 and init_rx_desc_type at
RX_DESC_TYPE_DEFAULT with no message.  On a 2 or 3 CPU machine
rounddown_pow_of_two(min(netif_get_num_default_rss_queues(),
hw_supp_num_rx_queues)) can be 1, so RSS stays off while 30 to 32 vectors
remain reserved.

Would a dev_info() on those paths help, and should the requested count match
the number of vectors the driver actually hooks up?

The one-line commit message does not describe this new allocation policy at
all — could it be mentioned there?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 6/7] r8169: move struct ethtool_ops

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] drivers/net/ethernet/realtek/r8169_main.c: this patch is pure
  code motion that relocates the `rtl8169_ethtool_ops` table ~4080…

--

r8169: move struct ethtool_ops

This is pure code motion: the rtl8169_ethtool_ops table is removed from
its place right after the ethtool callback block and re-added further
down the file, between r8169_init_napi() and rtl_init_one(). The stated
reason is to allow later additions of rtl8169_get_channels and
rtl8169_set_channels to be referenced without forward declarations.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index ad360f145076..3be6f6e708f2 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -2637,34 +2637,6 @@ static int rtl8169_nway_reset(struct net_device *dev)
>  	return phylink_ethtool_nway_reset(tp->phylink);
>  }
>  
> -static const struct ethtool_ops rtl8169_ethtool_ops = {
> -	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> -				     ETHTOOL_COALESCE_MAX_FRAMES,
> -	.get_drvinfo		= rtl8169_get_drvinfo,

[Severity: Low]
This isn't a bug, but does the commit message rationale match the rest of
the series?

The message says the move is needed "so that subsequent additions of
rtl8169_get_channels and rtl8169_set_channels can be referenced from the
ops struct without needing forward declarations".

The follow-up patch "r8169: add get_channel support for ethtool" adds only
rtl8169_get_channels(), and its body touches nothing defined after
rtl8169_nway_reset():

	static void rtl8169_get_channels(struct net_device *dev,
					 struct ethtool_channels *ch)
	{
		struct rtl8169_private *tp = netdev_priv(dev);

		if (tp->irq_nvecs > 1) {
			ch->max_rx = tp->hw_supp_num_rx_queues;
			ch->max_tx = 1;
			ch->rx_count = tp->num_rx_rings;
			ch->tx_count = 1;
		} else {
			ch->max_combined = 1;
			ch->combined_count = 1;
		}
	}

It only uses netdev_priv() and the tp->irq_nvecs, tp->hw_supp_num_rx_queues
and tp->num_rx_rings members of struct rtl8169_private, all of which are
declared far earlier in the file. Could rtl8169_get_channels() simply be
defined just above the original location of rtl8169_ethtool_ops, making this
move unnecessary and avoiding any forward declaration?

Also, rtl8169_set_channels is named as a reason for the move but is never
added anywhere in the series. Should that part of the message be dropped?

>  [ ... ]
> -	.get_eth_mac_stats	= rtl8169_get_eth_mac_stats,
> -	.get_eth_ctrl_stats	= rtl8169_get_eth_ctrl_stats,
> -};
> -
>  static const struct rtl_chip_info *rtl8169_get_chip_version(u32 xid, bool gmii)
>  {
>  	/* Chips combining a 1Gbps MAC with a 100Mbps PHY */
> @@ -6741,6 +6713,34 @@ static void r8169_init_napi(struct rtl8169_private *tp)
>  	}
>  }
>  
> +static const struct ethtool_ops rtl8169_ethtool_ops = {
> +	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> +				     ETHTOOL_COALESCE_MAX_FRAMES,
> +	.get_drvinfo		= rtl8169_get_drvinfo,

[Severity: Low]
The new position places the table roughly four thousand lines away from
every callback it references, between r8169_init_napi() and the PCI probe
function rtl_init_one(). Any future ethtool callback added next to the
existing callback block (rtl8169_get_drvinfo() .. rtl8169_nway_reset())
would then be the one separated from the table. Is keeping the table
adjacent to the callbacks preferable here, given the churn to git blame in
a driver that regularly takes stable backports?

> +	.get_eth_mac_stats	= rtl8169_get_eth_mac_stats,
> +	.get_eth_ctrl_stats	= rtl8169_get_eth_ctrl_stats,
> +};
> +
>  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>  {
>  	const struct rtl_chip_info *chip;

Since the diff is byte-identical motion with no behaviour change, would it
help to add the customary "No functional change intended." line to the
commit message?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn

Re: [PATCH net-next v14 7/7] r8169: add get_channel support for ethtool

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] rtl8169_get_channels()
  (drivers/net/ethernet/realtek/r8169_main.c:6716-6729) does not report…

--

r8169: add get_channel support for ethtool

Adds a .get_channels ethtool callback to the r8169 driver. In multi-vector
(MSI-X) mode it reports the RX and TX channel maximums and counts, and in
single-vector mode it reports one combined channel.

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 3be6f6e708f2..7a61c2ecc7d1 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -6713,6 +6713,22 @@ static void r8169_init_napi(struct rtl8169_private *tp)
>  	}
>  }
>  
> +static void rtl8169_get_channels(struct net_device *dev,
> +				 struct ethtool_channels *ch)
> +{
> +	struct rtl8169_private *tp = netdev_priv(dev);
> +
> +	if (tp->irq_nvecs > 1) {
> +		ch->max_rx = tp->hw_supp_num_rx_queues;
> +		ch->max_tx = 1;
> +		ch->rx_count = tp->num_rx_rings;
> +		ch->tx_count = 1;

[Severity: Low]
Should this branch also report the dedicated link-change vector via
max_other/other_count?

In multi-vector mode rtl8169_request_irq() installs a separate handler on a
vector that is neither an RX nor the TX queue vector:

	if (tp->irq_nvecs > 1) {
		if (i <= R8169_MAX_RX_QUEUES)
			rc = request_irq(irq, rtl8169_interrupt_msix,
					 irqflags, dev->name, napi);
		else if (i == MSIX_ID_VEC_MAP_LINKCHG)
			rc = request_irq(irq, rtl8169_interrupt_other,
					 irqflags, dev->name, tp);

and rtl_set_irq_mask() arms that source in the same mode:

	if (tp->irq_nvecs > 1) {
		tp->irq_mask = ISRIMR_LINKCHG | ISRIMR_TOK_Q0;

Since ch is zero-initialized by the caller and channels_fill_reply() in
net/ethtool/channels.c only emits the OTHER attributes when max_other is
non-zero:

	    (channels->max_other &&
	     (nla_put_u32(skb, ETHTOOL_A_CHANNELS_OTHER_MAX,
			  channels->max_other) ||
	      nla_put_u32(skb, ETHTOOL_A_CHANNELS_OTHER_COUNT,
			  channels->other_count))) ||

ethtool -l reports no "other" channel even though that interrupt is active.
ixgbe_get_channels() reports the equivalent non-queue vector:

	/* report info for other vector */
	if (adapter->flags & IXGBE_FLAG_MSIX_ENABLED) {
		ch->max_other = NON_Q_VECTORS;
		ch->other_count = NON_Q_VECTORS;
	}

Would setting ch->max_other = ch->other_count = 1 in the tp->irq_nvecs > 1
branch be more accurate here? The single-vector branch looks fine as is,
since link-change events share the one combined interrupt.

> +	} else {
> +		ch->max_combined = 1;
> +		ch->combined_count = 1;
> +	}
> +}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260918061923.313-1-javen_xu%40realsil.com.cn