* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Stephen Hemminger @ 2010-07-16 18:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Chetan Loke, netdev, Loke, Chetan
In-Reply-To: <1279300205.2097.9.camel@achroite.uk.solarflarecom.com>
On Fri, 16 Jul 2010 18:10:05 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Fri, 2010-07-16 at 09:18 -0400, Chetan Loke wrote:
> [...]
> > Requirement:
> > R1)Ability to address NICs/interfaces using a mac-addr in ioctls. This
> > is required because we don't have a consistent naming scheme for
> > Ethernet devices.Asking customers and/or field-engineers to change
> > udev rules and
> > other config files is not feasible.
>
> I don't know why they would need to change those. It might be useful to
> have a simple userland tool that will look up a device name by MAC
> address. But it's not like a MAC address is any more user-friendly than
> a net device name!
>
> > Existing pain-points:
> > P1) ioctl needs either i) if-name or ii) if-index before we can invoke
> > bind() etc.This works fine if you know your configuration and it is not going
> > to change.However,if we hot-add a NIC and if you have adapters from multiple
> > vendors(think:driver load order) then upon a reboot,the 'eth'
> > interfaces can be re-mapped.
>
> As you well know, udev makes the name/MAC-address association
> persistent, so this is no longer a problem.
>
> [...]
> > W2.1) If renaming were to even succeed then none of the existing
> > drivers re-register their msix-vectors.
>
> There is no need to do that, since the IRQ handler names are not copied
> but are held by the driver. So the driver only needs to rewrite the
> names when the device is renamed. However that does require registering
> a netdev notifier. It might be worth adding a netdev operation for
> renaming, to make this easier for driver maintainers.
>
> [...]
> > But there is no programmatic way of deriving the 'ethX' name.
> [...]
>
> Deriving it from what? Are you aiming to determine what the name of a
> device will be before the physical device is installed?
>
> Ben.
>
The additional API is not needed. It is trivial to find address for device
and do reverse mapping. Either with ioctl's or /sys/class/net/XXX/addr
^ permalink raw reply
* RE: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Loke, Chetan @ 2010-07-16 18:12 UTC (permalink / raw)
To: Stephen Hemminger, Ben Hutchings; +Cc: Chetan Loke, netdev
In-Reply-To: <20100716110407.316eb8f1@s6510>
> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: July 16, 2010 2:04 PM
> The additional API is not needed. It is trivial to find address for
> device and do reverse mapping. Either with ioctl's
Sorry, I might have missed it. But which ioctl would that be?
> or /sys/class/net/XXX/addr
So, is reading /sys/ nodes preferred over get-calls?
regards
Chetan Loke
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Stephen Hemminger @ 2010-07-16 18:31 UTC (permalink / raw)
To: Loke, Chetan; +Cc: Ben Hutchings, Chetan Loke, netdev
In-Reply-To: <D3F292ADF945FB49B35E96C94C2061B90C55C202@nsmail.netscout.com>
On Fri, 16 Jul 2010 14:12:24 -0400
"Loke, Chetan" <Chetan.Loke@netscout.com> wrote:
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: July 16, 2010 2:04 PM
>
> > The additional API is not needed. It is trivial to find address for
> > device and do reverse mapping. Either with ioctl's
> Sorry, I might have missed it. But which ioctl would that be?
Simple way:
Use SIOCGIFCONF to get list of interfaces
Use SIOCGIFHWADDR to read device addresss
If you want to handle the case where device address is set
by bonding or other protocols, use ETHTOOL_GPERMADDR to read
the original ethernet address.
> > or /sys/class/net/XXX/addr
> So, is reading /sys/ nodes preferred over get-calls?
No difference
^ permalink raw reply
* Re: Badness with the kernel version 2.6.35-rc1-git1 running on P6 box
From: David Rientjes @ 2010-07-16 19:19 UTC (permalink / raw)
To: Dave Hansen
Cc: Eric Dumazet, divya, LKML, linuxppc-dev, sachinp, benh, netdev,
David Miller, Jan-Bernd Themann
In-Reply-To: <1279301731.9207.239.camel@nimitz>
On Fri, 16 Jul 2010, Dave Hansen wrote:
> > > SLUB: Unable to allocate memory on node -1 (gfp=0x20)
> > > cache: kmalloc-16384, object size: 16384, buffer size: 16384,
> > default order: 2, min order: 0
> > > node 0: slabs: 28, objs: 292, free: 0
> > > ip: page allocation failure. order:0, mode:0x8020
> > > Call Trace:
> > > [c000000006a0eb40] [c000000000011c30] .show_stack+0x6c/0x16c (unreliable)
> > > [c000000006a0ebf0] [c00000000012129c] .__alloc_pages_nodemask+0x6a0/0x75c
> > > [c000000006a0ed70] [c0000000001527cc] .alloc_pages_current+0xc4/0x104
> > > [c000000006a0ee10] [c00000000011fca4] .__get_free_pages+0x18/0x90
> > > [c000000006a0ee90] [c0000000004f7058] .ehea_get_stats+0x4c/0x1bc
> > > [c000000006a0ef30] [c0000000005a0a04] .dev_get_stats+0x38/0x64
> > > [c000000006a0efc0] [c0000000005b456c] .rtnl_fill_ifinfo+0x35c/0x85c
> > > [c000000006a0f150] [c0000000005b5920] .rtmsg_ifinfo+0x164/0x204
> > > [c000000006a0f210] [c0000000005a6d6c] .dev_change_flags+0x4c/0x7c
> > > [c000000006a0f2a0] [c0000000005b50b4] .do_setlink+0x31c/0x750
> > > [c000000006a0f3b0] [c0000000005b6724] .rtnl_newlink+0x388/0x618
> > > [c000000006a0f5f0] [c0000000005b6350] .rtnetlink_rcv_msg+0x268/0x2b4
> > > [c000000006a0f6a0] [c0000000005cfdc0] .netlink_rcv_skb+0x74/0x108
> > > [c000000006a0f730] [c0000000005b60c4] .rtnetlink_rcv+0x38/0x5c
> > > [c000000006a0f7c0] [c0000000005cf8c8] .netlink_unicast+0x318/0x3f4
> > > [c000000006a0f890] [c0000000005d05b4] .netlink_sendmsg+0x2d0/0x310
> > > [c000000006a0f970] [c00000000058e1e8] .sock_sendmsg+0xd4/0x110
> > > [c000000006a0fb50] [c00000000058e514] .SyS_sendmsg+0x1f4/0x288
> > > [c000000006a0fd70] [c00000000058c2b8] .SyS_socketcall+0x214/0x280
> > > [c000000006a0fe30] [c0000000000085b4] syscall_exit+0x0/0x40
> > > Mem-Info:
> > > Node 0 DMA per-cpu:
> > > CPU 0: hi: 0, btch: 1 usd: 0
> > > CPU 1: hi: 0, btch: 1 usd: 0
> > > CPU 2: hi: 0, btch: 1 usd: 0
> > > CPU 3: hi: 0, btch: 1 usd: 0
> > >
> > > The mainline 2.6.35-rc5 worked fine.
> >
> > Maybe you were lucky with 2.6.35-rc5
> >
> > Anyway ehea should not use GFP_ATOMIC in its ehea_get_stats() method,
> > called in process context, but GFP_KERNEL.
> >
> > Another patch is needed for ehea_refill_rq_def() as well.
>
> You're right that this is abusing GFP_ATOMIC.
>
> But is, this is just a normal "GFP_ATOMIC" allocation failure? "SLUB:
> Unable to allocate memory on node -1" seems like a somewhat
> inappropriate error message for that.
>
The slub message is seperate and doesn't generate a call trace, even
though it is a (minimum) order-0 GFP_ATOMIC allocation as well. The page
allocation failure is seperate instance that is calling the page
allocator, not the slab allocator.
> It isn't immediately obvious where the -1 is coming from. Does it truly
> mean "allocate from any node" here, or is that a buglet in and of
> itself?
>
Yes, slub uses -1 to indicate that the allocation need not come from a
specific node.
^ permalink raw reply
* RE: [REGRESSION] e1000e stopped working [MANUALLY BISECTED]
From: Maxim Levitsky @ 2010-07-16 19:25 UTC (permalink / raw)
To: Tantilov, Emil S
Cc: Kirsher, Jeffrey T, netdev@vger.kernel.org, Allan, Bruce W,
Pieper, Jeffrey E
In-Reply-To: <1279220945.4411.6.camel@localhost.localdomain>
On Thu, 2010-07-15 at 22:09 +0300, Maxim Levitsky wrote:
> On Thu, 2010-07-15 at 13:02 -0600, Tantilov, Emil S wrote:
> > Maxim Levitsky wrote:
> > > On Thu, 2010-07-15 at 02:33 +0300, Maxim Levitsky wrote:
> > >> On Wed, 2010-07-14 at 16:56 -0600, Tantilov, Emil S wrote:
> > >>> Maxim Levitsky wrote:
> > >>>> On Mon, 2010-07-12 at 15:23 -0600, Tantilov, Emil S wrote:
> > >>>>> Maxim Levitsky wrote:
> > >>>>>> On Mon, 2010-07-05 at 12:58 +0300, Maxim Levitsky wrote:
> > >>>>>>> On Mon, 2010-07-05 at 01:13 -0700, Jeff Kirsher wrote:
> > >>>>>>>> On Sun, Jul 4, 2010 at 15:48, Maxim Levitsky
> > >>>>>>>> <maximlevitsky@gmail.com> wrote:
> > >>>>>>>>> Did few guesses, and now I see that reverting the below
> > >>>>>>>>> commit fixes the problem.
> > >>>>>>>>>
> > >>>>>>>>> "e1000e: Fix/cleanup PHY reset code for ICHx/PCHx"
> > >>>>>>>>> e98cac447cc1cc418dff1d610a5c79c4f2bdec7f.
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> Best regards,
> > >>>>>>>>> Maxim Levitsky
> > >>>>>>>>>
> > >>>>>>>>> --
> > >>>>>>>>
> > >>>>>>>> Can you give us till Tuesday to respond? I know that there are
> > >>>>>>>> some additional e1000e patches in my queue, which may resolve
> > >>>>>>>> the issue, but this weekend the power is down to do some
> > >>>>>>>> infrastructure upgrades which prevents us from doing any
> > >>>>>>>> investigation.debugging until Tuesday.
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>> Sure.
> > >>>>>>>
> > >>>>>>> Best regards,
> > >>>>>>> Maxim Levitsky
> > >>>>>>>
> > >>>>>>
> > >>>>>> Updates?
> > >>>>>
> > >>>>> We are working on reproducing the issue. So far we have not seen
> > >>>>> the problem when testing with net-next.
> > >>>>>
> > >>>>> I asked in previous email about some additional info from ethtool
> > >>>>> (-d, -e, -S) and kernel config. That would help us to narrow it
> > >>>>> down.
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Emil
> > >>>> I did send -e and -d output.
> > >>>
> > >>> Sorry, looks like I lost the email with the attachements.
> > >>>
> > >>> Could you provide the output of dmesg after the failure occurs?
> > >>>
> > >>>> Since you probably want -S output during failure, I need to
> > >>>> recompile kernel for that. I will do that soon.
> > >>>>
> > >>>>
> > >>>> One question, in two weeks I hope 2.6.35 won't be released?
> > >>>> If so, I will have enough free time then to narrow down this issue.
> > >>>>
> > >>>> Other solution, is to revert this commit.
> > >>>> (I have never seen this problem with it reverted).
> > >>>
> > >>> We have been running reboot tests on 2 separate systems with recent
> > >>> net-next kernels using your config and so far no luck in
> > >>> reproducing this issue.
> > >>>
> > >>> What is the make model of your system (or MB)?
> > >>
> > >> the motherboard is Intel DG965RY.
> > >>
> > >> However, I am using vanilla kernel.
> > >> net-next might contain further fixes.
> > >>
> > >> I see if net-next works here.
> > >
> > > Yep, net-next works here.
> > >
> > >
> > > I have the problem on vanilla kernel.
> > > Last revision of it, I tested is 2.6.35-rc4 exactly
> > > (815c4163b6c8ebf8152f42b0a5fd015cfdcedc78)
> > >
> > >
> > > Maybe vanilla git master works, I test it too soon.
> >
> > Thanks for the information! Good to know that this issue does not exist in the latest branch.
> >
> > Have you by any chance tested a stable branch (2.6.34.x)?
>
> I only did test plain 2.6.34 (v2.6.34)
And forgot to add, that it did work.
>
> Also I repeat that revert of e98cac447cc1cc418dff1d610a5c79c4f2bdec7f
> (e1000e: Fix/cleanup PHY reset code for ICHx/PCHx) fixes the bug on
> vanilla kernel.
>
> Also I just pulled latest vanilla git, and I according to diffstat I see
> no changes in e1000e, so its likely that bug remains there.
> I will test that soon.
Tested, broken as expected.
Best regards,
Maxim Levitsky
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Chetan Loke @ 2010-07-16 19:29 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Loke, Chetan, Ben Hutchings, netdev
In-Reply-To: <20100716113113.1e2b51c7@nehalam>
On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Fri, 16 Jul 2010 14:12:24 -0400
> "Loke, Chetan" <Chetan.Loke@netscout.com> wrote:
>
>> > -----Original Message-----
>> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
>> > Sent: July 16, 2010 2:04 PM
>>
>> > The additional API is not needed. It is trivial to find address for
>> > device and do reverse mapping. Either with ioctl's
>> Sorry, I might have missed it. But which ioctl would that be?
>
> Simple way:
> Use SIOCGIFCONF to get list of interfaces
> Use SIOCGIFHWADDR to read device addresss
>
And interfaces that don't have IP's?
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: David Miller @ 2010-07-16 19:33 UTC (permalink / raw)
To: chetanloke; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <AANLkTin3V7LtmhDAOktNF1UITM86sJwButxCQ3vo31lv@mail.gmail.com>
From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 15:29:06 -0400
> On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
> <shemminger@vyatta.com> wrote:
>> Simple way:
>> Use SIOCGIFCONF to get list of interfaces
>> Use SIOCGIFHWADDR to read device addresss
>>
> And interfaces that don't have IP's?
There is no requirement that an interface have configured IP addresses
in order to use those ioctl()'s.
You simply create an arbitrary socket, and use the resulting 'fd' to
run the ioctl's mentioned to fetch information about any and all
interfaces which exist in the system.
^ permalink raw reply
* Re: [RFC] LSM hook for post recvmsg.
From: David Miller @ 2010-07-16 19:35 UTC (permalink / raw)
To: penguin-kernel; +Cc: netdev
In-Reply-To: <201007170114.GFC57373.SQJHOVtLFMOFFO@I-love.SAKURA.ne.jp>
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 01:14:38 +0900
> Below is a patch for post recvmsg() operation. I modified the patch to call
> skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(), udpv6_recvmsg())
> if LSM dicided to drop the message. (Regarding rawv6_recvmsg(), I didn't do so
> in accordance with the comment at "csum_copy_err:".)
> What do you think about this verion?
This looks fine, but regardless of that comment I think the IPV6 raw recvmsg()
should loop just as the IPV4 one does in your patch.
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Chetan Loke @ 2010-07-16 19:35 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <20100716.123306.232886890.davem@davemloft.net>
On Fri, Jul 16, 2010 at 3:33 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 15:29:06 -0400
>
>> On Fri, Jul 16, 2010 at 2:31 PM, Stephen Hemminger
>> <shemminger@vyatta.com> wrote:
>>> Simple way:
>>> Use SIOCGIFCONF to get list of interfaces
>>> Use SIOCGIFHWADDR to read device addresss
>>>
>> And interfaces that don't have IP's?
>
> There is no requirement that an interface have configured IP addresses
> in order to use those ioctl()'s.
>
> You simply create an arbitrary socket, and use the resulting 'fd' to
> run the ioctl's mentioned to fetch information about any and all
> interfaces which exist in the system.
>
Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
that interfaces that didn't have an IP weren't returned.
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: David Miller @ 2010-07-16 19:40 UTC (permalink / raw)
To: chetanloke; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <AANLkTilhSSh1kE5ZfqVDbBJJWdP3hDcXRffpQwdyMQua@mail.gmail.com>
From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 15:35:48 -0400
> Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
> that interfaces that didn't have an IP weren't returned.
If you use the correct set of ioctl()'s it will, just as
"/sbin/ifconfig -a" lists all interfaces regardless of whether they
have IP addresses.
Run strace on 'ifconfig', see what it does :-)
And then you also have the option of using netlink as well.
This is how "ip l l" lists all interfaces, also regardless of
configuration.
^ permalink raw reply
* Re: [PATCH 0/1] Reviewing batman-adv for net/
From: David Miller @ 2010-07-16 19:41 UTC (permalink / raw)
To: sven.eckelmann; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1279291156-5297-1-git-send-email-sven.eckelmann@gmx.de>
So, what't the point of saying "PATCH 0/1" here if you don't
say "PATCH 1/1" in the actual patch posting? :-)
^ permalink raw reply
* Re: [PATCH 0/1] Reviewing batman-adv for net/
From: Sven Eckelmann @ 2010-07-16 19:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20100716.124105.124016801.davem@davemloft.net>
[-- Attachment #1: Type: Text/Plain, Size: 235 bytes --]
David Miller wrote:
> So, what't the point of saying "PATCH 0/1" here if you don't
> say "PATCH 1/1" in the actual patch posting? :-)
Sry, my fault. Happened when I removed another patch which wasn't for this ml.
Best regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [GIT PULL] vhost-net fixes
From: David Miller @ 2010-07-16 19:57 UTC (permalink / raw)
To: mst; +Cc: kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20100716122530.GA29478@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 16 Jul 2010 15:25:30 +0300
> David, please pull the following fixes for 2.6.35.
> Thanks!
>
> The following changes since commit 91a72a70594e5212c97705ca6a694bd307f7a26b:
>
> net/core: neighbour update Oops (2010-07-14 18:02:16 -0700)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net
>
> Michael S. Tsirkin (2):
> vhost-net: avoid flush under lock
> vhost: avoid pr_err on condition guest can trigger
Pulled, thanks!
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Chetan Loke @ 2010-07-16 20:02 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <20100716.124031.220080717.davem@davemloft.net>
On Fri, Jul 16, 2010 at 3:40 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 15:35:48 -0400
>
>> Yes, I opened a socket and then sent the IFCONF ioctl.What I meant was
>> that interfaces that didn't have an IP weren't returned.
>
> If you use the correct set of ioctl()'s it will, just as
> "/sbin/ifconfig -a" lists all interfaces regardless of whether they
> have IP addresses.
>
> Run strace on 'ifconfig', see what it does :-)
I grep the /sys nodes in my existing code already ;) and I didn't want
to do that.If we plop the single for-each-netdev loop then wouldn't it
be easier?
>
> And then you also have the option of using netlink as well.
> This is how "ip l l" lists all interfaces, also regardless of
> configuration.
>
So all user space apps should replicate code when all of that can be
replaced at the cost of a single for-loop in a non fast-path code
within the kernel?
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: David Miller @ 2010-07-16 20:04 UTC (permalink / raw)
To: chetanloke; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <AANLkTinAiJJqhrro8Jlrb0WgHzDSCx_dtWXL5N9ZKUbd@mail.gmail.com>
From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 16:02:23 -0400
> So all user space apps should replicate code when all of that can be
> replaced at the cost of a single for-loop in a non fast-path code
> within the kernel?
When there is a way to do something already, we don't add "yet another"
way to do it. The kernel already suffers from API masterbation as-is.
^ permalink raw reply
* Re: [PATCH] Drivers: net: 8139cp: Improved conformance to the Linux coding style guidelines.
From: David Miller @ 2010-07-16 20:13 UTC (permalink / raw)
To: joseph.kogut; +Cc: netdev, linux-kernel
In-Reply-To: <1279286391-31464-1-git-send-email-joseph.kogut@gmail.com>
From: Joseph Kogut <joseph.kogut@gmail.com>
Date: Fri, 16 Jul 2010 06:19:51 -0700
Please fix the clock on your computer, I don't think it's
6am in the -0700 timezone right now :-)
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Chetan Loke @ 2010-07-16 20:15 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <20100716.130445.149851386.davem@davemloft.net>
On Fri, Jul 16, 2010 at 4:04 PM, David Miller <davem@davemloft.net> wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 16:02:23 -0400
>
>> So all user space apps should replicate code when all of that can be
>> replaced at the cost of a single for-loop in a non fast-path code
>> within the kernel?
>
> When there is a way to do something already, we don't add "yet another"
> way to do it. The kernel already suffers from API masterbation as-is.
>
Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if the
ifindex is not found ? May be then in the else clause we could search
using the hw-addr before bailing out ;)? This way we can avoid adding
a new API. Still not a good idea?
^ permalink raw reply
* Re: [PATCH] ibmveth: lost IRQ while closing/opening device leads to service loss
From: David Miller @ 2010-07-16 20:16 UTC (permalink / raw)
To: rcj; +Cc: netdev, santil, brking
In-Reply-To: <20100716145724.GA25385@linux.vnet.ibm.com>
From: Robert Jennings <rcj@linux.vnet.ibm.com>
Date: Fri, 16 Jul 2010 09:57:25 -0500
> The order of freeing the IRQ and freeing the device in firmware
> in ibmveth_close can cause the adapter to become unusable after a
> subsequent ibmveth_open. Only a reboot of the OS will make the
> network device usable again. This is seen when cycling the adapter
> up and down while there is network activity.
>
> There is a window where an IRQ will be left unserviced (H_EOI will not
> be called). The solution is to make a VIO_IRQ_DISABLE h_call, free the
> device with firmware, and then call free_irq.
>
> Signed-off-by: Robert Jennings <rcj@linux.vnet.ibm.com>
Applied, thanks.
> No changes to the patch, I realized I left out '[PATCH]' in the subject
> and I didn't want any automated tools missing this fix.
Rules may be sometimes seen as anal, but they are not _that_ anal.
Also you can always look in patchwork to see if your change is
there, and your original patch was.
You actually make more work for me if you resubmit patches already
sitting in the queue :-/
^ permalink raw reply
* Re: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: David Miller @ 2010-07-16 20:18 UTC (permalink / raw)
To: chetanloke; +Cc: shemminger, Chetan.Loke, bhutchings, netdev
In-Reply-To: <AANLkTineSDEEKY-9ve2maSBmPWBxkW5XmqnHlLtIOClx@mail.gmail.com>
From: Chetan Loke <chetanloke@gmail.com>
Date: Fri, 16 Jul 2010 16:15:44 -0400
> Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if the
> ifindex is not found ? May be then in the else clause we could search
> using the hw-addr before bailing out ;)? This way we can avoid adding
> a new API. Still not a good idea?
I'm really not going to consider this seriously, sorry. I've
made my position pretty clear, and gave sufficient reasons for
it.
Myself and others have also shown you several ways to fetch the
information you need, and of them netlink is probably the most
efficient, but the others work just as well.
The new interface proposals are just not a good idea.
^ permalink raw reply
* Re: [PATCH 1/2] Remove REDWOOD_[456] config options and conditional code
From: David Miller @ 2010-07-16 20:45 UTC (permalink / raw)
To: jwboyer
Cc: qy03fugy, miltonm, mporter, benh, paulus, solomon, dwmw2, vapier,
jkosina, Artem.Bityutskiy, linux, randy.dunlap, john.linn,
florian, nico, joe, ladis, davidb, linuxppc-dev, linux-kernel,
linux-mtd, netdev, vamos-dev
In-Reply-To: <20100716142055.GA11736@zod.rchland.ibm.com>
From: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Date: Fri, 16 Jul 2010 10:20:55 -0400
> On Fri, Jul 16, 2010 at 02:29:02PM +0200, Christian Dietrich wrote:
>>The config options for REDWOOD_[456] were commented out in the powerpc
>>Kconfig. The ifdefs referencing this options therefore are dead and all
>>references to this can be removed (Also dependencies in other KConfig
>>files).
>>
>>Signed-off-by: Christian Dietrich <qy03fugy@stud.informatik.uni-erlangen.de>
>>Signed-off-by: Christoph Egger <siccegge@cs.fau.de>
>
> This seems fine with me.
>
> The only question is which tree it coms through. I'm happy to take it
> in via mine if the netdev and MTD people are fine with that. Otherwise,
> my ack is below.
>
> Acked-by: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Please take it:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* RE: [RFC] Enhance dev_ioctl to return <hwaddr>:<if_name::if_index> mapping
From: Loke, Chetan @ 2010-07-16 20:49 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, bhutchings, netdev
In-Reply-To: <20100716.131835.124042396.davem@davemloft.net>
Ooops... I got the 'CC' list this time
On Fri, Jul 16, 2010 at 4:33 PM, David Miller <davem@davemloft.net>
wrote:
> From: Chetan Loke <chetanloke@gmail.com>
> Date: Fri, 16 Jul 2010 16:24:16 -0400
>
>> Thanks for your time guys.
>
> You dropped everything from the CC: list, so nobody other
> than me saw this. :)
>
>
On Fri, Jul 16, 2010 at 4:24 PM, Chetan Loke <chetanloke@gmail.com>
wrote:
> On Fri, Jul 16, 2010 at 4:18 PM, David Miller <davem@davemloft.net>
wrote:
>> From: Chetan Loke <chetanloke@gmail.com>
>> Date: Fri, 16 Jul 2010 16:15:44 -0400
>>
>>> Ok, what if, i) ifindex == 0 in the call to SIOCGIFINDEX or ii) if
the
>>> ifindex is not found ? May be then in the else clause we could
search
>>> using the hw-addr before bailing out ;)? This way we can avoid
adding
>>> a new API. Still not a good idea?
>>
>> I'm really not going to consider this seriously, sorry. I've
>> made my position pretty clear, and gave sufficient reasons for
>> it.
>>
>> Myself and others have also shown you several ways to fetch the
>> information you need, and of them netlink is probably the most
>> efficient, but the others work just as well.
>>
>> The new interface proposals are just not a good idea.
>>
>
> No problem Dave. I will patch our internal iso and add dev_getbyhwaddr
> in the else clause.
>
> Thanks for your time guys.
>
> regards
> Chetan Loke
>
^ permalink raw reply
* Re: [PATCH v2 1/2] bnx2: allocate with GFP_KERNEL flag on RX path init
From: Michael Chan @ 2010-07-16 21:24 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100716105540.613d40cc@dhcp-lab-109.englab.brq.redhat.com>
On Fri, 2010-07-16 at 01:55 -0700, Stanislaw Gruszka wrote:
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Michael Chan <mchan@broadcom.com>
> ---
> v1->v2: use GFP_ATOMIC in bnx2_rx_skb
>
> drivers/net/bnx2.c | 17 +++++++++--------
> 1 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index a203f39..a7df539 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -2664,13 +2664,13 @@ bnx2_set_mac_addr(struct bnx2 *bp, u8 *mac_addr, u32 pos)
> }
>
> static inline int
> -bnx2_alloc_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
> +bnx2_alloc_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
> {
> dma_addr_t mapping;
> struct sw_pg *rx_pg = &rxr->rx_pg_ring[index];
> struct rx_bd *rxbd =
> &rxr->rx_pg_desc_ring[RX_RING(index)][RX_IDX(index)];
> - struct page *page = alloc_page(GFP_ATOMIC);
> + struct page *page = alloc_page(gfp);
>
> if (!page)
> return -ENOMEM;
> @@ -2705,7 +2705,7 @@ bnx2_free_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
> }
>
> static inline int
> -bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
> +bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp_t gfp)
> {
> struct sk_buff *skb;
> struct sw_bd *rx_buf = &rxr->rx_buf_ring[index];
> @@ -2713,7 +2713,7 @@ bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
> struct rx_bd *rxbd = &rxr->rx_desc_ring[RX_RING(index)][RX_IDX(index)];
> unsigned long align;
>
> - skb = netdev_alloc_skb(bp->dev, bp->rx_buf_size);
> + skb = __netdev_alloc_skb(bp->dev, bp->rx_buf_size, gfp);
> if (skb == NULL) {
> return -ENOMEM;
> }
> @@ -2974,7 +2974,7 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
> int err;
> u16 prod = ring_idx & 0xffff;
>
> - err = bnx2_alloc_rx_skb(bp, rxr, prod);
> + err = bnx2_alloc_rx_skb(bp, rxr, prod, GFP_ATOMIC);
> if (unlikely(err)) {
> bnx2_reuse_rx_skb(bp, rxr, skb, (u16) (ring_idx >> 16), prod);
> if (hdr_len) {
> @@ -3039,7 +3039,8 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
> rx_pg->page = NULL;
>
> err = bnx2_alloc_rx_page(bp, rxr,
> - RX_PG_RING_IDX(pg_prod));
> + RX_PG_RING_IDX(pg_prod),
> + GFP_ATOMIC);
> if (unlikely(err)) {
> rxr->rx_pg_cons = pg_cons;
> rxr->rx_pg_prod = pg_prod;
> @@ -5179,7 +5180,7 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
>
> ring_prod = prod = rxr->rx_pg_prod;
> for (i = 0; i < bp->rx_pg_ring_size; i++) {
> - if (bnx2_alloc_rx_page(bp, rxr, ring_prod) < 0) {
> + if (bnx2_alloc_rx_page(bp, rxr, ring_prod, GFP_KERNEL) < 0) {
> netdev_warn(bp->dev, "init'ed rx page ring %d with %d/%d pages only\n",
> ring_num, i, bp->rx_pg_ring_size);
> break;
> @@ -5191,7 +5192,7 @@ bnx2_init_rx_ring(struct bnx2 *bp, int ring_num)
>
> ring_prod = prod = rxr->rx_prod;
> for (i = 0; i < bp->rx_ring_size; i++) {
> - if (bnx2_alloc_rx_skb(bp, rxr, ring_prod) < 0) {
> + if (bnx2_alloc_rx_skb(bp, rxr, ring_prod, GFP_KERNEL) < 0) {
> netdev_warn(bp->dev, "init'ed rx ring %d with %d/%d skbs only\n",
> ring_num, i, bp->rx_ring_size);
> break;
^ permalink raw reply
* Re: [PATCH 2/2] bnx2: use device model DMA API
From: Michael Chan @ 2010-07-16 21:29 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100715142544.12504.283.send-patch@dhcp-lab-109.englab.brq.redhat.com>
On Thu, 2010-07-15 at 07:25 -0700, Stanislaw Gruszka wrote:
> Use DMA API as PCI equivalents will be deprecated. This change also allow
> to allocate with GFP_KERNEL in some places.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Michael Chan <mchan@broadcom.com>
> ---
> drivers/net/bnx2.c | 111 +++++++++++++++++++++++++++-------------------------
> 1 files changed, 58 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 6de4cb7..98aed05 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -692,9 +692,9 @@ bnx2_free_tx_mem(struct bnx2 *bp)
> struct bnx2_tx_ring_info *txr = &bnapi->tx_ring;
>
> if (txr->tx_desc_ring) {
> - pci_free_consistent(bp->pdev, TXBD_RING_SIZE,
> - txr->tx_desc_ring,
> - txr->tx_desc_mapping);
> + dma_free_coherent(&bp->pdev->dev, TXBD_RING_SIZE,
> + txr->tx_desc_ring,
> + txr->tx_desc_mapping);
> txr->tx_desc_ring = NULL;
> }
> kfree(txr->tx_buf_ring);
> @@ -714,9 +714,9 @@ bnx2_free_rx_mem(struct bnx2 *bp)
>
> for (j = 0; j < bp->rx_max_ring; j++) {
> if (rxr->rx_desc_ring[j])
> - pci_free_consistent(bp->pdev, RXBD_RING_SIZE,
> - rxr->rx_desc_ring[j],
> - rxr->rx_desc_mapping[j]);
> + dma_free_coherent(&bp->pdev->dev, RXBD_RING_SIZE,
> + rxr->rx_desc_ring[j],
> + rxr->rx_desc_mapping[j]);
> rxr->rx_desc_ring[j] = NULL;
> }
> vfree(rxr->rx_buf_ring);
> @@ -724,9 +724,9 @@ bnx2_free_rx_mem(struct bnx2 *bp)
>
> for (j = 0; j < bp->rx_max_pg_ring; j++) {
> if (rxr->rx_pg_desc_ring[j])
> - pci_free_consistent(bp->pdev, RXBD_RING_SIZE,
> - rxr->rx_pg_desc_ring[j],
> - rxr->rx_pg_desc_mapping[j]);
> + dma_free_coherent(&bp->pdev->dev, RXBD_RING_SIZE,
> + rxr->rx_pg_desc_ring[j],
> + rxr->rx_pg_desc_mapping[j]);
> rxr->rx_pg_desc_ring[j] = NULL;
> }
> vfree(rxr->rx_pg_ring);
> @@ -748,8 +748,8 @@ bnx2_alloc_tx_mem(struct bnx2 *bp)
> return -ENOMEM;
>
> txr->tx_desc_ring =
> - pci_alloc_consistent(bp->pdev, TXBD_RING_SIZE,
> - &txr->tx_desc_mapping);
> + dma_alloc_coherent(&bp->pdev->dev, TXBD_RING_SIZE,
> + &txr->tx_desc_mapping, GFP_KERNEL);
> if (txr->tx_desc_ring == NULL)
> return -ENOMEM;
> }
> @@ -776,8 +776,10 @@ bnx2_alloc_rx_mem(struct bnx2 *bp)
>
> for (j = 0; j < bp->rx_max_ring; j++) {
> rxr->rx_desc_ring[j] =
> - pci_alloc_consistent(bp->pdev, RXBD_RING_SIZE,
> - &rxr->rx_desc_mapping[j]);
> + dma_alloc_coherent(&bp->pdev->dev,
> + RXBD_RING_SIZE,
> + &rxr->rx_desc_mapping[j],
> + GFP_KERNEL);
> if (rxr->rx_desc_ring[j] == NULL)
> return -ENOMEM;
>
> @@ -795,8 +797,10 @@ bnx2_alloc_rx_mem(struct bnx2 *bp)
>
> for (j = 0; j < bp->rx_max_pg_ring; j++) {
> rxr->rx_pg_desc_ring[j] =
> - pci_alloc_consistent(bp->pdev, RXBD_RING_SIZE,
> - &rxr->rx_pg_desc_mapping[j]);
> + dma_alloc_coherent(&bp->pdev->dev,
> + RXBD_RING_SIZE,
> + &rxr->rx_pg_desc_mapping[j],
> + GFP_KERNEL);
> if (rxr->rx_pg_desc_ring[j] == NULL)
> return -ENOMEM;
>
> @@ -816,16 +820,16 @@ bnx2_free_mem(struct bnx2 *bp)
>
> for (i = 0; i < bp->ctx_pages; i++) {
> if (bp->ctx_blk[i]) {
> - pci_free_consistent(bp->pdev, BCM_PAGE_SIZE,
> - bp->ctx_blk[i],
> - bp->ctx_blk_mapping[i]);
> + dma_free_coherent(&bp->pdev->dev, BCM_PAGE_SIZE,
> + bp->ctx_blk[i],
> + bp->ctx_blk_mapping[i]);
> bp->ctx_blk[i] = NULL;
> }
> }
> if (bnapi->status_blk.msi) {
> - pci_free_consistent(bp->pdev, bp->status_stats_size,
> - bnapi->status_blk.msi,
> - bp->status_blk_mapping);
> + dma_free_coherent(&bp->pdev->dev, bp->status_stats_size,
> + bnapi->status_blk.msi,
> + bp->status_blk_mapping);
> bnapi->status_blk.msi = NULL;
> bp->stats_blk = NULL;
> }
> @@ -846,8 +850,8 @@ bnx2_alloc_mem(struct bnx2 *bp)
> bp->status_stats_size = status_blk_size +
> sizeof(struct statistics_block);
>
> - status_blk = pci_alloc_consistent(bp->pdev, bp->status_stats_size,
> - &bp->status_blk_mapping);
> + status_blk = dma_alloc_coherent(&bp->pdev->dev, bp->status_stats_size,
> + &bp->status_blk_mapping, GFP_KERNEL);
> if (status_blk == NULL)
> goto alloc_mem_err;
>
> @@ -885,9 +889,10 @@ bnx2_alloc_mem(struct bnx2 *bp)
> if (bp->ctx_pages == 0)
> bp->ctx_pages = 1;
> for (i = 0; i < bp->ctx_pages; i++) {
> - bp->ctx_blk[i] = pci_alloc_consistent(bp->pdev,
> + bp->ctx_blk[i] = dma_alloc_coherent(&bp->pdev->dev,
> BCM_PAGE_SIZE,
> - &bp->ctx_blk_mapping[i]);
> + &bp->ctx_blk_mapping[i],
> + GFP_KERNEL);
> if (bp->ctx_blk[i] == NULL)
> goto alloc_mem_err;
> }
> @@ -2674,9 +2679,9 @@ bnx2_alloc_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gf
>
> if (!page)
> return -ENOMEM;
> - mapping = pci_map_page(bp->pdev, page, 0, PAGE_SIZE,
> + mapping = dma_map_page(&bp->pdev->dev, page, 0, PAGE_SIZE,
> PCI_DMA_FROMDEVICE);
> - if (pci_dma_mapping_error(bp->pdev, mapping)) {
> + if (dma_mapping_error(&bp->pdev->dev, mapping)) {
> __free_page(page);
> return -EIO;
> }
> @@ -2697,8 +2702,8 @@ bnx2_free_rx_page(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index)
> if (!page)
> return;
>
> - pci_unmap_page(bp->pdev, dma_unmap_addr(rx_pg, mapping), PAGE_SIZE,
> - PCI_DMA_FROMDEVICE);
> + dma_unmap_page(&bp->pdev->dev, dma_unmap_addr(rx_pg, mapping),
> + PAGE_SIZE, PCI_DMA_FROMDEVICE);
>
> __free_page(page);
> rx_pg->page = NULL;
> @@ -2721,9 +2726,9 @@ bnx2_alloc_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, u16 index, gfp
> if (unlikely((align = (unsigned long) skb->data & (BNX2_RX_ALIGN - 1))))
> skb_reserve(skb, BNX2_RX_ALIGN - align);
>
> - mapping = pci_map_single(bp->pdev, skb->data, bp->rx_buf_use_size,
> - PCI_DMA_FROMDEVICE);
> - if (pci_dma_mapping_error(bp->pdev, mapping)) {
> + mapping = dma_map_single(&bp->pdev->dev, skb->data, bp->rx_buf_use_size,
> + PCI_DMA_FROMDEVICE);
> + if (dma_mapping_error(&bp->pdev->dev, mapping)) {
> dev_kfree_skb(skb);
> return -EIO;
> }
> @@ -2829,7 +2834,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
> }
> }
>
> - pci_unmap_single(bp->pdev, dma_unmap_addr(tx_buf, mapping),
> + dma_unmap_single(&bp->pdev->dev, dma_unmap_addr(tx_buf, mapping),
> skb_headlen(skb), PCI_DMA_TODEVICE);
>
> tx_buf->skb = NULL;
> @@ -2838,7 +2843,7 @@ bnx2_tx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
> for (i = 0; i < last; i++) {
> sw_cons = NEXT_TX_BD(sw_cons);
>
> - pci_unmap_page(bp->pdev,
> + dma_unmap_page(&bp->pdev->dev,
> dma_unmap_addr(
> &txr->tx_buf_ring[TX_RING_IDX(sw_cons)],
> mapping),
> @@ -2945,7 +2950,7 @@ bnx2_reuse_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr,
> cons_rx_buf = &rxr->rx_buf_ring[cons];
> prod_rx_buf = &rxr->rx_buf_ring[prod];
>
> - pci_dma_sync_single_for_device(bp->pdev,
> + dma_sync_single_for_device(&bp->pdev->dev,
> dma_unmap_addr(cons_rx_buf, mapping),
> BNX2_RX_OFFSET + BNX2_RX_COPY_THRESH, PCI_DMA_FROMDEVICE);
>
> @@ -2987,7 +2992,7 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
> }
>
> skb_reserve(skb, BNX2_RX_OFFSET);
> - pci_unmap_single(bp->pdev, dma_addr, bp->rx_buf_use_size,
> + dma_unmap_single(&bp->pdev->dev, dma_addr, bp->rx_buf_use_size,
> PCI_DMA_FROMDEVICE);
>
> if (hdr_len == 0) {
> @@ -3049,7 +3054,7 @@ bnx2_rx_skb(struct bnx2 *bp, struct bnx2_rx_ring_info *rxr, struct sk_buff *skb,
> return err;
> }
>
> - pci_unmap_page(bp->pdev, mapping_old,
> + dma_unmap_page(&bp->pdev->dev, mapping_old,
> PAGE_SIZE, PCI_DMA_FROMDEVICE);
>
> frag_size -= frag_len;
> @@ -3120,7 +3125,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
>
> dma_addr = dma_unmap_addr(rx_buf, mapping);
>
> - pci_dma_sync_single_for_cpu(bp->pdev, dma_addr,
> + dma_sync_single_for_cpu(&bp->pdev->dev, dma_addr,
> BNX2_RX_OFFSET + BNX2_RX_COPY_THRESH,
> PCI_DMA_FROMDEVICE);
>
> @@ -5338,7 +5343,7 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
> continue;
> }
>
> - pci_unmap_single(bp->pdev,
> + dma_unmap_single(&bp->pdev->dev,
> dma_unmap_addr(tx_buf, mapping),
> skb_headlen(skb),
> PCI_DMA_TODEVICE);
> @@ -5349,7 +5354,7 @@ bnx2_free_tx_skbs(struct bnx2 *bp)
> j++;
> for (k = 0; k < last; k++, j++) {
> tx_buf = &txr->tx_buf_ring[TX_RING_IDX(j)];
> - pci_unmap_page(bp->pdev,
> + dma_unmap_page(&bp->pdev->dev,
> dma_unmap_addr(tx_buf, mapping),
> skb_shinfo(skb)->frags[k].size,
> PCI_DMA_TODEVICE);
> @@ -5379,7 +5384,7 @@ bnx2_free_rx_skbs(struct bnx2 *bp)
> if (skb == NULL)
> continue;
>
> - pci_unmap_single(bp->pdev,
> + dma_unmap_single(&bp->pdev->dev,
> dma_unmap_addr(rx_buf, mapping),
> bp->rx_buf_use_size,
> PCI_DMA_FROMDEVICE);
> @@ -5732,9 +5737,9 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
> for (i = 14; i < pkt_size; i++)
> packet[i] = (unsigned char) (i & 0xff);
>
> - map = pci_map_single(bp->pdev, skb->data, pkt_size,
> - PCI_DMA_TODEVICE);
> - if (pci_dma_mapping_error(bp->pdev, map)) {
> + map = dma_map_single(&bp->pdev->dev, skb->data, pkt_size,
> + PCI_DMA_TODEVICE);
> + if (dma_mapping_error(&bp->pdev->dev, map)) {
> dev_kfree_skb(skb);
> return -EIO;
> }
> @@ -5772,7 +5777,7 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
>
> udelay(5);
>
> - pci_unmap_single(bp->pdev, map, pkt_size, PCI_DMA_TODEVICE);
> + dma_unmap_single(&bp->pdev->dev, map, pkt_size, PCI_DMA_TODEVICE);
> dev_kfree_skb(skb);
>
> if (bnx2_get_hw_tx_cons(tx_napi) != txr->tx_prod)
> @@ -5789,7 +5794,7 @@ bnx2_run_loopback(struct bnx2 *bp, int loopback_mode)
> rx_hdr = rx_buf->desc;
> skb_reserve(rx_skb, BNX2_RX_OFFSET);
>
> - pci_dma_sync_single_for_cpu(bp->pdev,
> + dma_sync_single_for_cpu(&bp->pdev->dev,
> dma_unmap_addr(rx_buf, mapping),
> bp->rx_buf_size, PCI_DMA_FROMDEVICE);
>
> @@ -6457,8 +6462,8 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
> } else
> mss = 0;
>
> - mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
> - if (pci_dma_mapping_error(bp->pdev, mapping)) {
> + mapping = dma_map_single(&bp->pdev->dev, skb->data, len, PCI_DMA_TODEVICE);
> + if (dma_mapping_error(&bp->pdev->dev, mapping)) {
> dev_kfree_skb(skb);
> return NETDEV_TX_OK;
> }
> @@ -6486,9 +6491,9 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
> txbd = &txr->tx_desc_ring[ring_prod];
>
> len = frag->size;
> - mapping = pci_map_page(bp->pdev, frag->page, frag->page_offset,
> - len, PCI_DMA_TODEVICE);
> - if (pci_dma_mapping_error(bp->pdev, mapping))
> + mapping = dma_map_page(&bp->pdev->dev, frag->page, frag->page_offset,
> + len, PCI_DMA_TODEVICE);
> + if (dma_mapping_error(&bp->pdev->dev, mapping))
> goto dma_error;
> dma_unmap_addr_set(&txr->tx_buf_ring[ring_prod], mapping,
> mapping);
> @@ -6527,7 +6532,7 @@ dma_error:
> ring_prod = TX_RING_IDX(prod);
> tx_buf = &txr->tx_buf_ring[ring_prod];
> tx_buf->skb = NULL;
> - pci_unmap_single(bp->pdev, dma_unmap_addr(tx_buf, mapping),
> + dma_unmap_single(&bp->pdev->dev, dma_unmap_addr(tx_buf, mapping),
> skb_headlen(skb), PCI_DMA_TODEVICE);
>
> /* unmap remaining mapped pages */
> @@ -6535,7 +6540,7 @@ dma_error:
> prod = NEXT_TX_BD(prod);
> ring_prod = TX_RING_IDX(prod);
> tx_buf = &txr->tx_buf_ring[ring_prod];
> - pci_unmap_page(bp->pdev, dma_unmap_addr(tx_buf, mapping),
> + dma_unmap_page(&bp->pdev->dev, dma_unmap_addr(tx_buf, mapping),
> skb_shinfo(skb)->frags[i].size,
> PCI_DMA_TODEVICE);
> }
^ permalink raw reply
* Re: [PATCH] Drivers: net: 8139cp: Improved conformance to the Linux coding style guidelines.
From: Jeff Garzik @ 2010-07-16 21:40 UTC (permalink / raw)
To: Joseph Kogut; +Cc: davem, netdev, linux-kernel
In-Reply-To: <1279286391-31464-1-git-send-email-joseph.kogut@gmail.com>
On 07/16/2010 09:19 AM, Joseph Kogut wrote:
> Fixed several issues that made the 8139C+ driver nonconformant to the Linux coding style guidelines.
>
> Signed-off-by: Joseph Kogut<joseph.kogut@gmail.com>
> ---
> drivers/net/8139cp.c | 304 +++++++++++++++++++++++++-------------------------
> 1 files changed, 153 insertions(+), 151 deletions(-)
This patch is still failing in places to recognize obvious intent of the
code writer.
> /*
> Copyright 2001-2004 Jeff Garzik<jgarzik@pobox.com>
>
> - Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com) [tg3.c]
> - Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com) [sungem.c]
> - Copyright 2001 Manfred Spraul [natsemi.c]
> - Copyright 1999-2001 by Donald Becker. [natsemi.c]
> - Written 1997-2001 by Donald Becker. [8139too.c]
> - Copyright 1998-2001 by Jes Sorensen,<jes@trained-monkey.org>. [acenic.c]
> + Copyright (C) 2001, 2002 David S. Miller (davem@redhat.com) [tg3.c]
> + Copyright (C) 2000, 2001 David S. Miller (davem@redhat.com) [sungem.c]
> + Copyright 2001 Manfred Spraul [natsemi.c]
> + Copyright 1999-2001 by Donald Becker. [natsemi.c]
> + Written 1997-2001 by Donald Becker. [8139too.c]
> + Copyright 1998-2001 by Jes Sorensen,<jes@trained-monkey.org>. [acenic.c]
what is the point of this? I would leave copyright messages as-is.
> @@ -1295,32 +1295,32 @@ static void mdio_write(struct net_device *dev, int phy_id, int location,
> }
>
> /* Set the ethtool Wake-on-LAN settings */
> -static int netdev_set_wol (struct cp_private *cp,
> +static int netdev_set_wol(struct cp_private *cp,
> const struct ethtool_wolinfo *wol)
> {
> u8 options;
>
> - options = cpr8 (Config3)& ~(LinkUp | MagicPacket);
> + options = cpr8(Config3)& ~(LinkUp | MagicPacket);
> /* If WOL is being disabled, no need for complexity */
> if (wol->wolopts) {
> - if (wol->wolopts& WAKE_PHY) options |= LinkUp;
> - if (wol->wolopts& WAKE_MAGIC) options |= MagicPacket;
> + if (wol->wolopts& WAKE_PHY) options |= LinkUp;
> + if (wol->wolopts& WAKE_MAGIC) options |= MagicPacket;
> }
>
> - cpw8 (Cfg9346, Cfg9346_Unlock);
> - cpw8 (Config3, options);
> - cpw8 (Cfg9346, Cfg9346_Lock);
> + cpw8(Cfg9346, Cfg9346_Unlock);
> + cpw8(Config3, options);
> + cpw8(Cfg9346, Cfg9346_Lock);
>
> options = 0; /* Paranoia setting */
> - options = cpr8 (Config5)& ~(UWF | MWF | BWF);
> + options = cpr8(Config5)& ~(UWF | MWF | BWF);
> /* If WOL is being disabled, no need for complexity */
> if (wol->wolopts) {
> - if (wol->wolopts& WAKE_UCAST) options |= UWF;
> - if (wol->wolopts& WAKE_BCAST) options |= BWF;
> - if (wol->wolopts& WAKE_MCAST) options |= MWF;
> + if (wol->wolopts& WAKE_UCAST) options |= UWF;
> + if (wol->wolopts& WAKE_BCAST) options |= BWF;
> + if (wol->wolopts& WAKE_MCAST) options |= MWF;
> }
you just un-aligned things that were nicely tab-aligned
> @@ -1328,35 +1328,36 @@ static int netdev_set_wol (struct cp_private *cp,
> }
>
> /* Get the ethtool Wake-on-LAN settings */
> -static void netdev_get_wol (struct cp_private *cp,
> - struct ethtool_wolinfo *wol)
> +static void netdev_get_wol(struct cp_private *cp,
> + struct ethtool_wolinfo *wol)
> {
> u8 options;
>
> wol->wolopts = 0; /* Start from scratch */
> - wol->supported = WAKE_PHY | WAKE_BCAST | WAKE_MAGIC |
> - WAKE_MCAST | WAKE_UCAST;
> + wol->supported = WAKE_PHY | WAKE_BCAST | WAKE_MAGIC | WAKE_MCAST | WAKE_UCAST;
> +
> /* We don't need to go on if WOL is disabled */
> - if (!cp->wol_enabled) return;
> + if (!cp->wol_enabled)
> + return;
>
> - options = cpr8 (Config3);
> - if (options & LinkUp) wol->wolopts |= WAKE_PHY;
> - if (options & MagicPacket) wol->wolopts |= WAKE_MAGIC;
> + options = cpr8(Config3);
> + if (options & LinkUp) wol->wolopts |= WAKE_PHY;
> + if (options & MagicPacket) wol->wolopts |= WAKE_MAGIC;
>
> options = 0; /* Paranoia setting */
> - options = cpr8 (Config5);
> - if (options & UWF) wol->wolopts |= WAKE_UCAST;
> - if (options & BWF) wol->wolopts |= WAKE_BCAST;
> - if (options & MWF) wol->wolopts |= WAKE_MCAST;
> + options = cpr8(Config5);
> + if (options & UWF) wol->wolopts |= WAKE_UCAST;
> + if (options & BWF) wol->wolopts |= WAKE_BCAST;
> + if (options & MWF) wol->wolopts |= WAKE_MCAST;
> }
>
> -static void cp_get_drvinfo (struct net_device *dev, struct ethtool_drvinfo *info)
ditto
Also, it's disappointing to see so much diff noise created by
s/func ()/func()/
but I don't suppose I'll win that battle.
Jeff
^ permalink raw reply
* RE: 2.6.34.1: kernel warning at igb_main.c:2080
From: Wyborny, Carolyn @ 2010-07-16 22:45 UTC (permalink / raw)
To: Ben Greear, NetDev
In-Reply-To: <4C3FA03E.9090802@candelatech.com>
>-----Original Message-----
>From: netdev-owner@vger.kernel.org
>[mailto:netdev-owner@vger.kernel.org] On Behalf Of Ben Greear
>Sent: Thursday, July 15, 2010 4:57 PM
>To: NetDev
>Subject: igb: 2.6.34.1: kernel warning at igb_main.c:2080
>
>We just saw this kernel warning on 2.6.34.1 + a few patches
>from the pending stable queue,
>plus our own hacks (though none to igb).
>
>We were running a modified version of pktgen traffic and at
>the same time
>bounced the port.
>
>This warning didn't seem to cause any real problems.
>
>Please let us know if you would like any additional information.
>
>]# lspci|grep Ethern
>01:00.0 Ethernet controller: Intel Corporation 82574L Gigabit
>Network Connection
>02:00.0 Ethernet controller: Intel Corporation 82574L Gigabit
>Network Connection
>05:00.0 Ethernet controller: Intel Corporation 82575EB Gigabit
>Network Connection (rev 02)
>05:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit
>Network Connection (rev 02)
>06:00.0 Ethernet controller: Intel Corporation 82575EB Gigabit
>Network Connection (rev 02)
>06:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit
>Network Connection (rev 02)
>
>
>Jul 15 16:36:01 localhost kernel: ------------[ cut here ]------------
>Jul 15 16:36:01 localhost kernel: WARNING: at
>/home/greearb/git/linux-2.6.dev.34.y/drivers/net/igb/igb_main.c
>:2080 igb_close+0x28/0x9f [igb]()
>Jul 15 16:36:01 localhost kernel: Hardware name: X8STi
>Jul 15 16:36:01 localhost kernel: Modules linked in: bridge
>arc4 michael_mic wanlink(P) 8021q garp xt_CT iptable_raw
>ipt_addrtype xt_DSCP xt_dscp xt_string
>xt_owner xt_NFQUEUE xt_multiport xt_mark xt_iprange
>xt_hashlimit xt_CONNMARK xt_connmark stp llc veth fuse macvlan
>bpctl_mod pktgen iscsi_tcp libiscsi_tcp
>libiscsi scsi_transport_iscsi nfs lockd fscache nfs_acl
>auth_rpcgss sunrpc ipv6 dm_multipath uinput i2c_i801 iTCO_wdt
>i2c_core ioatdma e1000e igb pcspkr
>iTCO_vendor_support dca ata_generic pata_acpi [last unloaded: nf_nat]
>Jul 15 16:36:01 localhost kernel: Pid: 17516, comm: ip
>Tainted: P 2.6.34.1 #2
>Jul 15 16:36:01 localhost kernel: Call Trace:
>Jul 15 16:36:01 localhost kernel: [<ffffffffa002a37f>] ?
>igb_close+0x28/0x9f [igb]
>Jul 15 16:36:01 localhost kernel: [<ffffffff81041bb6>]
>warn_slowpath_common+0x77/0x8f
>Jul 15 16:36:01 localhost kernel: [<ffffffff81041bdd>]
>warn_slowpath_null+0xf/0x11
>Jul 15 16:36:01 localhost kernel: [<ffffffffa002a37f>]
>igb_close+0x28/0x9f [igb]
>Jul 15 16:36:01 localhost kernel: [<ffffffff8133c626>]
>__dev_close+0x73/0x86
>Jul 15 16:36:01 localhost kernel: [<ffffffff8133a719>]
>__dev_change_flags+0xa8/0x12b
>Jul 15 16:36:01 localhost kernel: [<ffffffff8133c2aa>]
>dev_change_flags+0x1c/0x51
>Jul 15 16:36:01 localhost kernel: [<ffffffff813466bc>]
>do_setlink+0x273/0x482
>Jul 15 16:36:01 localhost kernel: [<ffffffff810ab1bb>] ?
>zone_statistics+0x5e/0x63
>Jul 15 16:36:01 localhost kernel: [<ffffffff8134755e>]
>rtnl_newlink+0x26c/0x422
>Jul 15 16:36:01 localhost kernel: [<ffffffff81345a06>] ?
>nla_nest_start+0x1d/0x31
>Jul 15 16:36:01 localhost kernel: [<ffffffff810ca997>] ?
>virt_to_head_page+0x9/0x2a
>Jul 15 16:36:01 localhost kernel: [<ffffffff813dd1f5>] ?
>__mutex_lock_common+0x38e/0x3ac
>Jul 15 16:36:01 localhost kernel: [<ffffffff81346ffb>]
>rtnetlink_rcv_msg+0x1d9/0x1f7
>Jul 15 16:36:01 localhost kernel: [<ffffffff81346e22>] ?
>rtnetlink_rcv_msg+0x0/0x1f7
>Jul 15 16:36:01 localhost kernel: [<ffffffff81356939>]
>netlink_rcv_skb+0x3e/0x8e
>Jul 15 16:36:01 localhost kernel: [<ffffffff81346cc9>]
>rtnetlink_rcv+0x20/0x29
>Jul 15 16:36:01 localhost kernel: [<ffffffff813567b2>]
>netlink_unicast+0xea/0x151
>Jul 15 16:36:01 localhost kernel: [<ffffffff8133545c>] ?
>memcpy_fromiovec+0x42/0x73
>Jul 15 16:36:01 localhost kernel: [<ffffffff81357bc8>]
>netlink_sendmsg+0x242/0x255
>Jul 15 16:36:01 localhost kernel: [<ffffffff8132a771>] ?
>__sock_recvmsg_nosec+0x29/0x2b
>Jul 15 16:36:01 localhost kernel: [<ffffffff8132bd02>]
>__sock_sendmsg+0x56/0x5f
>Jul 15 16:36:01 localhost kernel: [<ffffffff8132c127>]
>sock_sendmsg+0xa3/0xbc
>Jul 15 16:36:01 localhost kernel: [<ffffffff813351e5>] ?
>copy_from_user+0x28/0x30
>Jul 15 16:36:01 localhost kernel: [<ffffffff8133555e>] ?
>verify_iovec+0x52/0x95
>Jul 15 16:36:01 localhost kernel: [<ffffffff8132c368>]
>sys_sendmsg+0x1c6/0x22a
>Jul 15 16:36:01 localhost kernel: [<ffffffff810a277f>] ?
>lru_cache_add_lru+0x38/0x3d
>Jul 15 16:36:01 localhost kernel: [<ffffffff813de23e>] ?
>_raw_spin_unlock+0x2d/0x38
>Jul 15 16:36:01 localhost kernel: [<ffffffff810ad802>] ?
>spin_unlock+0x9/0xb
>Jul 15 16:36:01 localhost kernel: [<ffffffff810b0138>] ?
>handle_mm_fault+0x6d3/0x6f3
>Jul 15 16:36:01 localhost kernel: [<ffffffff810b3a0a>] ?
>__vma_link_rb+0x2b/0x2d
>Jul 15 16:36:01 localhost kernel: [<ffffffff810b45f7>] ?
>vma_link+0xcd/0xcf
>Jul 15 16:36:01 localhost kernel: [<ffffffff810d9369>] ?
>fget_light+0x39/0x87
>Jul 15 16:36:01 localhost kernel: [<ffffffff81083b91>] ?
>audit_syscall_entry+0xfe/0x12a
>Jul 15 16:36:01 localhost kernel: [<ffffffff81009ac2>]
>system_call_fastpath+0x16/0x1b
>Jul 15 16:36:01 localhost kernel: ---[ end trace 75242fae6dbfdf6d ]---
>
>
>Thanks,
>Ben
>
>--
>Ben Greear <greearb@candelatech.com>
>Candela Technologies Inc http://www.candelatech.com
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Thanks, I'll take a look at it.
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox