* [PATCH] vxge: Fix wrong boolean operator
From: Stefan Weil @ 2011-01-28 22:30 UTC (permalink / raw)
To: jon.mason
Cc: Stefan Weil, Ramkrishna Vepa, Sivakumar Subramani,
Sreenivasa Honnur, netdev, linux-kernel
This error is reported by cppcheck:
drivers/net/vxge/vxge-config.c:3693: warning: Mutual exclusion over || always evaluates to true. Did you intend to use && instead?
It looks like cppcheck is correct, so fix this. No test was run.
Cc: Ramkrishna Vepa <ramkrishna.vepa@exar.com>
Cc: Sivakumar Subramani <sivakumar.subramani@exar.com>
Cc: Sreenivasa Honnur <sreenivasa.honnur@exar.com>
Cc: Jon Mason <jon.mason@exar.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
drivers/net/vxge/vxge-config.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/vxge/vxge-config.c b/drivers/net/vxge/vxge-config.c
index 01c05f5..228d4f7 100644
--- a/drivers/net/vxge/vxge-config.c
+++ b/drivers/net/vxge/vxge-config.c
@@ -3690,7 +3690,7 @@ __vxge_hw_vpath_rts_table_get(struct __vxge_hw_vpath_handle *vp,
if (status != VXGE_HW_OK)
goto exit;
- if ((rts_table != VXGE_HW_RTS_ACCESS_STEER_CTRL_DATA_STRUCT_SEL_DA) ||
+ if ((rts_table != VXGE_HW_RTS_ACCESS_STEER_CTRL_DATA_STRUCT_SEL_DA) &&
(rts_table !=
VXGE_HW_RTS_ACS_STEER_CTRL_DATA_STRUCT_SEL_RTH_MULTI_IT))
*data1 = 0;
--
1.7.2.3
^ permalink raw reply related
* [PATCH] enc28j60: Fix reading of transmit status vector
From: Stefan Weil @ 2011-01-28 22:25 UTC (permalink / raw)
To: davem
Cc: Stefan Weil, Eric Dumazet, Tejun Heo, Jiri Pirko, netdev,
linux-kernel
This error was reported by cppcheck:
drivers/net/enc28j60.c:815: error: Using sizeof for array given as function argument returns the size of pointer.
The original code reads 4 or 8 bytes instead of TSV_SIZE (= 100) bytes.
I just fixed the code, but did not run any tests.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Jiri Pirko <jpirko@redhat.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
drivers/net/enc28j60.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
index 112c5aa..907b05a 100644
--- a/drivers/net/enc28j60.c
+++ b/drivers/net/enc28j60.c
@@ -812,7 +812,7 @@ static void enc28j60_read_tsv(struct enc28j60_net *priv, u8 tsv[TSV_SIZE])
if (netif_msg_hw(priv))
printk(KERN_DEBUG DRV_NAME ": reading TSV at addr:0x%04x\n",
endptr + 1);
- enc28j60_mem_read(priv, endptr + 1, sizeof(tsv), tsv);
+ enc28j60_mem_read(priv, endptr + 1, TSV_SIZE, tsv);
}
static void enc28j60_dump_tsv(struct enc28j60_net *priv, const char *msg,
--
1.7.2.3
^ permalink raw reply related
* Re: [net-2.6 v2 5/7] ixgbe: DDP last buffer size work around
From: David Miller @ 2011-01-28 21:45 UTC (permalink / raw)
To: amir.hanania; +Cc: jeffrey.t.kirsher, netdev, gospo, bphilips
In-Reply-To: <4C5E6457CD7911469A07260381288C28CBE83195@orsmsx502.amr.corp.intel.com>
From: "Hanania, Amir" <amir.hanania@intel.com>
Date: Fri, 28 Jan 2011 13:36:15 -0800
> Sorry I missed your mail from last night.
>
> You are right, it should be bufflen, which is 4096 and not PAGE_SIZE.
>
> I will fix that and resubmit.
Thanks for looking into this.
^ permalink raw reply
* RE: [net-2.6 v2 5/7] ixgbe: DDP last buffer size work around
From: Hanania, Amir @ 2011-01-28 21:36 UTC (permalink / raw)
To: David Miller, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, gospo@redhat.com, bphilips@novell.com
In-Reply-To: <20110128.115100.226772144.davem@davemloft.net>
David,
Sorry I missed your mail from last night.
You are right, it should be bufflen, which is 4096 and not PAGE_SIZE.
I will fix that and resubmit.
-Amir
-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Friday, January 28, 2011 11:51 AM
To: Kirsher, Jeffrey T
Cc: Hanania, Amir; netdev@vger.kernel.org; gospo@redhat.com; bphilips@novell.com
Subject: Re: [net-2.6 v2 5/7] ixgbe: DDP last buffer size work around
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 28 Jan 2011 04:29:01 -0800
> From: Amir Hanania <amir.hanania@intel.com>
>
> We found a hardware erratum on 82599 hardware that can lead to buffer
> overwriting if the last buffer in FCoE DDP is exactly PAGE_SIZE.
> If this is the case, we will make sure that there is no HW access to
> this buffer.
>
> Please see the 82599 Specification Update for more information.
>
> Signed-off-by: Amir Hanania <amir.hanania@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Jeff, I still think this change is bogus.
PAGE_SIZE is variable, so the chip can't possibly only BUG on that
specific value.
Maybe the condition is "any power-of-2 larger than or equal to 4096"?
Or something like that?
I pointed this out last night, and I was really hoping I'd get actual
feedback on this issue before you're respin your tree.
I'm not pulling in these changes until I get a human being telling me
what the situation is here with this bug.
^ permalink raw reply
* Re: [PATCH v2] gianfar: Fall back to software tcp/udp checksum on oldercontrollers
From: David Miller @ 2011-01-28 19:59 UTC (permalink / raw)
To: scottwood; +Cc: David.Laight, netdev, linuxppc-dev
In-Reply-To: <20110128105610.4a518456@udp111988uds.am.freescale.net>
From: Scott Wood <scottwood@freescale.com>
Date: Fri, 28 Jan 2011 10:56:10 -0600
> On Fri, 28 Jan 2011 09:10:46 +0000
> David Laight <David.Laight@ACULAB.COM> wrote:
>
>>
>> > + if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12)
>> > + && ((unsigned long)fcb % 0x20) > 0x18)) {
>>
>> You need to check the generated code, but I think you need:
>>
>> if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12))
>> && unlikely(((unsigned long)fcb % 0x20) > 0x18))
>>
>> ie unlikely() around both the primitive comparisons.
>
> Is the first condition actually unlikely? If you've got affected
> hardware, you'll hit it every time.
>
> If packets with the problematic alignment are rare, seems like it'd be
> better to check that first.
In cases like this gfar_has_errata() case, better to leave it's
likelyhood unmarked.
And yes, since it's cheaper, checking the alignment should be done
first.
^ permalink raw reply
* Re: [net-2.6 v2 5/7] ixgbe: DDP last buffer size work around
From: David Miller @ 2011-01-28 19:51 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: amir.hanania, netdev, gospo, bphilips
In-Reply-To: <1296217743-30093-6-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 28 Jan 2011 04:29:01 -0800
> From: Amir Hanania <amir.hanania@intel.com>
>
> We found a hardware erratum on 82599 hardware that can lead to buffer
> overwriting if the last buffer in FCoE DDP is exactly PAGE_SIZE.
> If this is the case, we will make sure that there is no HW access to
> this buffer.
>
> Please see the 82599 Specification Update for more information.
>
> Signed-off-by: Amir Hanania <amir.hanania@intel.com>
> Tested-by: Ross Brattain <ross.b.brattain@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Jeff, I still think this change is bogus.
PAGE_SIZE is variable, so the chip can't possibly only BUG on that
specific value.
Maybe the condition is "any power-of-2 larger than or equal to 4096"?
Or something like that?
I pointed this out last night, and I was really hoping I'd get actual
feedback on this issue before you're respin your tree.
I'm not pulling in these changes until I get a human being telling me
what the situation is here with this bug.
^ permalink raw reply
* Re: [RFC PATCH] ipsec: fix IPv4 AH alignment on 32 bits
From: David Miller @ 2011-01-28 19:46 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: herbert, netdev, christophe.gouault
In-Reply-To: <4D42839C.3050401@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Fri, 28 Jan 2011 09:51:40 +0100
> On 28/01/2011 05:51, Herbert Xu wrote:
>> So perhaps an SA configuration flag is needed?
> I agree. If David is ok, I will update the patch.
Sounds good to me.
^ permalink raw reply
* fwd: .35 vs .37 suspend/resume corrected defect
From: Joe Perches @ 2011-01-28 18:44 UTC (permalink / raw)
To: Leonardo L. P. da Mata; +Cc: netdev, linux-wireless
Hi Leonardo.
This is very much more appropriate for the mailing lists.
I'm forwarding your mail on to those lists.
-------- Forwarded Message --------
From: Leonardo L. P. da Mata <barroca@gmail.com>
[]
I have a 2.6.35 kernel that is showing some errors like this
(sorry, i don't have the exactly call trace, but is very similar to
this one):
[ 93.344045] Call Trace:
[ 93.344051] [<ffffffff8103aabb>] ? warn_slowpath_common+0x7b/0xc0
[ 93.344054] [<ffffffff8103abb5>] ? warn_slowpath_fmt+0x45/0x50
[ 93.344057] [<ffffffff813af93f>] ? ieee80211_work_work+0x2ef/0x13b0
[ 93.344061] [<ffffffff811d362c>] ? string+0x4c/0xe0
[ 93.344064] [<ffffffff81036c60>] ? default_wake_function+0x0/0x20
[ 93.344067] [<ffffffff813b0a74>] ? ieee80211_work_purge+0x74/0xd0
[ 93.344070] [<ffffffff813b1857>] ? ieee80211_stop+0x57/0x4e0
[ 93.344073] [<ffffffff8133c44a>] ? dev_deactivate+0x1aa/0x1d0
[ 93.344077] [<ffffffff81328551>] ? __dev_close+0x41/0x70
[ 93.344079] [<ffffffff81328592>] ? dev_close+0x12/0x40
[ 93.344082] [<ffffffff81328615>] ? rollback_registered_many+0x55/0x2c0
[ 93.344085] [<ffffffff8132888e>] ? unregister_netdevice_many+0xe/0x70
[ 93.344088] [<ffffffff813b0da4>] ? ieee80211_remove_interfaces+0xa4/0xe0
[ 93.344092] [<ffffffff813a2536>] ? ieee80211_unregister_hw+0x46/0x100
[ 93.344097] [<ffffffffa00dea29>] ? rt2x00lib_remove_dev+0xc9/0xd0 [rt2x00lib]
[ 93.344101] [<ffffffffa0054136>] ? rt2x00usb_disconnect+0x36/0x80 [rt2x00usb]
[ 93.344111] [<ffffffffa00a6254>] ? usb_unbind_interface+0xc4/0x120 [usbcore]
[ 93.344115] [<ffffffff81282090>] ? __device_release_driver+0x60/0xd0
[ 93.344118] [<ffffffff812821d5>] ? device_release_driver+0x25/0x40
[ 93.344126] [<ffffffffa00a64dc>] ? usb_rebind_intf+0x1c/0x60 [usbcore]
[ 93.344133] [<ffffffffa00a65bd>] ? do_unbind_rebind+0x7d/0xb0 [usbcore]
[ 93.344141] [<ffffffffa00a66ce>] ? usb_resume+0xde/0x180 [usbcore]
[ 93.344144] [<ffffffff81286ac2>] ? pm_op+0x1a2/0x1c0
[ 93.344147] [<ffffffff81286cc8>] ? device_resume+0x88/0x130
[ 93.344151] [<ffffffff81059740>] ? async_thread+0x0/0x260
[ 93.344153] [<ffffffff81286d84>] ? async_resume+0x14/0x40
[ 93.344156] [<ffffffff81059845>] ? async_thread+0x105/0x260
[ 93.344159] [<ffffffff81036c60>] ? default_wake_function+0x0/0x20
[ 93.344162] [<ffffffff81059740>] ? async_thread+0x0/0x260
[ 93.344165] [<ffffffff81052afe>] ? kthread+0x8e/0xa0
[ 93.344168] [<ffffffff81003b94>] ? kernel_thread_helper+0x4/0x10
[ 93.344171] [<ffffffff81052a70>] ? kthread+0x0/0xa0
[ 93.344178] [<ffffffff81003b90>] ? kernel_thread_helper+0x0/0x10
[ 93.344179] ---[ end trace 8223f2954477be04 ]---
I have to ask you if your patches are the responsible for solving this
bug on the kernel 2.6.37, because on the 37 the problem doesn't exist.
BTW this is a problem related with suspend and resuming. One thing
that i notice is that disabling the network and wifi modules, that
problem does not work.
Please, let me know if you think that this message is more useful on
the mailing list.
--
Leonardo Luiz Padovani da Mata
barroca@gmail.com
"May the force be with you, always"
"Nerd Pride... eu tenho. Voce tem?"
^ permalink raw reply
* Re: Network performance with small packets
From: Steve Dobbelstein @ 2011-01-28 18:29 UTC (permalink / raw)
To: mashirle; +Cc: kvm, Michael S. Tsirkin, netdev
In-Reply-To: <1296159305.1640.50.camel@localhost.localdomain>
mashirle@linux.vnet.ibm.com wrote on 01/27/2011 02:15:05 PM:
> On Thu, 2011-01-27 at 22:05 +0200, Michael S. Tsirkin wrote:
> > One simple theory is that guest net stack became faster
> > and so the host can't keep up.
>
> Yes, that's what I think here. Some qdisc code has been changed
> recently.
I ran a test with txqueuelen set to 128, instead of the default of 1000, in
the guest in an attempt to slow down the guest transmits. The change had
no effect on the throughput nor on the CPU usage.
On the other hand, I ran some tests with different CPU pinnings and
with/without hyperthreading enabled. Here is a summary of the results.
Pinning configuration 1: pin the VCPUs and pin the vhost thread to one of
the VCPU CPUs
Pinning configuration 2: pin the VCPUs and pin the vhost thread to a
separate CPU on the same socket
Pinning configuration 3: pin the VCPUs and pin the vhost thread to a
separate CPU a different socket
HT Pinning Throughput CPU
Yes config 1 - 40% - 40%
Yes config 2 - 37% - 35%
Yes config 3 - 37% - 36%
No none 0% - 5%
No config 1 - 41% - 43%
No config 2 + 32% - 4%
No config 3 + 34% + 9%
Pinning the vhost thread to the same CPU as a guest VCPU hurts performance.
Turning off hyperthreading and pinning the VPUS and vhost thread to
separate CPUs significantly improves performance, getting it into the
competitive range with other hypervisors.
Steve D.
^ permalink raw reply
* Re: [PATCH] r8169: use RxFIFO overflow workaround for 8168c chipset
From: Francois Romieu @ 2011-01-28 17:28 UTC (permalink / raw)
To: Ivan Vecera; +Cc: netdev, Hayes
In-Reply-To: <20110128172537.GA11064@electric-eye.fr.zoreil.com>
Ivan Vecera <ivecera@redhat.com> :
[...]
> The test case was: Migration of the several kvm guests at the same time
> between two hosts.
:o(
Could you check if simply leaving the irq handler when status == RxFIFOOver
works by any chance ?
Faced with pktgen, RTL_GIGA_MAC_VER_26 spits RxFIFO overflow errors quite
fast (10000 packets kill it, reliably). The same fix as your avoids the crash.
As is, RTL_GIGA_MAC_VER_12 seems to survive at 900kpps. It signals RXFIFO
overflow, loses half the packets and sends pause control frames but it does
not crash. I have made it leave the irq handler when status == RxFIFOOver.
I'll see if it can stand a few hours like that.
--
Ueimor
^ permalink raw reply
* Re: STMMAC driver: NFS Problem on 2.6.37
From: Chuck Lever @ 2011-01-28 16:58 UTC (permalink / raw)
To: Shiraz Hashim
Cc: Deepak SIKRI, Armando VISCONTI, Trond Myklebust,
netdev@vger.kernel.org, Linux NFS Mailing List, Viresh KUMAR,
Peppe CAVALLARO, amitgoel
In-Reply-To: <20110128124331.GD1140@DLHLAP0379>
Hi-
On Jan 28, 2011, at 7:43 AM, Shiraz Hashim wrote:
> Hello Chuck,
>
> On Wed, Jan 26, 2011 at 02:04:21AM +0800, Chuck Lever wrote:
>> See analysis in line.
>>
>> On Jan 25, 2011, at 6:56 AM, deepaksi wrote:
>
> [...]
>
>>> We have made following observations
>>> 1. It seems that the time taken by phy auto negotiation process is
>>> long and as soon as the link gets up rpc ping request is getting
>>> timed out and we receive "Unable to reach ICMP" error. The time
>>> out error is same even if you do not connect a network cable and
>>> do a nfs boot.
>>>
>>> 2. We tried to modify the rate at which the work queue is
>>> scheduled in the phy framework. instead of scheduling every HZ ( 1
>>> sec), we modified it to HZ/10. We did not received
>>> the error. This probably reduced the margin of the phy framework
>>> informing the kernel that the link is up.
>>>
>>> 3. We tried to use another network card and did a nfs boot. The
>>> only relevant difference we could find was the time of auto
>>> negotiation.
>>
>> Can you post a similar debugging dump of a root mount that succeeds
>> using a different network card?
>
> Following is the NFS boot log with a PCIe based e1000e nic card.
>
> ....
> ....
> [ 1.570000] e1000: Intel(R) PRO/1000 Network Driver - version 7.3.21-k8-NAPI
> [ 1.580000] e1000: Copyright (c) 1999-2006 Intel Corporation.
> [ 1.590000] e1000e: Intel(R) PRO/1000 Network Driver - 1.2.7-k2
> [ 1.590000] e1000e: Copyright (c) 1999 - 2010 Intel Corporation.
> [ 1.600000] e1000e 0000:01:00.0: Disabling ASPM L1
> [ 1.600000] PCI: enabling device 0000:01:00.0 (0140 -> 0142)
> [ 1.610000] e1000e 0000:01:00.0: (unregistered net_device): Failed to initialize MSI interrupts. Falling back to legacy interrupts.
> [ 1.850000] e1000e 0000:01:00.0: eth0: (PCI Express:2.5GB/s:Width x1) 00:15:17:ec:02:ff
> [ 1.860000] e1000e 0000:01:00.0: eth0: Intel(R) PRO/1000 Network Connection
> [ 1.870000] e1000e 0000:01:00.0: eth0: MAC: 1, PHY: 4, PBA No: d50861-004
> [ 1.870000] Intel(R) Gigabit Ethernet Network Driver - version 2.1.0-k2
> [ 1.880000] Copyright (c) 2007-2009 Intel Corporation.
> [ 1.880000] Intel(R) Virtual Function Network Driver - version 1.0.0-k0
> [ 1.890000] Copyright (c) 2009 Intel Corporation.
> [ 1.900000] CAN device driver interface
> [ 1.900000] STMMAC driver:
> [ 1.900000] platform registration...
> [ 1.910000] done!
> [ 1.910000] DWMAC1000 - user ID: 0x10, Synopsys ID: 0x35
> [ 1.920000] Enhanced descriptor structure
> [ 1.920000] eth1 - (dev. name: stmmaceth - id: 0, IRQ #65
> [ 1.920000] IO base addr: 0xd00f0000)
> [ 1.940000] STMMAC MII Bus: probed
> [ 1.940000] eth1: PHY ID 20005c7a at 5 IRQ -1 (0:05) active
> [ 1.950000] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> [ 1.950000] spear-ehci spear-ehci.0: SPEAr EHCI
> [ 1.960000] spear-ehci spear-ehci.0: new USB bus registered, assigned bus number 1
> [ 2.020000] spear-ehci spear-ehci.0: irq 96, io mem 0xe4800000
> [ 2.040000] spear-ehci spear-ehci.0: USB 0.0 started, EHCI 1.00
> [ 2.040000] hub 1-0:1.0: USB hub found
> [ 2.050000] hub 1-0:1.0: 1 port detected
> [ 2.050000] spear-ehci spear-ehci.1: SPEAr EHCI
> [ 2.060000] spear-ehci spear-ehci.1: new USB bus registered, assigned bus number 2
> [ 2.120000] spear-ehci spear-ehci.1: irq 98, io mem 0xe5800000
> [ 2.140000] spear-ehci spear-ehci.1: USB 0.0 started, EHCI 1.00
> [ 2.140000] hub 2-0:1.0: USB hub found
> [ 2.150000] hub 2-0:1.0: 1 port detected
> [ 2.150000] ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
> [ 2.160000] spear-ohci spear-ohci.0: SPEAr OHCI
> [ 2.160000] spear-ohci spear-ohci.0: new USB bus registered, assigned bus number 3
> [ 2.200000] spear-ohci spear-ohci.0: irq 97, io mem 0xe4000000
> [ 2.260000] hub 3-0:1.0: USB hub found
> [ 2.270000] hub 3-0:1.0: 1 port detected
> [ 2.270000] spear-ohci spear-ohci.1: SPEAr OHCI
> [ 2.280000] spear-ohci spear-ohci.1: new USB bus registered, assigned bus number 4
> [ 2.310000] spear-ohci spear-ohci.1: irq 99, io mem 0xe5000000
> [ 2.370000] hub 4-0:1.0: USB hub found
> [ 2.380000] hub 4-0:1.0: 1 port detected
> [ 2.380000] Initializing USB Mass Storage driver...
> [ 2.390000] usbcore: registered new interface driver usb-storage
> [ 2.390000] USB Mass Storage support registered.
> [ 2.400000] usbcore: registered new interface driver usbtest
> [ 2.400000] designware_udc designware_udc: Device Synopsys UDC probed csr d00fe000: plug d01c4000
> [ 2.410000] zero gadget: Gadget Zero, version: Cinco de Mayo 2008
> [ 2.420000] zero gadget: zero ready
> [ 2.420000] designware_udc designware_udc: reg gadget driver 'zero'
> [ 2.430000] mice: PS/2 mouse device common for all mice
> [ 2.440000] input: Spear Keyboard as /devices/platform/keyboard/input/input0
> [ 2.470000] usbcore: registered new interface driver usbtouchscreen
> [ 2.470000] input: STMPE610 Touchscreen as /devices/ssp-pl022/spi0.0/input/input1
> [ 2.670000] stmpe610-spi spi0.0: Detected Touch Screen with chip id: ffff and version: ff
> [ 2.690000] rtc-spear rtc-spear: rtc core: registered rtc-spear as rtc0
> [ 2.730000] i2c /dev entries driver
> [ 2.730000] cortexa9-wdt cortexa9-wdt: registration successful
> [ 2.740000] dw_dmac.0: DesignWare DMA Controller, 8 channels
> [ 2.750000] dw_dmac.1: DesignWare DMA Controller, 8 channels
> [ 2.760000] sdhci: Secure Digital Host Controller Interface driver
> [ 2.770000] sdhci: Copyright(c) Pierre Ossman
> [ 2.780000] mmc0: SDHCI controller on sdhci [platform] using DMA
> [ 2.780000] IPv4 over IPv4 tunneling driver
> [ 2.790000] TCP cubic registered
> [ 2.790000] NET: Registered protocol family 17
> [ 2.800000] can: controller area network core (rev 20090105 abi 8)
> [ 2.810000] NET: Registered protocol family 29
> [ 2.810000] can: raw protocol (rev 20090105)
> [ 2.810000] can: broadcast manager protocol (rev 20090105 t)
> [ 2.820000] rtc-spear rtc-spear: hctosys: invalid date/time
> [ 4.520000] IP-Config: Complete:
> [ 4.520000] device=eth0, addr=192.168.1.10, mask=255.255.255.0, gw=192.168.1.1,
> [ 4.530000] host=192.168.1.10, domain=, nis-domain=(none),
> [ 4.530000] bootserver=192.168.1.1, rootserver=192.168.1.1, rootpath=
> [ 4.540000] Root-NFS: nfsroot=/opt/STM/STLinux-2.4/devkit/armv7/target
> [ 4.550000] NFS: nfs mount opts='nolock,addr=192.168.1.1'
> [ 4.550000] NFS: parsing nfs mount option 'nolock'
> [ 4.560000] NFS: parsing nfs mount option 'addr=192.168.1.1'
> [ 4.560000] NFS: MNTPATH: '/opt/STM/STLinux-2.4/devkit/armv7/target'
> [ 4.570000] NFS: sending MNT request for 192.168.1.1:/opt/STM/STLinux-2.4/devkit/armv7/target
> [ 4.580000] Calling rpc_create
> [ 4.580000] RPC: set up xprt to 192.168.1.1 (autobind) via tcp
> [ 4.590000] RPC: created transport cfb13800 with 16 slots
> [ 4.590000] xprt_create_transport: RPC: created transport cfb13800 with 16 slots
> [ 4.600000] RPC: creating mount client for 192.168.1.1 (xprt cfb13800)
> [ 4.610000] RPC: creating UNIX authenticator for client cfaae800
> [ 4.620000] Calling rpc_ping
> [ 4.620000] RPC: new task initialized, procpid 1
> [ 4.620000] RPC: allocated task cfa93100
> [ 4.630000] RPC: 1 __rpc_execute flags=0x680
> [ 4.630000] RPC: 1 call_start mount3 proc NULL (sync)
> [ 4.640000] RPC: 1 call_reserve (status 0)
> [ 4.640000] RPC: 1 reserved req cfb20000 xid a026435b
> [ 4.650000] RPC: 1 call_reserveresult (status 0)
> [ 4.650000] RPC: 1 call_refresh (status 0)
> [ 4.660000] RPC: 1 holding NULL cred c0492798
> [ 4.660000] RPC: 1 refreshing NULL cred c0492798
> [ 4.670000] RPC: 1 call_refreshresult (status 0)
> [ 4.670000] RPC: 1 call_allocate (status 0)
> [ 4.680000] RPC: 1 allocated buffer of size 92 at cfb14000
> [ 4.680000] RPC: 1 call_bind (status 0)
> [ 4.690000] RPC: 1 rpcb_getport_async(192.168.1.1, 100005, 3, 6)
> [ 4.690000] RPC: 1 sleep_on(queue "xprt_binding" time 4294937765)
> [ 4.700000] RPC: 1 added to queue cfb138a4 "xprt_binding"
> [ 4.710000] RPC: 1 setting alarm for 60000 ms
> [ 4.710000] RPC: 1 rpcb_getport_async: trying rpcbind version 2
> [ 4.720000] Calling rpc_create
> [ 4.720000] RPC: set up xprt to 192.168.1.1 (port 111) via tcp
> [ 4.730000] RPC: created transport cfb14800 with 16 slots
> [ 4.730000] xprt_create_transport: RPC: created transport cfb14800 with 16 slots
> [ 4.740000] RPC: creating rpcbind client for 192.168.1.1 (xprt cfb14800)
> [ 4.750000] RPC: creating UNIX authenticator for client cfaaea00
> [ 4.750000] rpc_create returns 0xcfaaea00
> [ 4.760000] RPC: new task initialized, procpid 1
> [ 4.760000] RPC: allocated task cfa93180
> [ 4.770000] RPC: rpc_release_client(cfaaea00)
> [ 4.770000] RPC: 1 sync task going to sleep
> [ 4.780000] RPC: 2 __rpc_execute flags=0x681
> [ 4.780000] RPC: 2 call_start rpcbind2 proc GETPORT (async)
> [ 4.790000] RPC: 2 call_reserve (status 0)
> [ 4.790000] RPC: 2 reserved req cfb21000 xid aa41d674
> [ 4.800000] RPC: 2 call_reserveresult (status 0)
> [ 4.800000] RPC: 2 call_refresh (status 0)
> [ 4.810000] RPC: 2 looking up UNIX cred
> [ 4.810000] RPC: looking up UNIX cred
> [ 4.820000] RPC: allocating UNIX cred for uid 0 gid 0
> [ 4.820000] RPC: 2 refreshing UNIX cred cfa93200
> [ 4.830000] RPC: 2 call_refreshresult (status 0)
> [ 4.830000] RPC: 2 call_allocate (status 0)
> [ 4.840000] RPC: 2 allocated buffer of size 412 at cfb15000
> [ 4.840000] RPC: 2 call_bind (status 0)
> [ 4.850000] RPC: 2 call_connect xprt cfb14800 is not connected
> [ 4.850000] RPC: 2 xprt_connect xprt cfb14800 is not connected
> [ 4.860000] RPC: 2 sleep_on(queue "xprt_pending" time 4294937782)
> [ 4.860000] RPC: 2 added to queue cfb149dc "xprt_pending"
> [ 4.870000] RPC: 2 setting alarm for 60000 ms
>
> [ 4.880000] RPC: xs_connect scheduled xprt cfb14800
> [ 4.880000] RPC: xs_bind 0.0.0.0:0: ok (0)
> [ 4.890000] RPC: worker connecting xprt cfb14800 via tcp to 192.168.1.1 (port 111)
> [ 4.890000] RPC: cfb14800 connect status 115 connected 0 sock state 2
>
> [ 5.870000] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
>
> [ 5.900000] RPC: xs_tcp_state_change client cfb14800...
> [ 5.900000] RPC: state 1 conn 0 dead 0 zapped 1 sk_shutdown 0
In the earlier trace, the transport socket connection attempt returns EHOSTUNREACH when a socket connect operation is attempted before the interface is up. In this trace, the network layer allows the transport socket to connect straight away; there's no race with interface initialization. So your problem is not likely to be in the RPC client, but in the network layer or below (hardware driver, etc).
> [ 5.910000] RPC: 2 __rpc_wake_up_task (now 4294937887)
> [ 5.910000] RPC: 2 disabling timer
> [ 5.920000] RPC: 2 removed from queue cfb149dc "xprt_pending"
> [ 5.920000] RPC: __rpc_wake_up_task done
> [ 5.930000] RPC: 2 __rpc_execute flags=0x681
> [ 5.930000] RPC: 2 xprt_connect_status: retrying
> [ 5.940000] RPC: 2 call_connect_status (status -11)
> [ 5.940000] RPC: 2 call_transmit (status 0)
> [ 5.950000] RPC: 2 xprt_prepare_transmit
> [ 5.950000] RPC: 2 rpc_xdr_encode (status 0)
> [ 5.960000] RPC: 2 marshaling UNIX cred cfa93200
> [ 5.960000] RPC: 2 using AUTH_UNIX cred cfa93200 to wrap rpc data
> [ 5.970000] RPC: 2 encoding PMAP_GETPORT call (100005, 3, 6, 0)
> [ 5.980000] RPC: 2 xprt_transmit(92)
> [ 5.980000] RPC: xs_tcp_send_request(92) = 92
> [ 5.980000] RPC: xs_tcp_data_ready...
> [ 5.990000] RPC: xs_tcp_data_recv started
> [ 5.990000] RPC: reading TCP record fragment of length 28
> [ 6.000000] RPC: reading XID (4 bytes)
> [ 6.000000] RPC: reading request with XID aa41d674
> [ 6.010000] RPC: reading CALL/REPLY flag (4 bytes)
> [ 6.010000] RPC: read reply XID aa41d674
> [ 6.020000] RPC: XID aa41d674 read 20 bytes
> [ 6.020000] RPC: xprt = cfb14800, tcp_copied = 28, tcp_offset = 28, tcp_reclen = 28
> [ 6.030000] RPC: 2 xid aa41d674 complete (28 bytes received)
> [ 6.040000] RPC: xs_tcp_data_recv done
> [ 6.040000] RPC: 2 xmit complete
> [ 6.050000] RPC: wake_up_next(cfb14974 "xprt_resend")
> [ 6.050000] RPC: wake_up_next(cfb1490c "xprt_sending")
> [ 6.060000] RPC: 2 call_status (status 28)
> [ 6.060000] RPC: 2 call_decode (status 28)
> [ 6.070000] RPC: 2 validating UNIX cred cfa93200
> [ 6.070000] RPC: 2 using AUTH_UNIX cred cfa93200 to unwrap rpc data
> [ 6.080000] RPC: 2 PMAP_GETPORT result: 48734
> [ 6.080000] RPC: 2 call_decode result 0
> [ 6.090000] RPC: setting port for xprt cfb13800 to 48734
> [ 6.090000] RPC: 2 rpcb_getport_done(status 0, port 48734)
> [ 6.100000] RPC: 2 return 0, status 0
> [ 6.100000] RPC: 2 release task
> [ 6.110000] RPC: freeing buffer of size 412 at cfb15000
> [ 6.110000] RPC: 2 release request cfb21000
> [ 6.120000] RPC: wake_up_next(cfb14a44 "xprt_backlog")
> [ 6.120000] RPC: rpc_release_client(cfaaea00)
> [ 6.130000] RPC: destroying rpcbind client for 192.168.1.1
> [ 6.130000] RPC: destroying transport cfb14800
> [ 6.140000] RPC: xs_destroy xprt cfb14800
> [ 6.140000] RPC: xs_close xprt cfb14800
> [ 6.150000] RPC: disconnected transport cfb14800
> [ 6.150000] RPC: 2 freeing task
> [ 6.160000] RPC: 1 __rpc_wake_up_task (now 4294937912)
> [ 6.160000] RPC: 1 disabling timer
> [ 6.160000] RPC: 1 removed from queue cfb138a4 "xprt_binding"
> [ 6.170000] RPC: __rpc_wake_up_task done
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply
* Re: [PATCH v2] gianfar: Fall back to software tcp/udp checksum on oldercontrollers
From: Scott Wood @ 2011-01-28 16:56 UTC (permalink / raw)
To: David Laight; +Cc: netdev, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AC2E@saturn3.aculab.com>
On Fri, 28 Jan 2011 09:10:46 +0000
David Laight <David.Laight@ACULAB.COM> wrote:
>
> > + if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12)
> > + && ((unsigned long)fcb % 0x20) > 0x18)) {
>
> You need to check the generated code, but I think you need:
>
> if (unlikely(gfar_has_errata(priv, GFAR_ERRATA_12))
> && unlikely(((unsigned long)fcb % 0x20) > 0x18))
>
> ie unlikely() around both the primitive comparisons.
Is the first condition actually unlikely? If you've got affected
hardware, you'll hit it every time.
If packets with the problematic alignment are rare, seems like it'd be
better to check that first.
-Scott
^ permalink raw reply
* Re: [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Paul Moore @ 2011-01-28 15:46 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
In-Reply-To: <1296224232-8115-2-git-send-email-julia@diku.dk>
On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> genlmsg_cancel subtracts some constants from its second argument before
> calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
> arguments. nlmsg_trim tests for NULL before doing any computation, but a
> NULL second argument to genlmsg_cancel is no longer NULL due to the initial
> subtraction. Nothing else happens in this execution, so the call to
> genlmsg_cancel is simply unnecessary in this case.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression data;
> @@
>
> if (data == NULL) { ...
> * genlmsg_cancel(..., data);
> ...
> return ...;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
Acked-by: Paul Moore <paul.moore@hp.com>
> ---
> net/netlabel/netlabel_cipso_v4.c | 2 +-
> net/netlabel/netlabel_mgmt.c | 4 ++--
> net/netlabel/netlabel_unlabeled.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
> index 5f14c84..0a1f77b 100644
> --- a/net/netlabel/netlabel_cipso_v4.c
> +++ b/net/netlabel/netlabel_cipso_v4.c
> @@ -635,7 +635,7 @@ static int netlbl_cipsov4_listall_cb(struct cipso_v4_doi *doi_def, void *arg)
> cb_arg->seq, &netlbl_cipsov4_gnl_family,
> NLM_F_MULTI, NLBL_CIPSOV4_C_LISTALL);
> if (data == NULL)
> - goto listall_cb_failure;
> + return ret_val;
>
> ret_val = nla_put_u32(cb_arg->skb, NLBL_CIPSOV4_A_DOI, doi_def->doi);
> if (ret_val != 0)
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index 998e85e..daaa01d 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -452,7 +452,7 @@ static int netlbl_mgmt_listall_cb(struct netlbl_dom_map *entry, void *arg)
> cb_arg->seq, &netlbl_mgmt_gnl_family,
> NLM_F_MULTI, NLBL_MGMT_C_LISTALL);
> if (data == NULL)
> - goto listall_cb_failure;
> + return ret_val;
>
> ret_val = netlbl_mgmt_listentry(cb_arg->skb, entry);
> if (ret_val != 0)
> @@ -617,7 +617,7 @@ static int netlbl_mgmt_protocols_cb(struct sk_buff *skb,
> &netlbl_mgmt_gnl_family, NLM_F_MULTI,
> NLBL_MGMT_C_PROTOCOLS);
> if (data == NULL)
> - goto protocols_cb_failure;
> + return ret_val;
>
> ret_val = nla_put_u32(skb, NLBL_MGMT_A_PROTOCOL, protocol);
> if (ret_val != 0)
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index e2b0a68..b5d3945 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1141,7 +1141,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> cb_arg->seq, &netlbl_unlabel_gnl_family,
> NLM_F_MULTI, cmd);
> if (data == NULL)
> - goto list_cb_failure;
> + return ret_val;
>
> if (iface->ifindex > 0) {
> dev = dev_get_by_index(&init_net, iface->ifindex);
>
--
paul moore
linux @ hp
^ permalink raw reply
* [PATCH] include/net/genetlink.h: Allow genlmsg_cancel to accept a NULL argument
From: Julia Lawall @ 2011-01-28 15:43 UTC (permalink / raw)
To: davem, netdev, linux-kernel, paul.moore, kernel-janitors
nlmsg_cancel can accept NULL as its second argument, so for similarity,
this patch extends genlmsg_cancel to be able to accept a NULL second
argument as well.
Signed-off-by: Julia Lawall <julia@diku.dk>
---
include/net/genetlink.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index 8a64b81..b4c7c1c 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -195,7 +195,8 @@ static inline int genlmsg_end(struct sk_buff *skb, void *hdr)
*/
static inline void genlmsg_cancel(struct sk_buff *skb, void *hdr)
{
- nlmsg_cancel(skb, hdr - GENL_HDRLEN - NLMSG_HDRLEN);
+ if (hdr)
+ nlmsg_cancel(skb, hdr - GENL_HDRLEN - NLMSG_HDRLEN);
}
/**
^ permalink raw reply related
* Re: [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Julia Lawall @ 2011-01-28 15:29 UTC (permalink / raw)
To: Paul Moore; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
In-Reply-To: <1296228218.5511.13.camel@sifl>
On Fri, 28 Jan 2011, Paul Moore wrote:
> On Fri, 2011-01-28 at 15:58 +0100, Julia Lawall wrote:
> > On Fri, 28 Jan 2011, Paul Moore wrote:
> >
> > > On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> > > > genlmsg_cancel subtracts some constants from its second argument before
> > > > calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
> > > > arguments. nlmsg_trim tests for NULL before doing any computation, but a
> > > > NULL second argument to genlmsg_cancel is no longer NULL due to the initial
> > > > subtraction. Nothing else happens in this execution, so the call to
> > > > genlmsg_cancel is simply unnecessary in this case.
> > > >
> > > > The semantic match that finds this problem is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > > >
> > > > // <smpl>
> > > > @@
> > > > expression data;
> > > > @@
> > > >
> > > > if (data == NULL) { ...
> > > > * genlmsg_cancel(..., data);
> > > > ...
> > > > return ...;
> > > > }
> > > > // </smpl>
> > > >
> > > > Signed-off-by: Julia Lawall <julia@diku.dk>
> > >
> > > In all of the cases below, these functions are called multiple times to
> > > generate data chunks (additional netlink attributes) which are appended
> > > to an existing skbuff. I believe that the calls to genlmsg_cancel() are
> > > still needed to help cleanup in the case where the functions fail on the
> > > Nth call.
> > >
> > > If I'm wrong, feel free to enlighten me.
> >
> > Perhaps something is needed, but I don't see how the current code can
> > work. The call is genlmsg_cancel(cb_arg->skb, NULL) in each case.
>
> Ah yes, you're right. You will have to forgive me as it has been quite
> a while since I have looked at NetLabel's netlink code.
>
> You also might consider putting a NULL check in genlmsg_cancel() similar
> to the check nlmsg_trim(); that seems like a worthwhile addition.
OK, I'll do that.
julia
^ permalink raw reply
* Re: [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Paul Moore @ 2011-01-28 15:23 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64.1101281541030.8546@pc-004.diku.dk>
On Fri, 2011-01-28 at 15:58 +0100, Julia Lawall wrote:
> On Fri, 28 Jan 2011, Paul Moore wrote:
>
> > On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> > > genlmsg_cancel subtracts some constants from its second argument before
> > > calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
> > > arguments. nlmsg_trim tests for NULL before doing any computation, but a
> > > NULL second argument to genlmsg_cancel is no longer NULL due to the initial
> > > subtraction. Nothing else happens in this execution, so the call to
> > > genlmsg_cancel is simply unnecessary in this case.
> > >
> > > The semantic match that finds this problem is as follows:
> > > (http://coccinelle.lip6.fr/)
> > >
> > > // <smpl>
> > > @@
> > > expression data;
> > > @@
> > >
> > > if (data == NULL) { ...
> > > * genlmsg_cancel(..., data);
> > > ...
> > > return ...;
> > > }
> > > // </smpl>
> > >
> > > Signed-off-by: Julia Lawall <julia@diku.dk>
> >
> > In all of the cases below, these functions are called multiple times to
> > generate data chunks (additional netlink attributes) which are appended
> > to an existing skbuff. I believe that the calls to genlmsg_cancel() are
> > still needed to help cleanup in the case where the functions fail on the
> > Nth call.
> >
> > If I'm wrong, feel free to enlighten me.
>
> Perhaps something is needed, but I don't see how the current code can
> work. The call is genlmsg_cancel(cb_arg->skb, NULL) in each case.
Ah yes, you're right. You will have to forgive me as it has been quite
a while since I have looked at NetLabel's netlink code.
You also might consider putting a NULL check in genlmsg_cancel() similar
to the check nlmsg_trim(); that seems like a worthwhile addition.
> The definition of genlmsg_cancel is:
>
> static inline void genlmsg_cancel(struct sk_buff *skb, void *hdr)
> {
> nlmsg_cancel(skb, hdr - GENL_HDRLEN - NLMSG_HDRLEN);
> }
>
> Now the second argument to nlmsg_cancel is essentially a negative integer
> (or a very large pointer).
>
> nlmsg_cancel will call nlmsg_trim, which is defined as follows:
>
> static inline void nlmsg_trim(struct sk_buff *skb, const void *mark)
> {
> if (mark)
> skb_trim(skb, (unsigned char *) mark - skb->data);
> }
>
> I guess that the subtraction is going to result in an even larger negative
> number. The whole process is likely to end in doing nothing in the
> definition of skb_trim, which is as follows:
>
> void skb_trim(struct sk_buff *skb, unsigned int len)
> {
> if (skb->len > len)
> __skb_trim(skb, len);
> }
>
> since the result of casting a negative number to unsigned is likely to be
> larger than skb->len.
>
>
> > > ---
> > > net/netlabel/netlabel_cipso_v4.c | 2 +-
> > > net/netlabel/netlabel_mgmt.c | 4 ++--
> > > net/netlabel/netlabel_unlabeled.c | 2 +-
> > > 3 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
> > > index 5f14c84..0a1f77b 100644
> > > --- a/net/netlabel/netlabel_cipso_v4.c
> > > +++ b/net/netlabel/netlabel_cipso_v4.c
> > > @@ -635,7 +635,7 @@ static int netlbl_cipsov4_listall_cb(struct cipso_v4_doi *doi_def, void *arg)
> > > cb_arg->seq, &netlbl_cipsov4_gnl_family,
> > > NLM_F_MULTI, NLBL_CIPSOV4_C_LISTALL);
> > > if (data == NULL)
> > > - goto listall_cb_failure;
> > > + return ret_val;
> > >
> > > ret_val = nla_put_u32(cb_arg->skb, NLBL_CIPSOV4_A_DOI, doi_def->doi);
> > > if (ret_val != 0)
> > > diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> > > index 998e85e..daaa01d 100644
> > > --- a/net/netlabel/netlabel_mgmt.c
> > > +++ b/net/netlabel/netlabel_mgmt.c
> > > @@ -452,7 +452,7 @@ static int netlbl_mgmt_listall_cb(struct netlbl_dom_map *entry, void *arg)
> > > cb_arg->seq, &netlbl_mgmt_gnl_family,
> > > NLM_F_MULTI, NLBL_MGMT_C_LISTALL);
> > > if (data == NULL)
> > > - goto listall_cb_failure;
> > > + return ret_val;
> > >
> > > ret_val = netlbl_mgmt_listentry(cb_arg->skb, entry);
> > > if (ret_val != 0)
> > > @@ -617,7 +617,7 @@ static int netlbl_mgmt_protocols_cb(struct sk_buff *skb,
> > > &netlbl_mgmt_gnl_family, NLM_F_MULTI,
> > > NLBL_MGMT_C_PROTOCOLS);
> > > if (data == NULL)
> > > - goto protocols_cb_failure;
> > > + return ret_val;
> > >
> > > ret_val = nla_put_u32(skb, NLBL_MGMT_A_PROTOCOL, protocol);
> > > if (ret_val != 0)
> > > diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> > > index e2b0a68..b5d3945 100644
> > > --- a/net/netlabel/netlabel_unlabeled.c
> > > +++ b/net/netlabel/netlabel_unlabeled.c
> > > @@ -1141,7 +1141,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> > > cb_arg->seq, &netlbl_unlabel_gnl_family,
> > > NLM_F_MULTI, cmd);
> > > if (data == NULL)
> > > - goto list_cb_failure;
> > > + return ret_val;
> > >
> > > if (iface->ifindex > 0) {
> > > dev = dev_get_by_index(&init_net, iface->ifindex);
> > >
> >
> > --
> > paul moore
> > linux @ hp
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Julia Lawall @ 2011-01-28 14:58 UTC (permalink / raw)
To: Paul Moore; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
In-Reply-To: <1296225364.5511.6.camel@sifl>
On Fri, 28 Jan 2011, Paul Moore wrote:
> On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> > genlmsg_cancel subtracts some constants from its second argument before
> > calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
> > arguments. nlmsg_trim tests for NULL before doing any computation, but a
> > NULL second argument to genlmsg_cancel is no longer NULL due to the initial
> > subtraction. Nothing else happens in this execution, so the call to
> > genlmsg_cancel is simply unnecessary in this case.
> >
> > The semantic match that finds this problem is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression data;
> > @@
> >
> > if (data == NULL) { ...
> > * genlmsg_cancel(..., data);
> > ...
> > return ...;
> > }
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <julia@diku.dk>
>
> In all of the cases below, these functions are called multiple times to
> generate data chunks (additional netlink attributes) which are appended
> to an existing skbuff. I believe that the calls to genlmsg_cancel() are
> still needed to help cleanup in the case where the functions fail on the
> Nth call.
>
> If I'm wrong, feel free to enlighten me.
Perhaps something is needed, but I don't see how the current code can
work. The call is genlmsg_cancel(cb_arg->skb, NULL) in each case.
The definition of genlmsg_cancel is:
static inline void genlmsg_cancel(struct sk_buff *skb, void *hdr)
{
nlmsg_cancel(skb, hdr - GENL_HDRLEN - NLMSG_HDRLEN);
}
Now the second argument to nlmsg_cancel is essentially a negative integer
(or a very large pointer).
nlmsg_cancel will call nlmsg_trim, which is defined as follows:
static inline void nlmsg_trim(struct sk_buff *skb, const void *mark)
{
if (mark)
skb_trim(skb, (unsigned char *) mark - skb->data);
}
I guess that the subtraction is going to result in an even larger negative
number. The whole process is likely to end in doing nothing in the
definition of skb_trim, which is as follows:
void skb_trim(struct sk_buff *skb, unsigned int len)
{
if (skb->len > len)
__skb_trim(skb, len);
}
since the result of casting a negative number to unsigned is likely to be
larger than skb->len.
> > ---
> > net/netlabel/netlabel_cipso_v4.c | 2 +-
> > net/netlabel/netlabel_mgmt.c | 4 ++--
> > net/netlabel/netlabel_unlabeled.c | 2 +-
> > 3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
> > index 5f14c84..0a1f77b 100644
> > --- a/net/netlabel/netlabel_cipso_v4.c
> > +++ b/net/netlabel/netlabel_cipso_v4.c
> > @@ -635,7 +635,7 @@ static int netlbl_cipsov4_listall_cb(struct cipso_v4_doi *doi_def, void *arg)
> > cb_arg->seq, &netlbl_cipsov4_gnl_family,
> > NLM_F_MULTI, NLBL_CIPSOV4_C_LISTALL);
> > if (data == NULL)
> > - goto listall_cb_failure;
> > + return ret_val;
> >
> > ret_val = nla_put_u32(cb_arg->skb, NLBL_CIPSOV4_A_DOI, doi_def->doi);
> > if (ret_val != 0)
> > diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> > index 998e85e..daaa01d 100644
> > --- a/net/netlabel/netlabel_mgmt.c
> > +++ b/net/netlabel/netlabel_mgmt.c
> > @@ -452,7 +452,7 @@ static int netlbl_mgmt_listall_cb(struct netlbl_dom_map *entry, void *arg)
> > cb_arg->seq, &netlbl_mgmt_gnl_family,
> > NLM_F_MULTI, NLBL_MGMT_C_LISTALL);
> > if (data == NULL)
> > - goto listall_cb_failure;
> > + return ret_val;
> >
> > ret_val = netlbl_mgmt_listentry(cb_arg->skb, entry);
> > if (ret_val != 0)
> > @@ -617,7 +617,7 @@ static int netlbl_mgmt_protocols_cb(struct sk_buff *skb,
> > &netlbl_mgmt_gnl_family, NLM_F_MULTI,
> > NLBL_MGMT_C_PROTOCOLS);
> > if (data == NULL)
> > - goto protocols_cb_failure;
> > + return ret_val;
> >
> > ret_val = nla_put_u32(skb, NLBL_MGMT_A_PROTOCOL, protocol);
> > if (ret_val != 0)
> > diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> > index e2b0a68..b5d3945 100644
> > --- a/net/netlabel/netlabel_unlabeled.c
> > +++ b/net/netlabel/netlabel_unlabeled.c
> > @@ -1141,7 +1141,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> > cb_arg->seq, &netlbl_unlabel_gnl_family,
> > NLM_F_MULTI, cmd);
> > if (data == NULL)
> > - goto list_cb_failure;
> > + return ret_val;
> >
> > if (iface->ifindex > 0) {
> > dev = dev_get_by_index(&init_net, iface->ifindex);
> >
>
> --
> paul moore
> linux @ hp
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Paul Moore @ 2011-01-28 14:36 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
In-Reply-To: <1296224232-8115-2-git-send-email-julia@diku.dk>
On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> genlmsg_cancel subtracts some constants from its second argument before
> calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
> arguments. nlmsg_trim tests for NULL before doing any computation, but a
> NULL second argument to genlmsg_cancel is no longer NULL due to the initial
> subtraction. Nothing else happens in this execution, so the call to
> genlmsg_cancel is simply unnecessary in this case.
>
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression data;
> @@
>
> if (data == NULL) { ...
> * genlmsg_cancel(..., data);
> ...
> return ...;
> }
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
In all of the cases below, these functions are called multiple times to
generate data chunks (additional netlink attributes) which are appended
to an existing skbuff. I believe that the calls to genlmsg_cancel() are
still needed to help cleanup in the case where the functions fail on the
Nth call.
If I'm wrong, feel free to enlighten me.
> ---
> net/netlabel/netlabel_cipso_v4.c | 2 +-
> net/netlabel/netlabel_mgmt.c | 4 ++--
> net/netlabel/netlabel_unlabeled.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
> index 5f14c84..0a1f77b 100644
> --- a/net/netlabel/netlabel_cipso_v4.c
> +++ b/net/netlabel/netlabel_cipso_v4.c
> @@ -635,7 +635,7 @@ static int netlbl_cipsov4_listall_cb(struct cipso_v4_doi *doi_def, void *arg)
> cb_arg->seq, &netlbl_cipsov4_gnl_family,
> NLM_F_MULTI, NLBL_CIPSOV4_C_LISTALL);
> if (data == NULL)
> - goto listall_cb_failure;
> + return ret_val;
>
> ret_val = nla_put_u32(cb_arg->skb, NLBL_CIPSOV4_A_DOI, doi_def->doi);
> if (ret_val != 0)
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index 998e85e..daaa01d 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -452,7 +452,7 @@ static int netlbl_mgmt_listall_cb(struct netlbl_dom_map *entry, void *arg)
> cb_arg->seq, &netlbl_mgmt_gnl_family,
> NLM_F_MULTI, NLBL_MGMT_C_LISTALL);
> if (data == NULL)
> - goto listall_cb_failure;
> + return ret_val;
>
> ret_val = netlbl_mgmt_listentry(cb_arg->skb, entry);
> if (ret_val != 0)
> @@ -617,7 +617,7 @@ static int netlbl_mgmt_protocols_cb(struct sk_buff *skb,
> &netlbl_mgmt_gnl_family, NLM_F_MULTI,
> NLBL_MGMT_C_PROTOCOLS);
> if (data == NULL)
> - goto protocols_cb_failure;
> + return ret_val;
>
> ret_val = nla_put_u32(skb, NLBL_MGMT_A_PROTOCOL, protocol);
> if (ret_val != 0)
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index e2b0a68..b5d3945 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1141,7 +1141,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> cb_arg->seq, &netlbl_unlabel_gnl_family,
> NLM_F_MULTI, cmd);
> if (data == NULL)
> - goto list_cb_failure;
> + return ret_val;
>
> if (iface->ifindex > 0) {
> dev = dev_get_by_index(&init_net, iface->ifindex);
>
--
paul moore
linux @ hp
^ permalink raw reply
* Re: [PATCH 1/2] net/wireless/nl80211.c: Avoid call to genlmsg_cancel
From: Johannes Berg @ 2011-01-28 14:20 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA, John W. Linville,
David S. Miller, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <Pine.LNX.4.64.1101281515410.8546-h8frIrHk2441Y/6SN6b7/w@public.gmane.org>
On Fri, 2011-01-28 at 15:16 +0100, Julia Lawall wrote:
> > But why did you call the label differently? :)
>
> Because out is already used in this case, and I didn't want to change all
> of the other occurrences of nla_put_failure. It's a bit sloppy though,
> because this code is the actual nla_put_failure. I can change it if you
> prefer.
Oh, and I could've seen that from the patch itself too, I just missed
it, sorry.
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] net/netlabel: Avoid call to genlmsg_cancel
From: Julia Lawall @ 2011-01-28 14:17 UTC (permalink / raw)
To: Paul Moore; +Cc: kernel-janitors, David S. Miller, netdev, linux-kernel
genlmsg_cancel subtracts some constants from its second argument before
calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
arguments. nlmsg_trim tests for NULL before doing any computation, but a
NULL second argument to genlmsg_cancel is no longer NULL due to the initial
subtraction. Nothing else happens in this execution, so the call to
genlmsg_cancel is simply unnecessary in this case.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression data;
@@
if (data == NULL) { ...
* genlmsg_cancel(..., data);
...
return ...;
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
net/netlabel/netlabel_cipso_v4.c | 2 +-
net/netlabel/netlabel_mgmt.c | 4 ++--
net/netlabel/netlabel_unlabeled.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
index 5f14c84..0a1f77b 100644
--- a/net/netlabel/netlabel_cipso_v4.c
+++ b/net/netlabel/netlabel_cipso_v4.c
@@ -635,7 +635,7 @@ static int netlbl_cipsov4_listall_cb(struct cipso_v4_doi *doi_def, void *arg)
cb_arg->seq, &netlbl_cipsov4_gnl_family,
NLM_F_MULTI, NLBL_CIPSOV4_C_LISTALL);
if (data == NULL)
- goto listall_cb_failure;
+ return ret_val;
ret_val = nla_put_u32(cb_arg->skb, NLBL_CIPSOV4_A_DOI, doi_def->doi);
if (ret_val != 0)
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index 998e85e..daaa01d 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -452,7 +452,7 @@ static int netlbl_mgmt_listall_cb(struct netlbl_dom_map *entry, void *arg)
cb_arg->seq, &netlbl_mgmt_gnl_family,
NLM_F_MULTI, NLBL_MGMT_C_LISTALL);
if (data == NULL)
- goto listall_cb_failure;
+ return ret_val;
ret_val = netlbl_mgmt_listentry(cb_arg->skb, entry);
if (ret_val != 0)
@@ -617,7 +617,7 @@ static int netlbl_mgmt_protocols_cb(struct sk_buff *skb,
&netlbl_mgmt_gnl_family, NLM_F_MULTI,
NLBL_MGMT_C_PROTOCOLS);
if (data == NULL)
- goto protocols_cb_failure;
+ return ret_val;
ret_val = nla_put_u32(skb, NLBL_MGMT_A_PROTOCOL, protocol);
if (ret_val != 0)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index e2b0a68..b5d3945 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1141,7 +1141,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
cb_arg->seq, &netlbl_unlabel_gnl_family,
NLM_F_MULTI, cmd);
if (data == NULL)
- goto list_cb_failure;
+ return ret_val;
if (iface->ifindex > 0) {
dev = dev_get_by_index(&init_net, iface->ifindex);
^ permalink raw reply related
* [PATCH 1/2] net/wireless/nl80211.c: Avoid call to genlmsg_cancel
From: Julia Lawall @ 2011-01-28 14:17 UTC (permalink / raw)
To: Johannes Berg
Cc: kernel-janitors, John W. Linville, David S. Miller,
linux-wireless, netdev, linux-kernel
genlmsg_cancel subtracts some constants from its second argument before
calling nlmsg_cancel. nlmsg_cancel then calls nlmsg_trim on the same
arguments. nlmsg_trim tests for NULL before doing any computation, but a
NULL second argument to genlmsg_cancel is no longer NULL due to the initial
subtraction. Nothing else happens in this execution, so the call to
genlmsg_cancel is simply unnecessary in this case.
The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression data;
@@
if (data == NULL) { ...
* genlmsg_cancel(..., data);
...
return ...;
}
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
net/wireless/nl80211.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9b62710..864ddfb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2718,7 +2718,7 @@ static int nl80211_get_mesh_config(struct sk_buff *skb,
hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
NL80211_CMD_GET_MESH_CONFIG);
if (!hdr)
- goto nla_put_failure;
+ goto out;
pinfoattr = nla_nest_start(msg, NL80211_ATTR_MESH_CONFIG);
if (!pinfoattr)
goto nla_put_failure;
@@ -2759,6 +2759,7 @@ static int nl80211_get_mesh_config(struct sk_buff *skb,
nla_put_failure:
genlmsg_cancel(msg, hdr);
+ out:
nlmsg_free(msg);
return -ENOBUFS;
}
@@ -2954,7 +2955,7 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
NL80211_CMD_GET_REG);
if (!hdr)
- goto nla_put_failure;
+ goto put_failure;
NLA_PUT_STRING(msg, NL80211_ATTR_REG_ALPHA2,
cfg80211_regdomain->alpha2);
@@ -3001,6 +3002,7 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
nla_put_failure:
genlmsg_cancel(msg, hdr);
+put_failure:
nlmsg_free(msg);
err = -EMSGSIZE;
out:
^ permalink raw reply related
* Re: [PATCH 1/2] net/wireless/nl80211.c: Avoid call to genlmsg_cancel
From: Julia Lawall @ 2011-01-28 14:16 UTC (permalink / raw)
To: Johannes Berg
Cc: kernel-janitors, John W. Linville, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <1296223267.5118.7.camel@jlt3.sipsolutions.net>
On Fri, 28 Jan 2011, Johannes Berg wrote:
> On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
>
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 9b62710..864ddfb 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -2718,7 +2718,7 @@ static int nl80211_get_mesh_config(struct sk_buff *skb,
> > hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
> > NL80211_CMD_GET_MESH_CONFIG);
> > if (!hdr)
> > - goto nla_put_failure;
> > + goto out;
>
>
> > @@ -2954,7 +2955,7 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
> > hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
> > NL80211_CMD_GET_REG);
> > if (!hdr)
> > - goto nla_put_failure;
> > + goto put_failure;
> >
> > NLA_PUT_STRING(msg, NL80211_ATTR_REG_ALPHA2,
> > cfg80211_regdomain->alpha2);
>
> Seems fine. Actually, since the message is freed anyhow, the call to
> genlmsg_cancel is *completely* unnecessary, I just put it in to make it
> nest better and not rely on it not having side effects.
>
> But why did you call the label differently? :)
Because out is already used in this case, and I didn't want to change all
of the other occurrences of nla_put_failure. It's a bit sloppy though,
because this code is the actual nla_put_failure. I can change it if you
prefer.
julia
^ permalink raw reply
* Re: [PATCH 1/2] net/wireless/nl80211.c: Avoid call to genlmsg_cancel
From: Johannes Berg @ 2011-01-28 14:01 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, John W. Linville, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <1296224232-8115-1-git-send-email-julia@diku.dk>
On Fri, 2011-01-28 at 15:17 +0100, Julia Lawall wrote:
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 9b62710..864ddfb 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2718,7 +2718,7 @@ static int nl80211_get_mesh_config(struct sk_buff *skb,
> hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
> NL80211_CMD_GET_MESH_CONFIG);
> if (!hdr)
> - goto nla_put_failure;
> + goto out;
> @@ -2954,7 +2955,7 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
> hdr = nl80211hdr_put(msg, info->snd_pid, info->snd_seq, 0,
> NL80211_CMD_GET_REG);
> if (!hdr)
> - goto nla_put_failure;
> + goto put_failure;
>
> NLA_PUT_STRING(msg, NL80211_ATTR_REG_ALPHA2,
> cfg80211_regdomain->alpha2);
Seems fine. Actually, since the message is freed anyhow, the call to
genlmsg_cancel is *completely* unnecessary, I just put it in to make it
nest better and not rely on it not having side effects.
But why did you call the label differently? :)
johannes
^ permalink raw reply
* Re: [PATCH V10 01/15] time: Introduce timekeeping_inject_offset
From: Arnd Bergmann @ 2011-01-28 12:54 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Richard Cochran, John Stultz, Richard Cochran,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
Alan Cox, Christoph Lameter, David Miller, Krzysztof Halasa,
Rodolfo Giometti, Thomas Gleixner, Benjamin Herrenschmidt,
H. Peter Anvin, Ingo Molnar, Mike Frysinger, Paul Mackerras,
Russell King
In-Reply-To: <1296218630.15234.334.camel@laptop>
On Friday 28 January 2011, Peter Zijlstra wrote:
> >
> > The problem is step 6. The output of git format-patch does not work when
> > sending with mutt. The easiest solution is to send with git send-email,
> > which does the same as mutt -H, but gets it right.
>
> I use: formail -s sendmail -t < patches.mbox, but then, I use quilt mail
> to generate the mbox, not git.
I think in that case, quilt generates the correct 'From' headers both in
the actual email headers and in the body, so you can use any standard
email client to send it out.
Arnd
^ permalink raw reply
* Re: [PATCH V10 01/15] time: Introduce timekeeping_inject_offset
From: Peter Zijlstra @ 2011-01-28 12:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Richard Cochran, John Stultz, Richard Cochran, linux-kernel,
linux-api, netdev, Alan Cox, Christoph Lameter, David Miller,
Krzysztof Halasa, Rodolfo Giometti, Thomas Gleixner,
Benjamin Herrenschmidt, H. Peter Anvin, Ingo Molnar,
Mike Frysinger, Paul Mackerras, Russell King
In-Reply-To: <201101281305.37370.arnd@arndb.de>
On Fri, 2011-01-28 at 13:05 +0100, Arnd Bergmann wrote:
> On Friday 28 January 2011, Richard Cochran wrote:
> > I would like to get to the bottom of this. Here is what I did:
> >
> > 1. Saved your patch to disk in mbox format using Mutt.
> > 2. git am
> > 3. ... rebase, rebase, rebase, ...
> > 4. git format-patch [options] 1234..abcd
> > 5. Edit cover letter
> > 6. for x in 00*; do mutt -H $x; done
> >
> > Git format-patch places the "From: John Stultz <john.stultz@linaro.org>"
> > line with the other mail headers, and so I guess mutt just faithfully
> > preserves this.
> >
> > I don't like having to remember to fix this manually. There must be a
> > better way...
>
> The problem is step 6. The output of git format-patch does not work when
> sending with mutt. The easiest solution is to send with git send-email,
> which does the same as mutt -H, but gets it right.
I use: formail -s sendmail -t < patches.mbox, but then, I use quilt mail
to generate the mbox, not git.
^ 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