netdev - Re: [PATCH net] r8169: don't read interrupt mask register in interrupt handler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b908dbc6905f9894533680d7e62dc40b9001a369.camel@linux.intel.com>
Date:   Thu, 21 Mar 2019 13:30:26 -0700
From:   Alexander Duyck <alexander.h.duyck@...ux.intel.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Realtek linux nic maintainers <nic_swsd@...ltek.com>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        VDR User <user.vdr@...il.com>
Subject: Re: [PATCH net] r8169: don't read interrupt mask register in
 interrupt handler

On Thu, 2019-03-21 at 21:23 +0100, Heiner Kallweit wrote:
> After the original patch network starts to crash on heavy load.
> It's not fully clear why this additional register read has such side
> effects, but removing it fixes the issue.
> 
> Thanks also to Alex for his contribution and hints.
> 
> [0] https://marc.info/?t=155268170400002&r=1&w=2
> 
> Fixes: e782410ed237 ("r8169: improve spurious interrupt detection")
> Reported-by: VDR User <user.vdr@...il.com>
> Tested-by: VDR User <user.vdr@...il.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>

Looks good to me.

Reviewed-by: Alexander Duyck <alexander.h.duyck@...ux.intel.com>

> ---
>  drivers/net/ethernet/realtek/r8169.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
> index c29dde064..9dd1cd2c0 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -678,6 +678,7 @@ struct rtl8169_private {
>  		struct work_struct work;
>  	} wk;
>  
> +	unsigned irq_enabled:1;
>  	unsigned supports_gmii:1;
>  	dma_addr_t counters_phys_addr;
>  	struct rtl8169_counters *counters;
> @@ -1293,6 +1294,7 @@ static void rtl_ack_events(struct rtl8169_private *tp, u16 bits)
>  static void rtl_irq_disable(struct rtl8169_private *tp)
>  {
>  	RTL_W16(tp, IntrMask, 0);
> +	tp->irq_enabled = 0;
>  }
>  
>  #define RTL_EVENT_NAPI_RX	(RxOK | RxErr)
> @@ -1301,6 +1303,7 @@ static void rtl_irq_disable(struct rtl8169_private *tp)
>  
>  static void rtl_irq_enable(struct rtl8169_private *tp)
>  {
> +	tp->irq_enabled = 1;
>  	RTL_W16(tp, IntrMask, tp->irq_mask);
>  }
>  
> @@ -6520,9 +6523,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
>  {
>  	struct rtl8169_private *tp = dev_instance;
>  	u16 status = RTL_R16(tp, IntrStatus);
> -	u16 irq_mask = RTL_R16(tp, IntrMask);
>  
> -	if (status == 0xffff || !(status & irq_mask))
> +	if (!tp->irq_enabled || status == 0xffff || !(status & tp->irq_mask))
>  		return IRQ_NONE;
>  
>  	if (unlikely(status & SYSErr)) {