Re: [PATCH net-next v10 3/7] r8169: add support for new interrupt mapping
Jakub Kicinski <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 3 Aug 2026 10:13:01 +0800 javen wrote:
> +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);
> +
> + rtl_tx(dev, tp, budget);
This seems not to be ring aware? Is that because we can only have 1 Tx
ring at this point? maybe add a comment to this effect, it's somewhat
unusual.
> + if (napi_complete_done(napi, 0))
> + rtl8169_enable_hw_interrupt_msix(tp, (int)(napi - tp->rtl8169_napi));
> +
> + return 0;
> +}
> +
> +static int rtl8169_poll_msix_other(struct napi_struct *napi, int budget)
> +{
> + struct net_device *dev = napi->dev;
> + struct rtl8169_private *tp;
> +
> + tp = netdev_priv(dev);
> +
> + if (napi_complete_done(napi, 0))
> + rtl8169_enable_hw_interrupt_msix(tp, (int)(napi - tp->rtl8169_napi));
Why are we using a NAPI for "other" ? NAPI is for packet processing.
This handler only acks the IRQ and does nothing (even after all the
later patches in the series, AFAICT) so why schedule the NAPI in the
first place, and not handle all the work in the IRQ directly?
> + return 0;
> +}