public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail•com>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: Oleg Nesterov <oleg@redhat•com>,
	David Miller <davem@davemloft•net>, Ingo Molnar <mingo@elte•hu>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail•com>,
	Francois Romieu <romieu@fr•zoreil.com>,
	Peter Zijlstra <a.p.zijlstra@chello•nl>,
	netdev@vger•kernel.org,
	linux-kernel <linux-kernel@vger•kernel.org>
Subject: Re: [PATCH] r8169: Fix rtl8169_rx_interrupt()
Date: Tue, 16 Mar 2010 08:50:15 +0200	[thread overview]
Message-ID: <20100316065015.GA3336@swordfish.minsk.epam.com> (raw)
In-Reply-To: <1268699602.2824.14.camel@edumazet-laptop>

[-- Attachment #1: Type: text/plain, Size: 3237 bytes --]

Hello,

Sure. Give me a couple of days. By the way, should I test against .34
or .33?

	Sergey


On (03/16/10 01:33), Eric Dumazet wrote:
> Le lundi 15 mars 2010 à 22:01 +0100, Eric Dumazet a écrit :
> 
> > Yes, this is wrong. In this context (process context, not softirq), we
> > should use netif_rx() or just drop frames if we are in reset phase.
> > 
> 
> Sergey,
> 
> Here is a compiled but untested patch (I dont have the hardware), could
> you please test it ?
> 
> Thanks
> 
> [PATCH] r8169: Fix rtl8169_rx_interrupt()
> 
> In case a reset is performed, rtl8169_rx_interrupt() is called from
> process context instead of softirq context. Special care must be taken
> to call appropriate network core services (netif_rx() instead of
> netif_receive_skb()). VLAN handling also corrected.
> 
> Reported-by: Sergey Senozhatsky <sergey.senozhatsky@gmail•com>
> Diagnosed-by: Oleg Nesterov <oleg@redhat•com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail•com>
> ---
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 9d3ebf3..d873639 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -1038,14 +1038,14 @@ static void rtl8169_vlan_rx_register(struct net_device *dev,
>  }
>  
>  static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> -			       struct sk_buff *skb)
> +			       struct sk_buff *skb, int polling)
>  {
>  	u32 opts2 = le32_to_cpu(desc->opts2);
>  	struct vlan_group *vlgrp = tp->vlgrp;
>  	int ret;
>  
>  	if (vlgrp && (opts2 & RxVlanTag)) {
> -		vlan_hwaccel_receive_skb(skb, vlgrp, swab16(opts2 & 0xffff));
> +		__vlan_hwaccel_rx(skb, vlgrp, swab16(opts2 & 0xffff), polling);
>  		ret = 0;
>  	} else
>  		ret = -1;
> @@ -1062,7 +1062,7 @@ static inline u32 rtl8169_tx_vlan_tag(struct rtl8169_private *tp,
>  }
>  
>  static int rtl8169_rx_vlan_skb(struct rtl8169_private *tp, struct RxDesc *desc,
> -			       struct sk_buff *skb)
> +			       struct sk_buff *skb, int polling)
>  {
>  	return -1;
>  }
> @@ -4429,12 +4429,20 @@ out:
>  	return done;
>  }
>  
> +/*
> + * Warning : rtl8169_rx_interrupt() might be called :
> + * 1) from NAPI (softirq) context
> + *	(polling = 1 : we should call netif_receive_skb())
> + * 2) from process context (rtl8169_reset_task())
> + *	(polling = 0 : we must call netif_rx() instead)
> + */		
>  static int rtl8169_rx_interrupt(struct net_device *dev,
>  				struct rtl8169_private *tp,
>  				void __iomem *ioaddr, u32 budget)
>  {
>  	unsigned int cur_rx, rx_left;
>  	unsigned int delta, count;
> +	int polling = (budget != ~(u32)0) ? 1 : 0;
>  
>  	cur_rx = tp->cur_rx;
>  	rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx;
> @@ -4496,8 +4504,12 @@ static int rtl8169_rx_interrupt(struct net_device *dev,
>  			skb_put(skb, pkt_size);
>  			skb->protocol = eth_type_trans(skb, dev);
>  
> -			if (rtl8169_rx_vlan_skb(tp, desc, skb) < 0)
> -				netif_receive_skb(skb);
> +			if (rtl8169_rx_vlan_skb(tp, desc, skb, polling) < 0) {
> +				if (likely(polling))
> +					netif_receive_skb(skb);
> +				else
> +					netif_rx(skb);
> +			}
>  
>  			dev->stats.rx_bytes += pkt_size;
>  			dev->stats.rx_packets++;
> 
> 

[-- Attachment #2: Type: application/pgp-signature, Size: 316 bytes --]

  reply	other threads:[~2010-03-16  6:49 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20100307192305.GA598@elte.hu>
2010-03-08 12:51 ` inconsistent lock state Oleg Nesterov
2010-03-15 21:01   ` Eric Dumazet
2010-03-15 21:09     ` David Miller
2010-03-16  0:33     ` [PATCH] r8169: Fix rtl8169_rx_interrupt() Eric Dumazet
2010-03-16  6:50       ` Sergey Senozhatsky [this message]
2010-03-16  7:12         ` Eric Dumazet
2010-03-16 14:50       ` Sergey Senozhatsky
2010-03-16 15:00       ` Sergey Senozhatsky
2010-03-16 15:05         ` Eric Dumazet
2010-03-16 15:10           ` Sergey Senozhatsky
2010-03-16 15:20             ` Eric Dumazet
2010-03-16 18:26               ` Sergey Senozhatsky
2010-03-16 18:48                 ` Eric Dumazet
2010-03-16 19:02                   ` Sergey Senozhatsky
2010-03-17  7:25                   ` Sergey Senozhatsky
2010-03-17  7:37                     ` Eric Dumazet
2010-03-17  7:58                       ` Sergey Senozhatsky
2010-03-17 10:58                       ` Sergey Senozhatsky
2010-03-17 13:54                         ` Eric Dumazet
2010-03-18 12:28                           ` Sergey Senozhatsky
2010-03-17 23:55                       ` Francois Romieu
2010-03-18 12:32                         ` Sergey Senozhatsky
2010-03-18 13:31                         ` Sergey Senozhatsky
2010-03-25 11:30                   ` Sergey Senozhatsky
2010-03-25 13:19                     ` Eric Dumazet
2010-03-25 13:48                       ` Sergey Senozhatsky
2010-03-26 20:29                       ` François Romieu
2010-03-31 12:08                     ` Eric Dumazet
2010-04-02  1:43                       ` David Miller
2010-04-02 13:51                         ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100316065015.GA3336@swordfish.minsk.epam.com \
    --to=sergey.senozhatsky@gmail$(echo .)com \
    --cc=a.p.zijlstra@chello$(echo .)nl \
    --cc=davem@davemloft$(echo .)net \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=mingo@elte$(echo .)hu \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=oleg@redhat$(echo .)com \
    --cc=romieu@fr$(echo .)zoreil.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox