12 Feb, 2013
3 commits
-
Cc: Erik Hugne
Cc: YOSHIFUJI Hideaki
Signed-off-by: Hannes Frederic Sowa
Signed-off-by: David S. Miller -
v2:
a) moved before multicast source address check
b) changed comment to netdev styleCc: Erik Hugne
Cc: YOSHIFUJI Hideaki
Acked-by: YOSHIFUJI Hideaki
Signed-off-by: Hannes Frederic Sowa
Acked-by: YOSHIFUJI Hideaki
Signed-off-by: David S. Miller -
Reported-by: Erik Hugne
Cc: Erik Hugne
Cc: YOSHIFUJI Hideaki
Signed-off-by: Hannes Frederic Sowa
Acked-by: YOSHIFUJI Hideaki
Signed-off-by: David S. Miller
11 Feb, 2013
16 commits
-
o Do not read mailbox registers on timeout
o Add a helper function to handle mailbox responseSigned-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Signed-off-by: Manish Chopra
Signed-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
o Handle async events during diagnostic loopback test
o Clear loopback mode on failure to receive async eventsSigned-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Cleanly separate 83xx diagnostic IRQ test from 82xx
Signed-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Cleanly separate 83xx diagnostic loopback test routines from 82xx
Signed-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Create a helper routine to handle async events, as it is being called
from multiple placesSigned-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Driver needs to stop participating in firmware based Inter Driver
Communication (IDC) while unloading driverSigned-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
Register for firmware based Inter Driver Communication (IDC) using initialize
NIC as the first mailbox commandSigned-off-by: Himanshu Madhani
Signed-off-by: Jitendra Kalsaria
Signed-off-by: David S. Miller -
To allow both of protocol-specific data and device-specific data
attached with neighbour entry, and to eliminate size calculation
cost when allocating entry, sizeof protocol-speicic data must be
multiple of NEIGH_PRIV_ALIGN. On 64bit archs,
sizeof(struct dn_neigh) is multiple of NEIGH_PRIV_ALIGN, but on
32bit archs, it was not.Introduce NEIGH_ENTRY_SPACE() macro to ensure that protocol-specific
entry-size meets our requirement.Reported-by: Fengguang Wu
Signed-off-by: YOSHIFUJI Hideaki
Signed-off-by: David S. Miller -
RFC4291 (IPv6 addressing architecture) says that interface-Local scope
spans only a single interface on a node. We should not join L2 device
multicast list for addresses in interface-local (or smaller) scope.Signed-off-by: YOSHIFUJI Hideaki
Acked-by: Hannes Frederic Sowa
Signed-off-by: David S. Miller -
commit d0e2c55e7c940 (veth: avoid a NULL deref in veth_stats_one)
added another NULL deref in veth_dellink().# ip link add name veth1 type veth peer name veth0
# rmmod vethWe crash because veth_dellink() is called twice, so we must
take care of NULL peer.Signed-off-by: Eric Dumazet
Signed-off-by: David S. Miller -
David Ward says:
====================
The Linux kernel currently implements the GARP VLAN Registration
Protocol (GVRP) from IEEE 802.1Q-1998 (applicant-only participant).
When the GVRP flag is set for a VLAN interface on a Linux host, the
host advertises its membership in the VLAN to the attached bridge/
switch, so that it is not necessary to manually configure the bridge/
switch port to participate in the VLAN.GVRP has been superseded by the Multiple VLAN Registration Protocol
(MVRP) in IEEE 802.1Q-2011, which addresses scalability concerns about
the earlier protocol. The following patches add support for MVRP to
the Linux kernel and iproute2 utility. They are based largely off of
the existing implementation of GVRP, but have been modified for the
new PDU structure and state machine.This implementation was tested with two Juniper EX4200 switches.
====================Signed-off-by: David Ward
Acked-by: Patrick McHardy
Signed-off-by: David S. Miller -
Initial implementation of the Multiple VLAN Registration Protocol
(MVRP) from IEEE 802.1Q-2011, based on the existing implementation
of the GARP VLAN Registration Protocol (GVRP).Signed-off-by: David Ward
Acked-by: Patrick McHardy
Signed-off-by: David S. Miller -
Initial implementation of the Multiple Registration Protocol (MRP)
from IEEE 802.1Q-2011, based on the existing implementation of the
Generic Attribute Registration Protocol (GARP).Signed-off-by: David Ward
Acked-by: Patrick McHardy
Signed-off-by: David S. Miller -
Andy King says:
====================
In an effort to improve the out-of-the-box experience with Linux kernels for
VMware users, VMware is working on readying the VM Sockets (VSOCK, formerly
VMCI Sockets) (vsock) kernel module for inclusion in the Linux kernel. The
purpose of this post is to acquire feedback on the vsock kernel module.Unlike previous submissions, where the new socket family was entirely reliant
on VMware's VMCI PCI device (and thus VMware's hypervisor), VM Sockets is now
completely[1] separated out into two parts, each in its own module:o Core socket code, which is transport-neutral and invokes transport
callbacks to communicate with the hypervisor. This is vsock.ko.
o A VMCI transport, which communicates over VMCI with the VMware hypervisor.
This is vmw_vsock_vmci_transport.ko, and it registers with the core module
as a transport.This should provide a path to introducing additional transports, for example
virtio, with the ultimate goal being to make this new socket family
hypervisor-neutral.[1] If Gerd tries it and determines this to be false (still), I'll ship him
a keg of beer.
====================Signed-off-by: David S. Miller
-
VM Sockets allows communication between virtual machines and the hypervisor.
User level applications both in a virtual machine and on the host can use the
VM Sockets API, which facilitates fast and efficient communication between
guest virtual machines and their host. A socket address family, designed to be
compatible with UDP and TCP at the interface level, is provided.Today, VM Sockets is used by various VMware Tools components inside the guest
for zero-config, network-less access to VMware host services. In addition to
this, VMware's users are using VM Sockets for various applications, where
network access of the virtual machine is restricted or non-existent. Examples
of this are VMs communicating with device proxies for proprietary hardware
running as host applications and automated testing of applications running
within virtual machines.The VMware VM Sockets are similar to other socket types, like Berkeley UNIX
socket interface. The VM Sockets module supports both connection-oriented
stream sockets like TCP, and connectionless datagram sockets like UDP. The VM
Sockets protocol family is defined as "AF_VSOCK" and the socket operations
split for SOCK_DGRAM and SOCK_STREAM.For additional information about the use of VM Sockets, please refer to the
VM Sockets Programming Guide available at:https://www.vmware.com/support/developer/vmci-sdk/
Signed-off-by: George Zhang
Signed-off-by: Dmitry Torokhov
Signed-off-by: Andy king
Signed-off-by: David S. Miller
09 Feb, 2013
19 commits
-
Synchronize with 'net' in order to sort out some l2tp, wireless, and
ipv6 GRE fixes that will be built on top of in 'net-next'.Signed-off-by: David S. Miller
-
This was preventing GRO and RxCheckSum offload to happen.
Signed-off-by: Jay Hernandez
Signed-off-by: Vipul Pandya
Signed-off-by: David S. Miller -
In sctp_auth_make_key_vector, we allocate a temporary sctp_auth_bytes
structure with kmalloc instead of the sctp_auth_create_key allocator.
Change this to sctp_auth_create_key as it is the case everywhere else,
so that we also can properly free it via sctp_auth_key_put. This makes
it easier for future code changes in the structure and allocator itself,
since a single API is consistently used for this purpose. Also, by
using sctp_auth_create_key we're doing sanity checks over the arguments.Signed-off-by: Daniel Borkmann
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller -
Some multicast addresses are common to all macvlans,
so if a multicast message has a hash value collision, we
have to deliver a copy to all macvlans, adding significant
latency and possible packet drops if netdev_max_backlog
limit is hit.Having a per macvlan hash function permits to reduce the
impact of hash collisions.Suggested-by: Maciej Żenczykowski
Signed-off-by: Eric Dumazet
Signed-off-by: Maciej Żenczykowski
Signed-off-by: David S. Miller -
commit cd431e738509e (macvlan: add multicast filter) forgot
the broadcast case.Signed-off-by: Eric Dumazet
Reported-by: Maciej Żenczykowski
SIgned-off-by: Maciej Żenczykowski
Signed-off-by: David S. Miller -
This patch fixes the following RCU warning:
[ 51.680236] ===============================
[ 51.681914] [ INFO: suspicious RCU usage. ]
[ 51.683610] 3.8.0-rc6-next-20130206-sasha-00028-g83214f7-dirty #276 Tainted: G W
[ 51.686703] -------------------------------
[ 51.688281] net/ipv6/ip6_flowlabel.c:671 suspicious rcu_dereference_check() usage!we should use rcu_dereference_bh() when we hold rcu_read_lock_bh().
Reported-by: Sasha Levin
Cc: David S. Miller
Cc: YOSHIFUJI Hideaki
Signed-off-by: Cong Wang
Signed-off-by: David S. Miller -
alloc failures already get standardized OOM
messages and a dump_stack.For the affected mallocs around these OOM messages:
Converted kmallocs with multiplies to kmalloc_array.
Converted a kmalloc/memcpy to kmemdup.
Removed now unused stack variables.
Removed unnecessary parentheses.
Neatened alignment.Signed-off-by: Joe Perches
Acked-by: Arend van Spriel
Acked-by: Marc Kleine-Budde
Acked-by: John W. Linville
Signed-off-by: David S. Miller -
The last (bool) parameter in bgmac_cmdcfg_maskset says if the write
should be made, even if value didn't change. Currently driver doesn't
match the specs about (not) forcing some changes. This makes it follow
them.Reported-by: Nathan Hintz
Signed-off-by: Rafał Miłecki
Signed-off-by: David S. Miller -
This adds check for a valid Ethernet MAC address and in case it is not,
it will generate a valid random one, such that the adapter is still
usable.Signed-off-by: Rafał Miłecki
Signed-off-by: David S. Miller -
In order to address the fact that some devices cannot support the full 32K
frag size we need to have the value accessible somewhere so that we can use it
to do comparisons against what the device can support. As such I am moving
the values out of skbuff.c and into skbuff.h.Signed-off-by: Alexander Duyck
Signed-off-by: David S. Miller -
Pull ARM fixes from Russell King:
"I was going to hold these off until v3.8 was out, and send them with a
stable tag, but as everyone else is pushing much bigger fixes which
Linus is accepting, let's save people from the hastle of having to
patch v3.8 back into working or use a stable kernel.Looking at the diffstat, this really is high value for its size; this
is miniscule compared to how the -rc6 to tip diffstat currently looks.So, four patches in this set:
- Punit Agrawal reports that the kernel no longer boots on MPCore due
to a new assumption made in the GIC code which isn't true of
earlier GIC designs. This is the biggest change in this set.
- Punit's boot log also revealed a bunch of WARN_ON() dumps caused by
the DT-ification of the GIC support without fixing up non-DT
Realview - which now sees a greater number of interrupts than it
did before.
- A fix for the DMA coherent code from Marek which uses the wrong
check for atomic allocations; this can result in spinlock lockups
or other nasty effects.
- A fix from Will, which will affect all Android based platforms if
not applied (which use the 2G:2G VM split) - this causes
particularly 'make' to misbehave unless this bug is fixed."* 'fixes' of git://git.linaro.org/people/rmk/linux-arm:
ARM: 7641/1: memory: fix broken mmap by ensuring TASK_UNMAPPED_BASE is aligned
ARM: DMA mapping: fix bad atomic test
ARM: realview: ensure that we have sufficient IRQs available
ARM: GIC: fix GIC cpumask initialization -
Pull networking fixes from David Miller:
1) Revert iwlwifi reclaimed packet tracking, it causes problems for a
bunch of folks. From Emmanuel Grumbach.2) Work limiting code in brcmsmac wifi driver can clear tx status
without processing the event. From Arend van Spriel.3) rtlwifi USB driver processes wrong SKB, fix from Larry Finger.
4) l2tp tunnel delete can race with close, fix from Tom Parkin.
5) pktgen_add_device() failures are not checked at all, fix from Cong
Wang.6) Fix unintentional removal of carrier off from tun_detach(),
otherwise we confuse userspace, from Michael S. Tsirkin.7) Don't leak socket reference counts and ubufs in vhost-net driver,
from Jason Wang.8) vmxnet3 driver gets it's initial carrier state wrong, fix from Neil
Horman.9) Protect against USB networking devices which spam the host with 0
length frames, from Bjørn Mork.10) Prevent neighbour overflows in ipv6 for locally destined routes,
from Marcelo Ricardo. This is the best short-term fix for this, a
longer term fix has been implemented in net-next.11) L2TP uses ipv4 datagram routines in it's ipv6 code, whoops. This
mistake is largely because the ipv6 functions don't even have some
kind of prefix in their names to suggest they are ipv6 specific.
From Tom Parkin.12) Check SYN packet drops properly in tcp_rcv_fastopen_synack(), from
Yuchung Cheng.13) Fix races and TX skb freeing bugs in via-rhine's NAPI support, from
Francois Romieu and your's truly.14) Fix infinite loops and divides by zero in TCP congestion window
handling, from Eric Dumazet, Neal Cardwell, and Ilpo Järvinen.15) AF_PACKET tx ring handling can leak kernel memory to userspace, fix
from Phil Sutter.16) Fix error handling in ipv6 GRE tunnel transmit, from Tommi Rantala.
17) Protect XEN netback driver against hostile frontend putting garbage
into the rings, don't leak pages in TX GOP checking, and add proper
resource releasing in error path of xen_netbk_get_requests(). From
Ian Campbell.18) SCTP authentication keys should be cleared out and released with
kzfree(), from Daniel Borkmann.19) L2TP is a bit too clever trying to maintain skb->truesize, and ends
up corrupting socket memory accounting to the point where packet
sending is halted indefinitely. Just remove the adjustments
entirely, they aren't really needed. From Eric Dumazet.20) ATM Iphase driver uses a data type with the same name as the S390
headers, rename to fix the build. From Heiko Carstens.21) Fix a typo in copying the inner network header offset from one SKB
to another, from Pravin B Shelar.* git://git.kernel.org/pub/scm/linux/kernel/git/davem/net: (56 commits)
net: sctp: sctp_endpoint_free: zero out secret key data
net: sctp: sctp_setsockopt_auth_key: use kzfree instead of kfree
atm/iphase: rename fregt_t -> ffreg_t
net: usb: fix regression from FLAG_NOARP code
l2tp: dont play with skb->truesize
net: sctp: sctp_auth_key_put: use kzfree instead of kfree
netback: correct netbk_tx_err to handle wrap around.
xen/netback: free already allocated memory on failure in xen_netbk_get_requests
xen/netback: don't leak pages on failure in xen_netbk_tx_check_gop.
xen/netback: shutdown the ring if it contains garbage.
net: qmi_wwan: add more Huawei devices, including E320
net: cdc_ncm: add another Huawei vendor specific device
ipv6/ip6_gre: fix error case handling in ip6gre_tunnel_xmit()
tcp: fix for zero packets_in_flight was too broad
brcmsmac: rework of mac80211 .flush() callback operation
ssb: unregister gpios before unloading ssb
bcma: unregister gpios before unloading bcma
rtlwifi: Fix scheduling while atomic bug
net: usbnet: fix tx_dropped statistics
tcp: ipv6: Update MIB counters for drops
... -
John W. Linville says:
====================
Please accept this pull request intended for the 3.9 stream!Included are a mac80211 pull, an iwlwifi pull (actually two -- one
was a fast-forward), a wl12xx pull, and a couple of Bluetooth pulls.On mac80211, Johannes says:
"I've included
* AKM definitions from Bing,
* mesh fixes from Thomas, including a fix from him for me breaking his
patch while applying,
* channel check fix from Simon,
* an old patch from Yoni Divinsky who doesn't even work for TI any
more, to configure the WEP TX key for ARP offload etc.
* MAC ACL API from Vasanth
* a fix for the infamous chanctx_conf warning from Arnd
* from myself, a fix for my previous aggregation changes, some cleanup
and some improvements and fixes for WoWLAN"On iwlwifi, Johannes says:
"Two small changes for iwlwifi-next, one to update all our Copyright
notices and one to provide the RX page order."And also:
"So what I have here is some cleanups, preparations and the new MVM
(multi-virtual MAC) driver itself and (this is new) some work on the
transport API as well as a message flooding fix."On wl12xx, Luca says:
"Lots of bugfixes and improvements in our TI wireless drivers,
including support for multi-channel. Intended for 3.9."On Bluetooth, Gustavo says:
"This is my first pull request to 3.9. The biggest changes here are from Johan
Hedberg who made a lot of fixes in the Management interface. The issues arose
due to a new test tool we wrote and the usage of the Management interface as
default in BlueZ 5. The rest of the patches are more clean ups and small
fixes."And also:
"Here goes another batch intended for 3.9, the majority of the patch here are
from Johan who is fixing many issues in the management interface that have
appeared lately. The rest of the patches are just small improvements, fixes
and clean ups."Along with those are the usual variety of updates/enhancements to
the mwl8k, mwifiex, ath9k, rtlwifi, and rt2x00 drivers as well as
a few updates for the ssb and bcma busses. I don't think there are
any big headliners there.Please let me know if there are problems!
====================Signed-off-by: David S. Miller
-
Daniel Borkmann says:
====================
Cryptographically used keys should be zeroed out when our session
ends resp. memory is freed, thus do not leave them somewhere in the
memory.
====================Signed-off-by: David S. Miller
-
On sctp_endpoint_destroy, previously used sensitive keying material
should be zeroed out before the memory is returned, as we already do
with e.g. auth keys when released.Signed-off-by: Daniel Borkmann
Acked-by: Vlad Yasevich
Signed-off-by: David S. Miller -
In sctp_setsockopt_auth_key, we create a temporary copy of the user
passed shared auth key for the endpoint or association and after
internal setup, we free it right away. Since it's sensitive data, we
should zero out the key before returning the memory back to the
allocator. Thus, use kzfree instead of kfree, just as we do in
sctp_auth_key_put().Signed-off-by: Daniel Borkmann
Signed-off-by: David S. Miller -
We have conflicting type qualifiers for "freg_t" in s390's ptrace.h and the
iphase atm device driver, which causes the compile error below.
Unfortunately the s390 typedef can't be renamed, since it's a user visible api,
nor can I change the include order in s390 code to avoid the conflict.So simply rename the iphase typedef to a new name. Fixes this compile error:
In file included from drivers/atm/iphase.c:66:0:
drivers/atm/iphase.h:639:25: error: conflicting type qualifiers for 'freg_t'
In file included from next/arch/s390/include/asm/ptrace.h:9:0,
from next/arch/s390/include/asm/lowcore.h:12,
from next/arch/s390/include/asm/thread_info.h:30,
from include/linux/thread_info.h:54,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:50,
from include/linux/seqlock.h:29,
from include/linux/time.h:5,
from include/linux/stat.h:18,
from include/linux/module.h:10,
from drivers/atm/iphase.c:43:
next/arch/s390/include/uapi/asm/ptrace.h:197:3: note: previous declaration of 'freg_t' was hereSigned-off-by: Heiko Carstens
Acked-by: chas williams - CONTRACTOR
Signed-off-by: David S. Miller -
…wireless-next into for-davem
08 Feb, 2013
2 commits
-
We have received multiple reports of mmap failures when running with a
2:2 vm split. These manifest as either -EINVAL with a non page-aligned
address (ending 0xaaa) or a SEGV, depending on the application. The
issue is commonly observed in children of make, which appears to use
bottom-up mmap (assumedly because it changes the stack rlimit).Further investigation reveals that this regression was triggered by
394ef6403abc ("mm: use vm_unmapped_area() on arm architecture"), whereby
TASK_UNMAPPED_BASE is no longer page-aligned for bottom-up mmap, causing
get_unmapped_area to choke on misaligned addressed.This patch fixes the problem by defining TASK_UNMAPPED_BASE in terms of
TASK_SIZE and explicitly aligns the result to 16M, matching the other
end of the heap.Acked-by: Nicolas Pitre
Reported-by: Steve Capper
Reported-by: Jean-Francois Moine
Reported-by: Christoffer Dall
Signed-off-by: Will Deacon
Signed-off-by: Russell King -
Realview fails to boot with this warning:
BUG: spinlock lockup suspected on CPU#0, init/1
lock: 0xcf8bde10, .magic: dead4ead, .owner: init/1, .owner_cpu: 0
Backtrace:
[] (dump_backtrace+0x0/0x10c) from [] (dump_stack+0x18/0x1c) r6:cf8bde10 r5:cf83d1c0 r4:cf8bde10 r3:cf83d1c0
[] (dump_stack+0x0/0x1c) from [] (spin_dump+0x84/0x98)
[] (spin_dump+0x0/0x98) from [] (do_raw_spin_lock+0x100/0x198)
[] (do_raw_spin_lock+0x0/0x198) from [] (_raw_spin_lock+0x3c/0x44)
[] (_raw_spin_lock+0x0/0x44) from [] (pl011_console_write+0xe8/0x11c)
[] (pl011_console_write+0x0/0x11c) from [] (call_console_drivers.clone.7+0xdc/0x104)
[] (call_console_drivers.clone.7+0x0/0x104) from [] (console_unlock+0x2e8/0x454)
[] (console_unlock+0x0/0x454) from [] (vprintk_emit+0x2d8/0x594)
[] (vprintk_emit+0x0/0x594) from [] (printk+0x3c/0x44)
[] (printk+0x0/0x44) from [] (warn_slowpath_common+0x28/0x6c)
[] (warn_slowpath_common+0x0/0x6c) from [] (warn_slowpath_null+0x24/0x2c)
[] (warn_slowpath_null+0x0/0x2c) from [] (lockdep_trace_alloc+0xd8/0xf0)
[] (lockdep_trace_alloc+0x0/0xf0) from [] (kmem_cache_alloc+0x24/0x11c)
[] (kmem_cache_alloc+0x0/0x11c) from [] (__get_vm_area_node.clone.24+0x7c/0x16c)
[] (__get_vm_area_node.clone.24+0x0/0x16c) from [] (get_vm_area_caller+0x48/0x54)
[] (get_vm_area_caller+0x0/0x54) from [] (__alloc_remap_buffer.clone.15+0x38/0xb8)
[] (__alloc_remap_buffer.clone.15+0x0/0xb8) from [] (__dma_alloc+0x160/0x2c8)
[] (__dma_alloc+0x0/0x2c8) from [] (arm_dma_alloc+0x88/0xa0)[] (arm_dma_alloc+0x0/0xa0) from [] (dma_pool_alloc+0xcc/0x1a8)
[] (dma_pool_alloc+0x0/0x1a8) from [] (pl08x_fill_llis_for_desc+0x28/0x568)
[] (pl08x_fill_llis_for_desc+0x0/0x568) from [] (pl08x_prep_slave_sg+0x258/0x3b0)
[] (pl08x_prep_slave_sg+0x0/0x3b0) from [] (pl011_dma_tx_refill+0x140/0x288)
[] (pl011_dma_tx_refill+0x0/0x288) from [] (pl011_start_tx+0xe4/0x120)
[] (pl011_start_tx+0x0/0x120) from [] (__uart_start+0x48/0x4c)
[] (__uart_start+0x0/0x4c) from [] (uart_start+0x2c/0x3c)
[] (uart_start+0x0/0x3c) from [] (uart_write+0xcc/0xf4)
[] (uart_write+0x0/0xf4) from [] (n_tty_write+0x1c0/0x3e4)
[] (n_tty_write+0x0/0x3e4) from [] (tty_write+0x144/0x240)
[] (tty_write+0x0/0x240) from [] (redirected_tty_write+0x98/0xac)
[] (redirected_tty_write+0x0/0xac) from [] (vfs_write+0xbc/0x150)
[] (vfs_write+0x0/0x150) from [] (sys_write+0x4c/0x78)
[] (sys_write+0x0/0x78) from [] (ret_fast_syscall+0x0/0x3c)This happens because the DMA allocation code is not respecting atomic
allocations correctly.GFP flags should not be tested for GFP_ATOMIC to determine if an
atomic allocation is being requested. GFP_ATOMIC is not a flag but
a value. The GFP bitmask flags are all prefixed with __GFP_.The rest of the kernel tests for __GFP_WAIT not being set to indicate
an atomic allocation. We need to do the same.Signed-off-by: Russell King