Netdev List
 help / color / mirror / Atom feed
* RED tc qdisc not dropping?
From: Ken Savage @ 2012-10-03 20:14 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1643425136.11868012.1349294650298.JavaMail.root@cds046>

Hi there,

I'm running openSUSE 12.2, using the machine as a router/WANsim device.

Previously, I was running an older CentOS installation with a 2.6 kernel,
and my tc-red commands ran just fine, and imposed some bandwidth constraint
to the packets upon egress.  In 3.4.6, this doesn't seem to be the case
any longer.

Without any restrictions, there would be 25-30Mbps of traffic flowing out
the interface -- this is to give you a sense of the data rate.


Now, this said, I did notice that the latest RED code has the 'harddrop'
option that I didn't have under CentOS with kernel 2.6.  So in my attempt
to see ANYTHING happening with 3.4.6, I entered:

tc qdisc add dev eth0 root red limit 40000 min 3000 max 9000 avpkt 1000 burst 5 harddrop probability 1


Issuing 'tc -s -d qdisc show dev eth0', I obtain:

qdisc red 8006: root refcnt 2 limit 40000b min 3000b max 9000b harddrop ewma 2 probability 0.73242 Scell_log 12
 Sent 254028472 bytes 207494 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
  marked 0 early 0 pdrop 0 other 0


All those zeroes seem a little amiss to me  ;)


Any ideas?

Thanks,

Ken Savage
kens1835@shaw.ca

^ permalink raw reply

* Re: [PATCH 4/5] ucc_geth: Increase RX ring buffer from 32 to 64
From: Joakim Tjernlund @ 2012-10-03 20:13 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20121003.153855.325707079634367524.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote on 2012/10/03 21:38:55:
>
> From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> Date: Wed, 3 Oct 2012 15:17:58 +0200
>
> > Ping? Got no comments and I can see it in net or net-next trees either.
>
> No patch will be applied to the tree when one of the indiviual patches
> get feedback and request changes from you.
>
> When an individual patch must be redone, you must resend the entire
> series not just the individual patch which changed.

oh, I see what I did wrong. I sent them as series which may have internal dependencies.
However, each patch can be applied on its own.

>
> You also never need to ask the kind of question you are asking here,
> you simply need to look into patch work to see what the state of
> your patches is:
>
> I maintain this state exactly so people don't need to waste precious
> developer time asking "what is the state of my patch" like you are
> making me do right here.
>
> See:
>
> http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=ucc_geth

Ahh, better bookmark this page for future use.

>
> And as you can see this entire series is marked as either "RFC"
> or "Changes Requested"

Yes, I see that now.
I guess I should resend each patch separately then?
(Possibly wait until I have figured out the lockless TX stuff)

>
> Even worse, you did this for not one but several of the patches you
> posted.

Sorry about that.

 Jocke

^ permalink raw reply

* Re: [PATCH 4/5] ucc_geth: Increase RX ring buffer from 32 to 64
From: David Miller @ 2012-10-03 19:38 UTC (permalink / raw)
  To: joakim.tjernlund; +Cc: netdev
In-Reply-To: <OF4BE93D25.4AE817F3-ONC1257A8C.004909F7-C1257A8C.00490EAC@transmode.se>

From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
Date: Wed, 3 Oct 2012 15:17:58 +0200

> Ping? Got no comments and I can see it in net or net-next trees either.

No patch will be applied to the tree when one of the indiviual patches
get feedback and request changes from you.

When an individual patch must be redone, you must resend the entire
series not just the individual patch which changed.

You also never need to ask the kind of question you are asking here,
you simply need to look into patch work to see what the state of
your patches is:

I maintain this state exactly so people don't need to waste precious
developer time asking "what is the state of my patch" like you are
making me do right here.

See:

http://patchwork.ozlabs.org/project/netdev/list/?state=*&q=ucc_geth

And as you can see this entire series is marked as either "RFC"
or "Changes Requested"

Even worse, you did this for not one but several of the patches you
posted.

^ permalink raw reply

* Re: [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive
From: David Miller @ 2012-10-03 19:30 UTC (permalink / raw)
  To: dlstevens; +Cc: eric.dumazet, chris2553, davej, gpiez, ja, netdev, netdev-owner
In-Reply-To: <OFFFC99DDA.1BD04FA3-ON85257A8C.005A8E9B-85257A8C.00604444@us.ibm.com>

From: David Stevens <dlstevens@us.ibm.com>
Date: Wed, 3 Oct 2012 13:31:30 -0400

> Eric Dumazet <eric.dumazet@gmail.com> wrote on 10/03/2012 11:29:13 AM:
> 
>> >         Of course. I think our difference is on the definition of 
>> > "receives".
>> 
>> A receive is a packet delivered to this host.
>> Interface being promiscuous or not doesnt really matter.
> 
>         A receive is a packet *addressed* to this host.

Although I'm largely ambivalent, this one sentence tipped me over
towards David's side on this issue.

But this is easy to resolve Eric, just simply make a new custom
counter that counts these new cases you care about and document it
properly.

Thanks.

^ permalink raw reply

* Kernel recieves DNS reply, but doesn't deliver it to a waiting application
From: Andrew Savchenko @ 2012-10-03 19:25 UTC (permalink / raw)
  To: netdev

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

Hello,

I encountered a very weird bug: after a while of uptime kernel stops to deliver
DNS reply to applications. Tcpdump shows that correct reply is recieved, but 
strace shows inquiring application never recieves it and ends with timeout,
epoll_wait() always returns 0:
a slice from: $ host kernel.org 8.8.8.8:

sendmsg(20, {msg_name(16)={sa_family=AF_INET, sin_port=htons(53),
sin_addr=inet_addr("8.8.8.8")}, msg_iov(1)=[{"\266\344\1\0\0\1\0\0\0\0\0\0\6k
ernel\3org\0\0\1\0\1", 28}], msg_controllen=0, msg_flags=0}, 0) = 28            
epoll_wait(3, {}, 64, 0)                = 0                                     
epoll_wait(3, {}, 64, 4999)             = 0

Though tcpdump shows a normal reply:

20:28:44.162897 IP 10.7.74.7.43167 > 8.8.8.8.domain: 46820+ A? kernel.org. (28) 
20:28:44.221308 IP 8.8.8.8.domain > 10.7.74.7.43167: 46820 1/0/0 A 149.20.4.69
(44)

After this bug has occured, it is no longer possible to perform DNS request on
the crippled system. I tried to stop/restart all network-related daemons, to
recreate network interfaces whenever possible (e.g. pppX devices), but with no
help. I use iptables and ebtables on this host, but reseting them (flushing all
chains, removing user chains, setting all policies to ACCEPT) doesn't help. The
only worknig solution is to reboot the system.

This bug happens rarely and randomly (about once in 7-12 days on 24x7 available
production system), but I had it 5 times already. Due to rare and random nature
of the bug I can't bisect it.

This problem occured after I updated vanilla kernel from 2.6.39.4 to 3.4.6.
Afterward I updated kernel to 3.4.10 in the hope that this will fix the
problem, but with no result. (I updated kernel due to commit
2ce42ec4ef551b08d2e5d26775d838ac640f82ad, which describes somewhat similar
issue, though I don't use I/OAT engine due to lack of hardware support.)

More details, attached trace files and kernel configs are available at bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=48081

In a few days I'll try 3.4.12 (I need to rebuild kernel anyway due to unrelated
issue) and will report if this bug will occur again. But please note it may
take several weeks to check this.

Best regards,
Andrew Savchenko

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

^ permalink raw reply

* RE: [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Rose, Gregory V @ 2012-10-03 19:16 UTC (permalink / raw)
  To: Don Dutile, Duyck, Alexander H
  Cc: Yinghai Lu, Bjorn Helgaas, Greg Kroah-Hartman,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	yuvalmin@broadcom.com, bhutchings@solarflare.com,
	davem@davemloft.net--no-chain-reply-to, Kirsher, Jeffrey T,
	Brandeburg, Jesse, David S. Miller, Fastabend, John R,
	e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <506C8BCB.4080402@redhat.com>

> -----Original Message-----
> From: Don Dutile [mailto:ddutile@redhat.com]
> Sent: Wednesday, October 03, 2012 12:03 PM
> To: Duyck, Alexander H
> Cc: Yinghai Lu; Bjorn Helgaas; Greg Kroah-Hartman; linux-
> pci@vger.kernel.org; linux-kernel@vger.kernel.org; yuvalmin@broadcom.com;
> bhutchings@solarflare.com; Rose, Gregory V; davem@davemloft.net--no-chain-
> reply-to; Kirsher, Jeffrey T; Brandeburg, Jesse; David S. Miller;
> Fastabend, John R; e1000-devel@lists.sourceforge.net;
> netdev@vger.kernel.org
> Subject: Re: [PATCH 5/5] ixgbe: add driver set_max_vfs support
> 
> On 10/03/2012 02:47 PM, Alexander Duyck wrote:

[snip]

> >
> > The ixgbe_set_max_vfs function has several issues.  The two big ones
> > are that this function assumes it can just enable/disable SR-IOV
> > without any other changes being necessary which is not the case.  I
> > would recommend looking at ixgbe_setup_tc for how to do this properly.
> > Secondly is the fact that this code will change the PF network device
> > and as such sections of the code should be called with the RTNL lock
> > held.  In addition I believe you have to disable SR-IOV before
> > enabling it again with a different number of VFs.
> >
> > Below is a link to one of the early patches for igb when we were first
> > introducing SR-IOV, and the in-driver sysfs value had been rejected.
> > I figure it might be useful as it was also using sysfs to
> > enable/disable VFs.  It however doesn't have the correct locking on
> > changing the queues and as such will likely throw an error if you were
> > to implement it the same way now:
> > http://lists.openwall.net/netdev/2009/04/08/34
> >
> > Thanks,
> >
> > Alex
> 
> Alex,
> Thanks for patch set pointer.
> When I started to work on the ixgbe example use based on the RFC set I
> posted, I ran into the problem you outlined -- the PF uses/consumes all
> the queues & MSI intrs when sriov not enabled at driver load time, which
> required more network shutdown logic that I'm not familiar with... So, I
> was going to defer to Greg to work that magic. :)
> Greg: assume the 2 callback function interface in the RFC patch set I
> sent,
>        (primarily, just the include/linux/pci.h changes), and you can make
> the
>        necessary drivers mods from there.  In the meantime, I'll make the
> changes
>        to my original/v1 RFC to reflect the changes that GKH & Yinghai
> recommended/implemented
>        for sysfs attribute creation & removal in a v2 posting.
>        The end result is that the current module parameter setting for
> max_vfs should
>        continue to work, and the sysfs interface will work when those
> pieces are provided.

OK, I'll start work on it.

Thanks Don,

- Greg

> 
> -Don

^ permalink raw reply

* Re: [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Don Dutile @ 2012-10-03 19:02 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: Yinghai Lu, Bjorn Helgaas, Greg Kroah-Hartman, linux-pci,
	linux-kernel, yuvalmin, bhutchings, gregory.v.rose, davem,
	Jeff Kirsher, Jesse Brandeburg, David S. Miller, John Fastabend,
	e1000-devel, netdev
In-Reply-To: <506C8837.5070902@intel.com>

On 10/03/2012 02:47 PM, Alexander Duyck wrote:
> On 10/03/2012 10:51 AM, Yinghai Lu wrote:
>> Need ixgbe guys to close the loop to use set_max_vfs instead
>> kernel parameters.
>>
>> Signed-off-by: Yinghai Lu<yinghai@kernel.org>
>> Cc: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>> Cc: Jesse Brandeburg<jesse.brandeburg@intel.com>
>> Cc: Greg Rose<gregory.v.rose@intel.com>
>> Cc: "David S. Miller"<davem@davemloft.net>
>> Cc: John Fastabend<john.r.fastabend@intel.com>
>> Cc: e1000-devel@lists.sourceforge.net
>> Cc: netdev@vger.kernel.org
>> ---
>>   drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    2 +
>>   drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   44 +++++++++++++++++++-----
>>   2 files changed, 37 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> index b9623e9..d39d975 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
>> @@ -558,6 +558,8 @@ struct ixgbe_adapter {
>>   	u32 interrupt_event;
>>   	u32 led_reg;
>>
>> +	struct ixgbe_info *ixgbe_info;
>> +
>>   #ifdef CONFIG_IXGBE_PTP
>>   	struct ptp_clock *ptp_clock;
>>   	struct ptp_clock_info ptp_caps;
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> index ee61819..1c097c7 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifier = {
>>   };
>>   #endif
>>
>> -#ifdef CONFIG_PCI_IOV
>> -static unsigned int max_vfs;
>> -module_param(max_vfs, uint, 0);
>> -MODULE_PARM_DESC(max_vfs,
>> -		 "Maximum number of virtual functions to allocate per physical function - default is zero and maximum value is 63");
>> -#endif /* CONFIG_PCI_IOV */
>> -
>>   static unsigned int allow_unsupported_sfp;
>>   module_param(allow_unsupported_sfp, uint, 0);
>>   MODULE_PARM_DESC(allow_unsupported_sfp,
>> @@ -4496,7 +4489,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
>>   #ifdef CONFIG_PCI_IOV
>>   	/* assign number of SR-IOV VFs */
>>   	if (hw->mac.type != ixgbe_mac_82598EB)
>> -		adapter->num_vfs = (max_vfs>  63) ? 0 : max_vfs;
>> +		adapter->num_vfs = min_t(int, pdev->max_vfs, 63);
>>
>>   #endif
>>   	/* enable itr by default in dynamic mode */
>> @@ -7220,8 +7213,9 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
>>
>>   #ifdef CONFIG_PCI_IOV
>>   	ixgbe_enable_sriov(adapter, ii);
>> -
>>   #endif
>> +	adapter->ixgbe_info = ii;
>> +
>>   	netdev->features = NETIF_F_SG |
>>   			   NETIF_F_IP_CSUM |
>>   			   NETIF_F_IPV6_CSUM |
>> @@ -7683,11 +7677,43 @@ static const struct pci_error_handlers ixgbe_err_handler = {
>>   	.resume = ixgbe_io_resume,
>>   };
>>
>> +static void ixgbe_set_max_vfs(struct pci_dev *pdev)
>> +{
>> +#ifdef CONFIG_PCI_IOV
>> +	struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
>> +	struct ixgbe_hw *hw =&adapter->hw;
>> +	int num_vfs = 0;
>> +
>> +	/* assign number of SR-IOV VFs */
>> +	if (hw->mac.type != ixgbe_mac_82598EB)
>> +		num_vfs = min_t(int, pdev->max_vfs, 63);
>> +
>> +	/* no change */
>> +	if (adapter->num_vfs == num_vfs)
>> +		return;
>> +
>> +	if (!num_vfs) {
>> +		/* disable sriov */
>> +		ixgbe_disable_sriov(adapter);
>> +		adapter->num_vfs = 0;
>> +	} else if (!adapter->num_vfs&&  num_vfs) {
>> +		/* enable sriov */
>> +		adapter->num_vfs = num_vfs;
>> +		ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
>> +	} else {
>> +		/* increase or decrease */
>> +	}
>> +
>> +	pdev->max_vfs = adapter->num_vfs;
>> +#endif
>> +}
>> +
>>   static struct pci_driver ixgbe_driver = {
>>   	.name     = ixgbe_driver_name,
>>   	.id_table = ixgbe_pci_tbl,
>>   	.probe    = ixgbe_probe,
>>   	.remove   = __devexit_p(ixgbe_remove),
>> +	.set_max_vfs = ixgbe_set_max_vfs,
>>   #ifdef CONFIG_PM
>>   	.suspend  = ixgbe_suspend,
>>   	.resume   = ixgbe_resume,
>
> The ixgbe_set_max_vfs function has several issues.  The two big ones are
> that this function assumes it can just enable/disable SR-IOV without any
> other changes being necessary which is not the case.  I would recommend
> looking at ixgbe_setup_tc for how to do this properly.  Secondly is the
> fact that this code will change the PF network device and as such
> sections of the code should be called with the RTNL lock held.  In
> addition I believe you have to disable SR-IOV before enabling it again
> with a different number of VFs.
>
> Below is a link to one of the early patches for igb when we were first
> introducing SR-IOV, and the in-driver sysfs value had been rejected.  I
> figure it might be useful as it was also using sysfs to enable/disable
> VFs.  It however doesn't have the correct locking on changing the queues
> and as such will likely throw an error if you were to implement it the
> same way now:
> http://lists.openwall.net/netdev/2009/04/08/34
>
> Thanks,
>
> Alex

Alex,
Thanks for patch set pointer.
When I started to work on the ixgbe example use based on the RFC set I posted,
I ran into the problem you outlined -- the PF uses/consumes all the queues &
MSI intrs when sriov not enabled at driver load time, which required
more network shutdown logic that I'm not familiar with... So, I was going
to defer to Greg to work that magic. :)
Greg: assume the 2 callback function interface in the RFC patch set I sent,
       (primarily, just the include/linux/pci.h changes), and you can make the
       necessary drivers mods from there.  In the meantime, I'll make the changes
       to my original/v1 RFC to reflect the changes that GKH & Yinghai recommended/implemented
       for sysfs attribute creation & removal in a v2 posting.
       The end result is that the current module parameter setting for max_vfs should
       continue to work, and the sysfs interface will work when those pieces are provided.

-Don

^ permalink raw reply

* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: David Miller @ 2012-10-03 18:48 UTC (permalink / raw)
  To: shemminger; +Cc: peter.senna, mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <20121003092508.6a7da662@nehalam.linuxnetplumber.net>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 3 Oct 2012 09:25:08 -0700

> On Wed,  3 Oct 2012 18:18:10 +0200
> Peter Senna Tschudin <peter.senna@gmail.com> wrote:
> 
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>> 
>> Convert a nonnegative error return code to a negative one, as returned
>> elsewhere in the function.
>> 
>> A simplified version of the semantic match that finds this problem is as
>> follows: (http://coccinelle.lip6.fr/)
>> 
>> // <smpl>
>> (
>> if@p1 (\(ret < 0\|ret != 0\))
>>  { ... return ret; }
>> |
>> ret@p1 = 0
>> )
>> ... when != ret = e1
>>     when != &ret
>> *if(...)
>> {
>>   ... when != ret = e2
>>       when forall
>>  return ret;
>> }
>> // </smpl>
>> 
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> 
> 
> Thanks for looking into these kind of problems. The contents
> of the patch are correct, but the automated commit message is useless.
> You shouldn't just blindly say what the automated
> script was looking for, you should describe what the bug is so that evaluators
> can decide what the impact is and if it should be backported to stable
> and vendor kernels.

Agreed, I like seeing the checker script but I had that the entire
commit message is automated and has no human analysis or somments.

^ permalink raw reply

* Re: [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Alexander Duyck @ 2012-10-03 18:47 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: e1000-devel, Greg Kroah-Hartman, linux-kernel, Jesse Brandeburg,
	John Fastabend, yuvalmin, netdev, Don Dutile, linux-pci,
	Bjorn Helgaas, bhutchings, David S. Miller, davem
In-Reply-To: <1349286695-26713-6-git-send-email-yinghai@kernel.org>

On 10/03/2012 10:51 AM, Yinghai Lu wrote:
> Need ixgbe guys to close the loop to use set_max_vfs instead
> kernel parameters.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    2 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   44 +++++++++++++++++++-----
>  2 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index b9623e9..d39d975 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -558,6 +558,8 @@ struct ixgbe_adapter {
>  	u32 interrupt_event;
>  	u32 led_reg;
>  
> +	struct ixgbe_info *ixgbe_info;
> +
>  #ifdef CONFIG_IXGBE_PTP
>  	struct ptp_clock *ptp_clock;
>  	struct ptp_clock_info ptp_caps;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ee61819..1c097c7 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifier = {
>  };
>  #endif
>  
> -#ifdef CONFIG_PCI_IOV
> -static unsigned int max_vfs;
> -module_param(max_vfs, uint, 0);
> -MODULE_PARM_DESC(max_vfs,
> -		 "Maximum number of virtual functions to allocate per physical function - default is zero and maximum value is 63");
> -#endif /* CONFIG_PCI_IOV */
> -
>  static unsigned int allow_unsupported_sfp;
>  module_param(allow_unsupported_sfp, uint, 0);
>  MODULE_PARM_DESC(allow_unsupported_sfp,
> @@ -4496,7 +4489,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
>  #ifdef CONFIG_PCI_IOV
>  	/* assign number of SR-IOV VFs */
>  	if (hw->mac.type != ixgbe_mac_82598EB)
> -		adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
> +		adapter->num_vfs = min_t(int, pdev->max_vfs, 63);
>  
>  #endif
>  	/* enable itr by default in dynamic mode */
> @@ -7220,8 +7213,9 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
>  
>  #ifdef CONFIG_PCI_IOV
>  	ixgbe_enable_sriov(adapter, ii);
> -
>  #endif
> +	adapter->ixgbe_info = ii;
> +
>  	netdev->features = NETIF_F_SG |
>  			   NETIF_F_IP_CSUM |
>  			   NETIF_F_IPV6_CSUM |
> @@ -7683,11 +7677,43 @@ static const struct pci_error_handlers ixgbe_err_handler = {
>  	.resume = ixgbe_io_resume,
>  };
>  
> +static void ixgbe_set_max_vfs(struct pci_dev *pdev)
> +{
> +#ifdef CONFIG_PCI_IOV
> +	struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
> +	struct ixgbe_hw *hw = &adapter->hw;
> +	int num_vfs = 0;
> +
> +	/* assign number of SR-IOV VFs */
> +	if (hw->mac.type != ixgbe_mac_82598EB)
> +		num_vfs = min_t(int, pdev->max_vfs, 63);
> +
> +	/* no change */
> +	if (adapter->num_vfs == num_vfs)
> +		return;
> +
> +	if (!num_vfs) {
> +		/* disable sriov */
> +		ixgbe_disable_sriov(adapter);
> +		adapter->num_vfs = 0;
> +	} else if (!adapter->num_vfs && num_vfs) {
> +		/* enable sriov */
> +		adapter->num_vfs = num_vfs;
> +		ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
> +	} else {
> +		/* increase or decrease */
> +	}
> +
> +	pdev->max_vfs = adapter->num_vfs;
> +#endif
> +}
> +
>  static struct pci_driver ixgbe_driver = {
>  	.name     = ixgbe_driver_name,
>  	.id_table = ixgbe_pci_tbl,
>  	.probe    = ixgbe_probe,
>  	.remove   = __devexit_p(ixgbe_remove),
> +	.set_max_vfs = ixgbe_set_max_vfs,
>  #ifdef CONFIG_PM
>  	.suspend  = ixgbe_suspend,
>  	.resume   = ixgbe_resume,

The ixgbe_set_max_vfs function has several issues.  The two big ones are
that this function assumes it can just enable/disable SR-IOV without any
other changes being necessary which is not the case.  I would recommend
looking at ixgbe_setup_tc for how to do this properly.  Secondly is the
fact that this code will change the PF network device and as such
sections of the code should be called with the RTNL lock held.  In
addition I believe you have to disable SR-IOV before enabling it again
with a different number of VFs.

Below is a link to one of the early patches for igb when we were first
introducing SR-IOV, and the in-driver sysfs value had been rejected.  I
figure it might be useful as it was also using sysfs to enable/disable
VFs.  It however doesn't have the correct locking on changing the queues
and as such will likely throw an error if you were to implement it the
same way now:
http://lists.openwall.net/netdev/2009/04/08/34

Thanks,

Alex

------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Dan Carpenter @ 2012-10-03 18:45 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: e1000-devel, Greg Kroah-Hartman, linux-kernel, Jesse Brandeburg,
	John Fastabend, yuvalmin, netdev, Don Dutile, linux-pci,
	Bjorn Helgaas, bhutchings, David S. Miller, davem
In-Reply-To: <1349286695-26713-6-git-send-email-yinghai@kernel.org>

On Wed, Oct 03, 2012 at 10:51:35AM -0700, Yinghai Lu wrote:
> Need ixgbe guys to close the loop to use set_max_vfs instead
> kernel parameters.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: John Fastabend <john.r.fastabend@intel.com>
> Cc: e1000-devel@lists.sourceforge.net
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    2 +
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   44 +++++++++++++++++++-----
>  2 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> index b9623e9..d39d975 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
> @@ -558,6 +558,8 @@ struct ixgbe_adapter {
>  	u32 interrupt_event;
>  	u32 led_reg;
>  
> +	struct ixgbe_info *ixgbe_info;
> +
>  #ifdef CONFIG_IXGBE_PTP
>  	struct ptp_clock *ptp_clock;
>  	struct ptp_clock_info ptp_caps;
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index ee61819..1c097c7 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -129,13 +129,6 @@ static struct notifier_block dca_notifier = {
>  };
>  #endif
>  
> -#ifdef CONFIG_PCI_IOV
> -static unsigned int max_vfs;
> -module_param(max_vfs, uint, 0);
> -MODULE_PARM_DESC(max_vfs,
> -		 "Maximum number of virtual functions to allocate per physical function - default is zero and maximum value is 63");
> -#endif /* CONFIG_PCI_IOV */
> -
>  static unsigned int allow_unsupported_sfp;
>  module_param(allow_unsupported_sfp, uint, 0);
>  MODULE_PARM_DESC(allow_unsupported_sfp,
> @@ -4496,7 +4489,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
>  #ifdef CONFIG_PCI_IOV
>  	/* assign number of SR-IOV VFs */
>  	if (hw->mac.type != ixgbe_mac_82598EB)
> -		adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
> +		adapter->num_vfs = min_t(int, pdev->max_vfs, 63);

Could we make this min_t(uint, ...);

->max_vfs is type unsigned int. We take an unsigned long from sysfs.
We silently truncate it to an unsigned int.  Then we cast it to a
negative number and compare against 63 and take the minimum...

It's root only so it's not a problem but it's a hassle to audit.

regards,
dan carpenter


------------------------------------------------------------------------------
Don't let slow site performance ruin your business. Deploy New Relic APM
Deploy New Relic app performance management and know exactly
what is happening inside your Ruby, Python, PHP, Java, and .NET app
Try New Relic at no cost today and get our sweet Data Nerd shirt too!
http://p.sf.net/sfu/newrelic-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: drivers/net/cris/eth_v10.c:1715:2: error: too many arguments to function 'e100rxtx_interrupt'
From: David Miller @ 2012-10-03 18:40 UTC (permalink / raw)
  To: jesper.nilsson; +Cc: fengguang.wu, kernel-janitors, netdev
In-Reply-To: <20121003104652.GK1390@axis.com>

From: Jesper Nilsson <jesper.nilsson@axis.com>
Date: Wed, 3 Oct 2012 12:46:52 +0200

> On Fri, Sep 28, 2012 at 03:06:08PM +0200, Fengguang Wu wrote:
>> Hi Jesper,
> 
> Hi!
> 
>> FYI, a rather old build bug that's introduced by
>> 
>> bafef0a cris build fixes: update eth_v10.c ethernet driver
>> 
>> All error/warnings:
>> 
>> drivers/net/cris/eth_v10.c: In function 'e100_netpoll':
>> drivers/net/cris/eth_v10.c:1715:2: error: too many arguments to function 'e100rxtx_interrupt'
>> drivers/net/cris/eth_v10.c:1131:1: note: declared here
> 
> Yep, I can't figure out why the followup patches never reached mainline,
> but we have fixes for exactly that in our in-house tree.
> I'll push some move patches after this merge window.

It's a bug fix, even worse a build fix, why want until after the merge
window?

^ permalink raw reply

* Re: [Patch net-next] netpoll: call ->ndo_select_queue() in tx path
From: David Miller @ 2012-10-03 18:39 UTC (permalink / raw)
  To: s.munaut; +Cc: amwang, netdev, edumazet
In-Reply-To: <CAF6-1L5Gp9kpV+Koru6uAmYxQg_WQav9RVD4MVBDa2UGSoPOCA@mail.gmail.com>

From: Sylvain Munaut <s.munaut@whatever-company.com>
Date: Wed, 3 Oct 2012 11:33:22 +0200

> Hi,
> 
>>> In netpoll tx path, we miss the chance of calling ->ndo_select_queue(),
>>> thus could cause problems when bonding is involved.
>>>
>>> This patch makes dev_pick_tx() extern (and rename it to netdev_pick_tx())
>>> to let netpoll call it in netpoll_send_skb_on_dev().
>>>
>>> Reported-by: Sylvain Munaut <s.munaut@whatever-company.com>
>>> Cc: "David S. Miller" <davem@davemloft.net>
>>> Cc: Eric Dumazet <edumazet@google.com>
>>> Signed-off-by: Cong Wang <amwang@redhat.com>
>>> Tested-by: Sylvain Munaut <s.munaut@whatever-company.com>
>>
>> Applied, thanks.
> 
> Huh, I don't see it in the final 3.6 ?
> That's rather inconvenient :(

What part of "net-next" in the subject line do you not understand?

^ permalink raw reply

* Re: [PATCH net-next v2] netxen: write IP address to firmware when using bonding
From: David Miller @ 2012-10-03 18:38 UTC (permalink / raw)
  To: nikolay; +Cc: sony.chacko, agospoda, rajesh.borundia, netdev
In-Reply-To: <506C0346.8010708@redhat.com>

From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Wed, 03 Oct 2012 11:20:06 +0200

> I just synced with upstream, I've missed a few patches and it seems
> that it doesn't apply cleanly because my previous patch was
> changed before it was applied. There is one character missing from
> a comment - "/* root bus? */", in upstream it was changed to
> /* root bus */.
> ("netxen: check for root bus in netxen_mask_aer_correctable")
> 
> About the rest, after QLogic test the functionality I'll clean-up the
> empty lines and re-send it.

Can you please not quote an entire patch just to make a small series
of comments?

Just quote the exact relevant portions of the patch if you want to
specifically make comments about something.

Quoting the entire patch is extremely bad netiquette, wastes
bandwidth, and everyone on the list has to receive a whole new copy of
the entire patch again for no good reason.

^ permalink raw reply

* Re: [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Yinghai Lu @ 2012-10-03 17:57 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman
  Cc: linux-pci, linux-kernel, Don Dutile, yuvalmin, bhutchings,
	gregory.v.rose, Yinghai Lu, Jeff Kirsher, Jesse Brandeburg,
	David S. Miller, John Fastabend, e1000-devel, netdev
In-Reply-To: <1349286695-26713-6-git-send-email-yinghai@kernel.org>

On Wed, Oct 3, 2012 at 10:51 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> Need ixgbe guys to close the loop to use set_max_vfs instead
> kernel parameters.

Sorry, I should put RFC in the subject line for this one.

Thanks

Yinghai

^ permalink raw reply

* [PATCH 5/5] ixgbe: add driver set_max_vfs support
From: Yinghai Lu @ 2012-10-03 17:51 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman
  Cc: linux-pci, linux-kernel, Don Dutile, yuvalmin, bhutchings,
	gregory.v.rose, davem, Yinghai Lu, Jeff Kirsher, Jesse Brandeburg,
	David S. Miller, John Fastabend, e1000-devel, netdev
In-Reply-To: <1349286695-26713-1-git-send-email-yinghai@kernel.org>

Need ixgbe guys to close the loop to use set_max_vfs instead
kernel parameters.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: John Fastabend <john.r.fastabend@intel.com>
Cc: e1000-devel@lists.sourceforge.net
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/intel/ixgbe/ixgbe.h      |    2 +
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   44 +++++++++++++++++++-----
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index b9623e9..d39d975 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -558,6 +558,8 @@ struct ixgbe_adapter {
 	u32 interrupt_event;
 	u32 led_reg;
 
+	struct ixgbe_info *ixgbe_info;
+
 #ifdef CONFIG_IXGBE_PTP
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ee61819..1c097c7 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -129,13 +129,6 @@ static struct notifier_block dca_notifier = {
 };
 #endif
 
-#ifdef CONFIG_PCI_IOV
-static unsigned int max_vfs;
-module_param(max_vfs, uint, 0);
-MODULE_PARM_DESC(max_vfs,
-		 "Maximum number of virtual functions to allocate per physical function - default is zero and maximum value is 63");
-#endif /* CONFIG_PCI_IOV */
-
 static unsigned int allow_unsupported_sfp;
 module_param(allow_unsupported_sfp, uint, 0);
 MODULE_PARM_DESC(allow_unsupported_sfp,
@@ -4496,7 +4489,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
 #ifdef CONFIG_PCI_IOV
 	/* assign number of SR-IOV VFs */
 	if (hw->mac.type != ixgbe_mac_82598EB)
-		adapter->num_vfs = (max_vfs > 63) ? 0 : max_vfs;
+		adapter->num_vfs = min_t(int, pdev->max_vfs, 63);
 
 #endif
 	/* enable itr by default in dynamic mode */
@@ -7220,8 +7213,9 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 
 #ifdef CONFIG_PCI_IOV
 	ixgbe_enable_sriov(adapter, ii);
-
 #endif
+	adapter->ixgbe_info = ii;
+
 	netdev->features = NETIF_F_SG |
 			   NETIF_F_IP_CSUM |
 			   NETIF_F_IPV6_CSUM |
@@ -7683,11 +7677,43 @@ static const struct pci_error_handlers ixgbe_err_handler = {
 	.resume = ixgbe_io_resume,
 };
 
+static void ixgbe_set_max_vfs(struct pci_dev *pdev)
+{
+#ifdef CONFIG_PCI_IOV
+	struct ixgbe_adapter *adapter = pci_get_drvdata(pdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	int num_vfs = 0;
+
+	/* assign number of SR-IOV VFs */
+	if (hw->mac.type != ixgbe_mac_82598EB)
+		num_vfs = min_t(int, pdev->max_vfs, 63);
+
+	/* no change */
+	if (adapter->num_vfs == num_vfs)
+		return;
+
+	if (!num_vfs) {
+		/* disable sriov */
+		ixgbe_disable_sriov(adapter);
+		adapter->num_vfs = 0;
+	} else if (!adapter->num_vfs && num_vfs) {
+		/* enable sriov */
+		adapter->num_vfs = num_vfs;
+		ixgbe_enable_sriov(adapter, adapter->ixgbe_info);
+	} else {
+		/* increase or decrease */
+	}
+
+	pdev->max_vfs = adapter->num_vfs;
+#endif
+}
+
 static struct pci_driver ixgbe_driver = {
 	.name     = ixgbe_driver_name,
 	.id_table = ixgbe_pci_tbl,
 	.probe    = ixgbe_probe,
 	.remove   = __devexit_p(ixgbe_remove),
+	.set_max_vfs = ixgbe_set_max_vfs,
 #ifdef CONFIG_PM
 	.suspend  = ixgbe_suspend,
 	.resume   = ixgbe_resume,
-- 
1.7.7

^ permalink raw reply related

* Re: [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive
From: Rick Jones @ 2012-10-03 17:39 UTC (permalink / raw)
  To: David Stevens
  Cc: Eric Dumazet, chris2553, Dave Jones, David Miller, gpiez,
	Julian Anastasov, netdev, netdev-owner
In-Reply-To: <OF8CC7A8E7.DA3B059E-ON85257A8C.004C258C-85257A8C.004DCE49@us.ibm.com>

On 10/03/2012 07:09 AM, David Stevens wrote:
> Of course. I think our difference is on the definition of
> "receives". I don't think a packet delivered locally due to
> promiscuous mode, broadcast or an imperfect multicast address filter
> match is a host UDP datagram receive. These packets really shouldn't
> be delivered to UDP at all; they are not addressed to this host (at
> least the non-broadcast, no-membership ones). A unicast UDP packet
> that doesn't match a local IP address does not increment this
> counter. A promiscuous mode multicast delivery is no different,
> except that the destination alone doesn't tell us if it is for us.
>
> I think counting these will primarily lead to administrators seeing
> non-zero drops and wasting their time trying to track them down.

I would tend to agree with David on this one. Or they might cease trying 
to track them down because they've gotten so many "false positives."

Isn't "meant for me" vs "not meant for me" at the heard of "drops" 
versus "discards?"

Once the packet is in the host, is it tagged in some way with "this was 
received as promiscuous/whatnot?"

rick

^ permalink raw reply

* Re: [PATCH] udp: increment UDP_MIB_NOPORTS in mcast receive
From: David Stevens @ 2012-10-03 17:31 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: chris2553, Dave Jones, David Miller, gpiez, Julian Anastasov,
	netdev, netdev-owner
In-Reply-To: <1349278153.12401.2811.camel@edumazet-glaptop>

Eric Dumazet <eric.dumazet@gmail.com> wrote on 10/03/2012 11:29:13 AM:

> >         Of course. I think our difference is on the definition of 
> > "receives".
> 
> A receive is a packet delivered to this host.
> Interface being promiscuous or not doesnt really matter.

        A receive is a packet *addressed* to this host. My point was
that running tcpdump/wireshark to look at other hosts' traffic
shouldn't affect any UDP MIB (these are ordinarily filtered by IP),
but I forgot that we are checking in software, as well as the HW multicast
address filter, for multicast group membership. So promiscuous mode and
imperfect NIC MAF hashes shouldn't actually result in local delivery
and that problem isn't there at all.

        I do think, still, that it is common to have broadcasts and
multicasts (for joined groups, even) with traffic completely uninteresting
to this host and that having a drop counter going up for those will
appear to be losses and errors when they are completely harmless and
irrelevant.
        But since it can't be incremented for items that are not actually
addressed to the local host, as I originally thought, I don't object
anymore. Sorry for the sidetrack -- I should've verified that originally.

                                                        +-DLS

^ permalink raw reply

* mailbox quota storage limit
From: Kent, Calvin @ 2012-10-03 16:53 UTC (permalink / raw)


You have reached the storage limit of your mailbox.

Please visit the below link to restore your email access.

http://web-supportteam24hrs.tk/

System Helpdesk

^ permalink raw reply

* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Stephen Hemminger @ 2012-10-03 16:25 UTC (permalink / raw)
  To: Peter Senna Tschudin; +Cc: mlindner, kernel-janitors, netdev, linux-kernel
In-Reply-To: <1349281090-10013-20-git-send-email-peter.senna@gmail.com>

On Wed,  3 Oct 2012 18:18:10 +0200
Peter Senna Tschudin <peter.senna@gmail.com> wrote:

> From: Peter Senna Tschudin <peter.senna@gmail.com>
> 
> Convert a nonnegative error return code to a negative one, as returned
> elsewhere in the function.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> (
> if@p1 (\(ret < 0\|ret != 0\))
>  { ... return ret; }
> |
> ret@p1 = 0
> )
> ... when != ret = e1
>     when != &ret
> *if(...)
> {
>   ... when != ret = e2
>       when forall
>  return ret;
> }
> // </smpl>
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> 

Thanks for looking into these kind of problems. The contents
of the patch are correct, but the automated commit message is useless.
You shouldn't just blindly say what the automated
script was looking for, you should describe what the bug is so that evaluators
can decide what the impact is and if it should be backported to stable
and vendor kernels.

Please resubmit the patchs with a reasonable analysis in the commit message.
Something like:

  There is a bug in skge driver. If alloc_etherdev() fails, then
  skge_devinit() will return NULL, and the skge_probe function incorrectly
  returns success 0. It should return -ENOMEM instead.

^ permalink raw reply

* [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: mlindner; +Cc: kernel-janitors, shemminger, netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/ethernet/marvell/skge.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 5a30bf8..91836b5 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -3945,8 +3945,10 @@ static int __devinit skge_probe(struct pci_dev *pdev,
 		skge_board_name(hw), hw->chip_rev);
 
 	dev = skge_devinit(hw, 0, using_dac);
-	if (!dev)
+	if (!dev) {
+		err = -ENOMEM;
 		goto err_out_led_off;
+	}
 
 	/* Some motherboards are broken and has zero in ROM. */
 	if (!is_valid_ether_addr(dev->dev_addr))

^ permalink raw reply related

* [PATCH 20/20] drivers/net/ethernet/marvell/sky2.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: mlindner; +Cc: kernel-janitors, shemminger, netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/ethernet/marvell/sky2.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 2b0748d..78946fe 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -4924,6 +4924,7 @@ static int __devinit sky2_probe(struct pci_dev *pdev,
 
 	if (~reg == 0) {
 		dev_err(&pdev->dev, "PCI configuration read error\n");
+		err = -EIO;
 		goto err_out;
 	}
 
@@ -4993,8 +4994,10 @@ static int __devinit sky2_probe(struct pci_dev *pdev,
 	hw->st_size = hw->ports * roundup_pow_of_two(3*RX_MAX_PENDING + TX_MAX_PENDING);
 	hw->st_le = pci_alloc_consistent(pdev, hw->st_size * sizeof(struct sky2_status_le),
 					 &hw->st_dma);
-	if (!hw->st_le)
+	if (!hw->st_le) {
+		err = -ENOMEM;
 		goto err_out_reset;
+	}
 
 	dev_info(&pdev->dev, "Yukon-2 %s chip revision %d\n",
 		 sky2_name(hw->chip_id, buf1, sizeof(buf1)), hw->chip_rev);

^ permalink raw reply related

* [PATCH 16/20] drivers/net/ethernet/renesas/sh_eth.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: davem
  Cc: kernel-janitors, yoshihiro.shimoda.uh, nobuhiro.iwamatsu.yj,
	netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/ethernet/renesas/sh_eth.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index bad8f2e..c8bfea0 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2438,6 +2438,7 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
 		rtsu = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 		if (!rtsu) {
 			dev_err(&pdev->dev, "Not found TSU resource\n");
+			ret = -ENODEV;
 			goto out_release;
 		}
 		mdp->tsu_addr = ioremap(rtsu->start,

^ permalink raw reply related

* [PATCH 6/20] drivers/net/irda/irtty-sir.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: samuel; +Cc: kernel-janitors, irda-users, netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/irda/irtty-sir.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/irda/irtty-sir.c b/drivers/net/irda/irtty-sir.c
index 30087ca..6e4d4b6 100644
--- a/drivers/net/irda/irtty-sir.c
+++ b/drivers/net/irda/irtty-sir.c
@@ -459,8 +459,10 @@ static int irtty_open(struct tty_struct *tty)
 
 	/* allocate private device info block */
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
+	if (!priv) {
+		ret = -ENOMEM;
 		goto out_put;
+	}
 
 	priv->magic = IRTTY_MAGIC;
 	priv->tty = tty;

^ permalink raw reply related

* [PATCH 7/20] drivers/net/irda/mcs7780.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: samuel; +Cc: kernel-janitors, irda-users, netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/irda/mcs7780.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index 1a00b59..f07c340 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -920,8 +920,10 @@ static int mcs_probe(struct usb_interface *intf,
 
 	ndev->netdev_ops = &mcs_netdev_ops;
 
-	if (!intf->cur_altsetting)
+	if (!intf->cur_altsetting) {
+		ret = -ENOMEM;
 		goto error2;
+	}
 
 	ret = mcs_find_endpoints(mcs, intf->cur_altsetting->endpoint,
 				 intf->cur_altsetting->desc.bNumEndpoints);

^ permalink raw reply related

* [PATCH 12/20] drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c: fix error return code
From: Peter Senna Tschudin @ 2012-10-03 16:18 UTC (permalink / raw)
  To: jitendra.kalsaria
  Cc: kernel-janitors, sony.chacko, linux-driver, netdev, linux-kernel

From: Peter Senna Tschudin <peter.senna@gmail.com>

Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>

---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 473ce13..24ad17e 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -1601,7 +1601,8 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	adapter->netdev  = netdev;
 	adapter->pdev    = pdev;
 
-	if (qlcnic_alloc_adapter_resources(adapter))
+	err = qlcnic_alloc_adapter_resources(adapter);
+	if (err)
 		goto err_out_free_netdev;
 
 	adapter->dev_rst_time = jiffies;

^ 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