Netdev List
 help / color / mirror / Atom feed
* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Mike Frysinger @ 2013-01-16 17:28 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: libc-alpha, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund,
	Eric Blake, netdev, linux-kernel, libvirt-list, tgraf,
	David Miller, schwab, carlos
In-Reply-To: <1358356211.2923.25.camel@bwh-desktop.uk.solarflarecom.com>

[-- Attachment #1: Type: Text/Plain, Size: 2898 bytes --]

On Wednesday 16 January 2013 12:10:11 Ben Hutchings wrote:
> On Wed, 2013-01-16 at 12:04 -0500, Mike Frysinger wrote:
> > On Wednesday 16 January 2013 10:47:12 Ben Hutchings wrote:
> > > On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote:
> > > > Cong Wang wrote:
> > > > > (Cc'ing some glibc developers...)
> > > > > 
> > > > > Hello,
> > > > > 
> > > > > In glibc source file inet/netinet/in.h and kernel source file
> > > > > include/uapi/linux/in6.h, both define struct in6_addr, and both are
> > > > > visible to user applications. Thomas reported a conflict below.
> > > > > 
> > > > > So, how can we handle this? /me is wondering why we didn't see this
> > > > > before.
> > > 
> > > [...]
> > > 
> > > > This is not a new issue.  In addition to this,
> > > > netinet/in.h also conflits with linux/in.h.
> > > > 
> > > > We might have
> > > > 
> > > >  #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
> > > >  
> > > >  #endif
> > > > 
> > > > around those conflicting definitions in uapi/linux/in{,6}.h.
> > > 
> > > This only solves half the problem, as <netinet/in.h> might be included
> > > after <linux/in.h>.  Also, not all Linux userland uses glibc.
> > 
> > certainly true, but the current expectation is that you don't mix your
> > ABIs.
> 
> Whose expectation?  Which ABIs are being mixed?

the kernel's view of the world and the C library's view of the world.  there 
is a lot of overlap, but it's the C library's job to provide a POSIX compliant 
interface (and then some).

obvious examples:
 - the stat structures are not the same and require translation
 - ptrace structures are not the same and you really need to use the C lib one
 - the readdir function is completely different

but to this specific question, if you're calling the kernel's network functions 
directly, then you need to include the kernel's headers for its definition of 
how things are passed.  if you're calling the C library's network functions, 
then use the C library's headers.

> > if you're programming with the C library API, then use the C library
> > headers. if you're banging directly on the kernel, then use the kernel
> > headers.  not saying it's a perfect solution, but it works for the vast
> > majority of use cases.
> 
> In practice most C programs for Linux will use a mixture of thinly
> wrapped system calls and higher-level APIs from the C library, and never
> really call the kernel directly (as that requires inline assembler).
> Userland programmers will work around this historical mess by tweaking
> the #include order or splitting source files.  But they shouldn't have
> to.

if you're not calling the kernel directly, why are you including the kernel 
headers ?  what is the problem people are actually trying to address here (and 
no, "i want to include both headers" is not the answer) ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Ben Hutchings @ 2013-01-16 17:10 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: libc-alpha, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund,
	Eric Blake, netdev, linux-kernel, libvirt-list, tgraf,
	David Miller, schwab, carlos
In-Reply-To: <201301161205.04502.vapier@gentoo.org>

On Wed, 2013-01-16 at 12:04 -0500, Mike Frysinger wrote:
> On Wednesday 16 January 2013 10:47:12 Ben Hutchings wrote:
> > On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote:
> > > Cong Wang wrote:
> > > > (Cc'ing some glibc developers...)
> > > > 
> > > > Hello,
> > > > 
> > > > In glibc source file inet/netinet/in.h and kernel source file
> > > > include/uapi/linux/in6.h, both define struct in6_addr, and both are
> > > > visible to user applications. Thomas reported a conflict below.
> > > > 
> > > > So, how can we handle this? /me is wondering why we didn't see this
> > > > before.
> > 
> > [...]
> > 
> > > This is not a new issue.  In addition to this,
> > > netinet/in.h also conflits with linux/in.h.
> > > 
> > > We might have
> > > 
> > >  #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
> > >  
> > >  #endif
> > > 
> > > around those conflicting definitions in uapi/linux/in{,6}.h.
> > 
> > This only solves half the problem, as <netinet/in.h> might be included
> > after <linux/in.h>.  Also, not all Linux userland uses glibc.
> 
> certainly true, but the current expectation is that you don't mix your ABIs.

Whose expectation?  Which ABIs are being mixed?

> if you're programming with the C library API, then use the C library headers.  
> if you're banging directly on the kernel, then use the kernel headers.  not 
> saying it's a perfect solution, but it works for the vast majority of use 
> cases.

In practice most C programs for Linux will use a mixture of thinly
wrapped system calls and higher-level APIs from the C library, and never
really call the kernel directly (as that requires inline assembler).
Userland programmers will work around this historical mess by tweaking
the #include order or splitting source files.  But they shouldn't have
to.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Mike Frysinger @ 2013-01-16 17:04 UTC (permalink / raw)
  To: libc-alpha
  Cc: Ben Hutchings, YOSHIFUJI Hideaki, Cong Wang, Thomas Backlund,
	Eric Blake, netdev, linux-kernel, libvirt-list, tgraf,
	David Miller, schwab, carlos
In-Reply-To: <1358351232.2923.10.camel@bwh-desktop.uk.solarflarecom.com>

[-- Attachment #1: Type: Text/Plain, Size: 1302 bytes --]

On Wednesday 16 January 2013 10:47:12 Ben Hutchings wrote:
> On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote:
> > Cong Wang wrote:
> > > (Cc'ing some glibc developers...)
> > > 
> > > Hello,
> > > 
> > > In glibc source file inet/netinet/in.h and kernel source file
> > > include/uapi/linux/in6.h, both define struct in6_addr, and both are
> > > visible to user applications. Thomas reported a conflict below.
> > > 
> > > So, how can we handle this? /me is wondering why we didn't see this
> > > before.
> 
> [...]
> 
> > This is not a new issue.  In addition to this,
> > netinet/in.h also conflits with linux/in.h.
> > 
> > We might have
> > 
> >  #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
> >  
> >  #endif
> > 
> > around those conflicting definitions in uapi/linux/in{,6}.h.
> 
> This only solves half the problem, as <netinet/in.h> might be included
> after <linux/in.h>.  Also, not all Linux userland uses glibc.

certainly true, but the current expectation is that you don't mix your ABIs.  
if you're programming with the C library API, then use the C library headers.  
if you're banging directly on the kernel, then use the kernel headers.  not 
saying it's a perfect solution, but it works for the vast majority of use 
cases.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Michael S. Tsirkin @ 2013-01-16 17:03 UTC (permalink / raw)
  To: Jason Wang; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1358351078-58915-4-git-send-email-jasowang@redhat.com>

On Wed, Jan 16, 2013 at 11:44:38PM +0800, Jason Wang wrote:
> We forbid polling, writing and reading when the file were detached, this may
> complex the user in several cases:
> 
> - when guest pass some buffers to vhost/qemu and then disable some queues,
>   host/qemu needs to do its own cleanup on those buffers which is complex
>   sometimes. We can do this simply by allowing a user can still write to an
>   disabled queue. Write to an disabled queue will cause the packet pass to the
>   kernel and read will get nothing.
> - align the polling behavior with macvtap which never fails when the queue is
>   created. This can simplify the polling errors handling of its user (e.g vhost)
> 
> In order to achieve this, tfile->tun were not assign to NULL when detached. And
> tfile->tun were converted to be RCU protected in order to let the data path can
> check whether the file is deated in a lockless manner. This will be used to
> prevent the flow caches from being updated for a detached queue.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/net/tun.c |   45 ++++++++++++++++++++++++++-------------------
>  1 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index c81680d..ec539a9 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -139,7 +139,7 @@ struct tun_file {
>  	unsigned int flags;
>  	u16 queue_index;
>  	struct list_head next;
> -	struct tun_struct *detached;
> +	struct tun_struct __rcu *detached;
>  };
>  
>  struct tun_flow_entry {
> @@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
>  }
>  
>  static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
> -			    u16 queue_index)
> +			    struct tun_file *tfile)
>  {
>  	struct hlist_head *head;
>  	struct tun_flow_entry *e;
>  	unsigned long delay = tun->ageing_time;
> +	u16 queue_index = tfile->queue_index;
>  
>  	if (!rxhash)
>  		return;
> @@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
>  
>  	rcu_read_lock();
>  
> -	if (tun->numqueues == 1)
> +	if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
>  		goto unlock;
>  
>  	e = tun_flow_find(head, rxhash);

Sorry, still an issue with this one.


                u16 index = tfile->queue_index;
                BUG_ON(index >= tun->numqueues);
                dev = tun->dev;

                rcu_assign_pointer(tun->tfiles[index],
                                   tun->tfiles[tun->numqueues - 1]);
                rcu_assign_pointer(tfile->tun, NULL);
                ntfile = rtnl_dereference(tun->tfiles[index]);
                ntfile->queue_index = index;

                --tun->numqueues;
                if (clean)
                        sock_put(&tfile->sk);
                else
                        tun_disable_queue(tun, tfile);

You should first disable queue then synchronize network
only then play with tfiles array.
As it is you might have removed file from array but
did not set detached flag yet, so queue_index
above is stable.

On enable, clear detached last thing.

> @@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
>  
>  static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
>  {
> -	tfile->detached = tun;
> +	rcu_assign_pointer(tfile->detached, tun);
>  	list_add_tail(&tfile->next, &tun->disabled);
>  	++tun->numdisabled;
>  }
>  
>  static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>  {
> -	struct tun_struct *tun = tfile->detached;
> +	struct tun_struct *tun = rtnl_dereference(tfile->detached);
>  
> -	tfile->detached = NULL;
> +	rcu_assign_pointer(tfile->detached, NULL);
>  	list_del_init(&tfile->next);
>  	--tun->numdisabled;
>  	return tun;
> @@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
>  static void __tun_detach(struct tun_file *tfile, bool clean)
>  {
>  	struct tun_file *ntfile;
> -	struct tun_struct *tun;
> +	struct tun_struct *tun, *detached;
>  	struct net_device *dev;
>  
>  	tun = rtnl_dereference(tfile->tun);
> +	detached = rtnl_dereference(tfile->detached);
>  
> -	if (tun) {
> +	if (tun && !detached) {
>  		u16 index = tfile->queue_index;
>  		BUG_ON(index >= tun->numqueues);
>  		dev = tun->dev;
>  
>  		rcu_assign_pointer(tun->tfiles[index],
>  				   tun->tfiles[tun->numqueues - 1]);
> -		rcu_assign_pointer(tfile->tun, NULL);
>  		ntfile = rtnl_dereference(tun->tfiles[index]);
>  		ntfile->queue_index = index;
>  
>  		--tun->numqueues;
> -		if (clean)
> +		if (clean) {
> +			rcu_assign_pointer(tfile->tun, NULL);
>  			sock_put(&tfile->sk);
> -		else
> +		} else
>  			tun_disable_queue(tun, tfile);
>  
>  		synchronize_net();
> @@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>  		/* Drop read queue */
>  		skb_queue_purge(&tfile->sk.sk_receive_queue);
>  		tun_set_real_num_queues(tun);
> -	} else if (tfile->detached && clean) {
> +	} else if (detached && clean) {
>  		tun = tun_enable_queue(tfile);
>  		sock_put(&tfile->sk);
>  	}
> @@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
>  		rcu_assign_pointer(tfile->tun, NULL);
>  		--tun->numqueues;
>  	}
> +	list_for_each_entry(tfile, &tun->disabled, next) {
> +		wake_up_all(&tfile->wq.wait);
> +		rcu_assign_pointer(tfile->tun, NULL);
> +	}
>  	BUG_ON(tun->numqueues != 0);
>  
>  	synchronize_net();
> @@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>  		goto out;
>  
>  	err = -EINVAL;
> -	if (rtnl_dereference(tfile->tun))
> +	if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
>  		goto out;
>  
>  	err = -EBUSY;
> @@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>  		goto out;
>  
>  	err = -E2BIG;
> -	if (!tfile->detached &&
> +	if (!rtnl_dereference(tfile->detached) &&
>  	    tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
>  		goto out;
>  
> @@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>  	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
>  	tun->numqueues++;
>  
> -	if (tfile->detached)
> +	if (rtnl_dereference(tfile->detached))
>  		tun_enable_queue(tfile);
>  	else
>  		sock_hold(&tfile->sk);
> @@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>  	tun->dev->stats.rx_packets++;
>  	tun->dev->stats.rx_bytes += len;
>  
> -	tun_flow_update(tun, rxhash, tfile->queue_index);
> +	tun_flow_update(tun, rxhash, tfile);
>  	return total_len;
>  }
>  
> @@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  	struct net_device *dev;
>  	int err;
>  
> -	if (tfile->detached)
> +	if (rtnl_dereference(tfile->detached))
>  		return -EINVAL;
>  
>  	dev = __dev_get_by_name(net, ifr->ifr_name);
> @@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>  	rtnl_lock();
>  
>  	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
> -		tun = tfile->detached;
> +		tun = rtnl_dereference(tfile->detached);
>  		if (!tun) {
>  			ret = -EINVAL;
>  			goto unlock;
> @@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>  		ret = tun_attach(tun, file);
>  	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>  		tun = rtnl_dereference(tfile->tun);
> -		if (!tun || !(tun->flags & TUN_TAP_MQ))
> +		if (!tun || !(tun->flags & TUN_TAP_MQ) ||
> +		    rtnl_dereference(tfile->detached))
>  			ret = -EINVAL;
>  		else
>  			__tun_detach(tfile, false);
> -- 
> 1.7.1

^ permalink raw reply

* Re: [PATCH net-next] bnx2x: fix GRO parameters
From: Eric Dumazet @ 2013-01-16 16:50 UTC (permalink / raw)
  To: Yuval Mintz; +Cc: David Miller, netdev, Eilon Greenstein, ariele
In-Reply-To: <50F6BB70.8070409@broadcom.com>

On Wed, 2013-01-16 at 16:38 +0200, Yuval Mintz wrote:

> I think we have a misunderstanding here - when LRO is on, our FW works in
> TPA_MODE_LRO (LRO/GRO FW are mutually exclusive). In that mode, the bnx2x
> driver will not try to 'fake' GRO packets in the same manner.

I think you didn't really understand the issue then.

Call the hardware assist Receive Offload "LRO" if you really want, even
if not using net/ipv4/inet_lro.c , but provide :

gso_segs and gso_type as well. (Its useful for various things, check
netif_skb_features() for another example)

Or else we need to parse the headers again to check IPv4/ v6 / TCP / UDP
later.

There is no downside providing gso_segs and gso_type, you have all
needed information as shown in the patch I cooked and tested.

Our goal is to remove CONFIG_INET_LRO, so all multi-segments packets
should have the same set of parameters (gso_size, gso_segs, gso_type)

^ permalink raw reply

* Re: [BUG] Kernel recieves DNS reply, but doesn't deliver it to a waiting application
From: Andrew Savchenko @ 2013-01-16 16:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1356718263.21409.430.camel@edumazet-glaptop>


[-- Attachment #1.1: Type: text/plain, Size: 2135 bytes --]

Hello,

On Fri, 28 Dec 2012 10:11:03 -0800 Eric Dumazet wrote:
> On Sun, 2012-12-23 at 15:06 +0400, Andrew Savchenko wrote:
[...]
> > I hit this bug again on uptime 11 days on 3.7.0 vanilla kernel.
> > See kernel config, /prot/net/upd, netstat -s and dropwatch logs
> > attached to this mail. This bug happens on UDP DNS requests only,
> > TCP requests work fine, see dig.log attached.
> > 
> > Increasing of net.ipv4.udp_mem from
> > 24150        32201   48300
> > to
> > 100000       150000  200000
> > helps, but I'm afraid only temporary again.
> > 
> > Dropwatch data was collected in the following way:
> > - dropwatch.bug.* files contain data obtained after bug occurred;
> > - dropwatch.*.background files contain background data when no
> >   host or dig test was running; this system has active firewall
> >   and complicated routing, ipv6 disabled via sysctl, etc, so some
> >   drops are normal;
> > - dropwatch.*.host.request shows dropped packets recorded during
> >   host ya.ru request; of course, during this time some background
> >   packets were recorded as well (dropwatch doesn't support filtering
> >   at this moment);
> > - dropwatch.nobug.* data was collected after the bug was
> >   workarounded via net.ipv4.upd_mem as described above.
> > 
> > As can be seen from dropwatch logs, drop in __udp_queue_rcv_skb+61
> > happens only on host request on bug conditions, thus something is
> > wrong there.
> > 
> > Best regards,
> > Andrew Savchenko
> 
> Thanks a lot !
> 
> I see strange drops in dev_hard_start_xmit()
> 
> l2tp needs some care.
> 
> Please try the following patch, as skb_cow_head() API
> doesnt really ease skb->truesize exact tracking anyway, better not mess
> with it. 

Sorry for the delay, but I was able to reboot kernel only today.
Your patch is applied on top of the 3.7.2 vanilla kernel.

l2tp works fine and /proc/net/udp tx_queue values are normal now, see
attached /prot/net/udp output. This is a good hint that problem is
probably solved, but we need to wait at least several weeks to be
sure.

Best regards,
Andrew Savchenko

[-- Attachment #1.2: proc.net.udp --]
[-- Type: application/octet-stream, Size: 4224 bytes --]

  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode ref pointer drops             
    0: 00000000:06A5 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5013 2 ffff88003d605c00 0          
   89: 00000000:90FE 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5074 2 ffff88003bcba700 0          
   90: 7E18340A:88FF 1400320A:06A5 01 00000000:00000000 00:00000000 00000000     0        0 5034 4 ffff88003bcba000 0          
  136: 00000000:892D 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5049 2 ffff88003bcba380 0          
  160: 0100007F:2745 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4495 2 ffff88003d604380 0          
  183: 0100007F:035C 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4770 2 ffff88003d604a80 0          
  217: 00000000:857E 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5146 2 ffff88003bd34700 0          
  310: 00000000:03DB 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4520 2 ffff88003d604700 0          
  318: 00000000:A9E3 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4776 2 ffff88003d604e00 0          
  348: 00000000:0801 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5137 2 ffff88003bd34380 0          
  400: 010013AC:0035 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4876 2 ffff88003d605880 0          
  400: 0100007F:0035 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4875 2 ffff88003d605500 0          
  414: 00000000:0043 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5176 2 ffff88003bd35500 0          
  458: 00000000:006F 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4486 2 ffff88003d604000 0          
  466: 00000000:0277 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 4818 2 ffff88003d605180 0          
  470: 076A070A:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000   123        0 5526 2 ffff88003bd35c00 0          
  470: 010213AC:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5121 2 ffff88003bd34000 0          
  470: 010013AC:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5120 2 ffff88003bcbbc00 0          
  470: 00FCA8C0:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5119 2 ffff88003bcbb880 0          
  470: 7E18340A:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5118 2 ffff88003bcbb500 0          
  470: 0100007F:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5117 2 ffff88003bcbb180 0          
  470: 00000000:007B 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5111 2 ffff88003bcbaa80 0          
  484: FF0013AC:0089 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5298 2 ffff880039fbd500 0          
  484: 010013AC:0089 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5297 2 ffff880039fbd180 0          
  484: FF0213AC:0089 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5294 2 ffff880039fbc700 0          
  484: 010213AC:0089 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5293 2 ffff880039fbc380 0          
  484: 00000000:0089 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5290 2 ffff88003bcbae00 0          
  485: FF0013AC:008A 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5300 2 ffff880039fbdc00 0          
  485: 010013AC:008A 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5299 2 ffff880039fbd880 0          
  485: FF0213AC:008A 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5296 2 ffff880039fbce00 0          
  485: 010213AC:008A 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5295 2 ffff880039fbca80 0          
  485: 00000000:008A 00000000:0000 07 00000000:00000000 00:00000000 00000000     0        0 5291 2 ffff880039fbc000 0          

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

^ permalink raw reply

* Re: [RFC PATCH v2 10/10] ixgbe: Add support for set_channels ethtool operation
From: Waskiewicz Jr, Peter P @ 2013-01-16 16:30 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Duyck, Alexander H, netdev@vger.kernel.org, therbert@google.com,
	ycai@google.com, eric.dumazet@gmail.com, davem@davemloft.net
In-Reply-To: <1358353157.2923.18.camel@bwh-desktop.uk.solarflarecom.com>

On Wed, 2013-01-16 at 16:19 +0000, Ben Hutchings wrote:
> On Thu, 2013-01-10 at 10:58 -0800, Alexander Duyck wrote:
> > This change adds support for the ethtool set_channels operation.
> > 
> > Since the ixgbe driver has to support DCB as well as the other modes the
> > assumption I made here is that the number of channels in DCB modes refers
> > to the number of queues per traffic class, not the number of queues total.
> > 
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> In DCB mode are there separate IRQs for the different classes?

Yes.  The Rx packet buffer is split into multiple packet buffers, one
for each online class.  After that, it's just queues assigned to the
packet buffers, and interrupts assigned however you want them to be.

Cheers,
-PJ

^ permalink raw reply

* Re: [RFC PATCH v2 10/10] ixgbe: Add support for set_channels ethtool operation
From: Ben Hutchings @ 2013-01-16 16:19 UTC (permalink / raw)
  To: Alexander Duyck; +Cc: netdev, therbert, ycai, eric.dumazet, davem
In-Reply-To: <20130110185846.29578.94872.stgit@ahduyck-cp1.jf.intel.com>

On Thu, 2013-01-10 at 10:58 -0800, Alexander Duyck wrote:
> This change adds support for the ethtool set_channels operation.
> 
> Since the ixgbe driver has to support DCB as well as the other modes the
> assumption I made here is that the number of channels in DCB modes refers
> to the number of queues per traffic class, not the number of queues total.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

In DCB mode are there separate IRQs for the different classes?

[...]
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -2775,6 +2775,45 @@ static void ixgbe_get_channels(struct net_device *dev,
>  	ch->combined_count = adapter->ring_feature[RING_F_FDIR].indices;
>  }
>  
> +static int ixgbe_set_channels(struct net_device *dev,
> +			      struct ethtool_channels *ch)
> +{
> +	struct ixgbe_adapter *adapter = netdev_priv(dev);
> +	unsigned int count = ch->combined_count;
> +
> +	/* verify they are not requesting separate vectors */
> +	if (ch->rx_count || ch->tx_count)
> +		return -EINVAL;
> +
> +	/* ignore other_count since it is not changeable */
[...]

Please do return an error if the command specifies a change to
other_count.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped
From: Ben Hutchings @ 2013-01-16 16:00 UTC (permalink / raw)
  To: Amir Vadai; +Cc: David S. Miller, netdev, Or Gerlitz, Eugenia Emantayev
In-Reply-To: <1358350935-3281-5-git-send-email-amirv@mellanox.com>

On Wed, 2013-01-16 at 17:42 +0200, Amir Vadai wrote:
> From: Eugenia Emantayev <eugenia@mellanox.com>
> 
> Under heavy CPU load changing ring size/mtu/etc. could result in transmit
> timeout, since stop-start port might take more than 10 sec. Set
> netif_carrier_off to prevent tx queue transmit timeout.

A spurious link change can restart L3 auto-configuration (DHCP, SLAAC,
etc.)  netif_device_detach() also inhibits the watchdog and doesn't have
that problem.

Ben.

> Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
> Signed-off-by: Amir Vadai <amirv@mellanox.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 805e242..108c4cf 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
>  
>  	/* Synchronize with tx routine */
>  	netif_tx_lock_bh(dev);
> +	netif_carrier_off(dev);
>  	netif_tx_stop_all_queues(dev);
>  	netif_tx_unlock_bh(dev);
>  

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH 182/222] batman-adv: fix random jitter calculation
From: Herton Ronaldo Krzesinski @ 2013-01-16 15:56 UTC (permalink / raw)
  To: linux-kernel, stable, kernel-team
  Cc: Akinobu Mita, Marek Lindner, Simon Wunderlich, Antonio Quartulli,
	b.a.t.m.a.n, David S. Miller, netdev, Herton Ronaldo Krzesinski
In-Reply-To: <1358351822-7675-1-git-send-email-herton.krzesinski@canonical.com>

3.5.7.3 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Akinobu Mita <akinobu.mita@gmail.com>

commit 143cdd8f33909ff5a153e3f02048738c5964ba26 upstream.

batadv_iv_ogm_emit_send_time() attempts to calculates a random integer
in the range of 'orig_interval +- BATADV_JITTER' by the below lines.

        msecs = atomic_read(&bat_priv->orig_interval) - BATADV_JITTER;
        msecs += (random32() % 2 * BATADV_JITTER);

But it actually gets 'orig_interval' or 'orig_interval - BATADV_JITTER'
because '%' and '*' have same precedence and associativity is
left-to-right.

This adds the parentheses at the appropriate position so that it matches
original intension.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Acked-by: Antonio Quartulli <ordex@autistici.org>
Cc: Marek Lindner <lindner_marek@yahoo.de>
Cc: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Cc: Antonio Quartulli <ordex@autistici.org>
Cc: b.a.t.m.a.n@lists.open-mesh.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 net/batman-adv/bat_iv_ogm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index dc53798..bcf3760 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -117,7 +117,7 @@ static unsigned long bat_iv_ogm_emit_send_time(const struct bat_priv *bat_priv)
 {
 	return jiffies + msecs_to_jiffies(
 		   atomic_read(&bat_priv->orig_interval) -
-		   JITTER + (random32() % 2*JITTER));
+		   JITTER + (random32() % (2*JITTER)));
 }
 
 /* when do we schedule a ogm packet to be sent */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH 176/222] epoll: prevent missed events on EPOLL_CTL_MOD
From: Herton Ronaldo Krzesinski @ 2013-01-16 15:56 UTC (permalink / raw)
  To: linux-kernel, stable, kernel-team
  Cc: Eric Wong, Hans Verkuil, Jiri Olsa, Jonathan Corbet, Al Viro,
	Davide Libenzi, Hans de Goede, Mauro Carvalho Chehab,
	David Miller, Eric Dumazet, Andrew Morton, Andreas Voellmy,
	netdev, linux-fsdevel, Linus Torvalds, Herton Ronaldo Krzesinski
In-Reply-To: <1358351822-7675-1-git-send-email-herton.krzesinski@canonical.com>

3.5.7.3 -stable review patch.  If anyone has any objections, please let me know.

------------------

From: Eric Wong <normalperson@yhbt.net>

commit 128dd1759d96ad36c379240f8b9463e8acfd37a1 upstream.

EPOLL_CTL_MOD sets the interest mask before calling f_op->poll() to
ensure events are not missed.  Since the modifications to the interest
mask are not protected by the same lock as ep_poll_callback, we need to
ensure the change is visible to other CPUs calling ep_poll_callback.

We also need to ensure f_op->poll() has an up-to-date view of past
events which occured before we modified the interest mask.  So this
barrier also pairs with the barrier in wq_has_sleeper().

This should guarantee either ep_poll_callback or f_op->poll() (or both)
will notice the readiness of a recently-ready/modified item.

This issue was encountered by Andreas Voellmy and Junchang(Jason) Wang in:
http://thread.gmane.org/gmane.linux.kernel/1408782/

Signed-off-by: Eric Wong <normalperson@yhbt.net>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Davide Libenzi <davidel@xmailserver.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: David Miller <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Voellmy <andreas.voellmy@yale.edu>
Tested-by: "Junchang(Jason) Wang" <junchang.wang@yale.edu>
Cc: netdev@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Herton Ronaldo Krzesinski <herton.krzesinski@canonical.com>
---
 fs/eventpoll.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 1c8b556..dd1a55d 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1285,7 +1285,7 @@ static int ep_modify(struct eventpoll *ep, struct epitem *epi, struct epoll_even
 	 * otherwise we might miss an event that happens between the
 	 * f_op->poll() call and the new event set registering.
 	 */
-	epi->event.events = event->events;
+	epi->event.events = event->events; /* need barrier below */
 	pt._key = event->events;
 	epi->event.data = event->data; /* protected by mtx */
 	if (epi->event.events & EPOLLWAKEUP) {
@@ -1296,6 +1296,26 @@ static int ep_modify(struct eventpoll *ep, struct epitem *epi, struct epoll_even
 	}
 
 	/*
+	 * The following barrier has two effects:
+	 *
+	 * 1) Flush epi changes above to other CPUs.  This ensures
+	 *    we do not miss events from ep_poll_callback if an
+	 *    event occurs immediately after we call f_op->poll().
+	 *    We need this because we did not take ep->lock while
+	 *    changing epi above (but ep_poll_callback does take
+	 *    ep->lock).
+	 *
+	 * 2) We also need to ensure we do not miss _past_ events
+	 *    when calling f_op->poll().  This barrier also
+	 *    pairs with the barrier in wq_has_sleeper (see
+	 *    comments for wq_has_sleeper).
+	 *
+	 * This barrier will now guarantee ep_poll_callback or f_op->poll
+	 * (or both) will notice the readiness of an item.
+	 */
+	smp_mb();
+
+	/*
 	 * Get current event bits. We can safely use the file* here because
 	 * its usage count has been increased by the caller of this function.
 	 */
-- 
1.7.9.5

^ permalink raw reply related

* ip6_dst_lookup_tail oops
From: Dave Jones @ 2013-01-16 14:55 UTC (permalink / raw)
  To: netdev

Hit this after around 36 hours of fuzzing.

BUG: unable to handle kernel NULL pointer dereference at 000000000000017e
IP: [<ffffffff81626308>] ip6_dst_lookup_tail+0xe8/0x200
PGD be5fe067 PUD 93459067 PMD 0 
Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
Modules linked in: fuse l2tp_ppp l2tp_core 8021q garp bridge stp dlci binfmt_misc hidp bnep rfcomm ipt_ULOG scsi_transport_iscsi can_raw nfnetlink can_bcm can llc2 af_key netrom af_rxrpc phonet rose caif_socket caif pppoe pppox ax25 nfc ppp_generic decnet appletalk slhc ipx irda p8023 atm psnap x25 crc_ccitt p8022 llc lockd sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 xt_conntrack nf_conntrack ip6table_filter ip6_tables btusb bluetooth snd_hda_codec_realtek snd_hda_intel snd_hda_codec usb_debug rfkill microcode snd_pcm snd_page_alloc serio_raw snd_timer snd pcspkr edac_core soundcore r8169 mii vhost_net tun macvtap macvlan kvm_amd kvm
CPU 2 
Pid: 10098, comm: trinity-child2 Not tainted 3.8.0-rc3+ #52 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
RIP: 0010:[<ffffffff81626308>]  [<ffffffff81626308>] ip6_dst_lookup_tail+0xe8/0x200
RSP: 0018:ffff88008b5b3978  EFLAGS: 00010206
RAX: 0000000000000011 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff88008b5b3b10 RSI: ffff880124067600 RDI: ffff880065013800
RBP: ffff88008b5b3a08 R08: 0000000000000001 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff88008b5b3a20
R13: ffff88008b5b3b10 R14: ffff880065013800 R15: ffffffff81cb1980
FS:  00007fd98131d740(0000) GS:ffff88012f200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000000017e CR3: 000000009bef5000 CR4: 00000000000007e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process trinity-child2 (pid: 10098, threadinfo ffff88008b5b2000, task ffff880089e3a490)
Stack:
 ffff880065013800 0000000000000002 0000000000000001 0000000000000000
 ffff88008b5b39a8 ffffffff81332a28 ffff88008b5b39c8 ffffffff8106bdfc
 0000000000000001 ffff880124067600 ffff88008b5b3a08 0000000086d9021e
Call Trace:
 [<ffffffff81332a28>] ? __const_udelay+0x28/0x30
 [<ffffffff8106bdfc>] ? __rcu_read_unlock+0x5c/0xa0
 [<ffffffff8154f065>] ? sk_dst_check+0x5/0x260
 [<ffffffff816266cb>] ip6_sk_dst_lookup_flow+0xcb/0x1b0
 [<ffffffff8164803e>] udpv6_sendmsg+0x66e/0xb80
 [<ffffffff81332a28>] ? __const_udelay+0x28/0x30
 [<ffffffff8106bdfc>] ? __rcu_read_unlock+0x5c/0xa0
 [<ffffffff815e6391>] inet_sendmsg+0x111/0x220
 [<ffffffff815e6285>] ? inet_sendmsg+0x5/0x220
 [<ffffffff81547220>] sock_sendmsg+0xb0/0xe0
 [<ffffffff810b228e>] ? put_lock_stats.isra.23+0xe/0x40
 [<ffffffff810b73a7>] ? lock_release_non_nested+0x2b7/0x2f0
 [<ffffffff815486bc>] __sys_sendmsg+0x3ac/0x3c0
 [<ffffffff810b1ef8>] ? trace_hardirqs_off_caller+0x28/0xc0
 [<ffffffff810b1e22>] ? get_lock_stats+0x22/0x70
 [<ffffffff810b228e>] ? put_lock_stats.isra.23+0xe/0x40
 [<ffffffff8100a1b6>] ? native_sched_clock+0x26/0x90
 [<ffffffff810b1ef8>] ? trace_hardirqs_off_caller+0x28/0xc0
 [<ffffffff8104c494>] ? do_setitimer+0x1c4/0x300
 [<ffffffff810b228e>] ? put_lock_stats.isra.23+0xe/0x40
 [<ffffffff811cbe3a>] ? fget_light+0x3ca/0x500
 [<ffffffff810b830d>] ? trace_hardirqs_on+0xd/0x10
 [<ffffffff8154afb9>] sys_sendmsg+0x49/0x90
 [<ffffffff816a6802>] system_call_fastpath+0x16/0x1b
Code: 00 00 48 8b 5d d8 4c 8b 65 e0 4c 8b 6d e8 4c 8b 75 f0 4c 8b 7d f8 c9 c3 0f 1f 00 49 8b 34 24 48 8b 86 98 00 00 00 48 85 c0 74 c2 <f6> 80 6d 01 00 00 de 75 b9 48 8b 56 18 49 8d 75 24 b9 01 00 00 
RIP  [<ffffffff81626308>] ip6_dst_lookup_tail+0xe8/0x200
 RSP <ffff88008b5b3978>
CR2: 000000000000017e
---[ end trace 439801e1c30eed47 ]---


   0:	f6 80 6d 01 00 00 de 	testb  $0xde,0x16d(%rax)
   7:	75 b9                	jne    0xffffffffffffffc2
   9:	48 8b 56 18          	mov    0x18(%rsi),%rdx
   d:	49 8d 75 24          	lea    0x24(%r13),%rsi
  11:	b9                   	.byte 0xb9
  12:	01 00                	add    %eax,(%rax)
	...

This looks like the GPF in this function I reported last September.
http://www.spinics.net/lists/netdev/msg211894.html

In those reports, I ended up with an rt->n == 0x8000000000000011,
but this time, it's just 0x11.

	Dave

^ permalink raw reply

* Re: [RFC PATCH v3] cgroup: net_cls: traffic counter based on classification control cgroup
From: Alexey Perevalov @ 2013-01-16 15:48 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1358262309.8744.5760.camel@edumazet-glaptop>

Hello Eric

On 01/15/2013 07:05 PM, Eric Dumazet wrote:
> On Tue, 2013-01-15 at 17:33 +0400, Alexey Perevalov wrote:
>> Hello
>>
>> I would like to represent next version of patch I sent before
>> cgroup: "net_cls: traffic counter based on classification control cgroup"
>>
>> The main idea is the same as was. It keeping counter in control groups, but now uses atomic instead of resource_counters.
>>
>> Signed-off-by: Alexey Perevalov<a.perevalov-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> ---
>>   include/net/cls_cgroup.h         |  200 ++++++++++++++++++++++++++++++++++----
>>   include/net/cls_counter_holder.h |   26 +++++
>>   init/Kconfig                     |   25 +++++
>>   kernel/cgroup.c                  |    2 +
>>   kernel/res_counter.c             |    4 +
>>   net/core/dev.c                   |    6 ++
>>   net/ipv4/tcp.c                   |   27 ++++-
>>   net/ipv4/udp.c                   |    6 ++
>>   net/sched/Kconfig                |   11 ---
>>   net/sched/Makefile               |    1 +
>>   net/sched/cls_cgroup.c           |  194 +++++++++++++++++++++++++++++++++++-
>>   net/sched/cls_counter_holder.c   |  144 +++++++++++++++++++++++++++
>>   12 files changed, 611 insertions(+), 35 deletions(-)
>>   create mode 100644 include/net/cls_counter_holder.h
>>   create mode 100644 net/sched/cls_counter_holder.c
> Sorry, there are too many issues with this patch and I have not
> a lot of time to review it.
>
> 0) No changelog or documentation
Do you expect documentation in Documentation/cgroups/.
For example I'll create new file net_cls.txt
> 1) There is no way you need to add kludges in tcp and udp for this
> accounting. Retransmitted packets should not be ignored for example.
Retransmitted incoming packets will be ignored - right.
But counting on the 3rd layer of network stack (in netfilter) is 
expensive, we should twice resolve destination process.

> 2) Ugly #ifdef in c files. Don't do that please.
>
> 3) No check of kmalloc() returns (can be NULL)
>
This issues I'll fix.

>


-- 
Best regards,
Alexey Perevalov

^ permalink raw reply

* Re: Redefinition of struct in6_addr in <netinet/in.h> and <linux/in6.h>
From: Ben Hutchings @ 2013-01-16 15:47 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: Cong Wang, Thomas Backlund, Eric Blake, netdev, linux-kernel,
	libvirt-list, tgraf, David Miller, libc-alpha, schwab, carlos
In-Reply-To: <50F6B761.8070106@linux-ipv6.org>

On Wed, 2013-01-16 at 23:21 +0900, YOSHIFUJI Hideaki wrote:
> Cong Wang wrote:
> > (Cc'ing some glibc developers...)
> > 
> > Hello,
> > 
> > In glibc source file inet/netinet/in.h and kernel source file
> > include/uapi/linux/in6.h, both define struct in6_addr, and both are
> > visible to user applications. Thomas reported a conflict below.
> > 
> > So, how can we handle this? /me is wondering why we didn't see this
> > before.
[...]
> This is not a new issue.  In addition to this,
> netinet/in.h also conflits with linux/in.h.
> 
> We might have
>  #if !defined(__GLIBC__) || !defined(_NETINET_IN_H)
>  :
>  #endif
> around those conflicting definitions in uapi/linux/in{,6}.h.

This only solves half the problem, as <netinet/in.h> might be included
after <linux/in.h>.  Also, not all Linux userland uses glibc.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH V6 3/3] tuntap: allow polling/writing/reading when detached
From: Jason Wang @ 2013-01-16 15:44 UTC (permalink / raw)
  To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358351078-58915-1-git-send-email-jasowang@redhat.com>

We forbid polling, writing and reading when the file were detached, this may
complex the user in several cases:

- when guest pass some buffers to vhost/qemu and then disable some queues,
  host/qemu needs to do its own cleanup on those buffers which is complex
  sometimes. We can do this simply by allowing a user can still write to an
  disabled queue. Write to an disabled queue will cause the packet pass to the
  kernel and read will get nothing.
- align the polling behavior with macvtap which never fails when the queue is
  created. This can simplify the polling errors handling of its user (e.g vhost)

In order to achieve this, tfile->tun were not assign to NULL when detached. And
tfile->tun were converted to be RCU protected in order to let the data path can
check whether the file is deated in a lockless manner. This will be used to
prevent the flow caches from being updated for a detached queue.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c |   45 ++++++++++++++++++++++++++-------------------
 1 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index c81680d..ec539a9 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -139,7 +139,7 @@ struct tun_file {
 	unsigned int flags;
 	u16 queue_index;
 	struct list_head next;
-	struct tun_struct *detached;
+	struct tun_struct __rcu *detached;
 };
 
 struct tun_flow_entry {
@@ -295,11 +295,12 @@ static void tun_flow_cleanup(unsigned long data)
 }
 
 static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
-			    u16 queue_index)
+			    struct tun_file *tfile)
 {
 	struct hlist_head *head;
 	struct tun_flow_entry *e;
 	unsigned long delay = tun->ageing_time;
+	u16 queue_index = tfile->queue_index;
 
 	if (!rxhash)
 		return;
@@ -308,7 +309,7 @@ static void tun_flow_update(struct tun_struct *tun, u32 rxhash,
 
 	rcu_read_lock();
 
-	if (tun->numqueues == 1)
+	if (tun->numqueues == 1 || rcu_dereference(tfile->detached))
 		goto unlock;
 
 	e = tun_flow_find(head, rxhash);
@@ -384,16 +385,16 @@ static void tun_set_real_num_queues(struct tun_struct *tun)
 
 static void tun_disable_queue(struct tun_struct *tun, struct tun_file *tfile)
 {
-	tfile->detached = tun;
+	rcu_assign_pointer(tfile->detached, tun);
 	list_add_tail(&tfile->next, &tun->disabled);
 	++tun->numdisabled;
 }
 
 static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
 {
-	struct tun_struct *tun = tfile->detached;
+	struct tun_struct *tun = rtnl_dereference(tfile->detached);
 
-	tfile->detached = NULL;
+	rcu_assign_pointer(tfile->detached, NULL);
 	list_del_init(&tfile->next);
 	--tun->numdisabled;
 	return tun;
@@ -402,26 +403,27 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile)
 static void __tun_detach(struct tun_file *tfile, bool clean)
 {
 	struct tun_file *ntfile;
-	struct tun_struct *tun;
+	struct tun_struct *tun, *detached;
 	struct net_device *dev;
 
 	tun = rtnl_dereference(tfile->tun);
+	detached = rtnl_dereference(tfile->detached);
 
-	if (tun) {
+	if (tun && !detached) {
 		u16 index = tfile->queue_index;
 		BUG_ON(index >= tun->numqueues);
 		dev = tun->dev;
 
 		rcu_assign_pointer(tun->tfiles[index],
 				   tun->tfiles[tun->numqueues - 1]);
-		rcu_assign_pointer(tfile->tun, NULL);
 		ntfile = rtnl_dereference(tun->tfiles[index]);
 		ntfile->queue_index = index;
 
 		--tun->numqueues;
-		if (clean)
+		if (clean) {
+			rcu_assign_pointer(tfile->tun, NULL);
 			sock_put(&tfile->sk);
-		else
+		} else
 			tun_disable_queue(tun, tfile);
 
 		synchronize_net();
@@ -429,7 +431,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 		/* Drop read queue */
 		skb_queue_purge(&tfile->sk.sk_receive_queue);
 		tun_set_real_num_queues(tun);
-	} else if (tfile->detached && clean) {
+	} else if (detached && clean) {
 		tun = tun_enable_queue(tfile);
 		sock_put(&tfile->sk);
 	}
@@ -466,6 +468,10 @@ static void tun_detach_all(struct net_device *dev)
 		rcu_assign_pointer(tfile->tun, NULL);
 		--tun->numqueues;
 	}
+	list_for_each_entry(tfile, &tun->disabled, next) {
+		wake_up_all(&tfile->wq.wait);
+		rcu_assign_pointer(tfile->tun, NULL);
+	}
 	BUG_ON(tun->numqueues != 0);
 
 	synchronize_net();
@@ -496,7 +502,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
 		goto out;
 
 	err = -EINVAL;
-	if (rtnl_dereference(tfile->tun))
+	if (rtnl_dereference(tfile->tun) && !rtnl_dereference(tfile->detached))
 		goto out;
 
 	err = -EBUSY;
@@ -504,7 +510,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
 		goto out;
 
 	err = -E2BIG;
-	if (!tfile->detached &&
+	if (!rtnl_dereference(tfile->detached) &&
 	    tun->numqueues + tun->numdisabled == MAX_TAP_QUEUES)
 		goto out;
 
@@ -521,7 +527,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
 	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
 	tun->numqueues++;
 
-	if (tfile->detached)
+	if (rtnl_dereference(tfile->detached))
 		tun_enable_queue(tfile);
 	else
 		sock_hold(&tfile->sk);
@@ -1195,7 +1201,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	tun->dev->stats.rx_packets++;
 	tun->dev->stats.rx_bytes += len;
 
-	tun_flow_update(tun, rxhash, tfile->queue_index);
+	tun_flow_update(tun, rxhash, tfile);
 	return total_len;
 }
 
@@ -1552,7 +1558,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	struct net_device *dev;
 	int err;
 
-	if (tfile->detached)
+	if (rtnl_dereference(tfile->detached))
 		return -EINVAL;
 
 	dev = __dev_get_by_name(net, ifr->ifr_name);
@@ -1796,7 +1802,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 	rtnl_lock();
 
 	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
-		tun = tfile->detached;
+		tun = rtnl_dereference(tfile->detached);
 		if (!tun) {
 			ret = -EINVAL;
 			goto unlock;
@@ -1807,7 +1813,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 		ret = tun_attach(tun, file);
 	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
 		tun = rtnl_dereference(tfile->tun);
-		if (!tun || !(tun->flags & TUN_TAP_MQ))
+		if (!tun || !(tun->flags & TUN_TAP_MQ) ||
+		    rtnl_dereference(tfile->detached))
 			ret = -EINVAL;
 		else
 			__tun_detach(tfile, false);
-- 
1.7.1

^ permalink raw reply related

* [PATCH V6 2/3] vhost_net: handle polling errors when setting backend
From: Jason Wang @ 2013-01-16 15:44 UTC (permalink / raw)
  To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358351078-58915-1-git-send-email-jasowang@redhat.com>

Currently, the polling errors were ignored, which can lead following issues:

- vhost remove itself unconditionally from waitqueue when stopping the poll,
  this may crash the kernel since the previous attempt of starting may fail to
  add itself to the waitqueue
- userspace may think the backend were successfully set even when the polling
  failed.

Solve this by:

- check poll->wqh before trying to remove from waitqueue
- report polling errors in vhost_poll_start(), tx_poll_start(), the return value
  will be checked and returned when userspace want to set the backend

After this fix, there still could be a polling failure after backend is set, it
will addressed by the next patch.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c   |   27 ++++++++++++++++++---------
 drivers/vhost/vhost.c |   18 +++++++++++++++---
 drivers/vhost/vhost.h |    2 +-
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index d10ad6f..959b1cd 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -165,12 +165,16 @@ static void tx_poll_stop(struct vhost_net *net)
 }
 
 /* Caller must have TX VQ lock */
-static void tx_poll_start(struct vhost_net *net, struct socket *sock)
+static int tx_poll_start(struct vhost_net *net, struct socket *sock)
 {
+	int ret;
+
 	if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
-		return;
-	vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
-	net->tx_poll_state = VHOST_NET_POLL_STARTED;
+		return 0;
+	ret = vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
+	if (!ret)
+		net->tx_poll_state = VHOST_NET_POLL_STARTED;
+	return ret;
 }
 
 /* In case of DMA done not in order in lower device driver for some reason.
@@ -642,20 +646,23 @@ static void vhost_net_disable_vq(struct vhost_net *n,
 		vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
 }
 
-static void vhost_net_enable_vq(struct vhost_net *n,
+static int vhost_net_enable_vq(struct vhost_net *n,
 				struct vhost_virtqueue *vq)
 {
 	struct socket *sock;
+	int ret;
 
 	sock = rcu_dereference_protected(vq->private_data,
 					 lockdep_is_held(&vq->mutex));
 	if (!sock)
-		return;
+		return 0;
 	if (vq == n->vqs + VHOST_NET_VQ_TX) {
 		n->tx_poll_state = VHOST_NET_POLL_STOPPED;
-		tx_poll_start(n, sock);
+		ret = tx_poll_start(n, sock);
 	} else
-		vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+		ret = vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
+
+	return ret;
 }
 
 static struct socket *vhost_net_stop_vq(struct vhost_net *n,
@@ -833,7 +840,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 		r = vhost_init_used(vq);
 		if (r)
 			goto err_used;
-		vhost_net_enable_vq(n, vq);
+		r = vhost_net_enable_vq(n, vq);
+		if (r)
+			goto err_used;
 
 		oldubufs = vq->ubufs;
 		vq->ubufs = ubufs;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 34389f7..9759249 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -77,26 +77,38 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
 	init_poll_funcptr(&poll->table, vhost_poll_func);
 	poll->mask = mask;
 	poll->dev = dev;
+	poll->wqh = NULL;
 
 	vhost_work_init(&poll->work, fn);
 }
 
 /* Start polling a file. We add ourselves to file's wait queue. The caller must
  * keep a reference to a file until after vhost_poll_stop is called. */
-void vhost_poll_start(struct vhost_poll *poll, struct file *file)
+int vhost_poll_start(struct vhost_poll *poll, struct file *file)
 {
 	unsigned long mask;
+	int ret = 0;
 
 	mask = file->f_op->poll(file, &poll->table);
 	if (mask)
 		vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
+	if (mask & POLLERR) {
+		if (poll->wqh)
+			remove_wait_queue(poll->wqh, &poll->wait);
+		ret = -EINVAL;
+	}
+
+	return ret;
 }
 
 /* Stop polling a file. After this function returns, it becomes safe to drop the
  * file reference. You must also flush afterwards. */
 void vhost_poll_stop(struct vhost_poll *poll)
 {
-	remove_wait_queue(poll->wqh, &poll->wait);
+	if (poll->wqh) {
+		remove_wait_queue(poll->wqh, &poll->wait);
+		poll->wqh = NULL;
+	}
 }
 
 static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
@@ -792,7 +804,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
 		fput(filep);
 
 	if (pollstart && vq->handle_kick)
-		vhost_poll_start(&vq->poll, vq->kick);
+		r = vhost_poll_start(&vq->poll, vq->kick);
 
 	mutex_unlock(&vq->mutex);
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 2639c58..17261e2 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -42,7 +42,7 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
 
 void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
 		     unsigned long mask, struct vhost_dev *dev);
-void vhost_poll_start(struct vhost_poll *poll, struct file *file);
+int vhost_poll_start(struct vhost_poll *poll, struct file *file);
 void vhost_poll_stop(struct vhost_poll *poll);
 void vhost_poll_flush(struct vhost_poll *poll);
 void vhost_poll_queue(struct vhost_poll *poll);
-- 
1.7.1

^ permalink raw reply related

* [PATCH V6 1/3] vhost_net: correct error handling in vhost_net_set_backend()
From: Jason Wang @ 2013-01-16 15:44 UTC (permalink / raw)
  To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang
In-Reply-To: <1358351078-58915-1-git-send-email-jasowang@redhat.com>

Currently, when vhost_init_used() fails the sock refcnt and ubufs were
leaked. Correct this by calling vhost_init_used() before assign ubufs and
restore the oldsock when it fails.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ebd08b2..d10ad6f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -827,15 +827,16 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 			r = PTR_ERR(ubufs);
 			goto err_ubufs;
 		}
-		oldubufs = vq->ubufs;
-		vq->ubufs = ubufs;
+
 		vhost_net_disable_vq(n, vq);
 		rcu_assign_pointer(vq->private_data, sock);
-		vhost_net_enable_vq(n, vq);
-
 		r = vhost_init_used(vq);
 		if (r)
-			goto err_vq;
+			goto err_used;
+		vhost_net_enable_vq(n, vq);
+
+		oldubufs = vq->ubufs;
+		vq->ubufs = ubufs;
 
 		n->tx_packets = 0;
 		n->tx_zcopy_err = 0;
@@ -859,6 +860,11 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
 	mutex_unlock(&n->dev.mutex);
 	return 0;
 
+err_used:
+	rcu_assign_pointer(vq->private_data, oldsock);
+	vhost_net_enable_vq(n, vq);
+	if (ubufs)
+		vhost_ubuf_put_and_wait(ubufs);
 err_ubufs:
 	fput(sock->file);
 err_vq:
-- 
1.7.1

^ permalink raw reply related

* [PATCH V6 0/3] handle polling errors in vhost/vhost_net
From: Jason Wang @ 2013-01-16 15:44 UTC (permalink / raw)
  To: davem, mst, netdev, linux-kernel; +Cc: Jason Wang

This is an update version of last version to fix the handling of polling errors
in vhost/vhost_net.

Currently, vhost and vhost_net ignore polling errors which can lead kernel
crashing when it tries to remove itself from waitqueue after the polling
failure. Fix this by:

- examing the POLLERR when setting backend and report erros to userspace
- let tun always add to waitqueue in .poll() after the queue is created even if
  it was detached.

Changes from V5:
- use rcu_dereference() instead of the wrong rtnl_dereference() in data path
- test with CONFIG_PROVE_RCU

Changes from V4:
- check the detached state by tfile->detached and protect it by RCU

Changes from V3:
- make a smaller patch that doesn't touch the whole polling state and only check
  the polliner errors in backend setting.
- add a patch that allows tuntap to do polling/reading/writing when detached
  which could simplify the work of its user.

Changes from v2:
- check poll->wqh instead of the wrong assumption about POLLERR and waitqueue
- drop the whole tx polling state check since it was replaced by the wqh
  checking
- drop the buggy tuntap patch

Changes from v1:
- restore the state before the ioctl when vhost_init_used() fails
- log the error when meet polling errors in the data path
- don't put into waitqueue when tun_chr_poll() return POLLERR

Jason Wang (3):
  vhost_net: correct error handling in vhost_net_set_backend()
  vhost_net: handle polling errors when setting backend
  tuntap: allow polling/writing/reading when detached

 drivers/net/tun.c     |   45 ++++++++++++++++++++++++++-------------------
 drivers/vhost/net.c   |   41 ++++++++++++++++++++++++++++-------------
 drivers/vhost/vhost.c |   18 +++++++++++++++---
 drivers/vhost/vhost.h |    2 +-
 4 files changed, 70 insertions(+), 36 deletions(-)

^ permalink raw reply

* [PATCH net-next 7/7] net/mlx4_en: Initialize RFS filters lock and list in init_netdev
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

filters_lock might have been used while it was re-initialized.
Moved filters_lock and filters_list initialization to init_netdev instead of
alloc_resources which is called every time the device is configured.

Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 108c4cf..f297d4c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1455,9 +1455,6 @@ int mlx4_en_alloc_resources(struct mlx4_en_priv *priv)
 	priv->dev->rx_cpu_rmap = alloc_irq_cpu_rmap(priv->rx_ring_num);
 	if (!priv->dev->rx_cpu_rmap)
 		goto err;
-
-	INIT_LIST_HEAD(&priv->filters);
-	spin_lock_init(&priv->filters_lock);
 #endif
 
 	return 0;
@@ -1652,6 +1649,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	if (err)
 		goto out;
 
+#ifdef CONFIG_RFS_ACCEL
+	INIT_LIST_HEAD(&priv->filters);
+	spin_lock_init(&priv->filters_lock);
+#endif
+
 	/* Allocate page for receive rings */
 	err = mlx4_alloc_hwq_res(mdev->dev, &priv->res,
 				MLX4_EN_PAGE_SIZE, MLX4_EN_PAGE_SIZE);
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 0/7] Mellanox Ethernet driver updates 2013-01-16
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai

Hi Dave,

These are all small bug fixes to the Mellanox mlx4 driver, patches done against
net-next commit 757b8b1 "net_sched: fix qdisc_pkt_len_init()"

Thanks,
Amir

Amir Vadai (2):
  net/mlx4_en: Fix a race when closing TX queue
  net/mlx4_en: Initialize RFS filters lock and list in init_netdev

Aviad Yehezkel (1):
  net/mlx4_en: Fix traffic loss under promiscuous mode

Eugenia Emantayev (3):
  net/mlx4_en: Issue the dump eth statistics command under lock
  net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
  net/mlx4_en: Set carrier to off when a port is stopped

Jack Morgenstein (1):
  net/mlx4_core: Return proper error code when __mlx4_add_one fails

 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   56 ++++++++++++++++--------
 drivers/net/ethernet/mellanox/mlx4/en_tx.c     |    9 +++-
 drivers/net/ethernet/mellanox/mlx4/main.c      |    7 ++-
 3 files changed, 51 insertions(+), 21 deletions(-)

-- 
1.7.8.2

^ permalink raw reply

* [PATCH net-next 5/7] net/mlx4_core: Return proper error code when __mlx4_add_one fails
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Jack Morgenstein,
	Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Jack Morgenstein <jackm@dev.mellanox.co.il>

Returning 0 (success) when in fact we are aborting the load, leads to kernel
panic when unloading the module. Fix that by returning the actual error code.

Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index e1bafff..983fd3d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -2169,7 +2169,8 @@ slave_start:
 			dev->num_slaves = MLX4_MAX_NUM_SLAVES;
 		else {
 			dev->num_slaves = 0;
-			if (mlx4_multi_func_init(dev)) {
+			err = mlx4_multi_func_init(dev);
+			if (err) {
 				mlx4_err(dev, "Failed to init slave mfunc"
 					 " interface, aborting.\n");
 				goto err_cmd;
@@ -2193,7 +2194,8 @@ slave_start:
 	/* In master functions, the communication channel must be initialized
 	 * after obtaining its address from fw */
 	if (mlx4_is_master(dev)) {
-		if (mlx4_multi_func_init(dev)) {
+		err = mlx4_multi_func_init(dev);
+		if (err) {
 			mlx4_err(dev, "Failed to init master mfunc"
 				 "interface, aborting.\n");
 			goto err_close;
@@ -2210,6 +2212,7 @@ slave_start:
 	mlx4_enable_msi_x(dev);
 	if ((mlx4_is_mfunc(dev)) &&
 	    !(dev->flags & MLX4_FLAG_MSI_X)) {
+		err = -ENOSYS;
 		mlx4_err(dev, "INTx is not supported in multi-function mode."
 			 " aborting.\n");
 		goto err_free_eq;
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 6/7] net/mlx4_en: Fix a race when closing TX queue
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Yevgeny Petrilin,
	Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

There is a possible race where the TX completion handler can clean the
entire TX queue between the decision that the queue is full and actually
closing it. To avoid this situation, check again if the queue is really
full, if not, reopen the transmit and continue with sending the packet.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 2b799f4..1d17f5f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -592,7 +592,14 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif_tx_stop_queue(ring->tx_queue);
 		priv->port_stats.queue_stopped++;
 
-		return NETDEV_TX_BUSY;
+		/* Check again whether the queue was cleaned */
+		if (unlikely(((int)(ring->prod - ring->cons)) <=
+				ring->size - HEADROOM - MAX_DESC_TXBBS)) {
+			netif_tx_wake_queue(ring->tx_queue);
+			priv->port_stats.wake_queue++;
+		} else {
+			return NETDEV_TX_BUSY;
+		}
 	}
 
 	/* Track current inflight packets for performance analysis */
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 4/7] net/mlx4_en: Set carrier to off when a port is stopped
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

Under heavy CPU load changing ring size/mtu/etc. could result in transmit
timeout, since stop-start port might take more than 10 sec. Set
netif_carrier_off to prevent tx queue transmit timeout.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 805e242..108c4cf 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1212,6 +1212,7 @@ void mlx4_en_stop_port(struct net_device *dev)
 
 	/* Synchronize with tx routine */
 	netif_tx_lock_bh(dev);
+	netif_carrier_off(dev);
 	netif_tx_stop_all_queues(dev);
 	netif_tx_unlock_bh(dev);
 
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 2/7] net/mlx4_en: Fix traffic loss under promiscuous mode
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Or Gerlitz, Amir Vadai, Aviad Yehezkel, Eugenia Emantayev,
	Hadar Hen Zion
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Aviad Yehezkel <aviadye@mellanox.com>

When port is stopped and flow steering mode is not device managed: promisc QP
rule wasn't removed from MCG table.
Added code to remove it in all flow steering modes.
In addition, promsic rule removal should be in stop port and not in start
port - moved it accordingly.

Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |   35 +++++++++++++++++------
 1 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index b6c645f..bab8cec 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1167,15 +1167,6 @@ int mlx4_en_start_port(struct net_device *dev)
 
 	/* Must redo promiscuous mode setup. */
 	priv->flags &= ~(MLX4_EN_FLAG_PROMISC | MLX4_EN_FLAG_MC_PROMISC);
-	if (mdev->dev->caps.steering_mode ==
-	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_UPLINK);
-		mlx4_flow_steer_promisc_remove(mdev->dev,
-					       priv->port,
-					       MLX4_FS_PROMISC_ALL_MULTI);
-	}
 
 	/* Schedule multicast task to populate multicast list */
 	queue_work(mdev->workqueue, &priv->mcast_task);
@@ -1227,6 +1218,32 @@ void mlx4_en_stop_port(struct net_device *dev)
 	/* Set port as not active */
 	priv->port_up = false;
 
+	/* Promsicuous mode */
+	if (mdev->dev->caps.steering_mode ==
+	    MLX4_STEERING_MODE_DEVICE_MANAGED) {
+		priv->flags &= ~(MLX4_EN_FLAG_PROMISC |
+				 MLX4_EN_FLAG_MC_PROMISC);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_UPLINK);
+		mlx4_flow_steer_promisc_remove(mdev->dev,
+					       priv->port,
+					       MLX4_FS_PROMISC_ALL_MULTI);
+	} else if (priv->flags & MLX4_EN_FLAG_PROMISC) {
+		priv->flags &= ~MLX4_EN_FLAG_PROMISC;
+
+		/* Disable promiscouos mode */
+		mlx4_unicast_promisc_remove(mdev->dev, priv->base_qpn,
+					    priv->port);
+
+		/* Disable Multicast promisc */
+		if (priv->flags & MLX4_EN_FLAG_MC_PROMISC) {
+			mlx4_multicast_promisc_remove(mdev->dev, priv->base_qpn,
+						      priv->port);
+			priv->flags &= ~MLX4_EN_FLAG_MC_PROMISC;
+		}
+	}
+
 	/* Detach All multicasts */
 	memset(&mc_list[10], 0xff, ETH_ALEN);
 	mc_list[5] = priv->port; /* needed for B0 steering support */
-- 
1.7.8.2

^ permalink raw reply related

* [PATCH net-next 3/7] net/mlx4_en: Use the correct netif lock on ndo_set_rx_mode
From: Amir Vadai @ 2013-01-16 15:42 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Or Gerlitz, Amir Vadai, Eugenia Emantayev
In-Reply-To: <1358350935-3281-1-git-send-email-amirv@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

The device multicast list is protected by netif_addr_lock_bh in the networking core, we should
use this locking practice in mlx4_en too.

Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
Reviewed-by: Yevgeny Petrilin <yevgenyp@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bab8cec..805e242 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -767,9 +767,9 @@ static void mlx4_en_do_set_multicast(struct work_struct *work)
 
 		/* Update multicast list - we cache all addresses so they won't
 		 * change while HW is updated holding the command semaphor */
-		netif_tx_lock_bh(dev);
+		netif_addr_lock_bh(dev);
 		mlx4_en_cache_mclist(dev);
-		netif_tx_unlock_bh(dev);
+		netif_addr_unlock_bh(dev);
 		list_for_each_entry(mclist, &priv->mc_list, list) {
 			mcast_addr = mlx4_en_mac_to_u64(mclist->addr);
 			mlx4_SET_MCAST_FLTR(mdev->dev, priv->port,
-- 
1.7.8.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox