Netdev List
 help / color / mirror / Atom feed
* Re: TCP sockets stalling - help! (long)
From: Asdo @ 2009-11-25 16:38 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: e1000-devel, Netdev
In-Reply-To: <4B0D4EFC.4050302@shiftmail.org>

Asdo wrote:
> Asdo wrote:
>> Ilpo Järvinen wrote:
>>  
>>> ...I'd next try strace the sftp server to see what it was doing 
>>> during the stall.
>>>       
>> Thanks for your help Ilpo
>>
>> Isn't the strace equivalent to the stack trace I obtained via cat 
>> /proc/pid/stack reported previously? That was at the time of the stall
>>
>> I'm thinking the strace would slow down sftp-server very deeply...
>>   
>
> I found out that if I attach the strace I can see at least the last 
> function call.
> The SFTP it's hanged right now so I did that:
>
> root@mystorage:/root# strace -p 11475
> Process 11475 attached - interrupt to quit
> select(5, [3], [], NULL, NULL
>
> (stuck here forever... doesn't move)
> (it's strange the first option of select is 5, shouldn't it be 4 from 
> man select? A bug of strace maybe?)
>
> root@mystorage:/root# cat /proc/11475/stack
> [<ffffffff8112e644>] poll_schedule_timeout+0x34/0x50
> [<ffffffff8112ef4f>] do_select+0x58f/0x6b0
> [<ffffffff8112f8b5>] core_sys_select+0x185/0x2b0
> [<ffffffff8112fc32>] sys_select+0x42/0x110
> [<ffffffff8101225b>] tracesys+0xd9/0xde
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> And this is from cat /proc/net/tcp   2: 0F12A8C0:0016 2512A8C0:0FBD 01 
> 00000000:00000000 02:00009144 00000000     0        0 5326251 2 
> ffff88085408ce00 26 4 1 9 4
>
> The select refers to open files so here they are:
>
>
> root@mystorage:/proc/11475/fd# ll
> total 0
> lr-x------ 1 ccosentino wetlab 64 2009-11-25 14:43 0 -> pipe:[5326309]
> l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 1 -> pipe:[5326310]
> l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 2 -> pipe:[5326311]
> lr-x------ 1 ccosentino wetlab 64 2009-11-25 14:43 3 -> pipe:[5326309]
> l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 4 -> pipe:[5326310]
> l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 5 -> 
> /path/to/file_being_saved.filepart
>
> I tried to send SIGSTOP and then SIGCONT to see if I could make it 
> make a loop and then reenter into the select. I'm not sure it really 
> did that, what do you think? This is the strace:
> root@mystorage:/root# strace -p 11475 2>&1 | tee sftpstrace.dmp
> Process 11475 attached - interrupt to quit
> select(5, [3], [], NULL, NULL)          = ? ERESTARTNOHAND (To be 
> restarted)
> --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> select(5, [3], [], NULL, NULL)          = ? ERESTARTNOHAND (To be 
> restarted)
> --- SIGCONT (Continued) @ 0 (0) ---
> select(5, [3], [], NULL, NULL
>
> (hanged again here)
>
>
> Do you think this info is enough or I really have to strace it since 
> the beginning?
> If it is a race condition it might not happen if the sftp-server is 
> deeply slowed down by the strace.
> If I had a way to make it continue right now we could get the rest of 
> the strace... But it's not so easy, I tried starting a Samba transfer 
> but it did not unlock the SFTP this time. SIGSTOP + SIGCONT also 
> didn't work.
>
> BTW people using the Storage also experienced data loss while pushing 
> files in it: appartently data disappeared from the middle of a file 
> they were saving to the Storage.
> To me looks like another hint that application-level data which has 
> been received via network by TCP stack is trapped there and not being 
> pushed to the application.
> Or the data might even be trapped into the anonymous sockets between 
> sshd and sftp-server.
>
> Thanks for your help

I thought that it would be more meaningful to try the cat stack, the 
strace and the SIGSTOP SIGCONT tricks on the sshd which actually holds 
the TCP socket, instead of the sftp-server. Here they are, on the sshd:

root@mystorage:/root# strace -p 11449 2>&1 | tee -a sshd1strace1.dmp
Process 11449 attached - interrupt to quit
read(5, 0x7fff4e956220, 4)              = ? ERESTARTSYS (To be restarted)
--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
read(5, 0x7fff4e956220, 4)              = ? ERESTARTSYS (To be restarted)
--- SIGCONT (Continued) @ 0 (0) ---
read(5,

(hanged again here after the SIGSTOP + SIGCONT, unfortunately)

root@mystorage:/root# cat /proc/11449/stack
[<ffffffff814b428a>] unix_stream_data_wait+0xaa/0x110
[<ffffffff814b50cd>] unix_stream_recvmsg+0x36d/0x570
[<ffffffff81426399>] sock_aio_read+0x149/0x150
[<ffffffff8111e232>] do_sync_read+0xf2/0x130
[<ffffffff8111e8e1>] vfs_read+0x181/0x1a0
[<ffffffff8111edec>] sys_read+0x4c/0x80
[<ffffffff8101225b>] tracesys+0xd9/0xde
[<ffffffffffffffff>] 0xffffffffffffffff

root@mystorage:/proc/11449/fd# ll
total 0
lrwx------ 1 root root 64 2009-11-25 14:43 0 -> /dev/null
lrwx------ 1 root root 64 2009-11-25 14:43 1 -> /dev/null
lrwx------ 1 root root 64 2009-11-25 14:43 2 -> /dev/null
lrwx------ 1 root root 64 2009-11-25 14:43 3 -> socket:[5326251]
lrwx------ 1 root root 64 2009-11-25 14:43 5 -> socket:[5326303]


I would like to try to attach a program of mine (a python interactive) 
to "socket:[5326303]" and try to read data from there to see if 
something comes out of the TCP stack, but I don't know where to find the 
socket:[5326303] on the filesystem. Is it mapped to a file anywhere in 
Linux?

Thank you



------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

^ permalink raw reply

* Re: large packet loss take2 2.6.31.x
From: Caleb Cushing @ 2009-11-25 16:47 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: e1000-devel, netdev, Frans Pop, Jesse Brandeburg, linux-kernel,
	Andi Kleen, Jeff Kirsher
In-Reply-To: <81bfc67a0911250606k1ec76354n217da9ca20b3517c@mail.gmail.com>

> yeah 2.6.31.6 works on my laptop fine I'll just have to see about
> getting a direct connection to it. probably do that after I bring the
> other NIC back from home, on thanksgiving.

meh scratch that sorta.. screen mounting brackets/joint on laptop
broke... I'm thinking of scrapping it for a netbook. but replacing may
take a few weeks.

-- 
Caleb Cushing

http://xenoterracide.blogspot.com

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

^ permalink raw reply

* Re: Diagnostic Monitoring Interface Monitoring (DOM) PATCH 0/5 for net-next-2.6
From: robert @ 2009-11-25 16:22 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: Robert Olsson, David Miller, netdev
In-Reply-To: <9929d2390911250119h3e67fb50u2e9d8d3825b0496d@mail.gmail.com>


Jeff Kirsher writes:

 > FYI- This patch series is riddled with trailing whitespace, when I
 > imported that patches into my tree, I cleaned up all the whitespace
 > errors.  After some internal testing and review, 

 > I can re-post the entire series which has been cleaned up.

 Please do.

 Cheers
					--ro

^ permalink raw reply

* Re: Diagnostic Monitoring Interface Monitoring (DOM) PATCH 0/5 for net-next-2.6
From: robert @ 2009-11-25 16:21 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: Robert Olsson, David Miller, netdev
In-Reply-To: <9929d2390911250018o777405ddk257605e7d0b16d17@mail.gmail.com>

Jeff Kirsher writes:

 > Robert, to help get more eyes on this I have added it to my tree for
 > internal testing and review.

 Fine. The igb patch was tested with a 4-port 82576 board from Interface 
 Masters but they forgot to mount the pull-up resistors for the I2C-bus 
 on my prototype which took some weeks to debug. Anyway there are working 
 boards now.


 About the the Intel brand 82599 board with fixed SFP's. I don't see any DOM 
 support from them. It could be idea check if my reading from I2c is correct.

 Seems I'll have a SFP+ with DOM support soon.

 Cheers

	      	     	       	   	   --ro

^ permalink raw reply

* Re: dmfe/tulip device id overlap
From: Grant Grundler @ 2009-11-25 17:24 UTC (permalink / raw)
  To: Brandon Philips
  Cc: Tobias Ringstrom, Kyle McMartin, davem, netdev, Grant Grundler
In-Reply-To: <20091125044944.GL14043@jenkins.home.ifup.org>

On Tue, Nov 24, 2009 at 8:49 PM, Brandon Philips <brandon@ifup.org> wrote:
> Adding Tobias and Grant under (hopefully) working email addresses.
>
> Can you please submit patches to update MAINTAINERS with your
> preferred email?

grundler@parisc-linux.org is my preferred email for tulip driver
issues. Is that bouncing? :(


> On 20:36 Tue 24 Nov 2009, Brandon Philips wrote:
>> Hello All-
>>
>> dmfe and tulip have an overlap of device IDs and it has been discussed
>> before without resolution[1][2].
>>
>> The device ID in particular is:
>>
>>       { 0x1282, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
>>       { 0x1282, 0x9102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
>>
>> Users of Fedora[3] and openSUSE[4] are feeling pain from this but
>> Ubuntu users are avoiding the issue do to the patch below.
>>
>> This patch comes almost directly from the discussion on the
>> linux-sparc thread[2]. So, can we merge that patch? Or can one of the
>> maintainers come up with a less hacky solution (e.g. fix tulip)?

I'm totally ok with "clipping" the DMFE ids so tulip and dmfe drivers
have no overlap. I just can't test it since I don't have those cards.
It sounds like people know which driver works on sparc.


>>
>> Cheers,
>>
>>       Brandon
>>
>> [1] http://www.mail-archive.com/debian-sparc@lists.debian.org/msg21647.html
>> [2] http://marc.info/?l=linux-sparc&m=123698905214250&w=2
>> [3] https://bugzilla.redhat.com/show_bug.cgi?id=277731
>> [4] https://bugzilla.novell.com/show_bug.cgi?id=537016
>>
>> From: Ben Collins <bcollins@ubuntu.com>
>> Date: Wed, 18 Jul 2007 17:48:25 +0000 (-0400)
>> Subject: UBUNTU: SAUCE: tulip: Let dmfe handle davicom on non-sparc
>> X-Git-Tag: Ubuntu-2.6.31-10.31~680
>> X-Git-Url: http://kernel.ubuntu.com/git?p=ubuntu%2Fubuntu-karmic.git;a=commitdiff_plain;h=886595ab493b5c5fcf23b55b3ebf46bfe219a5d0
>>
>> UBUNTU: SAUCE: tulip: Let dmfe handle davicom on non-sparc
>>
>> Signed-off-by: Ben Collins <ben.collins@canonical.com>

I'm ok with this patch except the mention of Ubuntu in the comment is
superfluous. All the distro's will share this problem. I trust davem
to rewrite the comment and plase add my:
    Signed-off-by: Grant Grundler <grundler@parisc-linux.org>

cheers,
grant

>> ---
>>
>> diff --git a/drivers/net/tulip/tulip_core.c b/drivers/net/tulip/tulip_core.c
>> index 4cf9a65..b75f65c 100644
>> --- a/drivers/net/tulip/tulip_core.c
>> +++ b/drivers/net/tulip/tulip_core.c
>> @@ -228,8 +228,12 @@ static struct pci_device_id tulip_pci_tbl[] = {
>>       { 0x1259, 0xa120, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMET },
>>       { 0x11F6, 0x9881, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMPEX9881 },
>>       { 0x8086, 0x0039, PCI_ANY_ID, PCI_ANY_ID, 0, 0, I21145 },
>> +     /* Ubuntu: On non-sparc, this seems to be handled better by the
>> +      * dmfe driver. */
>> +#ifdef __sparc__
>>       { 0x1282, 0x9100, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
>>       { 0x1282, 0x9102, PCI_ANY_ID, PCI_ANY_ID, 0, 0, DM910X },
>> +#endif
>>       { 0x1113, 0x1216, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMET },
>>       { 0x1113, 0x1217, PCI_ANY_ID, PCI_ANY_ID, 0, 0, MX98715 },
>>       { 0x1113, 0x9511, PCI_ANY_ID, PCI_ANY_ID, 0, 0, COMET },
>

^ permalink raw reply

* Re: [RFC PATCH 00/10] Remove skb_dma_map/unmap calls
From: Alexander Duyck @ 2009-11-25 17:25 UTC (permalink / raw)
  To: Ajit Khaparde
  Cc: mcarlson@broadcom.com, mchan@broadcom.com,
	sathyap@serverengines.com, subbus@serverengines.com,
	davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20091125113050.GA19837@serverengines.com>

Ajit Khaparde wrote:
> On 24/11/09 17:20 -0800, Alexander Duyck wrote:
>> This patch series removes the skb_dma_map and skb_dma_unmap function calls.
>> The reason for this change is because the use of skb_dma_map/unmap can lead
>> to serious issues when HW IOMMU is enabled.  This is because each mapping
>> of the skb with a HW IOMMU enabled results in a new set of DMA mappings.
>> This in turn leads to issues when skbs are cloned for uses such as
>> bridging or pktgen because each transmitting device will update the skb
>> shared info structure resulting in some mappings being overwritten, and others
>> being freed multiple times.
> 
> If this is the case can the members related to the dma mapping stuff
> (skb_shinfo(skb)->dma_maps) be moved out of this shared info structure and
> we retain this "good" abstraction provided by this skb_dma_map/unmap api?

I had submitted a patch on November 5th that tried to move the maps out 
of the shared info structure and into the sk_buff itself.  That was 
rejected and it was suggested to go back to what was there before since 
not enough drivers were using the skb_dma_map/unmap calls to justify 
increasing the size of an sk_buff.

The biggest issue is that you end up needing to retain a copy of the DMA 
mapping output each time it is called and so the easiest place to retain 
the information is within the transmit buffer driver specific data 
structure.

>> I am looking for input specifically on the tg3, be2net, and bnx2 driver
>> patches as I am not very familiar with them and I am not certain if
>> additional changes are required.
> 
>>       be2net: remove use of skb_dma_map/unmap
>>
>>  drivers/net/benet/be_main.c    |   37 ++++++---
> 
> We have pulled the be2net specific patch for testing and review.
> We will send an update once done with it.
> 
> -Ajit

Thanks for taking a look at this.

Alex

^ permalink raw reply

* vlan: support "loose binding" to the underlying network device
From: Patrick McHardy @ 2009-11-25 17:54 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

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



[-- Attachment #2: 01.diff --]
[-- Type: text/x-patch, Size: 3496 bytes --]

commit 3cf29fff45d8671adf234c17e59681aab222a81d
Author: Patrick McHardy <kaber@trash.net>
Date:   Wed Nov 25 16:28:27 2009 +0100

    vlan: support "loose binding" to the underlying network device
    
    Currently the UP/DOWN state of VLANs is synchronized to the state of the
    underlying device, meaning all VLANs are set down once the underlying
    device is set down. This causes all routes to the VLAN devices to vanish.
    
    Add a flag to specify a "loose binding" mode, in which only the operstate
    is transfered, but the VLAN device state is independant.
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 153f6b9..3d870fd 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -339,6 +339,7 @@ enum vlan_ioctl_cmds {
 enum vlan_flags {
 	VLAN_FLAG_REORDER_HDR	= 0x1,
 	VLAN_FLAG_GVRP		= 0x2,
+	VLAN_FLAG_LOOSE_BINDING	= 0x4,
 };
 
 enum vlan_name_types {
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 1483243..225aa2f 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -431,6 +431,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 	struct vlan_group *grp;
 	int i, flgs;
 	struct net_device *vlandev;
+	struct vlan_dev_info *vlan;
 	LIST_HEAD(list);
 
 	if (is_vlan_dev(dev))
@@ -507,7 +508,9 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 			if (!(flgs & IFF_UP))
 				continue;
 
-			dev_change_flags(vlandev, flgs & ~IFF_UP);
+			vlan = vlan_dev_info(vlandev);
+			if (!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
+				dev_change_flags(vlandev, flgs & ~IFF_UP);
 			vlan_transfer_operstate(dev, vlandev);
 		}
 		break;
@@ -523,7 +526,9 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 			if (flgs & IFF_UP)
 				continue;
 
-			dev_change_flags(vlandev, flgs | IFF_UP);
+			vlan = vlan_dev_info(vlandev);
+			if (!(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
+				dev_change_flags(vlandev, flgs | IFF_UP);
 			vlan_transfer_operstate(dev, vlandev);
 		}
 		break;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index de0dc6b..b788978 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -431,7 +431,8 @@ int vlan_dev_change_flags(const struct net_device *dev, u32 flags, u32 mask)
 	struct vlan_dev_info *vlan = vlan_dev_info(dev);
 	u32 old_flags = vlan->flags;
 
-	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP))
+	if (mask & ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
+		     VLAN_FLAG_LOOSE_BINDING))
 		return -EINVAL;
 
 	vlan->flags = (old_flags & ~mask) | (flags & mask);
@@ -456,7 +457,8 @@ static int vlan_dev_open(struct net_device *dev)
 	struct net_device *real_dev = vlan->real_dev;
 	int err;
 
-	if (!(real_dev->flags & IFF_UP))
+	if (!(real_dev->flags & IFF_UP) &&
+	    !(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
 		return -ENETDOWN;
 
 	if (compare_ether_addr(dev->dev_addr, real_dev->dev_addr)) {
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
index 3c9cf6a..ddc1057 100644
--- a/net/8021q/vlan_netlink.c
+++ b/net/8021q/vlan_netlink.c
@@ -60,7 +60,8 @@ static int vlan_validate(struct nlattr *tb[], struct nlattr *data[])
 	if (data[IFLA_VLAN_FLAGS]) {
 		flags = nla_data(data[IFLA_VLAN_FLAGS]);
 		if ((flags->flags & flags->mask) &
-		    ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP))
+		    ~(VLAN_FLAG_REORDER_HDR | VLAN_FLAG_GVRP |
+		      VLAN_FLAG_LOOSE_BINDING))
 			return -EINVAL;
 	}
 

^ permalink raw reply related

* Re: large packet loss take2 2.6.31.x
From: Jarek Poplawski @ 2009-11-25 19:11 UTC (permalink / raw)
  To: Caleb Cushing
  Cc: e1000-devel, netdev, Frans Pop, Jesse Brandeburg, linux-kernel,
	Andi Kleen, Jeff Kirsher
In-Reply-To: <81bfc67a0911250606k1ec76354n217da9ca20b3517c@mail.gmail.com>

On Wed, Nov 25, 2009 at 09:06:30AM -0500, Caleb Cushing wrote:
> > Anyway, since there are problems with
> > getting stats from the router we still can't compare them, or check
> > for the dropped stats. (Btw, could you check for /proc/net/softnet_stat
> > yet?)
> 
> router? good kernel? bad kernel?

router.

> 
> > So, it might be the kernel problem you reported, but there is not
> > enough data to prove it. Then my proposal is to try to repeat this
> > problem in more "testing friendly" conditions - preferably against
> > some other, more up-to-date linux box, if possible?
> 
> yeah 2.6.31.6 works on my laptop fine I'll just have to see about
> getting a direct connection to it. probably do that after I bring the
> other NIC back from home, on thanksgiving.

This other NIC is a really good idea, so let's wait and see.

Happy Thanksgiving!
Jarek P.

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

^ permalink raw reply

* Re: [PATCH v2 0/7] via-velocity performance fixes
From: David Miller @ 2009-11-25 20:18 UTC (permalink / raw)
  To: simon.kagstrom; +Cc: netdev, davej, shemminger, romieu
In-Reply-To: <20091125083943.2ec05b70@marrow.netinsight.se>

From: Simon Kagstrom <simon.kagstrom@netinsight.net>
Date: Wed, 25 Nov 2009 08:39:43 +0100

> On Tue, 24 Nov 2009 12:13:39 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
>> Simon I'm still waiting for you to fix the transmit function return
>> value problems I pointed out in patch #5.
>> 
>> Please get to this so I can apply your patches.
> 
> Sorry, I missed that comment. I only subscribed to netdev after having
> sent out the patches, so I didn't get that reply. I've looked it up now
> (and I'm subscribed now!), so I'll send out a new version.

Your email address was the To: field in the reply, there is no reason
you should not have received the email, here are the email headers:

--------------------
Subject: Re: [PATCH v2 5/7] via-velocity: Re-enable transmit scatter-gather support
From: David Miller <davem@davemloft.net>
To: simon.kagstrom@netinsight.net
Cc: netdev@vger.kernel.org, davej@redhat.com, shemminger@vyatta.com,
 romieu@fr.zoreil.com
Date: Mon, 23 Nov 2009 10:52:21 -0800 (PST)
X-Mailer: Mew version 6.2.51 on Emacs 22.1 / Mule 5.0 (SAKAKI)
--------------------

So being subscribed to netdev has nothing to do with it, you simply
didn't get a direct email I sent to you for one reason or another.

^ permalink raw reply

* Re: Diagnostic Monitoring Interface Monitoring (DOM) PATCH 0/5 for net-next-2.6
From: Joe Perches @ 2009-11-25 20:38 UTC (permalink / raw)
  To: robert; +Cc: Jeff Kirsher, David Miller, netdev
In-Reply-To: <19213.23008.292298.804077@gargle.gargle.HOWL>

On Wed, 2009-11-25 at 17:22 +0100, robert@herjulf.net wrote:
> Jeff Kirsher writes:
>  > FYI- This patch series is riddled with trailing whitespace, when I
>  > imported that patches into my tree, I cleaned up all the whitespace
>  > errors.  After some internal testing and review, 
>  > I can re-post the entire series which has been cleaned up.
>  Please do.

Here's another attempt as a complete patch.

Differences from Robert's original:

Whitespace cleanups
Changed presumed coding error from:
	if (type & DOM_TYPE_DOM & DOM_TYPE_ADDR_CHNGE) {
to:
	if (type & DOM_TYPE_ADDR_CHNGE)  {
Added read_phy_diag_u32 function
Refactored multiple sequential register reads to
for loop over struct array with register read

Functional difference from my original patch:
Fixed ARRAY_SIZE(basic) misuse

Reduces code size:

was:
$ size drivers/net/igb/igb_ethtool.o drivers/net/ixgbe/ixgbe_ethtool.o
   text    data     bss     dec     hex filename
  26466    1544    8584   36594    8ef2 drivers/net/igb/igb_ethtool.o
  31726    2980    9128   43834    ab3a drivers/net/ixgbe/ixgbe_ethtool.o

is:
$ size drivers/net/igb/igb_ethtool.o drivers/net/ixgbe/ixgbe_ethtool.o
   text	   data	    bss	    dec	    hex	filename
  24642	   1544	   8008	  34194	   8592	drivers/net/igb/igb_ethtool.o
  29922	   2980	   8560	  41462	   a1f6	drivers/net/ixgbe/ixgbe_ethtool.o

Signed-off-by: Joe Perches <joe@perches.com>

 Documentation/networking/dom.txt  |   41 ++++++++
 drivers/net/igb/igb_ethtool.c     |  191 +++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethtool.c |  171 +++++++++++++++++++++++++++++++++-
 include/linux/ethtool.h           |   49 ++++++++++
 include/net/dom.h                 |  109 +++++++++++++++++++++
 net/core/ethtool.c                |   18 ++++
 6 files changed, 578 insertions(+), 1 deletions(-)

diff --git a/Documentation/networking/dom.txt b/Documentation/networking/dom.txt
new file mode 100644
index 0000000..4bf24de
--- /dev/null
+++ b/Documentation/networking/dom.txt
@@ -0,0 +1,41 @@
+Diagnostic Monitoring Interface Monitoring also called DOM is a specification
+for optical transceivers to allow link and other diagnostics related to the
+transceiver's to standardized and communicated. For communication the I2C
+bus is used. The SFF specifications is available at ftp://ftp.seagate.com/
+Specification is generic and should work for many types of optical modules
+as GBIC, SFP, SFp+, XFP etc
+
+In short DOM spec adds a memory page where the diagnostics's is kept (address
+0xA2 bytes 66 to 105) but there are lot's of option's and variants. For example
+alarm and warnings are optional. See example below.
+
+Not all SFP's (SFP is for GIGE) have DOM support normally long range supports
+DOM. And of course your board and driver needs this support too.
+For SFP+ (10G) DOM is mandatory.
+
+Linux kernel support is via ethertool.
+
+include/net/dom.h        # Basic definitions
+net/core/ethtool.c       # adds ethtool_phy_diag()
+include/linux/ethtool.h  # adds ETHTOOL_GPHYDIAG
+
+And drivers hooks exists currently in igb and ixgbe driver
+
+Usage example: ethtool -D eth5
+
+Ext-Calbr: Avr RX-Power: Alarm & Warn: RX_LOS: 	Wavelength: 1310 nm
+Alarms, warnings in beginning of line, Ie. AH = Alarm High, WL == Warn Low etc
+	Temp:  35.9 C			Thresh: Lo: -12.0/-8.0   Hi: 103.0/110.0 C
+	Vcc:  3.33 V			Thresh: Lo:   3.0/3.0    Hi:   3.7/4.0   V
+	Tx-Bias:  13.4 mA		Thresh: Lo:   2.0/4.0    Hi:  70.0/84.0  mA
+ALWL	TX-pwr:  -5.9 dBm ( 0.26 mW)	Thresh: Lo:  -4.0/-2.0   Hi:   7.0/8.2   dBm
+AHWH	RX-pwr:  -5.0 dBm ( 0.31 mW) 	Thresh: Lo: -35.2/-28.0  Hi:  -8.2/-6.0  dBm
+
+
+First line shows the options supported by the module. As we see this module
+supports Alarms and warnings as a consequence thresholds are printed. As example
+RX-pwr: -35.2 < no_alarm < -6.0 dBm and -28.0 < no_warning < -8.2 dBm. (dBm yields
+1 mW == 0 dBm)
+
+In the example above we see both warnings for both TX-pwr (low) and RX-Pwr high.
+The Rx side would need some attenuation.
diff --git a/drivers/net/igb/igb_ethtool.c b/drivers/net/igb/igb_ethtool.c
index ac9d527..63a297d 100644
--- a/drivers/net/igb/igb_ethtool.c
+++ b/drivers/net/igb/igb_ethtool.c
@@ -34,6 +34,7 @@
 #include <linux/interrupt.h>
 #include <linux/if_ether.h>
 #include <linux/ethtool.h>
+#include <net/dom.h>
 #include <linux/sched.h>
 
 #include "igb.h"
@@ -2063,6 +2064,195 @@ static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 	}
 }
 
+static s32 read_phy_diag(struct e1000_hw *hw, u8 page, u8 offset, u16 *data)
+{
+	u32 i, i2ccmd = 0;
+
+	if (offset > E1000_MAX_SGMII_PHY_REG_ADDR) {
+		hw_dbg("DOM Register Address %u is out of range\n", offset);
+		return -E1000_ERR_PARAM;
+	}
+
+	/*
+	 * Set up Op-code, Phy Address, and register address in the I2CCMD
+	 * register.  The MAC will take care of interfacing with the
+	 * PHY to retrieve the desired data.
+	 */
+
+	i2ccmd = (E1000_I2CCMD_OPCODE_READ) |
+		 (page << E1000_I2CCMD_PHY_ADDR_SHIFT) |
+		 (offset << E1000_I2CCMD_REG_ADDR_SHIFT);
+
+	wr32(E1000_I2CCMD, i2ccmd);
+
+	/* Poll the ready bit to see if the I2C read completed */
+	for (i = 0; i < E1000_I2CCMD_PHY_TIMEOUT; i++) {
+		udelay(50);
+		i2ccmd = rd32(E1000_I2CCMD);
+		//printk("DATA i2ccmd=0x%x\n", i2ccmd);
+		if (i2ccmd & E1000_I2CCMD_READY)
+			break;
+	}
+	if (!(i2ccmd & E1000_I2CCMD_READY)) {
+		hw_dbg("I2CCMD Read did not complete\n");
+		return -E1000_ERR_PHY;
+	}
+	if (i2ccmd & E1000_I2CCMD_ERROR) {
+		hw_dbg("I2CCMD Error bit set\n");
+		return -E1000_ERR_PHY;
+	}
+
+	/* Need to byte-swap the 16-bit value. */
+	*data = ((i2ccmd >> 8) & 0x00FF) | ((i2ccmd << 8) & 0xFF00);
+	return 0;
+}
+
+static s32 read_phy_diag_u32(struct e1000_hw *hw, u8 page, u8 offset, u32 *data)
+{
+	u16 p1;
+	u16 p2;
+	s32 res;
+
+	res = read_phy_diag(hw, page, offset, &p1);
+	if (res)
+		goto out;
+
+	res = read_phy_diag(hw, page, offset + 2, &p2);
+	if (res)
+		goto out;
+
+	*data = ((u32)p1) << 16 | p2;
+
+out:
+	return res;
+}
+
+struct reg_offset {
+	int reg;
+	size_t offset;
+};
+
+#define REG_OFFSET(a, b) \
+	{ .reg = a, .offset = offsetof(struct ethtool_phy_diag, b) }
+
+int igb_get_phy_diag(struct net_device *netdev, struct ethtool_phy_diag *pd)
+{
+	struct igb_adapter *adapter = netdev_priv(netdev);
+	struct e1000_hw *hw = &adapter->hw;
+	int res;
+	u8 type, eo;
+	int i;
+
+	static const struct reg_offset basic[] = {
+		REG_OFFSET(DOM_A2_TEMP, temp),
+		REG_OFFSET(DOM_A2_TEMP_SLOPE, temp_slope),
+		REG_OFFSET(DOM_A2_TEMP_OFFSET, temp_offset),
+		REG_OFFSET(DOM_A2_VCC, vcc),
+		REG_OFFSET(DOM_A2_VCC_SLOPE, vcc_slope),
+		REG_OFFSET(DOM_A2_VCC_OFFSET, vcc_offset),
+		REG_OFFSET(DOM_A2_TX_BIAS, tx_bias),
+		REG_OFFSET(DOM_A2_TX_I_SLOPE, tx_bias_slope),
+		REG_OFFSET(DOM_A2_TX_I_OFFSET, tx_bias_offset),
+		REG_OFFSET(DOM_A2_TX_PWR, tx_pwr),
+		REG_OFFSET(DOM_A2_TX_PWR_SLOPE, tx_pwr_slope),
+		REG_OFFSET(DOM_A2_TX_PWR_OFFSET, tx_pwr_offset),
+		REG_OFFSET(DOM_A2_RX_PWR, rx_pwr),
+	};
+
+	static const struct reg_offset power[] = {
+		REG_OFFSET(DOM_A2_RX_PWR_0, rx_pwr_cal[0]),
+		REG_OFFSET(DOM_A2_RX_PWR_1, rx_pwr_cal[1]),
+		REG_OFFSET(DOM_A2_RX_PWR_2, rx_pwr_cal[2]),
+		REG_OFFSET(DOM_A2_RX_PWR_3, rx_pwr_cal[3]),
+		REG_OFFSET(DOM_A2_RX_PWR_4, rx_pwr_cal[4]),
+	};
+
+	static const struct reg_offset aw[] = {
+		REG_OFFSET(DOM_A2_TEMP_AHT, temp_aht),
+		REG_OFFSET(DOM_A2_TEMP_ALT, temp_alt),
+		REG_OFFSET(DOM_A2_TEMP_WHT, temp_wht),
+		REG_OFFSET(DOM_A2_TEMP_WLT, temp_wlt),
+		REG_OFFSET(DOM_A2_VCC_AHT, vcc_aht),
+		REG_OFFSET(DOM_A2_VCC_ALT, vcc_alt),
+		REG_OFFSET(DOM_A2_VCC_WHT, vcc_wht),
+		REG_OFFSET(DOM_A2_VCC_WLT, vcc_wlt),
+		REG_OFFSET(DOM_A2_TX_BIAS_AHT, tx_bias_aht),
+		REG_OFFSET(DOM_A2_TX_BIAS_ALT, tx_bias_alt),
+		REG_OFFSET(DOM_A2_TX_BIAS_WHT, tx_bias_wht),
+		REG_OFFSET(DOM_A2_TX_BIAS_WLT, tx_bias_wlt),
+		REG_OFFSET(DOM_A2_TX_PWR_AHT, tx_pwr_aht),
+		REG_OFFSET(DOM_A2_TX_PWR_ALT, tx_pwr_alt),
+		REG_OFFSET(DOM_A2_TX_PWR_WHT, tx_pwr_wht),
+		REG_OFFSET(DOM_A2_TX_PWR_WLT, tx_pwr_wlt),
+		REG_OFFSET(DOM_A2_RX_PWR_AHT, rx_pwr_aht),
+		REG_OFFSET(DOM_A2_RX_PWR_ALT, rx_pwr_alt),
+		REG_OFFSET(DOM_A2_RX_PWR_WHT, rx_pwr_wht),
+		REG_OFFSET(DOM_A2_RX_PWR_WLT, rx_pwr_wlt),
+	};
+
+	res = read_phy_diag(hw, 0x0, DOM_A0_DOM_TYPE, &pd->type);
+	if (res)
+		goto out;
+
+	type = pd->type >> 8;
+
+	if ((~type & DOM_TYPE_DOM) || (type & DOM_TYPE_LEGAGY_DOM))
+		goto out;
+
+	if (type & DOM_TYPE_ADDR_CHNGE) {
+		hw_dbg("DOM module not supported (Address change)\n");
+		goto out;
+	}
+
+	eo = pd->type & 0xFF;
+
+	res = read_phy_diag(hw, 0x0, DOM_A0_WAVELENGTH, &pd->wavelength);
+	if (res)
+		goto out;
+
+	/* If supported. Read alarms and Warnings first*/
+	if (eo & DOM_EO_AW) {
+		res = read_phy_diag(hw, 0x1, DOM_A2_ALARM, &pd->alarm);
+		if (res)
+			goto out;
+		res = read_phy_diag(hw, 0x1, DOM_A2_WARNING, &pd->warning);
+		if (res)
+			goto out;
+	}
+
+	/* Basic diag */
+
+	for (i = 0; i < ARRAY_SIZE(basic); i++) {
+		res = read_phy_diag(hw, 0x1, basic[i].reg,
+				    (u16 *)((char *)pd + basic[i].offset));
+		if (res)
+			goto out;
+	}
+
+	/* Power */
+
+	for (i = 0; i < ARRAY_SIZE(power); i++) {
+		res = read_phy_diag_u32(hw, 0x1, power[i].reg,
+					(u32 *)((char *)pd + power[i].offset));
+		if (res)
+			goto out;
+	}
+
+	/* Thresholds for Alarms and Warnings */
+
+	if (eo & DOM_EO_AW) {
+		for (i = 0; i < ARRAY_SIZE(aw); i++) {
+			res = read_phy_diag(hw, 0x1, aw[i].reg,
+					    (u16 *)((char *)pd + aw[i].offset));
+			if (res)
+				goto out;
+		}
+	}
+
+out:
+	return res;
+}
+
 static const struct ethtool_ops igb_ethtool_ops = {
 	.get_settings           = igb_get_settings,
 	.set_settings           = igb_set_settings,
@@ -2097,6 +2287,7 @@ static const struct ethtool_ops igb_ethtool_ops = {
 	.get_ethtool_stats      = igb_get_ethtool_stats,
 	.get_coalesce           = igb_get_coalesce,
 	.set_coalesce           = igb_set_coalesce,
+	.get_phy_diag           = igb_get_phy_diag,
 };
 
 void igb_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ixgbe/ixgbe_ethtool.c b/drivers/net/ixgbe/ixgbe_ethtool.c
index 84ab4db..8435cd7 100644
--- a/drivers/net/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ixgbe/ixgbe_ethtool.c
@@ -34,9 +34,11 @@
 #include <linux/ethtool.h>
 #include <linux/vmalloc.h>
 #include <linux/uaccess.h>
+#include <net/dom.h>
 
 #include "ixgbe.h"
-
+#include "ixgbe_phy.h"
+#include "ixgbe_common.h"
 
 #define IXGBE_ALL_RAR_ENTRIES 16
 
@@ -2106,6 +2108,172 @@ static int ixgbe_set_flags(struct net_device *netdev, u32 data)
 
 }
 
+static s32 read_phy_diag(struct ixgbe_hw *hw, u8 page, u8 offset, u16 *data)
+{
+	s32 status;
+	u8 hi, lo;
+
+	status = ixgbe_read_i2c_byte_generic(hw, page, offset, &hi);
+	if (status)
+		goto out;
+
+	status = ixgbe_read_i2c_byte_generic(hw, page, offset, &lo);
+	if (status)
+		goto out;
+
+	*data = (((u16)hi) << 8) | lo;
+
+out:
+	return status;
+}
+
+static s32 read_phy_diag_u32(struct ixgbe_hw *hw, u8 page, u8 offset, u32 *data)
+{
+	u16 p1;
+	u16 p2;
+	s32 res;
+
+	res = read_phy_diag(hw, page, offset, &p1);
+	if (res)
+		goto out;
+
+	res = read_phy_diag(hw, page, offset + 2, &p2);
+	if (res)
+		goto out;
+
+	*data = ((u32)p1) << 16 | p2;
+
+out:
+	return res;
+}
+
+struct reg_offset {
+	int reg;
+	size_t offset;
+};
+
+#define REG_OFFSET(a, b) \
+	{ .reg = a, .offset = offsetof(struct ethtool_phy_diag, b) }
+
+int ixgb_get_phy_diag(struct net_device *netdev, struct ethtool_phy_diag *pd)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(netdev);
+	struct ixgbe_hw *hw = &adapter->hw;
+	int res;
+	u8 type, eo;
+	int i;
+
+	static const struct reg_offset basic[] = {
+		REG_OFFSET(DOM_A2_TEMP, temp),
+		REG_OFFSET(DOM_A2_TEMP_SLOPE, temp_slope),
+		REG_OFFSET(DOM_A2_TEMP_OFFSET, temp_offset),
+		REG_OFFSET(DOM_A2_VCC, vcc),
+		REG_OFFSET(DOM_A2_VCC_SLOPE, vcc_slope),
+		REG_OFFSET(DOM_A2_VCC_OFFSET, vcc_offset),
+		REG_OFFSET(DOM_A2_TX_BIAS, tx_bias),
+		REG_OFFSET(DOM_A2_TX_I_SLOPE, tx_bias_slope),
+		REG_OFFSET(DOM_A2_TX_I_OFFSET, tx_bias_offset),
+		REG_OFFSET(DOM_A2_TX_PWR, tx_pwr),
+		REG_OFFSET(DOM_A2_TX_PWR_SLOPE, tx_pwr_slope),
+		REG_OFFSET(DOM_A2_TX_PWR_OFFSET, tx_pwr_offset),
+		REG_OFFSET(DOM_A2_RX_PWR, rx_pwr),
+	};
+
+	static const struct reg_offset power[] = {
+		REG_OFFSET(DOM_A2_RX_PWR_0, rx_pwr_cal[0]),
+		REG_OFFSET(DOM_A2_RX_PWR_1, rx_pwr_cal[1]),
+		REG_OFFSET(DOM_A2_RX_PWR_2, rx_pwr_cal[2]),
+		REG_OFFSET(DOM_A2_RX_PWR_3, rx_pwr_cal[3]),
+		REG_OFFSET(DOM_A2_RX_PWR_4, rx_pwr_cal[4]),
+	};
+
+	static const struct reg_offset aw[] = {
+		REG_OFFSET(DOM_A2_TEMP_AHT, temp_aht),
+		REG_OFFSET(DOM_A2_TEMP_ALT, temp_alt),
+		REG_OFFSET(DOM_A2_TEMP_WHT, temp_wht),
+		REG_OFFSET(DOM_A2_TEMP_WLT, temp_wlt),
+		REG_OFFSET(DOM_A2_VCC_AHT, vcc_aht),
+		REG_OFFSET(DOM_A2_VCC_ALT, vcc_alt),
+		REG_OFFSET(DOM_A2_VCC_WHT, vcc_wht),
+		REG_OFFSET(DOM_A2_VCC_WLT, vcc_wlt),
+		REG_OFFSET(DOM_A2_TX_BIAS_AHT, tx_bias_aht),
+		REG_OFFSET(DOM_A2_TX_BIAS_ALT, tx_bias_alt),
+		REG_OFFSET(DOM_A2_TX_BIAS_WHT, tx_bias_wht),
+		REG_OFFSET(DOM_A2_TX_BIAS_WLT, tx_bias_wlt),
+		REG_OFFSET(DOM_A2_TX_PWR_AHT, tx_pwr_aht),
+		REG_OFFSET(DOM_A2_TX_PWR_ALT, tx_pwr_alt),
+		REG_OFFSET(DOM_A2_TX_PWR_WHT, tx_pwr_wht),
+		REG_OFFSET(DOM_A2_TX_PWR_WLT, tx_pwr_wlt),
+		REG_OFFSET(DOM_A2_RX_PWR_AHT, rx_pwr_aht),
+		REG_OFFSET(DOM_A2_RX_PWR_ALT, rx_pwr_alt),
+		REG_OFFSET(DOM_A2_RX_PWR_WHT, rx_pwr_wht),
+		REG_OFFSET(DOM_A2_RX_PWR_WLT, rx_pwr_wlt),
+	};
+
+	res = read_phy_diag(hw, 0x0, DOM_A0_DOM_TYPE, &pd->type);
+	if (res)
+		goto out;
+
+	type = pd->type >> 8;
+
+	if ((~type & DOM_TYPE_DOM) || (type & DOM_TYPE_LEGAGY_DOM))
+		goto out;
+
+	if (type & DOM_TYPE_ADDR_CHNGE)  {
+		hw_dbg(hw, "DOM module not supported (Address change)\n");
+		goto out;
+	}
+
+	eo = pd->type & 0xFF;
+
+	res = read_phy_diag(hw, 0x0, DOM_A0_WAVELENGTH, &pd->wavelength);
+	if (res)
+		goto out;
+
+	/* If supported. Read alarms and Warnings first*/
+	if (eo & DOM_EO_AW) {
+		res = read_phy_diag(hw, 0x1, DOM_A2_ALARM, &pd->alarm);
+		if (res)
+			goto out;
+
+		res = read_phy_diag(hw, 0x1, DOM_A2_WARNING, &pd->warning);
+		if (res)
+			goto out;
+	}
+
+	/* Basic diag */
+
+	for (i = 0; i < ARRAY_SIZE(basic); i++) {
+		res = read_phy_diag(hw, 0x1, basic[i].reg,
+				    (u16 *)((char *)pd + basic[i].offset));
+		if (res)
+			goto out;
+	}
+
+	/* Power */
+
+	for (i = 0; i < ARRAY_SIZE(power); i++) {
+		res = read_phy_diag_u32(hw, 0x1, power[i].reg,
+					(u32 *)((char *)pd + power[i].offset));
+		if (res)
+			goto out;
+	}
+
+	/* Thresholds for Alarms and Warnings */
+
+	if (eo & DOM_EO_AW) {
+		for (i = 0; i < ARRAY_SIZE(aw); i++) {
+			res = read_phy_diag(hw, 0x1, aw[i].reg,
+					    (u16 *)((char *)pd + aw[i].offset));
+			if (res)
+				goto out;
+		}
+	}
+
+out:
+	return res;
+}
+
 static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.get_settings           = ixgbe_get_settings,
 	.set_settings           = ixgbe_set_settings,
@@ -2141,6 +2309,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.set_coalesce           = ixgbe_set_coalesce,
 	.get_flags              = ethtool_op_get_flags,
 	.set_flags              = ixgbe_set_flags,
+	.get_phy_diag           = ixgb_get_phy_diag,
 };
 
 void ixgbe_set_ethtool_ops(struct net_device *netdev)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index edd03b7..4e45a94 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -272,6 +272,53 @@ struct ethtool_stats {
 	__u64	data[0];
 };
 
+/* Diagmostic Monitoring Interface Data -- DOM */
+struct ethtool_phy_diag {
+	__u32 cmd;
+	/* A0 page */
+	__u16 type;
+	__u16 wavelength;
+	/* A2 page */
+	__u16 alarm;
+	__u16 warning;
+	__s16 temp;
+	__u16 temp_slope;
+	__s16 temp_offset;
+	__u16 vcc;
+	__u16 vcc_slope;
+	__s16 vcc_offset;
+	__u16 tx_bias;
+	__u16 tx_bias_slope;
+	__s16 tx_bias_offset;
+	__u16 tx_pwr;
+	__u16 tx_pwr_slope;
+	__s16 tx_pwr_offset;
+	__u16 rx_pwr;
+	__u32 rx_pwr_cal[5];
+
+	/* Thresholds */
+	__s16 temp_alt;
+	__s16 temp_aht;
+	__s16 temp_wlt;
+	__s16 temp_wht;
+	__u16 vcc_alt;
+	__u16 vcc_aht;
+	__u16 vcc_wlt;
+	__u16 vcc_wht;
+	__u16 tx_bias_alt;
+	__u16 tx_bias_aht;
+	__u16 tx_bias_wlt;
+	__u16 tx_bias_wht;
+	__u16 tx_pwr_alt;
+	__u16 tx_pwr_aht;
+	__u16 tx_pwr_wlt;
+	__u16 tx_pwr_wht;
+	__u16 rx_pwr_alt;
+	__u16 rx_pwr_aht;
+	__u16 rx_pwr_wlt;
+	__u16 rx_pwr_wht;
+};
+
 struct ethtool_perm_addr {
 	__u32	cmd;		/* ETHTOOL_GPERMADDR */
 	__u32	size;
@@ -499,6 +546,7 @@ struct ethtool_ops {
 	int	(*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
 	int     (*flash_device)(struct net_device *, struct ethtool_flash *);
 	int	(*reset)(struct net_device *, u32 *);
+	int     (*get_phy_diag)(struct net_device *, struct ethtool_phy_diag *);
 };
 #endif /* __KERNEL__ */
 
@@ -557,6 +605,7 @@ struct ethtool_ops {
 #define	ETHTOOL_SRXCLSRLINS	0x00000032 /* Insert RX classification rule */
 #define	ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device */
 #define	ETHTOOL_RESET		0x00000034 /* Reset hardware */
+#define	ETHTOOL_GPHYDIAG	0x00000035 /* Get PHY diagnostics */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/include/net/dom.h b/include/net/dom.h
new file mode 100644
index 0000000..61540c3
--- /dev/null
+++ b/include/net/dom.h
@@ -0,0 +1,109 @@
+#ifndef _LINUX_DOM_H
+#define _LINUX_DOM_H
+
+/*
+   Diagnostic Monitoring Interface for Optical Tranceivers
+   SFF-8472 v. 10.4 (Jan 2009)
+   ftp://ftp.seagate.com/sff/SFF-8472.PDF
+
+   Licensce GPL. Copyright Robert Olsson robert@herjulf.net
+*/
+
+#define  DOM_A0_IDENTIFIER  0
+#define  DOM_A0_WAVELENGTH 60
+#define  DOM_A0_CC_BASE    63
+#define  DOM_A0_DOM_TYPE   92
+
+/* DOM_TYPE codings in DOM_A0_DOM_TYPE */
+#define     DOM_TYPE_LEGAGY_DOM     (1<<7)
+#define     DOM_TYPE_DOM            (1<<6)  /* Has DOM support */
+#define     DOM_TYPE_INT_CAL        (1<<5)  /* Internally calibrated */
+#define     DOM_TYPE_EXT_CAL        (1<<4)  /* Externally calibrated */
+#define     DOM_TYPE_RX_PWR         (1<<3)  /* Received Power OMA=0,  1=average */
+#define     DOM_TYPE_ADDR_CHNGE     (1<<2)  /* Address change required */
+
+#define  DOM_A0_EO   93                     /* Enhanced options */
+#define     DOM_EO_AW               (1<<7)  /* Alarm & Warnings */
+#define     DOM_EO_TX_DISABLE       (1<<6)
+#define     DOM_EO_TX_FAULT         (1<<5)
+#define     DOM_EO_RX_LOS           (1<<4)
+#define     DOM_EO_RATE_SELECT_MON  (1<<3)
+#define     DOM_EO_APP_SELECT       (1<<2)
+#define     DOM_EO_RATE_SELECT      (1<<1)
+
+#define  DOM_A0_CC_EXT     95
+
+#define  DOM_A2_TEMP_AHT    0  /* Temp alarm high threshold */
+#define  DOM_A2_TEMP_ALT    2
+#define  DOM_A2_TEMP_WHT    4  /* Temp warning high threshold */
+#define  DOM_A2_TEMP_WLT    6
+
+#define  DOM_A2_VCC_AHT    8  /* VCC alarm high threshold */
+#define  DOM_A2_VCC_ALT   10
+#define  DOM_A2_VCC_WHT   12  /* VCC warning high threshold */
+#define  DOM_A2_VCC_WLT   14
+
+#define  DOM_A2_TX_BIAS_AHT   16  /* TX_BIAS alarm high threshold */
+#define  DOM_A2_TX_BIAS_ALT   18
+#define  DOM_A2_TX_BIAS_WHT   20  /* TX_BIAS warning high threshold */
+#define  DOM_A2_TX_BIAS_WLT   22
+
+#define  DOM_A2_TX_PWR_AHT   24  /* TX_PWR alarm high threshold */
+#define  DOM_A2_TX_PWR_ALT   26
+#define  DOM_A2_TX_PWR_WHT   28  /* TX_PWR warning high threshold */
+#define  DOM_A2_TX_PWR_WLT   30
+
+#define  DOM_A2_RX_PWR_AHT   32  /* RX_PWR alarm high threshold */
+#define  DOM_A2_RX_PWR_ALT   34
+#define  DOM_A2_RX_PWR_WHT   36  /* RX_PWR warning high threshold */
+#define  DOM_A2_RX_PWR_WLT   38
+
+#define  DOM_A2_RX_PWR_4   56  /* 4 bytes  Calibration constants*/
+#define  DOM_A2_RX_PWR_3   60  /* 4 bytes */
+#define  DOM_A2_RX_PWR_2   64  /* 4 bytes */
+#define  DOM_A2_RX_PWR_1   68  /* 4 bytes */
+#define  DOM_A2_RX_PWR_0   72  /* 4 bytes */
+
+#define  DOM_A2_TX_I_SLOPE   76  /* 2 bytes */
+#define  DOM_A2_TX_I_OFFSET  78  /* 2 bytes */
+#define  DOM_A2_TX_PWR_SLOPE   80  /* 2 bytes */
+#define  DOM_A2_TX_PWR_OFFSET  82  /* 2 bytes */
+#define  DOM_A2_TEMP_SLOPE   84  /* 2 bytes */
+#define  DOM_A2_TEMP_OFFSET  86  /* 2 bytes */
+#define  DOM_A2_VCC_SLOPE   88  /* 2 bytes */
+#define  DOM_A2_VCC_OFFSET  90  /* 2 bytes */
+
+#define  DOM_A2_CC_DMI    95
+#define  DOM_A2_TEMP      96   /* 2 bytes */
+#define  DOM_A2_VCC       98   /* 2 bytes */
+#define  DOM_A2_TX_BIAS  100   /* 2 bytes */
+#define  DOM_A2_TX_PWR   102   /* 2 bytes */
+#define  DOM_A2_RX_PWR   104   /* 2 bytes */
+
+#define  DOM_A2_ALARM    112   /* 2 bytes */
+#define     DOM_TYPE_TEMP_AH       (1<<7)  /* Temp alarm high */
+#define     DOM_TYPE_TEMP_AL       (1<<6)  /* low */
+#define     DOM_TYPE_VCC_AH        (1<<5)
+#define     DOM_TYPE_VCC_AL        (1<<4)
+#define     DOM_TYPE_TX_BIAS_AH    (1<<3)
+#define     DOM_TYPE_TX_BIAS_AL    (1<<2)
+#define     DOM_TYPE_TX_PWR_AH     (1<<1)
+#define     DOM_TYPE_TX_PWR_AL     (1<<0)
+/* Next byte 113 */
+#define     DOM_TYPE_RX_PWR_AH     (1<<7)
+#define     DOM_TYPE_RX_PWR_AL     (1<<6)
+
+#define  DOM_A2_WARNING  116   /* 2 bytes */
+#define     DOM_TYPE_TEMP_WH       (1<<7)  /* Temp warning high */
+#define     DOM_TYPE_TEMP_WL       (1<<6)  /* low */
+#define     DOM_TYPE_VCC_WH        (1<<5)
+#define     DOM_TYPE_VCC_WL        (1<<4)
+#define     DOM_TYPE_TX_BIAS_WH    (1<<3)
+#define     DOM_TYPE_TX_BIAS_WL    (1<<2)
+#define     DOM_TYPE_TX_PWR_WH     (1<<1)
+#define     DOM_TYPE_TX_PWR_WL     (1<<0)
+/* Next byte 117 */
+#define     DOM_TYPE_RX_PWR_WH     (1<<7)
+#define     DOM_TYPE_RX_PWR_WL     (1<<6)
+
+#endif /* _LINUX_DOM_H */
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d8aee58..756434a 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -893,6 +893,21 @@ static int ethtool_flash_device(struct net_device *dev, char __user *useraddr)
 	return dev->ethtool_ops->flash_device(dev, &efl);
 }
 
+static int ethtool_phy_diag(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_phy_diag pd;
+
+	if (!dev->ethtool_ops->get_phy_diag)
+		return -EOPNOTSUPP;
+
+	dev->ethtool_ops->get_phy_diag(dev, &pd); /* FIXME */
+
+	if (copy_to_user(useraddr, &pd, sizeof(pd)))
+		 return -EFAULT;
+
+	return 0;
+}
+
 /* The main entry point in this file.  Called from net/core/dev.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -1112,6 +1127,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_RESET:
 		rc = ethtool_reset(dev, useraddr);
 		break;
+	case ETHTOOL_GPHYDIAG:
+		rc = ethtool_phy_diag(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}



^ permalink raw reply related

* Re: [Bugme-new] [Bug 14685] New: 2.6.30 -> 2.6.31: very high number of RX dropped in bnx2
From: Andrew Morton @ 2009-11-25 20:47 UTC (permalink / raw)
  To: Michael Chan, netdev; +Cc: bugzilla-daemon, bugme-daemon, olel
In-Reply-To: <bug-14685-10286@http.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 25 Nov 2009 00:16:00 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=14685
> 
>            Summary: 2.6.30 -> 2.6.31: very high number of RX dropped in
>                     bnx2
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.31.6
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: olel@ans.pl
>         Regression: Yes
> 
> 
> After upgradig my kernel from 2.6.30 to 2.6.31 I'm observing very high number
> of dropped packets on BCM5708(Dell PowerEdge 1950)/BCM5716(Dell PowerEdge R410)
> NICs.
> 
> # uptime ; ifconfig eth0|grep "RX p"; ethtool -S eth0|grep disc
>  01:07:31 up 6 min,  1 user,  load average: 0.00, 0.07, 0.04
>           RX packets:2234 errors:0 dropped:527 overruns:0 frame:0
>      rx_discards: 0
>      rx_fw_discards: 527
> 
> Everything seems to work just fine so I suspect that the driver/firmware is
> counting something wrong.
> 


^ permalink raw reply

* Re: [Bugme-new] [Bug 14691] New: Complete NAPI IRQ lockup in b44 driver: code fundamentally incompatible with netconsole requirements
From: Andrew Morton @ 2009-11-25 21:16 UTC (permalink / raw)
  To: Gary Zambrano; +Cc: bugzilla-daemon, bugme-daemon, netdev, andi
In-Reply-To: <bug-14691-10286@http.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 25 Nov 2009 20:15:33 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=14691
> 
>            Summary: Complete NAPI IRQ lockup in b44 driver: code
>                     fundamentally incompatible with netconsole
>                     requirements
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.32-rc8
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: blocking
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: andi@lisas.de
>         Regression: No
> 
> 
> Hi,
> 
> severity "blocking" (== "Blocks development and/or testing work"): it's my
> primary window to the world on this router - netconsole - and crashes multiple
> times when loading various drivers, thus for me there's no productive
> proceeding possible, plus there have been multiple reports on this issue since
> at least 2004.
> 
> The kernel version I'm testing it on is 2.6.30.9 (OpenWrt MIPSEL, on ASUS
> WL-500gP v2), but since b44 interrupt handler is unchanged I assume it's still
> unfixed.
> 
> Frequently when doing some medium activity such as loading modules such as
> ftdi_sio, usb_audio etc. (especially with kobject debugging enabled), the
> network LED starts blinking like crazy and when re-listening to netconsole
> output, there's simply a flood of
> %s: Error, poll already scheduled
> and the box is DEAD (again! *curse*).
> 
> Productive debugging of multiple USB and ftdi_sio issues is simply impossible
> with this kind of problem remaining.
> 
> See also:
> "RE:hostname freeze" (2004!!):
> http://lists.linuxcoding.com/rhl/2004/msg51048.html
> 
> "b44 driver suspend/resume (was Re: [ACPI] Re: Re: various problems with Acer
> TM654, suspend, ACAD, radeon)":
> http://osdir.com/ml/network.general/2004-08/msg00208.html
> 
> "acpi and b44: irq disabled, b44: Error, poll already scheduled" (2004!!):
> http://lkml.indiana.edu/hypermail/linux/kernel/0408.3/0078.html
> 
> And a very related fix and long discussion is at:
> "[PATCH 2.6.30-rc4] r8169: avoid losing MSI interrupts":
> http://kerneltrap.org/mailarchive/linux-netdev/2009/5/23/5791863
> 
> I'm very uncertain whether the interrupt handler method of simply screaming
> this error message on every interrupt without doing any actual remedies is a
> good idea.
> HAH! Now I've got it: the fact that we're on netconsole means that this error
> message gets sent _immediately_ (out to the netconsole device!!), thus this
> very network transmission encounters an IRQ handler at a time where the
> previous scheduled NAPI processing had no chance yet of being done, thus
> napi_schedule_prep() FAIL, thus error message again, thus... ad nauseam!
> Or, IOW, now I'm _certain_ that it is _NOT_ a good idea to scream this message
> on every IRQ without doing anything about it.
> 
> 
> I want to report this issue now to make sure it can get the attention it
> deserves, but for now I'll try to come up with a bandaid locally (simply
> silencing the error message) in order to be able to debug the other driver
> issues mentioned above.
> 
> Thanks!


^ permalink raw reply

* [PATCH] b44.c box lockup fix (netconsole): ratelimit NAPI poll error message
From: Andreas Mohr @ 2009-11-25 21:35 UTC (permalink / raw)
  To: davem; +Cc: zambrano, dave, mb, netdev, linux-kernel

Hello all,

my OpenWrt MIPSEL ASUS WL-500gP v2 kept locking up during driver
debugging with kobject debugging turned on (when debugging non-working
drivers such as USB, ftdi_sio and usb_audio).
Turns out that it's not very helpful to have a netconsole-deployed
network driver's interrupt handler message spew noise permanently
(see comment in patch).

See
http://bugzilla.kernel.org/show_bug.cgi?id=14691
for background information.

Tested on 2.6.30.9 and working (now normal debug messages manage
to come through, too, yet box still locked up, simply due to excessive
_normal_ log messages such as kobject logging, but that's _expected_
and user-correctable).

checkpatch.pl'd, applies cleanly to -rc8.
Likely -stable candidate material.

r8169 (and perhaps other netconsole-capable drivers?)
might need such a correction, too, if the
                        if (likely(napi_schedule_prep(&tp->napi)))
                                __napi_schedule(&tp->napi);
                        else if (netif_msg_intr(tp)) {
                                printk(KERN_INFO "%s: interrupt %04x in poll\n",
                                dev->name, status);
                        }
is similarly problematic.
BTW: this patch here is to be seen separate from the r8169 interrupt handler
rework as discussed at "r8169: avoid losing MSI interrupts", since even with a
"working" interrupt handler (i.e. one that closes the "new interrupt" race window,
which the one in b44 currently probably doesn't) such error messages
have a fatal feedback loop in the case of netconsole deployment.

Thanks,

Signed-off-by: Andreas Mohr <andi@lisas.de>

--- linux-2.6.30.9/drivers/net/b44.c.orig	2009-11-25 20:49:48.000000000 +0100
+++ linux-2.6.30.9/drivers/net/b44.c	2009-11-25 21:26:34.000000000 +0100
@@ -913,8 +913,18 @@
 			__b44_disable_ints(bp);
 			__napi_schedule(&bp->napi);
 		} else {
-			printk(KERN_ERR PFX "%s: Error, poll already scheduled\n",
-			       dev->name);
+			/* netconsole fix!!:
+			 * without ratelimiting, this message would:
+			 * immediately find its way out to b44 netconsole ->
+			 * new IRQ -> re-encounter failed napi_schedule_prep()
+			 * -> error message -> ad nauseam -> box LOCKUP.
+			 * See thread "r8169: avoid losing MSI interrupts"
+			 * for further inspiration. */
+			if (printk_ratelimit())
+				printk(KERN_ERR PFX
+					"%s: Error, poll already scheduled; "
+					"istat 0x%x, imask 0x%x\n",
+						dev->name, istat, imask);
 		}
 
 irq_ack:

^ permalink raw reply

* [PATCH] r8169: move PHY regs tables to .rodata
From: Alexey Dobriyan @ 2009-11-25 21:39 UTC (permalink / raw)
  To: davem; +Cc: netdev

As side effect, consume less stack.

	-rtl8169_get_mac_version [vmlinux]:	432
	-rtl8169_init_one [vmlinux]:		376
	+rtl8169_init_one [vmlinux]:		136

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/net/r8169.c |   42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -756,7 +756,7 @@ static int rtl8169_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
 	struct rtl8169_private *tp = netdev_priv(dev);
 	void __iomem *ioaddr = tp->mmio_addr;
 	unsigned int i;
-	static struct {
+	static const struct {
 		u32 opt;
 		u16 reg;
 		u8  mask;
@@ -1247,7 +1247,7 @@ static void rtl8169_get_mac_version(struct rtl8169_private *tp,
 	 *
 	 * (RTL_R32(TxConfig) & 0x700000) == 0x200000 ? 8101Eb : 8101Ec
 	 */
-	const struct {
+	static const struct {
 		u32 mask;
 		u32 val;
 		int mac_version;
@@ -1318,7 +1318,7 @@ struct phy_reg {
 	u16 val;
 };
 
-static void rtl_phy_write(void __iomem *ioaddr, struct phy_reg *regs, int len)
+static void rtl_phy_write(void __iomem *ioaddr, const struct phy_reg *regs, int len)
 {
 	while (len-- > 0) {
 		mdio_write(ioaddr, regs->reg, regs->val);
@@ -1328,7 +1328,7 @@ static void rtl_phy_write(void __iomem *ioaddr, struct phy_reg *regs, int len)
 
 static void rtl8169s_hw_phy_config(void __iomem *ioaddr)
 {
-	struct {
+	static const struct {
 		u16 regs[5]; /* Beware of bit-sign propagation */
 	} phy_magic[5] = { {
 		{ 0x0000,	//w 4 15 12 0
@@ -1380,7 +1380,7 @@ static void rtl8169s_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8169sb_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0002 },
 		{ 0x01, 0x90d0 },
 		{ 0x1f, 0x0000 }
@@ -1391,7 +1391,7 @@ static void rtl8169sb_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168bb_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x10, 0xf41b },
 		{ 0x1f, 0x0000 }
 	};
@@ -1404,7 +1404,7 @@ static void rtl8168bb_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168bef_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x10, 0xf41b },
 		{ 0x1f, 0x0000 }
@@ -1415,7 +1415,7 @@ static void rtl8168bef_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168cp_1_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0000 },
 		{ 0x1d, 0x0f00 },
 		{ 0x1f, 0x0002 },
@@ -1428,7 +1428,7 @@ static void rtl8168cp_1_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168cp_2_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x1d, 0x3d98 },
 		{ 0x1f, 0x0000 }
@@ -1443,7 +1443,7 @@ static void rtl8168cp_2_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168c_1_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x12, 0x2300 },
 		{ 0x1f, 0x0002 },
@@ -1472,7 +1472,7 @@ static void rtl8168c_1_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168c_2_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x12, 0x2300 },
 		{ 0x03, 0x802f },
@@ -1500,7 +1500,7 @@ static void rtl8168c_2_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168c_3_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x12, 0x2300 },
 		{ 0x1d, 0x3d98 },
@@ -1527,7 +1527,7 @@ static void rtl8168c_4_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8168d_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init_0[] = {
+	static const struct phy_reg phy_reg_init_0[] = {
 		{ 0x1f, 0x0001 },
 		{ 0x09, 0x2770 },
 		{ 0x08, 0x04d0 },
@@ -1556,7 +1556,7 @@ static void rtl8168d_hw_phy_config(void __iomem *ioaddr)
 	rtl_phy_write(ioaddr, phy_reg_init_0, ARRAY_SIZE(phy_reg_init_0));
 
 	if (mdio_read(ioaddr, 0x06) == 0xc400) {
-		struct phy_reg phy_reg_init_1[] = {
+		static const struct phy_reg phy_reg_init_1[] = {
 			{ 0x1f, 0x0005 },
 			{ 0x01, 0x0300 },
 			{ 0x1f, 0x0000 },
@@ -1597,7 +1597,7 @@ static void rtl8168d_hw_phy_config(void __iomem *ioaddr)
 
 static void rtl8102e_hw_phy_config(void __iomem *ioaddr)
 {
-	struct phy_reg phy_reg_init[] = {
+	static const struct phy_reg phy_reg_init[] = {
 		{ 0x1f, 0x0003 },
 		{ 0x08, 0x441d },
 		{ 0x01, 0x9100 },
@@ -2370,7 +2370,7 @@ static void rtl_set_rx_max_size(void __iomem *ioaddr, unsigned int rx_buf_sz)
 
 static void rtl8169_set_magic_reg(void __iomem *ioaddr, unsigned mac_version)
 {
-	struct {
+	static const struct {
 		u32 mac_version;
 		u32 clk;
 		u32 val;
@@ -2494,7 +2494,7 @@ struct ephy_info {
 	u16 bits;
 };
 
-static void rtl_ephy_init(void __iomem *ioaddr, struct ephy_info *e, int len)
+static void rtl_ephy_init(void __iomem *ioaddr, const struct ephy_info *e, int len)
 {
 	u16 w;
 
@@ -2565,7 +2565,7 @@ static void __rtl_hw_start_8168cp(void __iomem *ioaddr, struct pci_dev *pdev)
 
 static void rtl_hw_start_8168cp_1(void __iomem *ioaddr, struct pci_dev *pdev)
 {
-	static struct ephy_info e_info_8168cp[] = {
+	static const struct ephy_info e_info_8168cp[] = {
 		{ 0x01, 0,	0x0001 },
 		{ 0x02, 0x0800,	0x1000 },
 		{ 0x03, 0,	0x0042 },
@@ -2609,7 +2609,7 @@ static void rtl_hw_start_8168cp_3(void __iomem *ioaddr, struct pci_dev *pdev)
 
 static void rtl_hw_start_8168c_1(void __iomem *ioaddr, struct pci_dev *pdev)
 {
-	static struct ephy_info e_info_8168c_1[] = {
+	static const struct ephy_info e_info_8168c_1[] = {
 		{ 0x02, 0x0800,	0x1000 },
 		{ 0x03, 0,	0x0002 },
 		{ 0x06, 0x0080,	0x0000 }
@@ -2626,7 +2626,7 @@ static void rtl_hw_start_8168c_1(void __iomem *ioaddr, struct pci_dev *pdev)
 
 static void rtl_hw_start_8168c_2(void __iomem *ioaddr, struct pci_dev *pdev)
 {
-	static struct ephy_info e_info_8168c_2[] = {
+	static const struct ephy_info e_info_8168c_2[] = {
 		{ 0x01, 0,	0x0001 },
 		{ 0x03, 0x0400,	0x0220 }
 	};
@@ -2767,7 +2767,7 @@ static void rtl_hw_start_8168(struct net_device *dev)
 
 static void rtl_hw_start_8102e_1(void __iomem *ioaddr, struct pci_dev *pdev)
 {
-	static struct ephy_info e_info_8102e_1[] = {
+	static const struct ephy_info e_info_8102e_1[] = {
 		{ 0x01,	0, 0x6e65 },
 		{ 0x02,	0, 0x091f },
 		{ 0x03,	0, 0xc2f9 },

^ permalink raw reply

* Re: [Bugme-new] [Bug 14688] New: alignment of rx_skb in r8169 driver
From: Andrew Morton @ 2009-11-25 21:55 UTC (permalink / raw)
  To: meyer; +Cc: bugzilla-daemon, bugme-daemon, Francois Romieu, netdev
In-Reply-To: <bug-14688-10286@http.bugzilla.kernel.org/>


(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 25 Nov 2009 15:01:16 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=14688
> 
>            Summary: alignment of rx_skb in r8169 driver
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.29
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: meyer@fsr.tu-darmstadt.de
>         Regression: No
> 
> 
> Created an attachment (id=23933)
>  --> (http://bugzilla.kernel.org/attachment.cgi?id=23933)
> patch for rtl8169_alloc_rx_skb
> 
> Dear Maintainers,
> 
> I think there is a bug in the rtl8169_alloc_rx_skb function of the RealTek
> 8169/8168/8101 ethernet driver in kernel version 2.6.29.6, but also in current
> 2.6.31 versions.
> 
> For the case the align argument is different from 0, the skb_reserve call is
> expected to align the data field accordingly by padding. The number of bytes to
> reserve is calculated by the expression
> 
>             (align - 1) & (unsigned long)skb->data
> 
> which is wrong. I would suggest the following correction:
> 
> pad = align ? align - ((align - 1) & (unsigned long)skb->data) : NET_IP_ALIGN;
> if (pad == align) pad = 0; // optional
> skb_reserve(skb, pad);
> 
> A patch for r8169.c is provided as attachment.
> 

Thanks, but..

- please always submit patches via email, not via bugzilla. 
  Documentation/SubmittingPatches goes into some detail.

- prepare patches as a unified diff, in `patch -p1' form.  It should
  look like:

--- a/drivers/net/r8169.c~a
+++ a/drivers/net/r8169.c
@@ -207,7 +207,9 @@ enum rtl_registers {
 	ChipCmd		= 0x37,
 	TxPoll		= 0x38,
 	IntrMask	= 0x3c,
+wibble
 	IntrStatus	= 0x3e,
+wobble
 	TxConfig	= 0x40,
 	RxConfig	= 0x44,
 	RxMissed	= 0x4c,
_


- Please cc at least the following on the patch:

	Francois Romieu <romieu@fr.zoreil.com>
	"David S. Miller" <davem@davemloft.net>
	netdev@vger.kernel.org

- Don't forget the changelog and a Singed-off-by:!

^ permalink raw reply

* Re: [rfc 1/3 v2] [E1000-devel] [rfc 1/4] igb: Add igb_cleanup_vf()
From: Simon Horman @ 2009-11-25 22:11 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: e1000-devel, netdev, Arnd Bergmann, Alexander Duyck
In-Reply-To: <9929d2390911242340t2e639719kc5ed80725f74a31d@mail.gmail.com>

On Tue, Nov 24, 2009 at 11:40:36PM -0800, Jeff Kirsher wrote:
> On Tue, Nov 24, 2009 at 22:32, Simon Horman <horms@verge.net.au> wrote:
> > Move virtual finction cleanup code into igb_cleanup_vf() and for the sake
> > of symmetry rename igb_probe_vfs() as igb_init_vf().
> >
> > Although these functions aren't entirely symmetrical it should aid
> > maintenance by making the relationship between initialisation and cleanup
> > more obvious.
> >
> > Note that there appears to be no way for adapter->vfs_allocated_count to be
> > non-zero for the case where CONFIG_PCI_IOV is not set, so reseting this
> > value was moved to inside the relvant #ifdef.
> >
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> > Thu, 05 Nov 2009 11:58:48 +1100
> > * Initial post
> >
> > Wed, 25 Nov 2009 15:43:45 +1100
> > * Actually remove adapter->vfs_allocated_count = 0 from outside of
> >  igb_cleanup_vf()
> > * Up-port to current net-next
> 
> Thanks Simon and congrats on the new addition!  I have added this
> three patch series to my tree for review and testing.

Thanks

^ permalink raw reply

* Re: [rfc 1/3 v2] [E1000-devel] [rfc 1/4] igb: Add igb_cleanup_vf()
From: Alexander Duyck @ 2009-11-25 22:27 UTC (permalink / raw)
  To: Simon Horman
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Arnd Bergmann, Kirsher, Jeffrey T
In-Reply-To: <20091125063530.796967189@vergenet.net>

Simon Horman wrote:
> Move virtual finction cleanup code into igb_cleanup_vf() and for the sake
> of symmetry rename igb_probe_vfs() as igb_init_vf().
> 
> Although these functions aren't entirely symmetrical it should aid
> maintenance by making the relationship between initialisation and cleanup
> more obvious.
> 
> Note that there appears to be no way for adapter->vfs_allocated_count to be
> non-zero for the case where CONFIG_PCI_IOV is not set, so reseting this
> value was moved to inside the relvant #ifdef.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>

The one thing I noticed with this patch is that it didn't remove the vf 
cleanup from igb_remove.  You may want to go back and also add the 
replacement of that function with igb_cleanup_vf to this patch.

Thanks,

Alex

> --- 
> Thu, 05 Nov 2009 11:58:48 +1100
> * Initial post
> 
> Wed, 25 Nov 2009 15:43:45 +1100
> * Actually remove adapter->vfs_allocated_count = 0 from outside of
>   igb_cleanup_vf()
> * Up-port to current net-next
> Index: net-next-2.6/drivers/net/igb/igb_main.c
> ===================================================================
> --- net-next-2.6.orig/drivers/net/igb/igb_main.c	2009-11-25 16:59:19.000000000 +1100
> +++ net-next-2.6/drivers/net/igb/igb_main.c	2009-11-25 17:05:03.000000000 +1100
> @@ -92,6 +92,7 @@ void igb_update_stats(struct igb_adapter
>  static int igb_probe(struct pci_dev *, const struct pci_device_id *);
>  static void __devexit igb_remove(struct pci_dev *pdev);
>  static int igb_sw_init(struct igb_adapter *);
> +static void igb_cleanup_vf(struct igb_adapter * adapter);
>  static int igb_open(struct net_device *);
>  static int igb_close(struct net_device *);
>  static void igb_configure_tx(struct igb_adapter *);
> @@ -701,22 +702,7 @@ static void igb_set_interrupt_capability
>  
>  	/* If we can't do MSI-X, try MSI */
>  msi_only:
> -#ifdef CONFIG_PCI_IOV
> -	/* disable SR-IOV for non MSI-X configurations */
> -	if (adapter->vf_data) {
> -		struct e1000_hw *hw = &adapter->hw;
> -		/* disable iov and allow time for transactions to clear */
> -		pci_disable_sriov(adapter->pdev);
> -		msleep(500);
> -
> -		kfree(adapter->vf_data);
> -		adapter->vf_data = NULL;
> -		wr32(E1000_IOVCTL, E1000_IOVCTL_REUSE_VFQ);
> -		msleep(100);
> -		dev_info(&adapter->pdev->dev, "IOV Disabled\n");
> -	}
> -#endif
> -	adapter->vfs_allocated_count = 0;
> +	igb_cleanup_vf(adapter);
>  	adapter->rss_queues = 1;
>  	adapter->flags |= IGB_FLAG_QUEUE_PAIRS;
>  	adapter->num_rx_queues = 1;
> @@ -1755,7 +1741,7 @@ static void __devexit igb_remove(struct 
>  }
>  
>  /**
> - * igb_probe_vfs - Initialize vf data storage and add VFs to pci config space
> + * igb_init_vf - Initialize vf data storage and add VFs to pci config space
>   * @adapter: board private structure to initialize
>   *
>   * This function initializes the vf specific data storage and then attempts to
> @@ -1763,7 +1749,7 @@ static void __devexit igb_remove(struct 
>   * mor expensive time wise to disable SR-IOV than it is to allocate and free
>   * the memory for the VFs.
>   **/
> -static void __devinit igb_probe_vfs(struct igb_adapter * adapter)
> +static void __devinit igb_init_vf(struct igb_adapter * adapter)
>  {
>  #ifdef CONFIG_PCI_IOV
>  	struct pci_dev *pdev = adapter->pdev;
> @@ -1909,6 +1895,35 @@ static void igb_init_hw_timer(struct igb
>  }
>  
>  /**
> + * igb_cleanup_vf - Clean up vf data and remove vfs from pci config space
> + * @adapter: board private structure to initialize
> + *
> + * This function cleans-up the vf specific data storage and then attempts to
> + * deallocate the VFs.
> + **/
> +static void igb_cleanup_vf(struct igb_adapter * adapter)
> +{
> +#ifdef CONFIG_PCI_IOV
> +	struct e1000_hw *hw = &adapter->hw;
> +
> +	if (!adapter->vf_data)
> +		return;
> +
> +	/* disable iov and allow time for transactions to clear */
> +	pci_disable_sriov(adapter->pdev);
> +	msleep(500);
> +
> +	kfree(adapter->vf_data);
> +	adapter->vf_data = NULL;
> +	adapter->vfs_allocated_count = 0;
> +
> +	wr32(E1000_IOVCTL, E1000_IOVCTL_REUSE_VFQ);
> +	msleep(100);
> +	dev_info(&adapter->pdev->dev, "IOV Disabled\n");
> +#endif
> +}
> +
> +/**
>   * igb_sw_init - Initialize general software structures (struct igb_adapter)
>   * @adapter: board private structure to initialize
>   *
> @@ -1955,7 +1970,7 @@ static int __devinit igb_sw_init(struct 
>  	}
>  
>  	igb_init_hw_timer(adapter);
> -	igb_probe_vfs(adapter);
> +	igb_init_vf(adapter);
>  
>  	/* Explicitly disable IRQ since the NIC can be in any state. */
>  	igb_irq_disable(adapter);
> 


^ permalink raw reply

* Re: [E1000-devel] TCP sockets stalling - help! (long)
From: Ilpo Järvinen @ 2009-11-25 22:29 UTC (permalink / raw)
  To: Asdo; +Cc: e1000-devel, Netdev
In-Reply-To: <4B0D5D86.8010509@shiftmail.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6731 bytes --]

On Wed, 25 Nov 2009, Asdo wrote:

> Asdo wrote:
> > Asdo wrote:
> > > Ilpo Järvinen wrote:
> > >  
> > > > ...I'd next try strace the sftp server to see what it was doing during
> > > > the stall.
> > > >       
> > > Thanks for your help Ilpo
> > > 
> > > Isn't the strace equivalent to the stack trace I obtained via cat
> > > /proc/pid/stack reported previously? That was at the time of the stall
> > > 
> > > I'm thinking the strace would slow down sftp-server very deeply...
> > >   
> > 
> > I found out that if I attach the strace I can see at least the last function
> > call.
> > The SFTP it's hanged right now so I did that:
> > 
> > root@mystorage:/root# strace -p 11475
> > Process 11475 attached - interrupt to quit
> > select(5, [3], [], NULL, NULL
> > 
> > (stuck here forever... doesn't move)
> > (it's strange the first option of select is 5, shouldn't it be 4 from man
> > select? A bug of strace maybe?)
> > 
> > root@mystorage:/root# cat /proc/11475/stack
> > [<ffffffff8112e644>] poll_schedule_timeout+0x34/0x50
> > [<ffffffff8112ef4f>] do_select+0x58f/0x6b0
> > [<ffffffff8112f8b5>] core_sys_select+0x185/0x2b0
> > [<ffffffff8112fc32>] sys_select+0x42/0x110
> > [<ffffffff8101225b>] tracesys+0xd9/0xde
> > [<ffffffffffffffff>] 0xffffffffffffffff
> > 
> > And this is from cat /proc/net/tcp   2: 0F12A8C0:0016 2512A8C0:0FBD 01
> > 00000000:00000000 02:00009144 00000000     0        0 5326251 2
> > ffff88085408ce00 26 4 1 9 4
> > 
> > The select refers to open files so here they are:
> > 
> > 
> > root@mystorage:/proc/11475/fd# ll
> > total 0
> > lr-x------ 1 ccosentino wetlab 64 2009-11-25 14:43 0 -> pipe:[5326309]
> > l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 1 -> pipe:[5326310]
> > l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 2 -> pipe:[5326311]
> > lr-x------ 1 ccosentino wetlab 64 2009-11-25 14:43 3 -> pipe:[5326309]
> > l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 4 -> pipe:[5326310]
> > l-wx------ 1 ccosentino wetlab 64 2009-11-25 14:43 5 ->
> > /path/to/file_being_saved.filepart
> > 
> > I tried to send SIGSTOP and then SIGCONT to see if I could make it make a
> > loop and then reenter into the select. I'm not sure it really did that, what
> > do you think? This is the strace:
> > root@mystorage:/root# strace -p 11475 2>&1 | tee sftpstrace.dmp
> > Process 11475 attached - interrupt to quit
> > select(5, [3], [], NULL, NULL)          = ? ERESTARTNOHAND (To be restarted)
> > --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> > --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> > select(5, [3], [], NULL, NULL)          = ? ERESTARTNOHAND (To be restarted)
> > --- SIGCONT (Continued) @ 0 (0) ---
> > select(5, [3], [], NULL, NULL
> > 
> > (hanged again here)
> > 
> > 
> > Do you think this info is enough or I really have to strace it since the
> > beginning?
> > If it is a race condition it might not happen if the sftp-server is deeply
> > slowed down by the strace.
> > If I had a way to make it continue right now we could get the rest of the
> > strace... But it's not so easy, I tried starting a Samba transfer but it did
> > not unlock the SFTP this time. SIGSTOP + SIGCONT also didn't work.
> > 
> > BTW people using the Storage also experienced data loss while pushing files
> > in it: appartently data disappeared from the middle of a file they were
> > saving to the Storage.
> > To me looks like another hint that application-level data which has been
> > received via network by TCP stack is trapped there and not being pushed to
> > the application.
> > Or the data might even be trapped into the anonymous sockets between sshd
> > and sftp-server.
> > 
> > Thanks for your help
> 
> I thought that it would be more meaningful to try the cat stack, the strace
> and the SIGSTOP SIGCONT tricks on the sshd which actually holds the TCP
> socket, instead of the sftp-server. Here they are, on the sshd:
> 
> root@mystorage:/root# strace -p 11449 2>&1 | tee -a sshd1strace1.dmp
> Process 11449 attached - interrupt to quit
> read(5, 0x7fff4e956220, 4)              = ? ERESTARTSYS (To be restarted)
> --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> --- SIGSTOP (Stopped (signal)) @ 0 (0) ---
> read(5, 0x7fff4e956220, 4)              = ? ERESTARTSYS (To be restarted)
> --- SIGCONT (Continued) @ 0 (0) ---
> read(5,
> 
> (hanged again here after the SIGSTOP + SIGCONT, unfortunately)
> 
> root@mystorage:/root# cat /proc/11449/stack
> [<ffffffff814b428a>] unix_stream_data_wait+0xaa/0x110
> [<ffffffff814b50cd>] unix_stream_recvmsg+0x36d/0x570
> [<ffffffff81426399>] sock_aio_read+0x149/0x150
> [<ffffffff8111e232>] do_sync_read+0xf2/0x130
> [<ffffffff8111e8e1>] vfs_read+0x181/0x1a0
> [<ffffffff8111edec>] sys_read+0x4c/0x80
> [<ffffffff8101225b>] tracesys+0xd9/0xde
> [<ffffffffffffffff>] 0xffffffffffffffff
> 
> root@mystorage:/proc/11449/fd# ll
> total 0
> lrwx------ 1 root root 64 2009-11-25 14:43 0 -> /dev/null
> lrwx------ 1 root root 64 2009-11-25 14:43 1 -> /dev/null
> lrwx------ 1 root root 64 2009-11-25 14:43 2 -> /dev/null
> lrwx------ 1 root root 64 2009-11-25 14:43 3 -> socket:[5326251]
> lrwx------ 1 root root 64 2009-11-25 14:43 5 -> socket:[5326303]
> 
> 
> I would like to try to attach a program of mine (a python interactive) to
> "socket:[5326303]" and try to read data from there to see if something comes
> out of the TCP stack, but I don't know where to find the socket:[5326303] on
> the filesystem. Is it mapped to a file anywhere in Linux?

This line still tells that there is nothing to read from the socket:

> > And this is from cat /proc/net/tcp   2: 0F12A8C0:0016 2512A8C0:0FBD 01    
> > 00000000:00000000 02:00009144 00000000     0        0 5326251 2

Let me reiterate, the first zeros here are the write side queue, the 
latter are for the receive (not including ofo queue but if tcpdump doesn't 
show ofo that's not significant detail here).

> > ffff88085408ce00 26 4 1 9 4

To me it seems that this particular problem so far shows very little 
problem on TCP side. Ie., TCP just "stalls" because it is not fed with 
anything.

What I would want to make absolutely sure, is that sftp-server was waiting 
for a pipe which comes from sshd, I don't think we have confirmed that 
just yet.

Also, I'd want to get a tcpdump, strace (the latter attach might be ok 
but I'm not sure just yet) and /proc/net/tcp from a consistent case rather 
than trying to mix multiple captures from different stalls. This is very 
important since your captures are already in disagreement on what is 
happening.

In addition, I want to see TCP timestamps (probably you need more -vvv to 
tcpdump to show them), however, if there are no timestamps in use at all
and 1gpbs speeds, it could potentially explain the missing data already.


-- 
 i.

^ permalink raw reply

* Re: [rfc 2/3 v2] [rfc 3/4] igb: Common error path in igb_init_vfs()
From: Alexander Duyck @ 2009-11-25 22:35 UTC (permalink / raw)
  To: Simon Horman
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Arnd Bergmann, Kirsher, Jeffrey T
In-Reply-To: <20091125063531.218106764@vergenet.net>

Simon Horman wrote:
> Drop out into an error path on error.
> 
> This is a bit superfluous as things stand, though arguably
> it already makes the code cleaner. But it should help things a lot
> if igb_init_vfs() has a several things to unwind on error.
> 
> This patch eliminates resetting adapter->vfs_allocated_count to 0
> in the case where CONFIG_PCI_IOV is not set, because in that
> case adapter->vfs_allocated_count can never be non-zero.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> --- 
> Thu, 05 Nov 2009 11:58:50 +1100
> * Initial post
> 
> Wed, 25 Nov 2009 16:44:15 +1100
> * Merged with " Initialise adapter->vfs_allocated_count in igb_init_vf()",
>   however the initialisation of adapter->vfs_allocated_count is no
>   longer moved into igb_init_vf() as doing so results in a panic.
> 
> Index: net-next-2.6/drivers/net/igb/igb_main.c
> ===================================================================
> --- net-next-2.6.orig/drivers/net/igb/igb_main.c	2009-11-25 17:05:43.000000000 +1100
> +++ net-next-2.6/drivers/net/igb/igb_main.c	2009-11-25 17:05:44.000000000 +1100
> @@ -1753,38 +1753,37 @@ static void __devinit igb_init_vf(struct
>  {
>  #ifdef CONFIG_PCI_IOV
>  	struct pci_dev *pdev = adapter->pdev;
> +	unsigned char mac_addr[ETH_ALEN];
> +	int i;
>  
>  	if (adapter->vfs_allocated_count > 7)
>  		adapter->vfs_allocated_count = 7;
>  
> -	if (adapter->vfs_allocated_count) {
> -		adapter->vf_data = kcalloc(adapter->vfs_allocated_count,
> -		                           sizeof(struct vf_data_storage),
> -		                           GFP_KERNEL);
> -		/* if allocation failed then we do not support SR-IOV */
> -		if (!adapter->vf_data) {
> -			adapter->vfs_allocated_count = 0;
> -			dev_err(&pdev->dev, "Unable to allocate memory for VF "
> -			        "Data Storage\n");
> -		}
> +	adapter->vf_data = kcalloc(adapter->vfs_allocated_count,
> +				   sizeof(struct vf_data_storage), GFP_KERNEL);
> +	/* if allocation failed then we do not support SR-IOV */
> +	if (!adapter->vf_data) {
> +		dev_err(&pdev->dev, "Unable to allocate memory for VF "
> +		        "Data Storage\n");
> +		goto err_zero;
>  	}
>  
The only issue I see with the changes above is that if SR-IOV is not 
enabled via the max_vfs option you will end up returning an error even 
though there is nothing wrong.  You might want to go and keep the if 
statement near the start and do something like:
	if (!adapter->vfs_allocated_count)
		goto err_zero;

This way you can avoid the false error report.

> -	if (pci_enable_sriov(pdev, adapter->vfs_allocated_count)) {
> -		kfree(adapter->vf_data);
> -		adapter->vf_data = NULL;
> -#endif /* CONFIG_PCI_IOV */
> -		adapter->vfs_allocated_count = 0;
> -#ifdef CONFIG_PCI_IOV
> -	} else {
> -		unsigned char mac_addr[ETH_ALEN];
> -		int i;
> -		dev_info(&pdev->dev, "%d vfs allocated\n",
> -		         adapter->vfs_allocated_count);
> -		for (i = 0; i < adapter->vfs_allocated_count; i++) {
> -			random_ether_addr(mac_addr);
> -			igb_set_vf_mac(adapter, i, mac_addr);
> -		}
> +	if (pci_enable_sriov(pdev, adapter->vfs_allocated_count))
> +		goto err_free;
> +
> +	dev_info(&pdev->dev, "%d vfs allocated\n",
> +		 adapter->vfs_allocated_count);
> +	for (i = 0; i < adapter->vfs_allocated_count; i++) {
> +		random_ether_addr(mac_addr);
> +		igb_set_vf_mac(adapter, i, mac_addr);
>  	}
> +
> +	return;
> +err_free:
> +	kfree(adapter->vf_data);
> +err_zero:
> +	adapter->vf_data = NULL;
> +	adapter->vfs_allocated_count = 0;
>  #endif /* CONFIG_PCI_IOV */
>  }
>  
> 

My preference here would be to bump the #endif up two lines so that 
adapter->vf_data = NULL and adapter->vfs_allocated_count = 0 are ran in 
the non-iov case.  That way we know that SR-IOV is explicitly disabled.

Thanks,

Alex

------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july

^ permalink raw reply

* [PATCH net-next-2.6] net: use net_eq to compare nets
From: Octavian Purdila @ 2009-11-25 22:51 UTC (permalink / raw)
  To: netdev; +Cc: Julia Lawall


Generated with the following semantic patch

@@
struct net *n1;
struct net *n2;
@@
- n1 == n2
+ net_eq(n1, n2)

@@
struct net *n1;
struct net *n2;
@@
- n1 != n2
+ !net_eq(n1, n2)

applied over {include,net,drivers/net}.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 drivers/net/hamradio/bpqether.c |    4 ++--
 drivers/net/loopback.c          |    2 +-
 drivers/net/wan/hdlc.c          |    4 ++--
 net/appletalk/ddp.c             |    2 +-
 net/atm/svc.c                   |    2 +-
 net/ax25/af_ax25.c              |    2 +-
 net/can/af_can.c                |    2 +-
 net/core/dev.c                  |    4 ++--
 net/core/neighbour.c            |    2 +-
 net/core/net-sysfs.c            |    4 ++--
 net/core/net_namespace.c        |    2 +-
 net/core/sysctl_net_core.c      |    2 +-
 net/dcb/dcbnl.c                 |    2 +-
 net/decnet/af_decnet.c          |    2 +-
 net/decnet/dn_dev.c             |    6 +++---
 net/decnet/dn_fib.c             |    4 ++--
 net/decnet/dn_route.c           |    4 ++--
 net/decnet/dn_table.c           |    2 +-
 net/econet/af_econet.c          |    2 +-
 net/ieee802154/af_ieee802154.c  |    2 +-
 net/ipv4/devinet.c              |    2 +-
 net/ipv4/fib_semantics.c        |    4 ++--
 net/ipv4/inet_connection_sock.c |    4 ++--
 net/ipv4/inet_hashtables.c      |    3 ++-
 net/ipv4/ip_fragment.c          |    4 ++--
 net/ipv4/ip_input.c             |    2 +-
 net/ipv4/netfilter/ip_queue.c   |    2 +-
 net/ipv4/route.c                |    4 ++--
 net/ipv4/sysctl_net_ipv4.c      |    4 ++--
 net/ipv6/addrconf.c             |    4 ++--
 net/ipv6/ip6_flowlabel.c        |    9 +++++----
 net/ipv6/netfilter/ip6_queue.c  |    2 +-
 net/ipv6/reassembly.c           |    4 ++--
 net/ipx/af_ipx.c                |    2 +-
 net/llc/af_llc.c                |    2 +-
 net/netfilter/nfnetlink_log.c   |    2 +-
 net/netlink/af_netlink.c        |    2 +-
 net/netrom/af_netrom.c          |    2 +-
 net/packet/af_packet.c          |    6 +++---
 net/rose/af_rose.c              |    2 +-
 net/rxrpc/af_rxrpc.c            |    2 +-
 net/sched/act_api.c             |    4 ++--
 net/sched/cls_api.c             |    4 ++--
 net/sched/sch_api.c             |   10 +++++-----
 net/tipc/socket.c               |    2 +-
 net/x25/af_x25.c                |    2 +-
 46 files changed, 74 insertions(+), 72 deletions(-)

diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index fe893c9..76abed9 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -186,7 +186,7 @@ static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_ty
 	struct ethhdr *eth;
 	struct bpqdev *bpq;
 
-	if (dev_net(dev) != &init_net)
+	if (!net_eq(dev_net(dev), &init_net))
 		goto drop;
 
 	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
@@ -552,7 +552,7 @@ static int bpq_device_event(struct notifier_block *this,unsigned long event, voi
 {
 	struct net_device *dev = (struct net_device *)ptr;
 
-	if (dev_net(dev) != &init_net)
+	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
 
 	if (!dev_is_ethdev(dev))
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 1bc654a..c9f6557 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -207,7 +207,7 @@ static __net_init int loopback_net_init(struct net *net)
 out_free_netdev:
 	free_netdev(dev);
 out:
-	if (net == &init_net)
+	if (net_eq(net, &init_net))
 		panic("loopback: Failed to register netdevice: %d\n", err);
 	return err;
 }
diff --git a/drivers/net/wan/hdlc.c b/drivers/net/wan/hdlc.c
index cc07236..9937bba 100644
--- a/drivers/net/wan/hdlc.c
+++ b/drivers/net/wan/hdlc.c
@@ -57,7 +57,7 @@ static int hdlc_rcv(struct sk_buff *skb, struct net_device *dev,
 {
 	struct hdlc_device *hdlc = dev_to_hdlc(dev);
 
-	if (dev_net(dev) != &init_net) {
+	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(skb);
 		return 0;
 	}
@@ -102,7 +102,7 @@ static int hdlc_device_event(struct notifier_block *this, unsigned long event,
 	unsigned long flags;
 	int on;
 
-	if (dev_net(dev) != &init_net)
+	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
 
 	if (!(dev->priv_flags & IFF_WAN_HDLC))
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 73ca4d5..9fc4da5 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1023,7 +1023,7 @@ static int atalk_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	int rc = -ESOCKTNOSUPPORT;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	/*
diff --git a/net/atm/svc.c b/net/atm/svc.c
index c739507..66e1d9b 100644
--- a/net/atm/svc.c
+++ b/net/atm/svc.c
@@ -655,7 +655,7 @@ static int svc_create(struct net *net, struct socket *sock, int protocol,
 {
 	int error;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	sock->ops = &svc_proto_ops;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index d6ddfa4..5588ba6 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -805,7 +805,7 @@ static int ax25_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	ax25_cb *ax25;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	switch (sock->type) {
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 833bd83..f306717 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -126,7 +126,7 @@ static int can_create(struct net *net, struct socket *sock, int protocol,
 	if (protocol < 0 || protocol >= CAN_NPROTO)
 		return -EINVAL;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 #ifdef CONFIG_MODULES
diff --git a/net/core/dev.c b/net/core/dev.c
index ccefa24..e65af60 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -985,7 +985,7 @@ rollback:
 	/* For now only devices in the initial network namespace
 	 * are in sysfs.
 	 */
-	if (net == &init_net) {
+	if (net_eq(net, &init_net)) {
 		ret = device_rename(&dev->dev, dev->name);
 		if (ret) {
 			memcpy(dev->name, oldname, IFNAMSIZ);
@@ -4792,7 +4792,7 @@ static void rollback_registered_many(struct list_head *head)
 	list_for_each_entry_safe(dev, aux, head, unreg_list) {
 		int new_net = 1;
 		list_for_each_entry(fdev, &pernet_list, unreg_list) {
-			if (dev_net(dev) == dev_net(fdev)) {
+			if (net_eq(dev_net(dev), dev_net(fdev))) {
 				new_net = 0;
 				dev_put(dev);
 				break;
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e587e68..a08a35b 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2092,7 +2092,7 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 		if (h > s_h)
 			s_idx = 0;
 		for (n = tbl->hash_buckets[h], idx = 0; n; n = n->next) {
-			if (dev_net(n->dev) != net)
+			if (!net_eq(dev_net(n->dev), net))
 				continue;
 			if (idx < s_idx)
 				goto next;
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 157645c..fbc1c74 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -525,7 +525,7 @@ void netdev_unregister_kobject(struct net_device * net)
 
 	kobject_get(&dev->kobj);
 
-	if (dev_net(net) != &init_net)
+	if (!net_eq(dev_net(net), &init_net))
 		return;
 
 	device_del(dev);
@@ -559,7 +559,7 @@ int netdev_register_kobject(struct net_device *net)
 #endif
 #endif /* CONFIG_SYSFS */
 
-	if (dev_net(net) != &init_net)
+	if (!net_eq(dev_net(net), &init_net))
 		return 0;
 
 	return device_add(dev);
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 1c1af27..86ed7f4 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -280,7 +280,7 @@ out_undo:
 	list_del(&ops->list);
 	if (ops->exit) {
 		for_each_net(undo_net) {
-			if (undo_net == net)
+			if (net_eq(undo_net, net))
 				goto undone;
 			ops->exit(undo_net);
 		}
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index 7db1de0..fcfc545 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -134,7 +134,7 @@ static __net_init int sysctl_core_net_init(struct net *net)
 	net->core.sysctl_somaxconn = SOMAXCONN;
 
 	tbl = netns_core_table;
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		tbl = kmemdup(tbl, sizeof(netns_core_table), GFP_KERNEL);
 		if (tbl == NULL)
 			goto err_dup;
diff --git a/net/dcb/dcbnl.c b/net/dcb/dcbnl.c
index ac1205d..2afd617 100644
--- a/net/dcb/dcbnl.c
+++ b/net/dcb/dcbnl.c
@@ -1126,7 +1126,7 @@ static int dcb_doit(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	u32 pid = skb ? NETLINK_CB(skb).pid : 0;
 	int ret = -EINVAL;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	ret = nlmsg_parse(nlh, sizeof(*dcb), tb, DCB_ATTR_MAX,
diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 9ade3a6..2b494fa 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -680,7 +680,7 @@ static int dn_create(struct net *net, struct socket *sock, int protocol,
 {
 	struct sock *sk;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	switch(sock->type) {
diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index 6c916e2..f20dec9 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -636,7 +636,7 @@ static int dn_nl_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	struct dn_ifaddr *ifa, **ifap;
 	int err = -EINVAL;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		goto errout;
 
 	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, dn_ifa_policy);
@@ -675,7 +675,7 @@ static int dn_nl_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	struct dn_ifaddr *ifa;
 	int err;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, dn_ifa_policy);
@@ -789,7 +789,7 @@ static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	struct dn_dev *dn_db;
 	struct dn_ifaddr *ifa;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	skip_ndevs = cb->args[0];
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index fd641f6..e9d4870 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -509,7 +509,7 @@ static int dn_fib_rtm_delroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *
 	struct rtattr **rta = arg;
 	struct rtmsg *r = NLMSG_DATA(nlh);
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	if (dn_fib_check_attr(r, rta))
@@ -529,7 +529,7 @@ static int dn_fib_rtm_newroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *
 	struct rtattr **rta = arg;
 	struct rtmsg *r = NLMSG_DATA(nlh);
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	if (dn_fib_check_attr(r, rta))
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 860286a..a032840 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1517,7 +1517,7 @@ static int dn_cache_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh, void
 	struct sk_buff *skb;
 	struct flowi fl;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	memset(&fl, 0, sizeof(fl));
@@ -1602,7 +1602,7 @@ int dn_cache_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	int h, s_h;
 	int idx, s_idx;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	if (NLMSG_PAYLOAD(cb->nlh, 0) < sizeof(struct rtmsg))
diff --git a/net/decnet/dn_table.c b/net/decnet/dn_table.c
index 67054b0..f281e0f 100644
--- a/net/decnet/dn_table.c
+++ b/net/decnet/dn_table.c
@@ -471,7 +471,7 @@ int dn_fib_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	struct hlist_node *node;
 	int dumped = 0;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	if (NLMSG_PAYLOAD(cb->nlh, 0) >= sizeof(struct rtmsg) &&
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 5966798..29b4931 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -612,7 +612,7 @@ static int econet_create(struct net *net, struct socket *sock, int protocol,
 	struct econet_sock *eo;
 	int err;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	/* Econet only provides datagram services. */
diff --git a/net/ieee802154/af_ieee802154.c b/net/ieee802154/af_ieee802154.c
index de6e34d..bad1c49 100644
--- a/net/ieee802154/af_ieee802154.c
+++ b/net/ieee802154/af_ieee802154.c
@@ -241,7 +241,7 @@ static int ieee802154_create(struct net *net, struct socket *sock,
 	struct proto *proto;
 	const struct proto_ops *ops;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	switch (sock->type) {
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7620382..c100709 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1605,7 +1605,7 @@ static __net_init int devinet_init_net(struct net *net)
 	all = &ipv4_devconf;
 	dflt = &ipv4_devconf_dflt;
 
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		all = kmemdup(all, sizeof(ipv4_devconf), GFP_KERNEL);
 		if (all == NULL)
 			goto err_alloc_all;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 9b096d6..ed19aa6 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -228,7 +228,7 @@ static struct fib_info *fib_find_info(const struct fib_info *nfi)
 	head = &fib_info_hash[hash];
 
 	hlist_for_each_entry(fi, node, head, fib_hash) {
-		if (fi->fib_net != nfi->fib_net)
+		if (!net_eq(fi->fib_net, nfi->fib_net))
 			continue;
 		if (fi->fib_nhs != nfi->fib_nhs)
 			continue;
@@ -1047,7 +1047,7 @@ int fib_sync_down_addr(struct net *net, __be32 local)
 		return 0;
 
 	hlist_for_each_entry(fi, node, head, fib_lhash) {
-		if (fi->fib_net != net)
+		if (!net_eq(fi->fib_net, net))
 			continue;
 		if (fi->fib_prefsrc == local) {
 			fi->fib_flags |= RTNH_F_DEAD;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 26fb50e..9b35c56 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -112,7 +112,7 @@ again:
 					hashinfo->bhash_size)];
 			spin_lock(&head->lock);
 			inet_bind_bucket_for_each(tb, node, &head->chain)
-				if (ib_net(tb) == net && tb->port == rover) {
+				if (net_eq(ib_net(tb), net) && tb->port == rover) {
 					if (tb->fastreuse > 0 &&
 					    sk->sk_reuse &&
 					    sk->sk_state != TCP_LISTEN &&
@@ -158,7 +158,7 @@ have_snum:
 				hashinfo->bhash_size)];
 		spin_lock(&head->lock);
 		inet_bind_bucket_for_each(tb, node, &head->chain)
-			if (ib_net(tb) == net && tb->port == snum)
+			if (net_eq(ib_net(tb), net) && tb->port == snum)
 				goto tb_found;
 	}
 	tb = NULL;
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 47ad7aa..94ef51a 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -454,7 +454,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 			 * unique enough.
 			 */
 			inet_bind_bucket_for_each(tb, node, &head->chain) {
-				if (ib_net(tb) == net && tb->port == port) {
+				if (net_eq(ib_net(tb), net) &&
+				    tb->port == port) {
 					if (tb->fastreuse >= 0)
 						goto next_port;
 					WARN_ON(hlist_empty(&tb->owners));
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index b007f8a..1472d8e 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -658,7 +658,7 @@ static int ip4_frags_ns_ctl_register(struct net *net)
 	struct ctl_table_header *hdr;
 
 	table = ip4_frags_ns_ctl_table;
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		table = kmemdup(table, sizeof(ip4_frags_ns_ctl_table), GFP_KERNEL);
 		if (table == NULL)
 			goto err_alloc;
@@ -676,7 +676,7 @@ static int ip4_frags_ns_ctl_register(struct net *net)
 	return 0;
 
 err_reg:
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		kfree(table);
 err_alloc:
 	return -ENOMEM;
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index fdf51ba..c29de98 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -164,7 +164,7 @@ int ip_call_ra_chain(struct sk_buff *skb)
 		if (sk && inet_sk(sk)->inet_num == protocol &&
 		    (!sk->sk_bound_dev_if ||
 		     sk->sk_bound_dev_if == dev->ifindex) &&
-		    sock_net(sk) == dev_net(dev)) {
+		    net_eq(sock_net(sk), dev_net(dev))) {
 			if (ip_hdr(skb)->frag_off & htons(IP_MF | IP_OFFSET)) {
 				if (ip_defrag(skb, IP_DEFRAG_CALL_RA_CHAIN)) {
 					read_unlock(&ip_ra_lock);
diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index c156db2..884f085 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -500,7 +500,7 @@ ipq_rcv_nl_event(struct notifier_block *this,
 	if (event == NETLINK_URELEASE &&
 	    n->protocol == NETLINK_FIREWALL && n->pid) {
 		write_lock_bh(&queue_lock);
-		if ((n->net == &init_net) && (n->pid == peer_pid))
+		if ((net_eq(n->net, &init_net)) && (n->pid == peer_pid))
 			__ipq_reset();
 		write_unlock_bh(&queue_lock);
 	}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 7547944..aea7bb3 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -703,7 +703,7 @@ static inline int compare_keys(struct flowi *fl1, struct flowi *fl2)
 
 static inline int compare_netns(struct rtable *rt1, struct rtable *rt2)
 {
-	return dev_net(rt1->u.dst.dev) == dev_net(rt2->u.dst.dev);
+	return net_eq(dev_net(rt1->u.dst.dev), dev_net(rt2->u.dst.dev));
 }
 
 static inline int rt_is_expired(struct rtable *rth)
@@ -3310,7 +3310,7 @@ static __net_init int sysctl_route_net_init(struct net *net)
 	struct ctl_table *tbl;
 
 	tbl = ipv4_route_flush_table;
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		tbl = kmemdup(tbl, sizeof(ipv4_route_flush_table), GFP_KERNEL);
 		if (tbl == NULL)
 			goto err_dup;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 2dcf04d..c00323b 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -818,7 +818,7 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 	struct ctl_table *table;
 
 	table = ipv4_net_table;
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		table = kmemdup(table, sizeof(ipv4_net_table), GFP_KERNEL);
 		if (table == NULL)
 			goto err_alloc;
@@ -849,7 +849,7 @@ static __net_init int ipv4_sysctl_init_net(struct net *net)
 	return 0;
 
 err_reg:
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		kfree(table);
 err_alloc:
 	return -ENOMEM;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 522bdc7..b1ce8fc 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4498,7 +4498,7 @@ static int addrconf_init_net(struct net *net)
 	all = &ipv6_devconf;
 	dflt = &ipv6_devconf_dflt;
 
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		all = kmemdup(all, sizeof(ipv6_devconf), GFP_KERNEL);
 		if (all == NULL)
 			goto err_alloc_all;
@@ -4546,7 +4546,7 @@ static void addrconf_exit_net(struct net *net)
 	__addrconf_sysctl_unregister(net->ipv6.devconf_dflt);
 	__addrconf_sysctl_unregister(net->ipv6.devconf_all);
 #endif
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		kfree(net->ipv6.devconf_dflt);
 		kfree(net->ipv6.devconf_all);
 	}
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 593a67e..6e7bffa 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -67,7 +67,7 @@ static inline struct ip6_flowlabel *__fl_lookup(struct net *net, __be32 label)
 	struct ip6_flowlabel *fl;
 
 	for (fl=fl_ht[FL_HASH(label)]; fl; fl = fl->next) {
-		if (fl->label == label && fl->fl_net == net)
+		if (fl->label == label && net_eq(fl->fl_net, net))
 			return fl;
 	}
 	return NULL;
@@ -163,7 +163,8 @@ static void ip6_fl_purge(struct net *net)
 		struct ip6_flowlabel *fl, **flp;
 		flp = &fl_ht[i];
 		while ((fl = *flp) != NULL) {
-			if (fl->fl_net == net && atomic_read(&fl->users) == 0) {
+			if (net_eq(fl->fl_net, net) &&
+			    atomic_read(&fl->users) == 0) {
 				*flp = fl->next;
 				fl_free(fl);
 				atomic_dec(&fl_size);
@@ -630,7 +631,7 @@ static struct ip6_flowlabel *ip6fl_get_first(struct seq_file *seq)
 	for (state->bucket = 0; state->bucket <= FL_HASH_MASK; ++state->bucket) {
 		fl = fl_ht[state->bucket];
 
-		while (fl && fl->fl_net != net)
+		while (fl && !net_eq(fl->fl_net, net))
 			fl = fl->next;
 		if (fl)
 			break;
@@ -645,7 +646,7 @@ static struct ip6_flowlabel *ip6fl_get_next(struct seq_file *seq, struct ip6_flo
 
 	fl = fl->next;
 try_again:
-	while (fl && fl->fl_net != net)
+	while (fl && !net_eq(fl->fl_net, net))
 		fl = fl->next;
 
 	while (!fl) {
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index 1cf3f0c..4c7a18a 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -502,7 +502,7 @@ ipq_rcv_nl_event(struct notifier_block *this,
 	if (event == NETLINK_URELEASE &&
 	    n->protocol == NETLINK_IP6_FW && n->pid) {
 		write_lock_bh(&queue_lock);
-		if ((n->net == &init_net) && (n->pid == peer_pid))
+		if ((net_eq(n->net, &init_net)) && (n->pid == peer_pid))
 			__ipq_reset();
 		write_unlock_bh(&queue_lock);
 	}
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index dce699f..45efc39 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -681,7 +681,7 @@ static int ip6_frags_ns_sysctl_register(struct net *net)
 	struct ctl_table_header *hdr;
 
 	table = ip6_frags_ns_ctl_table;
-	if (net != &init_net) {
+	if (!net_eq(net, &init_net)) {
 		table = kmemdup(table, sizeof(ip6_frags_ns_ctl_table), GFP_KERNEL);
 		if (table == NULL)
 			goto err_alloc;
@@ -699,7 +699,7 @@ static int ip6_frags_ns_sysctl_register(struct net *net)
 	return 0;
 
 err_reg:
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		kfree(table);
 err_alloc:
 	return -ENOMEM;
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 975c5a3..f9759b5 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1362,7 +1362,7 @@ static int ipx_create(struct net *net, struct socket *sock, int protocol,
 	int rc = -ESOCKTNOSUPPORT;
 	struct sock *sk;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	/*
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 5266c28..3a66546 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -158,7 +158,7 @@ static int llc_ui_create(struct net *net, struct socket *sock, int protocol,
 	if (!capable(CAP_NET_RAW))
 		return -EPERM;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	if (likely(sock->type == SOCK_DGRAM || sock->type == SOCK_STREAM)) {
diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index f900dc3..4900548 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -678,7 +678,7 @@ nfulnl_rcv_nl_event(struct notifier_block *this,
 			struct hlist_head *head = &instance_table[i];
 
 			hlist_for_each_entry_safe(inst, tmp, t2, head, hlist) {
-				if ((n->net == &init_net) &&
+				if ((net_eq(n->net, &init_net)) &&
 				    (n->pid == inst->peer_pid))
 					__instance_destroy(inst);
 			}
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index eff5b0d..a4957bf 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1092,7 +1092,7 @@ static inline int do_one_set_err(struct sock *sk,
 	if (sk == p->exclude_sk)
 		goto out;
 
-	if (sock_net(sk) != sock_net(p->exclude_sk))
+	if (!net_eq(sock_net(sk), sock_net(p->exclude_sk)))
 		goto out;
 
 	if (nlk->pid == p->pid || p->group - 1 >= nlk->ngroups ||
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 4bdd569..71604c6 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -431,7 +431,7 @@ static int nr_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	struct nr_sock *nr;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	if (sock->type != SOCK_SEQPACKET || protocol != 0)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index c620bd9..940fc20 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -365,7 +365,7 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
 	if (skb->pkt_type == PACKET_LOOPBACK)
 		goto out;
 
-	if (dev_net(dev) != sock_net(sk))
+	if (!net_eq(dev_net(dev), sock_net(sk)))
 		goto out;
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
@@ -553,7 +553,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 	sk = pt->af_packet_priv;
 	po = pkt_sk(sk);
 
-	if (dev_net(dev) != sock_net(sk))
+	if (!net_eq(dev_net(dev), sock_net(sk)))
 		goto drop;
 
 	skb->dev = dev;
@@ -674,7 +674,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	sk = pt->af_packet_priv;
 	po = pkt_sk(sk);
 
-	if (dev_net(dev) != sock_net(sk))
+	if (!net_eq(dev_net(dev), sock_net(sk)))
 		goto drop;
 
 	if (dev->header_ops) {
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index 4de4287..8feb9e5 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -518,7 +518,7 @@ static int rose_create(struct net *net, struct socket *sock, int protocol,
 	struct sock *sk;
 	struct rose_sock *rose;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	if (sock->type != SOCK_SEQPACKET || protocol != 0)
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index f978d02..287b141 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -616,7 +616,7 @@ static int rxrpc_create(struct net *net, struct socket *sock, int protocol,
 
 	_enter("%p,%d", sock, protocol);
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	/* we support transport protocol UDP only */
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index ca2e1fd..2a74003 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -969,7 +969,7 @@ static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	u32 pid = skb ? NETLINK_CB(skb).pid : 0;
 	int ret = 0, ovr = 0;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	ret = nlmsg_parse(n, sizeof(struct tcamsg), tca, TCA_ACT_MAX, NULL);
@@ -1052,7 +1052,7 @@ tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
 	struct tcamsg *t = (struct tcamsg *) NLMSG_DATA(cb->nlh);
 	struct nlattr *kind = find_dump_kind(cb->nlh);
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	if (kind == NULL) {
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c024da7..3725d8f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -137,7 +137,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	int err;
 	int tp_created = 0;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 replay:
@@ -418,7 +418,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 	const struct Qdisc_class_ops *cops;
 	struct tcf_dump_args arg;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	if (cb->nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*tcm)))
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 876ba4b..75fd1c6 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -947,7 +947,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	struct Qdisc *p = NULL;
 	int err;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	if ((dev = __dev_get_by_index(&init_net, tcm->tcm_ifindex)) == NULL)
@@ -1009,7 +1009,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	struct Qdisc *q, *p;
 	int err;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 replay:
@@ -1274,7 +1274,7 @@ static int tc_dump_qdisc(struct sk_buff *skb, struct netlink_callback *cb)
 	int s_idx, s_q_idx;
 	struct net_device *dev;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	s_idx = cb->args[0];
@@ -1334,7 +1334,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n, void *arg)
 	u32 qid = TC_H_MAJ(clid);
 	int err;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EINVAL;
 
 	if ((dev = __dev_get_by_index(&init_net, tcm->tcm_ifindex)) == NULL)
@@ -1576,7 +1576,7 @@ static int tc_dump_tclass(struct sk_buff *skb, struct netlink_callback *cb)
 	struct net_device *dev;
 	int t, s_t;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return 0;
 
 	if (cb->nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*tcm)))
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index d00c211..eca5eb0 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -195,7 +195,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol,
 
 	/* Validate arguments */
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	if (unlikely(protocol != 0))
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index ac7dba4..2a3a513 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -514,7 +514,7 @@ static int x25_create(struct net *net, struct socket *sock, int protocol,
 	struct x25_sock *x25;
 	int rc = -ESOCKTNOSUPPORT;
 
-	if (net != &init_net)
+	if (!net_eq(net, &init_net))
 		return -EAFNOSUPPORT;
 
 	if (sock->type != SOCK_SEQPACKET || protocol)
-- 
1.5.6.5


^ permalink raw reply related

* Re: [PATCH] b44.c box lockup fix (netconsole): ratelimit NAPI poll error message
From: David Miller @ 2009-11-25 22:56 UTC (permalink / raw)
  To: andi; +Cc: zambrano, dave, mb, netdev, linux-kernel
In-Reply-To: <20091125213546.GA6168@rhlx01.hs-esslingen.de>

From: Andreas Mohr <andi@lisas.de>
Date: Wed, 25 Nov 2009 22:35:46 +0100

> Turns out that it's not very helpful to have a netconsole-deployed
> network driver's interrupt handler message spew noise permanently
> (see comment in patch).

It's not even an illegal state, especially with shared interrupts.
The warning should never be emitted.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: use net_eq to compare nets
From: David Miller @ 2009-11-25 23:14 UTC (permalink / raw)
  To: opurdila; +Cc: netdev, julia
In-Reply-To: <200911260051.17430.opurdila@ixiacom.com>

From: Octavian Purdila <opurdila@ixiacom.com>
Date: Thu, 26 Nov 2009 00:51:17 +0200

> Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>

Applied, thanks.

^ permalink raw reply

* Re: how expensive are mallocs?
From: David Miller @ 2009-11-25 23:18 UTC (permalink / raw)
  To: andy.grover; +Cc: netdev
In-Reply-To: <c0a09e5c0911240957x45abfb26k1611e53f435e5179@mail.gmail.com>

From: Andrew Grover <andy.grover@gmail.com>
Date: Tue, 24 Nov 2009 09:57:34 -0800

> How much effort generally makes sense to avoid mallocs? For example,
> rds has a function that does a kmalloc for an array of scatterlist
> entries, which is freed at the bottom of the function. We couldn't
> allocate off the stack in all cases (too big), but we could allocate
> an array of say 8 scatterlists, and use it if that's big enough,
> falling back to kmalloc if it's not.
> 
> Is this a good idea?

This is a poor idea, especially if this function executes frequently.

Better to have a per-socket or per-cpu (softirq protected, if
necessary) work area to do such things if you really can't fit it on
the stack.

> Also, RDS has its own per-cpu page remainder allocator (see
> net/rds/page.c) for kernel send buffers. Would cutting this code and
> just using kmalloc be recommended? Doesn't SL?B already do per-cpu
> pools?
> 
> Does this stuff even matter enough to rise above the noise in benchmarks?

We already have a per-socket page allocation scheme for sendmsg handling.

In general adding more and more specialized allocators is heavily
discouraged.  If the generic kernel facilities don't fit the bill,
fix them.


^ permalink raw reply

* Re: [rfc 1/3 v2] [E1000-devel] [rfc 1/4] igb: Add igb_cleanup_vf()
From: Simon Horman @ 2009-11-25 23:26 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Arnd Bergmann, Kirsher, Jeffrey T
In-Reply-To: <4B0DAF55.4060400@intel.com>

On Wed, Nov 25, 2009 at 02:27:33PM -0800, Alexander Duyck wrote:
> Simon Horman wrote:
> >Move virtual finction cleanup code into igb_cleanup_vf() and for the sake
> >of symmetry rename igb_probe_vfs() as igb_init_vf().
> >
> >Although these functions aren't entirely symmetrical it should aid
> >maintenance by making the relationship between initialisation and cleanup
> >more obvious.
> >
> >Note that there appears to be no way for adapter->vfs_allocated_count to be
> >non-zero for the case where CONFIG_PCI_IOV is not set, so reseting this
> >value was moved to inside the relvant #ifdef.
> >
> >Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> The one thing I noticed with this patch is that it didn't remove the
> vf cleanup from igb_remove.  You may want to go back and also add
> the replacement of that function with igb_cleanup_vf to this patch.

Ooops, yes that would be a good idea.
I'll send a revised patch after testing it.


^ permalink raw reply

* Re: [rfc 2/3 v2] [E1000-devel] [rfc 3/4] igb: Common error path in igb_init_vfs()
From: Simon Horman @ 2009-11-25 23:27 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Arnd Bergmann, Kirsher, Jeffrey T
In-Reply-To: <4B0DB12C.3030709@intel.com>

On Wed, Nov 25, 2009 at 02:35:24PM -0800, Alexander Duyck wrote:
> Simon Horman wrote:
> >Drop out into an error path on error.
> >
> >This is a bit superfluous as things stand, though arguably
> >it already makes the code cleaner. But it should help things a lot
> >if igb_init_vfs() has a several things to unwind on error.
> >
> >This patch eliminates resetting adapter->vfs_allocated_count to 0
> >in the case where CONFIG_PCI_IOV is not set, because in that
> >case adapter->vfs_allocated_count can never be non-zero.
> >
> >Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> >--- Thu, 05 Nov 2009 11:58:50 +1100
> >* Initial post
> >
> >Wed, 25 Nov 2009 16:44:15 +1100
> >* Merged with " Initialise adapter->vfs_allocated_count in igb_init_vf()",
> >  however the initialisation of adapter->vfs_allocated_count is no
> >  longer moved into igb_init_vf() as doing so results in a panic.
> >
> >Index: net-next-2.6/drivers/net/igb/igb_main.c
> >===================================================================
> >--- net-next-2.6.orig/drivers/net/igb/igb_main.c	2009-11-25 17:05:43.000000000 +1100
> >+++ net-next-2.6/drivers/net/igb/igb_main.c	2009-11-25 17:05:44.000000000 +1100
> >@@ -1753,38 +1753,37 @@ static void __devinit igb_init_vf(struct
> > {
> > #ifdef CONFIG_PCI_IOV
> > 	struct pci_dev *pdev = adapter->pdev;
> >+	unsigned char mac_addr[ETH_ALEN];
> >+	int i;
> > 	if (adapter->vfs_allocated_count > 7)
> > 		adapter->vfs_allocated_count = 7;
> >-	if (adapter->vfs_allocated_count) {
> >-		adapter->vf_data = kcalloc(adapter->vfs_allocated_count,
> >-		                           sizeof(struct vf_data_storage),
> >-		                           GFP_KERNEL);
> >-		/* if allocation failed then we do not support SR-IOV */
> >-		if (!adapter->vf_data) {
> >-			adapter->vfs_allocated_count = 0;
> >-			dev_err(&pdev->dev, "Unable to allocate memory for VF "
> >-			        "Data Storage\n");
> >-		}
> >+	adapter->vf_data = kcalloc(adapter->vfs_allocated_count,
> >+				   sizeof(struct vf_data_storage), GFP_KERNEL);
> >+	/* if allocation failed then we do not support SR-IOV */
> >+	if (!adapter->vf_data) {
> >+		dev_err(&pdev->dev, "Unable to allocate memory for VF "
> >+		        "Data Storage\n");
> >+		goto err_zero;
> > 	}
> The only issue I see with the changes above is that if SR-IOV is not
> enabled via the max_vfs option you will end up returning an error
> even though there is nothing wrong.  You might want to go and keep
> the if statement near the start and do something like:
> 	if (!adapter->vfs_allocated_count)
> 		goto err_zero;
> 
> This way you can avoid the false error report.

Good idea, I will add that.

> >-	if (pci_enable_sriov(pdev, adapter->vfs_allocated_count)) {
> >-		kfree(adapter->vf_data);
> >-		adapter->vf_data = NULL;
> >-#endif /* CONFIG_PCI_IOV */
> >-		adapter->vfs_allocated_count = 0;
> >-#ifdef CONFIG_PCI_IOV
> >-	} else {
> >-		unsigned char mac_addr[ETH_ALEN];
> >-		int i;
> >-		dev_info(&pdev->dev, "%d vfs allocated\n",
> >-		         adapter->vfs_allocated_count);
> >-		for (i = 0; i < adapter->vfs_allocated_count; i++) {
> >-			random_ether_addr(mac_addr);
> >-			igb_set_vf_mac(adapter, i, mac_addr);
> >-		}
> >+	if (pci_enable_sriov(pdev, adapter->vfs_allocated_count))
> >+		goto err_free;
> >+
> >+	dev_info(&pdev->dev, "%d vfs allocated\n",
> >+		 adapter->vfs_allocated_count);
> >+	for (i = 0; i < adapter->vfs_allocated_count; i++) {
> >+		random_ether_addr(mac_addr);
> >+		igb_set_vf_mac(adapter, i, mac_addr);
> > 	}
> >+
> >+	return;
> >+err_free:
> >+	kfree(adapter->vf_data);
> >+err_zero:
> >+	adapter->vf_data = NULL;
> >+	adapter->vfs_allocated_count = 0;
> > #endif /* CONFIG_PCI_IOV */
> > }
> >
> 
> My preference here would be to bump the #endif up two lines so that
> adapter->vf_data = NULL and adapter->vfs_allocated_count = 0 are ran
> in the non-iov case.  That way we know that SR-IOV is explicitly
> disabled.

I'm not sure that its necessary but it certainly does seem to be
a clean and safe way to handle things. I'll update the patch.


^ permalink raw reply


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