* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-04 23:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Joe Perches, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <20100304143837.af39845d.akpm@linux-foundation.org>
On Thu, 4 Mar 2010, Andrew Morton wrote:
>
> What would I need to do to make it recur more than once? Include a %pV
> in a string, like dev_printk("%s", %%pV")?
I would argue that if somebody does that, they're just broken.
The single level of recursion is really nice - it solves a real annoyance.
But if somebody starts trying to do multiple levels, that's just crazy.
Of course, I guess we don't relly have any way of sanely _counting_ the
recursion level, so we'd have to just trust people to not do crazy things.
And that's a big thing to take on trust ;)
Linus
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Jan Engelhardt @ 2010-03-04 22:55 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Serge E. Hallyn, Sukadev Bhattiprolu, Pavel Emelyanov,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <m1pr3j92x8.fsf@fess.ebiederm.org>
On Thursday 2010-03-04 22:45, Eric W. Biederman wrote:
>
>So an unshare of the pid namespace that doesn't really take effect
>until we fork may actually be usable from pam, and in fact is probably
>the preferred implementation. It looks like neither openssh nor login
>from util-linux-ng will cope properly with getting any pid back from
>wait() except the pid of their child.
Correct; I can tell from experience with pam_mount. GDM for example is
very unhappy if you fork/exit processes in PAM modules and don't hide
the fact by bending SIGCHLD from gdm_handler to mypam_handler (which
itself is racy, suppose GDM re-set the SIGCHLD handler midway through).
(In this particular case however, I'd prefer if login programs like GDM
just ignored any PIDs they did not spawn in the first place instead of
moaning around.)
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Andrew Morton @ 2010-03-04 22:38 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <cover.1267682641.git.joe@perches.com>
On Wed, 3 Mar 2010 22:27:21 -0800
Joe Perches <joe@perches.com> wrote:
> (Typo'ed Linus' email address, added Greg KH and netdev)
What is the networking significance here?
> dev_<level> macros use a lot of repetitive string space.
>
> Eliminate the string prefixes and function arguments from all the macro uses
> and consolidate them in functions.
>
> This patchset saves about 60K.
>
> This implementation also adds the ability to use a struct va_format to
> emit a format string along with va_list arguments.
>
> This %pV implementation should not be used without a wrapper that
> does printf argument verification like the dev_<level> functions.
>
> Inspired a bit by Nick Andrew's patches and Linus' comments in December 2008
> http://lkml.org/lkml/2008/12/6/15
> http://lkml.org/lkml/2008/12/6/101
>
Looks like a reasonable approach, although I didn't check how much
additional stack the recursion will take. Bear in mind that printk()
can be called from super-deep contexts.
What would I need to do to make it recur more than once? Include a %pV
in a string, like dev_printk("%s", %%pV")?
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-04 21:45 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Sukadev Bhattiprolu, Pavel Emelyanov, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear
In-Reply-To: <m13a0hmblr.fsf@fess.ebiederm.org>
ebiederm@xmission.com (Eric W. Biederman) writes:
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
>
>> Quoting Eric W. Biederman (ebiederm@xmission.com):
>>> Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes:
>>>
>>> > Eric W. Biederman [ebiederm@xmission.com] wrote:
>>> > |
>>> > | I think replacing a struct pid for another struct pid allocated in
>>> > | descendant pid_namespace (but has all of the same struct upid values
>>> > | as the first struct pid) is a disastrous idea. It destroys the
>>> >
>>> > True. Sorry, I did not mean we would need a new 'struct pid' for an
>>> > existing process. I think we talked earlier of finding a way of attaching
>>> > additional pid numbers to the same struct pid.
>>>
>>> I just played with this and if you make the semantics of unshare(CLONE_NEWPID)
>>> to be that you become the idle task aka pid 0, and not the init task pid 1 the
>>> implementation is trivial.
>>
>> Heh, and then (browsing through your copy_process() patch hunks) the next
>> forked task becomes the child reaper for the new pidns? <shrug> why not
>> I guess.
>>
>> Now if that child reaper then gets killed, will the idle task get killed too?
>
> No.
>
>> And if not, then idle task can just re-populating the new pidns with new
>> idle tasks...
>
> After zap_pid_namespace interesting...
>
>> If this brought us a step closer to entering an existing pidns that would
>> be one thing, but is there actually any advantage to being able to
>> unshare a new pidns? Oh, I guess there is - PAM can then use it at
>> login, which might be neat.
>
> I have to say that the semantics of my patch are unworkable for
> unshare. Unless I am mistaken for PAM to use it requires that the
> current process fully change and become what it needs to be.
> Requiring an extra fork to fully complete the process is a problem.
>
> Scratch one bright idea.
Maybe not. I just looked and in the vast majority of cases the login
process goes like this.
{
setup stuff include pam
child = fork();
if (!child) {
setuid()
exec /bin/bash
}
waitpid(child);
pam and other cleanup
}
So an unshare of the pid namespace that doesn't really take effect
until we fork may actually be usable from pam, and in fact is probably
the preferred implementation. It looks like neither openssh nor login
from util-linux-ng will cope properly with getting any pid back from
wait() except the pid of their child. It looks like they both with
terminate. Which means if you login in a new pid namespace (where the
unsharing process becomes pid 1) and call nohup everything will get
killed and you will be logged out.
Eric
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Eric W. Biederman @ 2010-03-04 21:14 UTC (permalink / raw)
To: Octavian Purdila
Cc: Cong Wang, David Miller, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <201003042211.23460.opurdila@ixiacom.com>
Octavian Purdila <opurdila@ixiacom.com> writes:
> On Thursday 04 March 2010 21:14:07 you wrote:
>> Cong Wang <amwang@redhat.com> writes:
>> > David Miller wrote:
>> >> Eric B., could you look over the first two patches (which touch the
>> >> sysctl core) and give some review and ACK/NACK?
>> >
>> > ping Eric W. Biederman ... :)
>>
>> I have looked and it is not easy to tell by simple review if
>> correctness has been maintained in the proc cleanup patch.
>> Furthermore the code even after the cleanups still feels like code
>> that is trying too hard to do too much.
>>
>>
>> I think the example set by bitmap_parse_user in it's user interface is
>> a good example to follow when constructing bitmap helper functions.
>> Including having the actual parsing code should live in lib/bitmap.c
>>
>> The users of bitmap_parse_user do something very nice. They allocate
>> a temporary bitmap. Parse the userspace value into the temporary
>> bitmap, and then move that new bitmap into the kernel data structures.
>> For your large bitmap this seems like the idea way to handle it. That
>> guarantees no weird intermediate failure states, and really makes the
>> the bitmap feel like a single value.
>>
>> I would add the restriction that the values in the list of ranges
>> always must be increasing, and in general restrict the set of accepted
>> values as much as possible. If we don't accept it now we don't have
>> to worry about some userspace application relying on some unitended
>> side effect a few years into the future.
>>
>
> Eric, thanks for taking the time to go over it again. I now do share you
> opinion that we need to be more atomic. How about this simple approach:
>
> 1. Allocate new kernel buffer for the text and copy the whole userspace buffer
> into it.
> 2. Allocate temporary buffer for bitmap.
> 3. Parse the kernel text buffer into the temp bitmap.
> 4. Copy the temp bitmap into the final bitmap.
>
> This is simple and clean but it has the disadvantage of potentially allocating
> a large chunk of memory, although even in the case that all ports are going to
> be set the temporary buffer will not go over 390K, which is now not an issue
> anymore for kmalloc, right?
More than page size will always be an issue when memory is
sufficiently fragmented. I expect we can do just about as simply
with a small sliding window. Perhaps 50 characters.
However that might not be worth worrying about. Anything set by
a human being and anything we expect in practice is going to be
much shorter.
>> I think it is a serious bug that you clear the destination bitmap
>> in the middle of parsing it. That will either open or close all
>> ports in the middle of parsing, and I can't see how that would
>> ever be a good thing.
>>
>
> Even when doing the copy from the temp bitmap you still have some inconsistent
> bitmap state while copying.
>
> We could solve by replacing the old buffer with the new one + RCU.
True.
I was thinking a simple pass through that updated it a bit at a time
(if it was different), but rcu or some other form of locking would be
even more consistent. The joy with splitting the parser for the rest
of the sysctl code is that this is a separate decision that can be made
to accommodate the specific user.
Eric
^ permalink raw reply
* [PATCH] s2io: Fixing debug message
From: leitao @ 2010-03-04 20:40 UTC (permalink / raw)
To: sreenivasa.honnur; +Cc: netdev, Breno Leitao
Currently s2io is dumping debug messages using the interface name
before it was allocated, showing a message like the following:
s2io: eth%d: Ring Mem PHY: 0x7ef80000
s2io: s2io_reset: Resetting XFrame card eth%d
This patch just fixes it, printing the pci bus information for
the card instead of the interface name.
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
---
drivers/net/s2io.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 3c4836d..2a913be 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -923,8 +923,8 @@ static int init_shared_mem(struct s2io_nic *nic)
tmp_v_addr = mac_control->stats_mem;
mac_control->stats_info = (struct stat_block *)tmp_v_addr;
memset(tmp_v_addr, 0, size);
- DBG_PRINT(INIT_DBG, "%s: Ring Mem PHY: 0x%llx\n", dev->name,
- (unsigned long long)tmp_p_addr);
+ DBG_PRINT(INIT_DBG, "%s: Ring Mem PHY: 0x%llx\n",
+ dev_name(&nic->pdev->dev), (unsigned long long)tmp_p_addr);
mac_control->stats_info->sw_stat.mem_allocated += mem_allocated;
return SUCCESS;
}
@@ -3480,7 +3480,7 @@ static void s2io_reset(struct s2io_nic *sp)
struct swStat *swstats;
DBG_PRINT(INIT_DBG, "%s: Resetting XFrame card %s\n",
- __func__, sp->dev->name);
+ __func__, pci_name(sp->pdev));
/* Back up the PCI-X CMD reg, dont want to lose MMRBC, OST settings */
pci_read_config_word(sp->pdev, PCIX_COMMAND_REGISTER, &(pci_cmd));
--
1.6.0.2
^ permalink raw reply related
* Re: [Open-FCoE] Request for Advice from Steve Ma
From: Steve Ma @ 2010-03-04 20:40 UTC (permalink / raw)
To: Joe Eykholt; +Cc: berthiaume_wayne, devel, netdev
In-Reply-To: <4B8EA858.8020308@cisco.com>
On Wed, Mar 3, 2010 at 10:20 AM, Joe Eykholt <jeykholt@cisco.com> wrote:
> berthiaume_wayne@emc.com wrote:
>>
>> Wouldn't it be better to pass this as part of the SCSI block so it would
>> be transport agnostic?
>
> I'm not sure I see what you mean.
>
> This is transport-specific already, where we're putting
> a FCP command together and sending it out via FCoE. When the NIC gets the
> sk_buff
> to send, it only has the FCP command and any meta-data in the sk_buff.
> There's no
> access to the scsi_cmnd, if that's what you mean. Putting a pointer to that
> in
> the sk_buff would solve the problem, but seems like a layering problem.
> What am I overlooking?
>
> See more comments at end.
>
>>
>> Regards,
>> Wayne.
>> -----Original Message-----
>> From: devel-bounces@open-fcoe.org [mailto:devel-bounces@open-fcoe.org] On
>> Behalf Of Steve Ma
>> Sent: Monday, March 01, 2010 6:17 PM
>> To: Joe Eykholt
>> Cc: devel@open-fcoe.org
>> Subject: Re: [Open-FCoE] Request for Advice from Steve Ma
>>
>> On Mon, Mar 1, 2010 at 2:39 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>
>>> Steve Ma wrote:
>>>>
>>>> On Mon, Mar 1, 2010 at 12:08 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>>>
>>>>> Steve Ma wrote:
>>>>>>
>>>>>> On Mon, Mar 1, 2010 at 11:38 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Steve Ma wrote:
>>>>>>>>
>>>>>>>> On Mon, Mar 1, 2010 at 10:49 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Steve Ma wrote:
>>>>>>>>>>
>>>>>>>>>> On Mon, Mar 1, 2010 at 10:06 AM, Robert Love
>>>>>>>>>> <robert.w.love@intel.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Mon, 2010-03-01 at 10:04 -0800, Robert Love wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, 2010-03-01 at 09:26 -0800, Steve Ma wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Guys,
>>>>>>>>>>>>>
>>>>>>>>>>>> Hi Steve
>>>>>>>>>>>>
>>>>>>>>>>>>> When the SCSI mid-layer passes a SCSI command to the Open-FCoE,
>>>>>>>>>>>>> the
>>>>>>>>>>>>> fc_queuecomamnd receives the pointer of struct scsi_cmnd. This
>>>>>>>>>>>>> data
>>>>>>>>>>>>> structure contains two scatter-gather lists:
>>>>>>>>>>>>>
>>>>>>>>>>>>> struct scsi_data_buffer sdb;
>>>>>>>>>>>>> struct scsi_data_buffer *prot_sdb;
>>>>>>>>>>>>>
>>>>>>>>>>>>> The Open-FCoE stack does transmitting and receiving of sdb
>>>>>>>>>>>>> to/from
>>>>>>>>>>>>> the
>>>>>>>>>>>>> NIC driver. Now I am in need of transmitting and receiving both
>>>>>>>>>>>>> of
>>>>>>>>>>>>> them to the NIC driver and, in addition, I also need to pass
>>>>>>>>>>>>> down
>>>>>>>>>>>>> the
>>>>>>>>>>>>> return value of the mid layer routine
>>>>>>>>>>>>>
>>>>>>>>>>>>> unsigned char scsi_get_prot_op(struct scsi_cmnd *scmd)
>>>>>>>>>>>>>
>>>>>>>>>>>>> What would be the best way to implement?
>>>>>>>>>>>>
>>>>>>>>>>>> At a high level you're going to need to go through netdev so
>>>>>>>>>>>> engaging
>>>>>>>>>>>> their mailing list would be a good start (CC'ing this list would
>>>>>>>>>>>> be
>>>>>>>>>>>> nice). For FCoE offloaded devices that are netdev based we have
>>>>>>>>>>>> these
>>>>>>>>>>>> routines in the netdev structure-
>>>>>>>>>>>
>>>>>>>>>>> This should say, "in struct net_device_ops."
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I have already seen those ndo routines. I may be able to make use
>>>>>>>>>> of
>>>>>>>>>> the DDP rotuines. But for transmit, I am still not sure what the
>>>>>>>>>> best
>>>>>>>>>> way to do.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Steve
>>>>>>>>>
>>>>>>>>> Is the extra buffer descriptor for DIF bytes or what?
>>>>>>>>
>>>>>>>> Yes, there are 8-bytes of protection data for DIF/DIX.
>>>>>>>>
>>>>>>>>> How would it be supported and used in a non-offloaded case?
>>>>>>>>
>>>>>>>> Currently we are intended to do this in offloaded case.
>>>>>>>> I would disable the protection if offload (LSO/DDP) is disabled.
>>>>>>>>
>>>>>>>>> I mean, what would the code look like in fc_fcp.c?
>>>>>>>>
>>>>>>>> Not sure yet. I will try to make the code as simple as possible in
>>>>>>>> the
>>>>>>>> fcoe level, and do as much work as possible in the NIC driver and
>>>>>>>> the
>>>>>>>> ASIC.
>>>>>>>>
>>>>>>>>> What should the frames look like? Would there just be additional
>>>>>>>>> DIF bytes sent/received after every so many data bytes? If so,
>>>>>>>>> would it work (from a protocol standpoint, maybe not efficiently)
>>>>>>>>> to just merge the sg_lists?
>>>>>>>>
>>>>>>>> I have the same thought of merging the sglists, and in the NIC
>>>>>>>> driver
>>>>>>>> if I can figure out the correct pointer to the 8-bytes at the end, I
>>>>>>>> can assign an additional TxD for it. I am not sure if I have better
>>>>>>>> choice.
>>>>>>>
>>>>>>> If the NIC driver is just going to merge the SG lists, then why
>>>>>>> not do it at the fc_fcp layer? That way it would work for the
>>>>>>> non-offloaded case as well, and you wouldn't need any additional
>>>>>>> offload interfaces.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Joe
>>>>>>>
>>>>>>>
>>>>>> No, what I am thinking is to merge the two sglists in fc_fcp, thus the
>>>>>> 8-byte protection data will be part of the frame (skb) passed down to
>>>>>> the NIC driver.
>>>>>
>>>>> That's what I was suggesting. I thought you were suggesting passing
>>>>> both SG lists separately to the NIC. Perhaps I misunderstood.
>>>>>
>>>>>> It will be a driver for our future NIC. In the NIC driver, I will have
>>>>>> to assign TxDs for the frame data, and an additional TxD for the
>>>>>> 8-byte. So to my understanding, the NIC driver is to separate the
>>>>>> frame data and the 8-byte tag fields. I would let the ASIC to to the
>>>>>> validation job for the protection data. If there is no-offload, the
>>>>>> validation would be a big performance burden if they are done in the
>>>>>> OS driver.
>>>>>
>>>>> Wouldn't DIF need to be validated again when the data is used?
>>>>
>>>> This depends on the value returned from scsi_get_prot_op(). Either
>>>> OS<-->HBA is protected or HBA<-->Target is protected or both
>>>> (end-to-end). HBA is a checkpoint we are responsible for.
>>>>
>>>>> Otherwise other parts of the data path aren't covered ... the
>>>>> DIF wouldn't be completely end-to-end. But verification at
>>>>> multiple points along the path is good.
>>>>>
>>>>> I was mostly thinking about sending the DIF where it's already
>>>>> been verified. Receiving the DIF is more difficult since you
>>>>> don't know exactly how the data will be divided into frames,
>>>>> but it's the same effect: a merged SG list.
>>>>
>>>> With your support, this is the best approach I can work on.
>>>>
>>>> BTW. I want to pass the return value of scsi_get_prot_op() from fc_fcp
>>>> to the NIC driver during transmitting of a SCSI command, what would be
>>>> the best approach? It is only a byte.
>>>
>>> You could pass it to fcoe_xmit() or whatever LLD in the fcoe_rcv_info.
>>> There's only one byte free there (without stretching skb->cb[]),
>>> but you could encode it in fr_flags, which has 7 bits free.
>>> skb->cb[] can't be used for this outside of libfc/fcoe though,
>>> because it's allowed to be reused by any intermediate layers.
>>> But since skb->cb also contains the fr_fsp, you could put it in there,
>>> and call scsi_get_prot_op() from the fcoe if needed.
>>>
>>> After fcoe_xmit(), prot_op could be passed to the NIC some other way.
>>> Perhaps by appending it after the data (to be stripped off later),
>>> or using some existing field in the sk_buff.
>>>
>>> This is a long-winded way of saying "I don't know." Someone who
>>> deals more with the guts of sk_buffs will probably suggest something.
>>>
>>> Joe
>>>
>>>
>>>
>> Appending seems to be a clean way to do. So I will have 9 bytes in
>> total appended to the data frame.
>>
>> Thanks
>> Steve
>
> I regret suggesting appending. I no longer thing that's such a good idea.
> If it is after the FCoE Encapsulation, then it might be OK, but we would
> have to know that the NIC will take it off. Also, it will look wrong
> for tcpdump.
>
> Look into other ways of attaching meta-data to sk_buffs. There might
> be something already available.
>
> Joe
>
>
I'll keep on looking.
Thanks.
Steve Ma
^ permalink raw reply
* Re: 2.6.33 dies on modprobe
From: M G Berberich @ 2010-03-04 20:38 UTC (permalink / raw)
To: Américo Wang
Cc: linux-kernel, Linux Kernel Network Developers, Greg Kroah-Hartman
In-Reply-To: <20100304153042.GA4614@hack>
Am Donnerstag, den 04. März schrieb Américo Wang:
> On Thu, Mar 04, 2010 at 12:04:38AM +0100, M G Berberich wrote:
> >Am Mittwoch, den 03. März schrieb Américo Wang:
> >> On Wed, Mar 3, 2010 at 5:08 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Wed, Mar 3, 2010 at 4:58 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> >> Ok, below is my patch, I am not sure it could fix the BUG for you,
> >> >> but it could fix the WARNING. But perhaps they are related.
> >> >>
> >> >> Please give it a try.
> >> >>
> >> >
> >> > Oops! Ignore the patch, it should not be correct, I will send a
> >> > correct version soon.
> >> > Sorry.
> >> >
> >>
> >> Here we go:
> >
> >Seems to fix the warning, but not the modprobe-Oops.
> >
>
> Thanks much for your testing!
>
> Could you please try to turn on DEBUG_KOBJECT and boot your kernel
> with "initcall_debug"? And then show us your dmesg.
If I'm doing this (enabled CONFIG_DEBUG_KERNEL and
CONFIG_DEBUG_KOBJECT), the kernel dies immediately - no output.
MfG
bmg
--
„Des is völlig wurscht, was heut beschlos- | M G Berberich
sen wird: I bin sowieso dagegn!“ | berberic@fmi.uni-passau.de
(SPD-Stadtrat Kurt Schindler; Regensburg) | www.fmi.uni-passau.de/~berberic
^ permalink raw reply
* Re: BNX2: Kernel crashes with 2.6.31 and 2.6.31.9
From: Brian Haley @ 2010-03-04 20:31 UTC (permalink / raw)
To: Michael Chan; +Cc: Bruno Prémont, Benjamin Li, NetDEV, Linux-Kernel
In-Reply-To: <1267567926.19491.175.camel@nseg_linux_HP1.broadcom.com>
Hi Michael,
Michael Chan wrote:
> Do we have timers running in this environment? The timer in the bnx2
> driver, bnx2_timer(), needs to run to provide a heart beat to the
> firmware. In netpoll mode without timer interrupts, if we are regularly
> calling the NAPI poll function, it should also be able to provide the
> heartbeat. Without the heartbeat, the firmware will reset the chip and
> result in the NETDEV WATCHDOG.
We have also been seeing watchdog timeouts with bnx2, below is a
stack trace with Benjamin's debug patch applied. Normally we were
only seeing them under heavy load, but this one was at boot. We haven't
tried the latest firmware/driver from 2.6.33 yet. You can contact me
offline if you need more detailed info.
Thanks,
-Brian
[ 2.428093] bnx2 0000:04:00.0: firmware: requesting bnx2/bnx2-rv2p-06-5.0.0.j3.fw
[ 2.432526] eth0: Broadcom NetXtreme II BCM5708 1000Base-T (B2) PCI-X 64-bit 133MHz found at mem f6000000, IRQ 41, node addr 00:1c:c4:e1:cc:ea
[ 2.439520] bnx2 0000:42:00.0: PCI INT A -> GSI 34 (level, low) -> IRQ 34
<snip>
[ 223.805014] ------------[ cut here ]------------
[ 223.805023] WARNING: at net/sched/sch_generic.c:261 dev_watchdog+0x12d/0x1d5()
[ 223.805026] Hardware name: ProLiant DL385 G2
[ 223.805028] NETDEV WATCHDOG: eth0 (bnx2): transmit queue 0 timed out
[ 223.805031] Modules linked in: itapi iptable_filter ip_tables x_tables mptctl ipmi_devintf deflate zlib_deflate ctr twofish twofish_common camellia serpent blowfish cast5 des_generic cbc cryptd aes_x86_64 aes_generic xcbc rmd160 sha256_generic sha1_generic crypto_null af_key dm_snapshot dm_mirror dm_region_hash dm_log dm_mod sg bonding sctp crc32c libcrc32c loop psmouse serio_raw amd64_edac_mod edac_core k8temp container i2c_piix4 i2c_core ipmi_si ipmi_msghandler shpchp pci_hotplug hpilo processor evdev ext3 jbd mbcache ses enclosure sd_mod crc_t10dif ide_cd_mod cdrom ata_generic libata ide_pci_generic usbhid hid mptsas bnx2 mptscsih mptbase scsi_transport_sas serverworks ehci_hcd scsi_mod ide_core ohci_hcd uhci_hcd button thermal fan thermal_sys edd [last unloaded: scsi_wait_scan]
[ 223.805102] Pid: 0, comm: swapper Not tainted 2.6.32-clim-4-amd64 #1
[ 223.805105] Call Trace:
[ 223.805108] <IRQ> [<ffffffff812697a0>] ? dev_watchdog+0x12d/0x1d5
[ 223.805118] [<ffffffff81049914>] warn_slowpath_common+0x77/0xa4
[ 223.805123] [<ffffffff810499b6>] warn_slowpath_fmt+0x64/0x66
[ 223.805128] [<ffffffff81045df7>] ? default_wake_function+0xd/0xf
[ 223.805133] [<ffffffff81035fa7>] ? __wake_up_common+0x46/0x76
[ 223.805138] [<ffffffff8103b414>] ? __wake_up+0x43/0x50
[ 223.805143] [<ffffffff81253829>] ? netdev_drivername+0x43/0x4b
[ 223.805147] [<ffffffff812697a0>] dev_watchdog+0x12d/0x1d5
[ 223.805152] [<ffffffff8105e84a>] ? delayed_work_timer_fn+0x0/0x3d
[ 223.805156] [<ffffffff8105e7ee>] ? __queue_work+0x35/0x3d
[ 223.805159] [<ffffffff81269673>] ? dev_watchdog+0x0/0x1d5
[ 223.805164] [<ffffffff8105655a>] run_timer_softirq+0x1ff/0x2a1
[ 223.805169] [<ffffffff810205a1>] ? lapic_next_event+0x18/0x1c
[ 223.805174] [<ffffffff8104f9e3>] __do_softirq+0xde/0x19f
[ 223.805179] [<ffffffff8100ccec>] call_softirq+0x1c/0x28
[ 223.805183] [<ffffffff8100e8b1>] do_softirq+0x41/0x81
[ 223.805187] [<ffffffff8104f7bd>] irq_exit+0x36/0x75
[ 223.805191] [<ffffffff81020f33>] smp_apic_timer_interrupt+0x88/0x96
[ 223.805195] [<ffffffff8100c6b3>] apic_timer_interrupt+0x13/0x20
[ 223.805198] <EOI> [<ffffffff81027740>] ? native_safe_halt+0x6/0x8
[ 223.805207] [<ffffffff81012da3>] ? default_idle+0x55/0x74
[ 223.805210] [<ffffffff810131ce>] ? c1e_idle+0xf4/0xfb
[ 223.805215] [<ffffffff81065529>] ? atomic_notifier_call_chain+0x13/0x15
[ 223.805219] [<ffffffff8100aeec>] ? cpu_idle+0x5b/0x93
[ 223.805225] [<ffffffff81304144>] ? start_secondary+0x1a8/0x1ac
[ 223.805228] ---[ end trace b04d103e6c8c23de ]---
[ 223.805231] bnx2: eth0 DEBUG: intr_sem[0]
[ 223.805236] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 223.805242] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 223.805245] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 228.805016] bnx2: eth0 DEBUG: intr_sem[0]
[ 228.805023] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 228.805029] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 228.805033] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 233.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 233.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 233.805024] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 233.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 238.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 238.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 238.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 238.805029] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 243.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 243.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 243.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 243.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 248.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 248.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 248.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 248.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 253.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 253.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 253.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 253.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 258.805016] bnx2: eth0 DEBUG: intr_sem[0]
[ 258.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 258.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 258.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 263.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 263.805018] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 263.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 263.805027] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 268.805014] bnx2: eth0 DEBUG: intr_sem[0]
[ 268.805019] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 268.805025] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 268.805028] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 273.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 273.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 273.805028] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 273.805032] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 278.805012] bnx2: eth0 DEBUG: intr_sem[0]
[ 278.805017] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 278.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 278.805026] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 283.805012] bnx2: eth0 DEBUG: intr_sem[0]
[ 283.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 283.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 283.805025] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 288.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 288.805022] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 288.805028] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 288.805031] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 293.805011] bnx2: eth0 DEBUG: intr_sem[0]
[ 293.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 293.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 293.805026] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 298.805015] bnx2: eth0 DEBUG: intr_sem[0]
[ 298.805021] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 298.805027] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 298.805030] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 303.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 303.805020] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 303.805026] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 303.805030] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 308.805013] bnx2: eth0 DEBUG: intr_sem[0]
[ 308.805018] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 308.805023] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 308.805027] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
[ 313.805011] bnx2: eth0 DEBUG: intr_sem[0]
[ 313.805016] bnx2: eth0 DEBUG: EMAC_TX_STATUS[00000008] RPM_MGMT_PKT_CTRL[00000000]
[ 313.805022] bnx2: eth0 DEBUG: MCP_STATE_P0[00000000] MCP_STATE_P1[00000000]
[ 313.805025] bnx2: eth0 DEBUG: HC_STATS_INTERRUPT_STATUS[00000000]
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Octavian Purdila @ 2010-03-04 20:11 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Cong Wang, David Miller, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <m1vddbdhmo.fsf@fess.ebiederm.org>
On Thursday 04 March 2010 21:14:07 you wrote:
> Cong Wang <amwang@redhat.com> writes:
> > David Miller wrote:
> >> Eric B., could you look over the first two patches (which touch the
> >> sysctl core) and give some review and ACK/NACK?
> >
> > ping Eric W. Biederman ... :)
>
> I have looked and it is not easy to tell by simple review if
> correctness has been maintained in the proc cleanup patch.
> Furthermore the code even after the cleanups still feels like code
> that is trying too hard to do too much.
>
>
> I think the example set by bitmap_parse_user in it's user interface is
> a good example to follow when constructing bitmap helper functions.
> Including having the actual parsing code should live in lib/bitmap.c
>
> The users of bitmap_parse_user do something very nice. They allocate
> a temporary bitmap. Parse the userspace value into the temporary
> bitmap, and then move that new bitmap into the kernel data structures.
> For your large bitmap this seems like the idea way to handle it. That
> guarantees no weird intermediate failure states, and really makes the
> the bitmap feel like a single value.
>
> I would add the restriction that the values in the list of ranges
> always must be increasing, and in general restrict the set of accepted
> values as much as possible. If we don't accept it now we don't have
> to worry about some userspace application relying on some unitended
> side effect a few years into the future.
>
Eric, thanks for taking the time to go over it again. I now do share you
opinion that we need to be more atomic. How about this simple approach:
1. Allocate new kernel buffer for the text and copy the whole userspace buffer
into it.
2. Allocate temporary buffer for bitmap.
3. Parse the kernel text buffer into the temp bitmap.
4. Copy the temp bitmap into the final bitmap.
This is simple and clean but it has the disadvantage of potentially allocating
a large chunk of memory, although even in the case that all ports are going to
be set the temporary buffer will not go over 390K, which is now not an issue
anymore for kmalloc, right?
> I think it is a serious bug that you clear the destination bitmap
> in the middle of parsing it. That will either open or close all
> ports in the middle of parsing, and I can't see how that would
> ever be a good thing.
>
Even when doing the copy from the temp bitmap you still have some inconsistent
bitmap state while copying.
We could solve by replacing the old buffer with the new one + RCU.
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Eric W. Biederman @ 2010-03-04 19:14 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, opurdila, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <4B8F6FF0.5000306@redhat.com>
Cong Wang <amwang@redhat.com> writes:
> David Miller wrote:
>> Eric B., could you look over the first two patches (which touch the
>> sysctl core) and give some review and ACK/NACK?
>>
>
> ping Eric W. Biederman ... :)
I have looked and it is not easy to tell by simple review if
correctness has been maintained in the proc cleanup patch.
Furthermore the code even after the cleanups still feels like code
that is trying too hard to do too much.
I think the example set by bitmap_parse_user in it's user interface is
a good example to follow when constructing bitmap helper functions.
Including having the actual parsing code should live in lib/bitmap.c
The users of bitmap_parse_user do something very nice. They allocate
a temporary bitmap. Parse the userspace value into the temporary
bitmap, and then move that new bitmap into the kernel data structures.
For your large bitmap this seems like the idea way to handle it. That
guarantees no weird intermediate failure states, and really makes the
the bitmap feel like a single value.
I would add the restriction that the values in the list of ranges
always must be increasing, and in general restrict the set of accepted
values as much as possible. If we don't accept it now we don't have
to worry about some userspace application relying on some unitended
side effect a few years into the future.
I think it is a serious bug that you clear the destination bitmap
in the middle of parsing it. That will either open or close all
ports in the middle of parsing, and I can't see how that would
ever be a good thing.
Eric
^ permalink raw reply
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: jamal @ 2010-03-04 18:30 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Alexander Duyck
In-Reply-To: <20100304094323.05fce73a@nehalam>
On Thu, 2010-03-04 at 09:43 -0800, Stephen Hemminger wrote:
> I put the first bit in back in December. It was too early since
> kernel wasn't ready yet, but generally harmless
Ah, my bad. git whatchanged didnt show that.
cheers,
jamal
^ permalink raw reply
* [PATCH] Re: [PATCH v4] ethtool: Add direct access to ops->get_sset_count
From: Jeff Garzik @ 2010-03-04 18:21 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Jeff Kirsher, davem, netdev, gospo
In-Reply-To: <1267712797.2819.149.camel@localhost>
[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]
On 03/04/2010 09:26 AM, Ben Hutchings wrote:
> On Thu, 2010-03-04 at 00:51 -0800, Jeff Kirsher wrote:
>> From: Jeff Garzik<jgarzik@redhat.com>
>>
>> This patch is an alternative approach for accessing string
>> counts, vs. the drvinfo indirect approach. This way the drvinfo
>> space doesn't run out, and we don't break ABI later.
> [...]
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -214,6 +214,10 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
>> info.cmd = ETHTOOL_GDRVINFO;
>> ops->get_drvinfo(dev,&info);
>>
>> + /*
>> + * this method of obtaining string set info is deprecated;
>> + * consider using ETHTOOL_GSSET_INFO instead
>> + */
>
> This comment belongs on the interface (ethtool.h) not the
> implementation.
Debatable -- the current comment is located at the callsite of
ops->get_sset_count(), which is where an implementor might think to add
a new call. Not all the numeric fields in ethtool_drvinfo are obtained
from ->get_sset_count().
Hence the "some" in the attached patch to include/linux/ethtool.h,
addressing your comment.
> [...]
>> +static noinline int ethtool_get_sset_info(struct net_device *dev,
>> + void __user *useraddr)
>> +{
> [...]
>> + /* calculate size of return buffer */
>> + for (i = 0; i< 64; i++)
>> + if (sset_mask& (1ULL<< i))
>> + n_bits++;
> [...]
>
> We have a function for this:
>
> n_bits = hweight64(sset_mask);
Agreed.
I've attached a follow-up patch, which should enable my/Jeff's kernel
patch to be applied, followed by this one.
Jeff
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1755 bytes --]
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
include/linux/ethtool.h | 7 +++++++
net/core/ethtool.c | 7 +++----
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f6f961f..b33f316 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -61,6 +61,13 @@ struct ethtool_drvinfo {
/* For PCI devices, use pci_name(pci_dev). */
char reserved1[32];
char reserved2[12];
+ /*
+ * Some struct members below are filled in
+ * using ops->get_sset_count(). Obtaining
+ * this info from ethtool_drvinfo is now
+ * deprecated; Use ETHTOOL_GSSET_INFO
+ * instead.
+ */
__u32 n_priv_flags; /* number of flags valid in ETHTOOL_GPFLAGS */
__u32 n_stats; /* number of u64's from ETHTOOL_GSTATS */
__u32 testinfo_len;
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 70075c4..33d2ded 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -17,6 +17,7 @@
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
+#include <linux/bitops.h>
#include <asm/uaccess.h>
/*
@@ -216,7 +217,7 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
/*
* this method of obtaining string set info is deprecated;
- * consider using ETHTOOL_GSSET_INFO instead
+ * Use ETHTOOL_GSSET_INFO instead.
*/
if (ops->get_sset_count) {
int rc;
@@ -265,9 +266,7 @@ static noinline int ethtool_get_sset_info(struct net_device *dev,
return 0;
/* calculate size of return buffer */
- for (i = 0; i < 64; i++)
- if (sset_mask & (1ULL << i))
- n_bits++;
+ n_bits = hweight64(sset_mask);
memset(&info, 0, sizeof(info));
info.cmd = ETHTOOL_GSSET_INFO;
^ permalink raw reply related
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: Stephen Hemminger @ 2010-03-04 17:43 UTC (permalink / raw)
To: hadi; +Cc: netdev, Alexander Duyck
In-Reply-To: <1267721912.4821.37.camel@bigi>
On Thu, 04 Mar 2010 11:58:32 -0500
jamal <hadi@cyberus.ca> wrote:
> On Thu, 2010-03-04 at 08:48 -0800, Stephen Hemminger wrote:
> > On Wed, 03 Mar 2010 21:51:31 -0500
> > jamal <hadi@cyberus.ca> wrote:
> >
> > > You applied 2/2 but not 1/2.
>
> > > >
> >
> > The first was the help message text, and that was already there.
> > Or was there some other bit.
>
> I can see that now. The git logs dont show how that change
> made it in - thats how i concluded it wasnt there. How did that
> sneak in there?
I put the first bit in back in December. It was too early since
kernel wasn't ready yet, but generally harmless
commit e04dd30a38130a3d85065a747cc33274766a9cb6
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Sat Dec 26 11:12:43 2009 -0800
skbedit: Add support to mark packets
This adds support for setting the skb mark.
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* uninterruptible sleep in unix_dgram_recvmsg
From: Michal Schmidt @ 2010-03-04 17:41 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Hello David.
When multiple tasks call recv() on a AF_UNIX/SOCK_DGRAM socket where
noone sends anything, only the first one will sleep interruptibly. The
others are in uninterruptible sleep, causing artificial increase of
loadavg. After two minutes, the hung task watchdog triggers and prints
ugly warnings.
The bug is reported here (with a reproducer attached):
https://bugzilla.redhat.com/show_bug.cgi?id=529202
While the first task awaits the arrival of a packet in
skb_recv_datagram(), it holds the u->readlock mutex, on which the
other tasks will be waiting.
My first idea was to simply replace mutex_lock with
mutex_lock_interruptible. This solves the problem, but one
issue still remains - the receiving timeout (SO_RCVTIMEO) would start
ticking only after the process got the mutex and entered into
skb_recv_datagram().
So instead of that I started to think about why u->readlock is held
across skb_recv_datagram() anyway. I found that it was added in 2.6.10
by your patch "[AF_UNIX]: Serialize dgram read using semaphore just
like stream" which apparently fixed an exploitable race condition
(CAN-2004-1068).
I don't know what exactly u->readlock protects here.
IOW, what race would this patch cause?:
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f255119..01387da 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1660,9 +1660,9 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
msg->msg_namelen = 0;
+ skb = skb_recv_datagram(sk, flags, noblock, &err);
mutex_lock(&u->readlock);
- skb = skb_recv_datagram(sk, flags, noblock, &err);
if (!skb) {
unix_state_lock(sk);
/* Signal EOF on disconnected non-blocking SEQPACKET socket. */
^ permalink raw reply related
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: jamal @ 2010-03-04 16:58 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Alexander Duyck
In-Reply-To: <20100304084829.22a80ace@nehalam>
On Thu, 2010-03-04 at 08:48 -0800, Stephen Hemminger wrote:
> On Wed, 03 Mar 2010 21:51:31 -0500
> jamal <hadi@cyberus.ca> wrote:
>
> > You applied 2/2 but not 1/2.
> > >
>
> The first was the help message text, and that was already there.
> Or was there some other bit.
I can see that now. The git logs dont show how that change
made it in - thats how i concluded it wasnt there. How did that
sneak in there?
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 2/2] iproute2 skbedit: Add support to mark packets
From: Stephen Hemminger @ 2010-03-04 16:48 UTC (permalink / raw)
To: hadi; +Cc: netdev, Alexander Duyck
In-Reply-To: <1267671091.4821.27.camel@bigi>
On Wed, 03 Mar 2010 21:51:31 -0500
jamal <hadi@cyberus.ca> wrote:
> You applied 2/2 but not 1/2.
>
> cheers,
> jamal
> On Wed, 2010-03-03 at 16:36 -0800, Stephen Hemminger wrote:
> > On Thu, 03 Dec 2009 08:32:48 -0500
> > jamal <hadi@cyberus.ca> wrote:
> >
> > > I did this originally because i was getting too many support
> > > questions on ipt with mark. It still makes sense since skbedit
> > > seems to be the mother action of skb metadata (should probably
> > > have been called skbmeta).
> > >
> > > cheers,
> > > jamal
> >
The first was the help message text, and that was already there.
Or was there some other bit.
^ permalink raw reply
* Re: Iproute2 tc pfifo_fast not work
From: Stephen Hemminger @ 2010-03-04 16:40 UTC (permalink / raw)
To: Петухов Дмитрий
Cc: netdev
In-Reply-To: <4B8F7537.7090008@electro-com.ru>
On Thu, 04 Mar 2010 11:54:15 +0300
Петухов Дмитрий <d.petuhov@electro-com.ru> wrote:
> Hi Stephen.
>
> I've found that I can't add pfifo_fast qdisc to my tc class tree. It
> just says:
> -----------
> # /sbin/tc qdisc add dev eth3 parent 2:299B handle 299B: pfifo_fast
> qdisc 'pfifo_fast' does not support option parsing
> -----------
> And nothing else happends.
> strace /sbin/tc qdisc add dev eth3 parent 2:299B handle 299B: pfifo_fast
> &> tc_pfifo_strace.txt
> attached.
>
> Kernel version 2.6.31, .config attached.
> tc version 2.6.31.
>
> I do something wrong or this is a bug?
no
> For now I've switched to PRIO QDISC which seems to work fine, but it
> would be great to use lithweigth pfifo_fast instead of prio...
pfifo_fast is very limited to being just a root qdisc, and with no
children. If you want to do the same thing but in other context
just use pfifo.
^ permalink raw reply
* Re: [PATCH] gianfar: Fix TX ring processing on SMP machines
From: Kumar Gala @ 2010-03-04 16:34 UTC (permalink / raw)
To: Anton Vorontsov
Cc: linuxppc-dev list, Netdev, Martyn Welch, Paul Gortmaker,
Sandeep Gopalpet, David Miller
In-Reply-To: <20100304.004157.18721737.davem@davemloft.net>
On Mar 4, 2010, at 2:41 AM, David Miller wrote:
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Date: Wed, 3 Mar 2010 21:18:58 +0300
>
>> Starting with commit a3bc1f11e9b867a4f49505 ("gianfar: Revive SKB
>> recycling") gianfar driver sooner or later stops transmitting any
>> packets on SMP machines.
>>
>> start_xmit() prepares new skb for transmitting, generally it does
>> three things:
>>
>> 1. sets up all BDs (marks them ready to send), except the first one.
>> 2. stores skb into tx_queue->tx_skbuff so that clean_tx_ring()
>> would cleanup it later.
>> 3. sets up the first BD, i.e. marks it ready.
>>
>> Here is what clean_tx_ring() does:
>>
>> 1. reads skbs from tx_queue->tx_skbuff
>> 2. checks if the *last* BD is ready. If it's still ready [to send]
>> then it it isn't transmitted, so clean_tx_ring() returns.
>> Otherwise it actually cleanups BDs. All is OK.
>>
>> Now, if there is just one BD, code flow:
>>
>> - start_xmit(): stores skb into tx_skbuff. Note that the first BD
>> (which is also the last one) isn't marked as ready, yet.
>> - clean_tx_ring(): sees that skb is not null, *and* its lstatus
>> says that it is NOT ready (like if BD was sent), so it cleans
>> it up (bad!)
>> - start_xmit(): marks BD as ready [to send], but it's too late.
>>
>> We can fix this simply by reordering lstatus/tx_skbuff writes.
>>
>> Reported-by: Martyn Welch <martyn.welch@ge.com>
>> Bisected-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
>> Tested-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> Tested-by: Martyn Welch <martyn.welch@ge.com>
>
> Applied.
Anton,
Once this makes it into Linus's tree can you make sure we get it added to -stable.
- k
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2010-03-04 16:18 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b1003040806m5946c9e3xc3cd7b8adade899c@mail.gmail.com>
On Thu, Mar 04, 2010 at 11:06:59AM -0500, Bob Beers wrote:
> On Thu, Mar 4, 2010 at 10:47 AM, Greg KH <greg@kroah.com> wrote:
> > On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
> >> Obviously still needs serious attention, but it compiles.
> >
> > Great, this looks wonderful, I'll queue it up.
> >
> >> Original author: Rick Dobbs
> >
> > Hm, do we have an email address for Rick anywhere?
>
> I knew you were going to ask that, and I asked at OSS.
> They said, "Rick has asked me not to give out his e-mail."
Heh, ok, I guess "pride of ownership" is not something that he has for
this body of code :)
I'll take it as-is.
thanks,
greg k-h
^ permalink raw reply
* Re: Help On function to get IPV6 address of an interface in kernel
From: Brian Haley @ 2010-03-04 16:17 UTC (permalink / raw)
To: Mohan Reddy; +Cc: netdev
In-Reply-To: <20100304094103.GA6599@mohan-laptop>
Mohan Reddy wrote:
> Hi,
>
> Is there a function or a way in kernel to get an interface ipv6 address if interface name or net_device object is known? I searched in the kernel i got a function ipv6_get_ifaddr(). But it is expecting an ipv6 address as a parameter.
What type of IPv6 address? For link-locals you can use ipv6_get_lladdr(), but
for others ipv6_dev_get_saddr() is more comprehensive, but you'll need a
destination address. There's lots of reference code in addrconf.c to do
other things.
-Brian
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Bob Beers @ 2010-03-04 16:06 UTC (permalink / raw)
To: Greg KH; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <20100304154707.GB30191@kroah.com>
On Thu, Mar 4, 2010 at 10:47 AM, Greg KH <greg@kroah.com> wrote:
> On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
>> Obviously still needs serious attention, but it compiles.
>
> Great, this looks wonderful, I'll queue it up.
>
>> Original author: Rick Dobbs
>
> Hm, do we have an email address for Rick anywhere?
I knew you were going to ask that, and I asked at OSS.
They said, "Rick has asked me not to give out his e-mail."
--
thanks,
-Bob Beers
^ permalink raw reply
* Re: wanPMC-CxT1E1
From: Greg KH @ 2010-03-04 15:47 UTC (permalink / raw)
To: Bob Beers; +Cc: netdev, Krzysztof Halasa
In-Reply-To: <4f6ba3b1003040540v4619afa8v7f28ef5d75514c47@mail.gmail.com>
On Thu, Mar 04, 2010 at 08:40:46AM -0500, Bob Beers wrote:
> Obviously still needs serious attention, but it compiles.
Great, this looks wonderful, I'll queue it up.
> Original author: Rick Dobbs
Hm, do we have an email address for Rick anywhere?
thanks,
greg k-h
^ permalink raw reply
* Re: 2.6.33 dies on modprobe
From: Américo Wang @ 2010-03-04 15:30 UTC (permalink / raw)
To: M G Berberich
Cc: Américo Wang, linux-kernel, Linux Kernel Network Developers,
Greg Kroah-Hartman
In-Reply-To: <20100303230438.GA3363@invalid>
On Thu, Mar 04, 2010 at 12:04:38AM +0100, M G Berberich wrote:
>Hello,
>
>Am Mittwoch, den 03. März schrieb Américo Wang:
>> On Wed, Mar 3, 2010 at 5:08 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> > On Wed, Mar 3, 2010 at 4:58 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
>> >> Ok, below is my patch, I am not sure it could fix the BUG for you,
>> >> but it could fix the WARNING. But perhaps they are related.
>> >>
>> >> Please give it a try.
>> >>
>> >
>> > Oops! Ignore the patch, it should not be correct, I will send a
>> > correct version soon.
>> > Sorry.
>> >
>>
>> Here we go:
>
>Seems to fix the warning, but not the modprobe-Oops.
>
Thanks much for your testing!
Could you please try to turn on DEBUG_KOBJECT and boot your kernel
with "initcall_debug"? And then show us your dmesg.
Regards.
^ permalink raw reply
* [PATCH] typhoon: fix incorrect use of smp_wmb()
From: David Dillow @ 2010-03-04 14:37 UTC (permalink / raw)
To: netdev; +Cc: David Miller
The typhoon driver was incorrectly using smp_wmb() to order memory
accesses against IO to the NIC in a few instances. Use wmb() instead,
which is required to actually order between memory types.
Signed-off-by: David Dillow <dave@thedillows.org>
---
This is against Linus's tree, which doesn't appear to have net-next in
it yet. It should apply with offsets, though. I can redo against
net-next if needed, but likely won't have quick access to that tree
until the weekend.
diff --git a/drivers/net/typhoon.c b/drivers/net/typhoon.c
index 39f1fc6..a192852 100644
--- a/drivers/net/typhoon.c
+++ b/drivers/net/typhoon.c
@@ -487,7 +487,7 @@ typhoon_hello(struct typhoon *tp)
typhoon_inc_cmd_index(&ring->lastWrite, 1);
INIT_COMMAND_NO_RESPONSE(cmd, TYPHOON_CMD_HELLO_RESP);
- smp_wmb();
+ wmb();
iowrite32(ring->lastWrite, tp->ioaddr + TYPHOON_REG_CMD_READY);
spin_unlock(&tp->command_lock);
}
@@ -1321,13 +1321,15 @@ typhoon_init_interface(struct typhoon *tp)
tp->txlo_dma_addr = le32_to_cpu(iface->txLoAddr);
tp->card_state = Sleeping;
- smp_wmb();
tp->offload = TYPHOON_OFFLOAD_IP_CHKSUM | TYPHOON_OFFLOAD_TCP_CHKSUM;
tp->offload |= TYPHOON_OFFLOAD_UDP_CHKSUM | TSO_OFFLOAD_ON;
spin_lock_init(&tp->command_lock);
spin_lock_init(&tp->state_lock);
+
+ /* Force the writes to the shared memory area out before continuing. */
+ wmb();
}
static void
^ permalink raw reply related
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