Subject: Re: [RFC Patch net-next v2 4/8] r8169: add support for new interrupt mapping

From: Vadim Fedorenko <[email-masked]>
Date: Wed, 29 Apr 2026 15:32:39 +0100

On 29/04/2026 08:07, javen wrote:
> From: Javen Xu <[email-masked]>
> 
> 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_VER_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 <[email-masked]>

[...]

>   
>   		napi = &tp->r8169napi[i];
>   		snprintf(irq->name, len, "%s-%d", dev->name, i);
> @@ -5664,10 +5717,17 @@ 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->features & RTL_VEC_MAP_ENABLE) {
> +		tp->irq_mask = ISRIMR_LINKCHG;
> +		tp->irq_mask |= ISRIMR_TOK_Q0;

       nit: you can set it in one line

> +		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)
> @@ -5695,6 +5755,16 @@ static int rtl_alloc_irq(struct rtl8169_private *tp)
>   	if (nvecs < 0)
>   		nvecs = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>   
> +	tp->features &= ~RTL_VEC_MAP_ENABLE;
> +
> +	if (nvecs > 0) {
> +		tp->irq_nvecs = nvecs;
> +		tp->irq = pci_irq_vector(pdev, 0);
> +		if (nvecs > 1)
> +			tp->features |= RTL_VEC_MAP_ENABLE;
> +		return 0;
> +	}
> +
>   	tp->irq = pdev->irq;
>   	tp->irq_nvecs = 1;

now these 2 lines are not needed, because in success they are never
executed, but in error path they provide wrong information.

the whole can be rewritten with error path in case both tries of
pci_alloc_irq_vectors failed and the common code for success path:

	if (nvecs < 0)
		nvecs = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);

	if (nvecs < 0)
		return nvecs;

	tp->irq_nvecs = nvecs;
	tp->irq = pci_irq_vector(pdev, 0);

	if (nvecs > 1)
		tp->features |= RTL_VEC_MAP_ENABLE;

	return 0;

>   
> @@ -5965,6 +6035,53 @@ static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
>   	return false;
>   }
>   
> +static int rtl8169_poll_msix_rx(struct napi_struct *napi, int budget)
> +{
> +	struct rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi);
> +	struct rtl8169_private *tp = r8169_napi->priv;
> +	struct net_device *dev = tp->dev;
> +	const int message_id = r8169_napi->index;

reverse xmass tree, please

> +	int work_done = 0;
> +
> +	if (message_id < tp->num_rx_rings)
> +		work_done += rtl_rx(dev, tp, &tp->rx_ring[message_id], budget);
> +
> +	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 rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi);
> +	struct rtl8169_private *tp = r8169_napi->priv;
> +	struct net_device *dev = tp->dev;
> +	unsigned int work_done = 0;
> +	const int message_id = r8169_napi->index;
> +	int tx_ring_idx = message_id - 8;

ditto

> +
> +	if (tx_ring_idx >= 0)
> +		rtl_tx(dev, tp, budget);
> +
> +	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_other(struct napi_struct *napi, int budget)
> +{
> +	struct rtl8169_napi *r8169_napi = container_of(napi, struct rtl8169_napi, napi);
> +	struct rtl8169_private *tp = r8169_napi->priv;
> +	const int message_id = r8169_napi->index;
> +
> +	napi_complete_done(napi, budget);
> +	rtl8169_enable_hw_interrupt_msix(tp, message_id);
> +
> +	return 1;
> +}
> +
>   static void r8169_init_napi(struct rtl8169_private *tp)
>   {
>   	for (int i = 0; i < tp->irq_nvecs; i++) {
> @@ -5972,6 +6089,20 @@ static void r8169_init_napi(struct rtl8169_private *tp)
>   		int (*poll)(struct napi_struct *napi, int budget);
>   
>   		poll = rtl8169_poll;
> +		if (tp->features & RTL_VEC_MAP_ENABLE) {
> +			switch (tp->hw_curr_isr_ver) {
> +			case 6:
> +				if (i < R8127_MAX_RX_QUEUES)
> +					poll = rtl8169_poll_msix_rx;
> +				else if (i > 7 && i < 16)

	magic constants?

> +					poll = rtl8169_poll_msix_tx;
> +				else
> +					poll = rtl8169_poll_msix_other;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
>   		netif_napi_add(tp->dev, &r8169napi->napi, poll);
>   		r8169napi->priv = tp;
>   		r8169napi->index = i;