* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Joe Perches @ 2011-05-12 15:52 UTC (permalink / raw)
To: Vitalii Demianets
Cc: Ben Hutchings, Jay Vosburgh, Andy Gospodarek,
Arnaldo Carvalho de Melo, netdev, bonding-devel
In-Reply-To: <201105121845.49090.vitas@nppfactor.kiev.ua>
On Thu, 2011-05-12 at 18:45 +0300, Vitalii Demianets wrote:
> On Thursday 12 May 2011 17:33:36 Ben Hutchings wrote:
> [...]
> > The '__packed' macro is preferred.
> Also, I'm slightly in doubt if I should split the patch in 2 parts, one for
> bonding an one for LLC?
What arch needs __packed for
struct mac_addr {
unsigned char addr[6];
};
?
^ permalink raw reply
* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Vitalii Demianets @ 2011-05-12 15:45 UTC (permalink / raw)
To: Ben Hutchings
Cc: Jay Vosburgh, Andy Gospodarek, Arnaldo Carvalho de Melo, netdev,
bonding-devel
In-Reply-To: <1305210816.5214.9.camel@bwh-desktop>
On Thursday 12 May 2011 17:33:36 Ben Hutchings wrote:
[...]
> The '__packed' macro is preferred.
Ok, it looks more compatible, i"ll resubmit the patch with this macro.
Also, I'm slightly in doubt if I should split the patch in 2 parts, one for
bonding an one for LLC?
^ permalink raw reply
* Re: [PATCH net-next] net:set valid name before calling ndo_init()
From: Changli Gao @ 2011-05-12 15:32 UTC (permalink / raw)
To: Weiping Pan(潘卫平)
Cc: davem, eric.dumazet, mirq-linux, jpirko, netdev, linux-kernel
In-Reply-To: <1300936598-3971-1-git-send-email-panweiping3@gmail.com>
On Thu, Mar 24, 2011 at 11:16 AM, Weiping Pan(潘卫平)
<panweiping3@gmail.com> wrote:
> From: Pan Weiping <panweiping3@gmail.com>
>
> A bug of bonding was invloved by e815d19ffe02bdfda1260949ef2b1806171,
> see example 1 and 2.
>
> In register_netdevice(), the name of net_device is not valid until
> dev_get_valid_name() is called. But dev->netdev_ops->ndo_init(that is
> bond_init) is called before dev_get_valid_name(),
> and it uses the invalid name of net_device.
>
> I think register_netdevice() should make sure that the name of net_device is
> valid before calling ndo_init().
>
> example 1:
> modprobe bonding
> ls /proc/net/bonding/bond%d
>
> ps -eLf
> root 3398 2 3398 0 1 21:34 ? 00:00:00 [bond%d]
>
> example 2:
> modprobe bonding max_bonds=3
>
> [ 170.100292] bonding: Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
> [ 170.101090] bonding: Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.
> [ 170.102469] ------------[ cut here ]------------
> [ 170.103150] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
> [ 170.104075] Hardware name: VirtualBox
> [ 170.105065] proc_dir_entry 'bonding/bond%d' already registered
> [ 170.105613] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
> [ 170.108397] Pid: 3457, comm: modprobe Not tainted 2.6.39-rc2+ #14
> [ 170.108935] Call Trace:
> [ 170.109382] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
> [ 170.109911] [<c051a42a>] ? proc_register+0x126/0x157
> [ 170.110329] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
> [ 170.110846] [<c051a42a>] proc_register+0x126/0x157
> [ 170.111870] [<c051a4dd>] proc_create_data+0x82/0x98
> [ 170.112335] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
> [ 170.112905] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
> [ 170.113319] [<c0721ac6>] register_netdevice+0x8c/0x1d3
> [ 170.113848] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
> [ 170.114322] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
> [ 170.114879] [<c0401240>] do_one_initcall+0x76/0x122
> [ 170.115317] [<f94f4000>] ? 0xf94f3fff
> [ 170.115799] [<c0463f1e>] sys_init_module+0x1286/0x140d
> [ 170.116879] [<c07c6d9f>] sysenter_do_call+0x12/0x28
> [ 170.117404] ---[ end trace 64e4fac3ae5fff1a ]---
> [ 170.117924] bond%d: Warning: failed to register to debugfs
> [ 170.128728] ------------[ cut here ]------------
> [ 170.129360] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
> [ 170.130323] Hardware name: VirtualBox
> [ 170.130797] proc_dir_entry 'bonding/bond%d' already registered
> [ 170.131315] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
> [ 170.133731] Pid: 3457, comm: modprobe Tainted: G W 2.6.39-rc2+ #14
> [ 170.134308] Call Trace:
> [ 170.134743] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
> [ 170.135305] [<c051a42a>] ? proc_register+0x126/0x157
> [ 170.135820] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
> [ 170.137168] [<c051a42a>] proc_register+0x126/0x157
> [ 170.137700] [<c051a4dd>] proc_create_data+0x82/0x98
> [ 170.138174] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
> [ 170.138745] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
> [ 170.139278] [<c0721ac6>] register_netdevice+0x8c/0x1d3
> [ 170.139828] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
> [ 170.140361] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
> [ 170.140927] [<c0401240>] do_one_initcall+0x76/0x122
> [ 170.141494] [<f94f4000>] ? 0xf94f3fff
> [ 170.141975] [<c0463f1e>] sys_init_module+0x1286/0x140d
> [ 170.142463] [<c07c6d9f>] sysenter_do_call+0x12/0x28
> [ 170.142974] ---[ end trace 64e4fac3ae5fff1b ]---
> [ 170.144949] bond%d: Warning: failed to register to debugfs
>
> Signed-off-by: Pan Weiping <panweiping3@gmail.com>
Acked-by: Changli Gao <xiaosuo@gmail.com>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH net-next] net:set valid name before calling ndo_init()
From: Eric Dumazet @ 2011-05-12 15:33 UTC (permalink / raw)
To: Weiping Pan(潘卫平)
Cc: David S. Miller, Tom Herbert, Michał Mirosław,
Ben Hutchings, open list:NETWORKING [GENERAL], open list
In-Reply-To: <1305207541-13311-1-git-send-email-panweiping3@gmail.com>
Le jeudi 12 mai 2011 à 21:39 +0800, Weiping Pan(潘卫平) a écrit :
> From: Pan Weiping <panweiping3@gmail.com>
>
> A bug of bonding was invloved by e815d19ffe02bdfda1260949ef2b1806171,
> see example 1 and 2.
>
I cant find e815d19ffe02bdfda1260949ef2b1806171 in net-next-2.6
but I do find 1c5cae815d19ffe02bdfda1260949ef2b1806171
Please always use following when referring to a commit :
... in commit 1c5cae815d19 (net: call dev_alloc_name from
register_netdevice) ...
- just limit to _first_ 12 chars, no need to have full length
- give the commit title
^ permalink raw reply
* Re: [PATCH net-next] net:set valid name before calling ndo_init()
From: Jiri Pirko @ 2011-05-12 15:17 UTC (permalink / raw)
To: Weiping Pan(潘卫平)
Cc: davem, eric.dumazet, xiaosuo, mirq-linux, netdev, linux-kernel
In-Reply-To: <1300936598-3971-1-git-send-email-panweiping3@gmail.com>
Thu, Mar 24, 2011 at 04:16:38AM CET, panweiping3@gmail.com wrote:
>From: Pan Weiping <panweiping3@gmail.com>
>
>A bug of bonding was invloved by e815d19ffe02bdfda1260949ef2b1806171,
>see example 1 and 2.
>
>In register_netdevice(), the name of net_device is not valid until
>dev_get_valid_name() is called. But dev->netdev_ops->ndo_init(that is
>bond_init) is called before dev_get_valid_name(),
>and it uses the invalid name of net_device.
>
>I think register_netdevice() should make sure that the name of net_device is
>valid before calling ndo_init().
>
>example 1:
>modprobe bonding
>ls /proc/net/bonding/bond%d
>
>ps -eLf
>root 3398 2 3398 0 1 21:34 ? 00:00:00 [bond%d]
>
>example 2:
>modprobe bonding max_bonds=3
>
>[ 170.100292] bonding: Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
>[ 170.101090] bonding: Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.
>[ 170.102469] ------------[ cut here ]------------
>[ 170.103150] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
>[ 170.104075] Hardware name: VirtualBox
>[ 170.105065] proc_dir_entry 'bonding/bond%d' already registered
>[ 170.105613] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
>[ 170.108397] Pid: 3457, comm: modprobe Not tainted 2.6.39-rc2+ #14
>[ 170.108935] Call Trace:
>[ 170.109382] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
>[ 170.109911] [<c051a42a>] ? proc_register+0x126/0x157
>[ 170.110329] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
>[ 170.110846] [<c051a42a>] proc_register+0x126/0x157
>[ 170.111870] [<c051a4dd>] proc_create_data+0x82/0x98
>[ 170.112335] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
>[ 170.112905] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
>[ 170.113319] [<c0721ac6>] register_netdevice+0x8c/0x1d3
>[ 170.113848] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
>[ 170.114322] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
>[ 170.114879] [<c0401240>] do_one_initcall+0x76/0x122
>[ 170.115317] [<f94f4000>] ? 0xf94f3fff
>[ 170.115799] [<c0463f1e>] sys_init_module+0x1286/0x140d
>[ 170.116879] [<c07c6d9f>] sysenter_do_call+0x12/0x28
>[ 170.117404] ---[ end trace 64e4fac3ae5fff1a ]---
>[ 170.117924] bond%d: Warning: failed to register to debugfs
>[ 170.128728] ------------[ cut here ]------------
>[ 170.129360] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
>[ 170.130323] Hardware name: VirtualBox
>[ 170.130797] proc_dir_entry 'bonding/bond%d' already registered
>[ 170.131315] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
>[ 170.133731] Pid: 3457, comm: modprobe Tainted: G W 2.6.39-rc2+ #14
>[ 170.134308] Call Trace:
>[ 170.134743] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
>[ 170.135305] [<c051a42a>] ? proc_register+0x126/0x157
>[ 170.135820] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
>[ 170.137168] [<c051a42a>] proc_register+0x126/0x157
>[ 170.137700] [<c051a4dd>] proc_create_data+0x82/0x98
>[ 170.138174] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
>[ 170.138745] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
>[ 170.139278] [<c0721ac6>] register_netdevice+0x8c/0x1d3
>[ 170.139828] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
>[ 170.140361] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
>[ 170.140927] [<c0401240>] do_one_initcall+0x76/0x122
>[ 170.141494] [<f94f4000>] ? 0xf94f3fff
>[ 170.141975] [<c0463f1e>] sys_init_module+0x1286/0x140d
>[ 170.142463] [<c07c6d9f>] sysenter_do_call+0x12/0x28
>[ 170.142974] ---[ end trace 64e4fac3ae5fff1b ]---
>[ 170.144949] bond%d: Warning: failed to register to debugfs
>
>Signed-off-by: Pan Weiping <panweiping3@gmail.com>
>---
> net/core/dev.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 75898a3..f289117 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -5412,6 +5412,10 @@ int register_netdevice(struct net_device *dev)
>
> dev->iflink = -1;
>
>+ ret = dev_get_valid_name(dev, dev->name);
>+ if (ret < 0)
>+ goto out;
>+
> /* Init, if this function is available */
> if (dev->netdev_ops->ndo_init) {
> ret = dev->netdev_ops->ndo_init(dev);
>@@ -5422,10 +5426,6 @@ int register_netdevice(struct net_device *dev)
> }
> }
>
>- ret = dev_get_valid_name(dev, dev->name);
>- if (ret < 0)
>- goto err_uninit;
>-
> dev->ifindex = dev_new_index(net);
> if (dev->iflink == -1)
> dev->iflink = dev->ifindex;
>--
>1.7.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
This is looking good to me.
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Oliver Hartkopp @ 2011-05-12 14:41 UTC (permalink / raw)
To: Subhasish Ghosh
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Arnd Bergmann, Netdev-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0,
open list, CAN NETWORK DRIVERS, Alan Cox, Marc Kleine-Budde,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
m-watkins-l0cyMroinI0, Wolfgang Grandegger
In-Reply-To: <4DCB88A4.2010901-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On 12.05.2011 09:13, Wolfgang Grandegger wrote:
> On 05/11/2011 11:44 PM, Arnd Bergmann wrote:
>> On Wednesday 11 May 2011, Arnd Bergmann wrote:
>>> If that interpretation is right, I would seriously recommend rethinking
>>> the design of the CAN firmware for pruss, so you can start doing something
>>> useful with the offload engine that fits into the Socket CAN API, or that
>>> would be a useful extension to Socket CAN that is also implementable in
>>> the kernel for all other drivers in a meaningful way.
>>
>> I've looked some more into the CAN socket implementation, and I suppose that
>> the idea of the pruss driver was really to help do the work from the
>> can_rcv_filter function in hardware.
>
> That software filter is per socket while the hardware filter will be per
> device.
Hi all,
i took some while to get behind all the arguments for me :-)
Wolfgangs suggestion
> A simple interface using:
>
> ip link set can0 type can filter <id>:<mask> [<id>:<mask> ...]
>
indeed would just be fine - but IMHO it doesn't help for the pruss CAN driver.
The problem is, that you plan to filter on a CAN-identifier base. This is not
only very application dependent (as Kurt already pointed out) - it also does
not bring any safety that your system does not explode on heavy CAN load.
E.g. assume you need the CAN-IDs 0x100, 0x200 and 0x300 in your application
and for that reason you configure these IDs in the pruss CAN driver.
What if someone generates a 100% CAN busload exactly on CAN-ID 0x100 then?
Worst case (1MBit/s, DLC=0) you would need to handle about 21.000 irqs/s for
the correctly received CAN frames with the filtered CAN-ID 0x100 ...
At the beginnig of the SocketCAN development, we had a PowerPC (E603) @ 133MHz
that was able to handle 4 (dumb) SJA1000 CAN controllers without problems.
Maybe i should tell a bit more about what's happening at CAN frame receive time:
1. IRQ happens
2. Read CAN frame from CAN controller registers
3. Allocate a socketbuffer (skb) and queue it into the netdevice rx queue
4. Softirq handles the new skb
5. Check the (specialized/optimized) CAN-ID filters for this CAN device
6. Enqueue the data to the sockets recv buffer(s) and/or drop the skb.
This all depends heavily on Linux networking (skb handling, caching, etc) and
is pretty fast and optimized!! That was also the reason why it ran on the old
PowerPC that smoothly. The mostly seen effect if anything drops is when the
application (holding the socket) was not fast enough to handle the incoming
data. NB: For that reason we implemented a CAN content filter (CAN_BCM) that
is able to do content filtering and timeout monitoring in Kernelspace - all
performed in the SoftIRQ.
Having 'Mailboxes' bound to CAN-IDs is something that's useful for 8/16 bit
CPUs where an application is tightly bound to the embedded ECUs functionality.
IMO you should try to implement an 'open' pruss CAN driver without filtering
as it doesn't really help (see above). The networking stack can cope with the
load. In all cases (with/without filters) the reduction the irqload could be a
way to investigate (e.g. think about dropping CAN frames based on the irqload).
But you should try the 'open' way first. And i think with your current driver
you could just filter for one CAN-ID (e.g. 0x100) and then produce the heavy
CAN busload with this CAN-ID 0x100 and a second CAN node, right?
Regards,
Oliver
^ permalink raw reply
* Re: future developments of usbnet
From: Alan Stern @ 2011-05-12 14:37 UTC (permalink / raw)
To: Oliver Neukum
Cc: David Miller, shemminger-ZtmgI6mnKB3QT0dZR+AlfA,
tom.leiming-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <201105120959.28473.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
On Thu, 12 May 2011, Oliver Neukum wrote:
> Am Mittwoch, 11. Mai 2011, 19:47:27 schrieb David Miller:
>
> > Basically once you take you interrupt, and disable device interrupts,
> > the generic net device layer calls your ->poll() routing with a "weight"
> > You should not process more RX packets than this value.
> >
> > If you have less than "weight" work to do, you should do a napi_complete(),
> > which takes you out of the polling group, and re-enable device interrupts.
> >
> > So the idea is that you keep getting ->poll()'d until there is no more
> > RX work to do.
> >
> > The "weight" argument implements fairness amongst competing, actively
> > polling, devices on the same CPU.
> >
>
> Thank you, this is very informative. Our problem here is that USB doesn't work
> sanely without interrupts. We can stop IO regarding rx, but we cannot stop
> interrupts if we want to do rx.
The idea behind NAPI can be adapted to the usbnet context. The basic
idea is that the driver is allowed to accept only a limited number of
packets, driven by polling from the NAPI core.
Therefore usbnet's poll routine should take the "weight" argument as an
indication of how many outstanding rx URBs are allowed. Each time the
poll routine is called, it should check to see if any rx URBs have
completed since the previous poll. If not then there is no network
traffic, so usbnet can take itself out of the poll loop. Otherwise,
the number of outstanding URBs should be adjusted (by unlinking some or
submitting more -- subject to some fixed maximum limit) to match the
new "weight".
Does that make sense?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [Bugme-new] [Bug 33502] New: Caught 64-bit read from uninitialized memory in __alloc_skb
From: Christoph Lameter @ 2011-05-12 14:36 UTC (permalink / raw)
To: Eric Dumazet
Cc: Vegard Nossum, Pekka Enberg, casteyde.christian, Andrew Morton,
netdev, bugzilla-daemon, bugme-daemon
In-Reply-To: <1305083543.2437.39.camel@edumazet-laptop>
On Wed, 11 May 2011, Eric Dumazet wrote:
> > The cpu sets a page flag called PageFrozen() and points a per cpu pointer
> > to the page.
> >
> >
>
> So, if I understand you, there is no problem at all and no patch even
> needed ? I can start a stress test and you guarantee there wont be a
> crash ?
>
> Sorry, its 5h11 in the morning here ;)
There is a problem if an interrupt or a preemption occurs and there is no
object left on the page. Then the current page will be unfrozen and a new
page put into place for allocation. The old page may then be freed by some other
process on another processor before we continue the interrupted
slab_alloc().
When slab_alloc() resumes in this scenario then it will ultimately see
that the tid was incremented and so the cmpxchg will fail. But before we
do the cmpxchgwe determine the pointer to the next object. And for that
we access the old page. The access must not cause a page fault (which it
currently does with CONFIG_DEBUG_PAGEALLOC). That is why we need the patch introducing
get_freepointer_safe()
The result does not matter since we will repeat the cmpxchg loop.
^ permalink raw reply
* Re: [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Ben Hutchings @ 2011-05-12 14:33 UTC (permalink / raw)
To: Vitalii Demianets
Cc: Jay Vosburgh, Andy Gospodarek, Arnaldo Carvalho de Melo, netdev,
bonding-devel
In-Reply-To: <201105121557.06679.vitas@nppfactor.kiev.ua>
On Thu, 2011-05-12 at 15:57 +0300, Vitalii Demianets wrote:
> With some combinations of arch/compiler the sizeof operator on structure
> returns value greater than expected.
Yes, some architecture ABIs set a minimum alignment for structures so
they can have greater alignment than any of their members.
> In cases when the structure is used for
> mapping PDU fields it may lead to unexpected results (e.g. holes and
> alignment problems in skb data). Attribute "packed" prevents this undesired
> behavior.
[...]
The '__packed' macro is preferred.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next-2.6] l2tp: fix potential rcu race
From: Paul E. McKenney @ 2011-05-12 14:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, James Chapman
In-Reply-To: <1305174156.3232.26.camel@edumazet-laptop>
On Thu, May 12, 2011 at 06:22:36AM +0200, Eric Dumazet wrote:
> While trying to remove useless synchronize_rcu() calls, I found l2tp is
> indeed incorrectly using two of such calls, but also bumps tunnel
> refcount after list insertion.
>
> tunnel refcount must be incremented before being made publically visible
> by rcu readers.
>
> This fix can be applied to 2.6.35+ and might need a backport for older
> kernels, since things were shuffled in commit fd558d186df2c
> (l2tp: Split pppol2tp patch into separate l2tp and ppp parts)
Ouch! Good catch, Eric!
Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> CC: James Chapman <jchapman@katalix.com>
> ---
>
> I based this patch on net-next-2.6 because of recent commits in this
> file and linux-2.6 being on late RC phase. But its a stable candidate.
>
> net/l2tp/l2tp_core.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 9be095e..ed8a233 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1435,16 +1435,15 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
>
> /* Add tunnel to our list */
> INIT_LIST_HEAD(&tunnel->list);
> - spin_lock_bh(&pn->l2tp_tunnel_list_lock);
> - list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list);
> - spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
> - synchronize_rcu();
> atomic_inc(&l2tp_tunnel_count);
>
> /* Bump the reference count. The tunnel context is deleted
> - * only when this drops to zero.
> + * only when this drops to zero. Must be done before list insertion
> */
> l2tp_tunnel_inc_refcount(tunnel);
> + spin_lock_bh(&pn->l2tp_tunnel_list_lock);
> + list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list);
> + spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
>
> err = 0;
> err:
> @@ -1636,7 +1635,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
> hlist_add_head_rcu(&session->global_hlist,
> l2tp_session_id_hash_2(pn, session_id));
> spin_unlock_bh(&pn->l2tp_session_hlist_lock);
> - synchronize_rcu();
> }
>
> /* Ignore management session in session count value */
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] bonding,llc: Fix structure sizeof incompatibility for some PDUs
From: Vitalii Demianets @ 2011-05-12 12:57 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, Arnaldo Carvalho de Melo
Cc: netdev, bonding-devel
With some combinations of arch/compiler the sizeof operator on structure
returns value greater than expected. In cases when the structure is used for
mapping PDU fields it may lead to unexpected results (e.g. holes and
alignment problems in skb data). Attribute "packed" prevents this undesired
behavior.
Signed-off-by: Vitalii Demianets <vitas@nppfactor.kiev.ua>
---
drivers/net/bonding/bond_3ad.h | 10 +++++-----
include/net/llc_pdu.h | 8 ++++----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 291dbd4..6d6327a 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -39,7 +39,7 @@
typedef struct mac_addr {
u8 mac_addr_value[ETH_ALEN];
-} mac_addr_t;
+} __attribute__((packed)) mac_addr_t;
enum {
BOND_AD_STABLE = 0,
@@ -134,12 +134,12 @@ typedef struct lacpdu {
u8 tlv_type_terminator; // = terminator
u8 terminator_length; // = 0
u8 reserved_50[50]; // = 0
-} lacpdu_t;
+} __attribute__((packed)) lacpdu_t;
typedef struct lacpdu_header {
struct ethhdr hdr;
struct lacpdu lacpdu;
-} lacpdu_header_t;
+} __attribute__((packed)) lacpdu_header_t;
// Marker Protocol Data Unit(PDU) structure(43.5.3.2 in the 802.3ad standard)
typedef struct bond_marker {
@@ -155,12 +155,12 @@ typedef struct bond_marker {
u8 tlv_type_terminator; // = 0x00
u8 terminator_length; // = 0x00
u8 reserved_90[90]; // = 0
-} bond_marker_t;
+} __attribute__((packed)) bond_marker_t;
typedef struct bond_marker_header {
struct ethhdr hdr;
struct bond_marker marker;
-} bond_marker_header_t;
+} __attribute__((packed)) bond_marker_header_t;
#pragma pack()
diff --git a/include/net/llc_pdu.h b/include/net/llc_pdu.h
index 75b8e29..a8eeb3f 100644
--- a/include/net/llc_pdu.h
+++ b/include/net/llc_pdu.h
@@ -199,7 +199,7 @@ struct llc_pdu_sn {
u8 ssap;
u8 ctrl_1;
u8 ctrl_2;
-};
+} __attribute__((packed));
static inline struct llc_pdu_sn *llc_pdu_sn_hdr(struct sk_buff *skb)
{
@@ -211,7 +211,7 @@ struct llc_pdu_un {
u8 dsap;
u8 ssap;
u8 ctrl_1;
-};
+} __attribute__((packed));
static inline struct llc_pdu_un *llc_pdu_un_hdr(struct sk_buff *skb)
{
@@ -359,7 +359,7 @@ struct llc_xid_info {
u8 fmt_id; /* always 0x81 for LLC */
u8 type; /* different if NULL/non-NULL LSAP */
u8 rw; /* sender receive window */
-};
+} __attribute__((packed));
/**
* llc_pdu_init_as_xid_cmd - sets bytes 3, 4 & 5 of LLC header as XID
@@ -415,7 +415,7 @@ struct llc_frmr_info {
u8 curr_ssv; /* current send state variable val */
u8 curr_rsv; /* current receive state variable */
u8 ind_bits; /* indicator bits set with macro */
-};
+} __attribute__((packed));
extern void llc_pdu_set_cmd_rsp(struct sk_buff *skb, u8 type);
extern void llc_pdu_set_pf_bit(struct sk_buff *skb, u8 bit_value);
--
1.7.3.4
^ permalink raw reply related
* Get wireless network card or check whether an interface is a wireless card
From: Chin Shi Hong @ 2011-05-12 13:51 UTC (permalink / raw)
To: netdev
Dear all,
My application must display only wireless (802.11) network interface
card. Currently, by using some ioctl calls and ifreq structure, my
application currently can detect any network interface. But, I only
want my application to detect wireless network interface card.
What are the ioctl calls or c/c++ codes to do this?
Regards,
^ permalink raw reply
* [PATCH net-next] net:set valid name before calling ndo_init()
From: Weiping Pan(潘卫平) @ 2011-05-12 13:39 UTC (permalink / raw)
Cc: Pan Weiping, David S. Miller, Eric Dumazet, Tom Herbert,
Michał Mirosław, Ben Hutchings,
open list:NETWORKING [GENERAL], open list
From: Pan Weiping <panweiping3@gmail.com>
A bug of bonding was invloved by e815d19ffe02bdfda1260949ef2b1806171,
see example 1 and 2.
In register_netdevice(), the name of net_device is not valid until
dev_get_valid_name() is called. But dev->netdev_ops->ndo_init(that is
bond_init) is called before dev_get_valid_name(),
and it uses the invalid name of net_device.
I think register_netdevice() should make sure that the name of net_device is
valid before calling ndo_init().
example 1:
modprobe bonding
ls /proc/net/bonding/bond%d
ps -eLf
root 3398 2 3398 0 1 21:34 ? 00:00:00 [bond%d]
example 2:
modprobe bonding max_bonds=3
[ 170.100292] bonding: Ethernet Channel Bonding Driver: v3.7.1 (April 27, 2011)
[ 170.101090] bonding: Warning: either miimon or arp_interval and arp_ip_target module parameters must be specified, otherwise bonding will not detect link failures! see bonding.txt for details.
[ 170.102469] ------------[ cut here ]------------
[ 170.103150] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
[ 170.104075] Hardware name: VirtualBox
[ 170.105065] proc_dir_entry 'bonding/bond%d' already registered
[ 170.105613] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
[ 170.108397] Pid: 3457, comm: modprobe Not tainted 2.6.39-rc2+ #14
[ 170.108935] Call Trace:
[ 170.109382] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
[ 170.109911] [<c051a42a>] ? proc_register+0x126/0x157
[ 170.110329] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
[ 170.110846] [<c051a42a>] proc_register+0x126/0x157
[ 170.111870] [<c051a4dd>] proc_create_data+0x82/0x98
[ 170.112335] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
[ 170.112905] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
[ 170.113319] [<c0721ac6>] register_netdevice+0x8c/0x1d3
[ 170.113848] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
[ 170.114322] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
[ 170.114879] [<c0401240>] do_one_initcall+0x76/0x122
[ 170.115317] [<f94f4000>] ? 0xf94f3fff
[ 170.115799] [<c0463f1e>] sys_init_module+0x1286/0x140d
[ 170.116879] [<c07c6d9f>] sysenter_do_call+0x12/0x28
[ 170.117404] ---[ end trace 64e4fac3ae5fff1a ]---
[ 170.117924] bond%d: Warning: failed to register to debugfs
[ 170.128728] ------------[ cut here ]------------
[ 170.129360] WARNING: at /home/pwp/net-next-2.6/fs/proc/generic.c:586 proc_register+0x126/0x157()
[ 170.130323] Hardware name: VirtualBox
[ 170.130797] proc_dir_entry 'bonding/bond%d' already registered
[ 170.131315] Modules linked in: bonding(+) sunrpc ipv6 uinput microcode ppdev parport_pc parport joydev e1000 pcspkr i2c_piix4 i2c_core [last unloaded: bonding]
[ 170.133731] Pid: 3457, comm: modprobe Tainted: G W 2.6.39-rc2+ #14
[ 170.134308] Call Trace:
[ 170.134743] [<c0438f3b>] warn_slowpath_common+0x6a/0x7f
[ 170.135305] [<c051a42a>] ? proc_register+0x126/0x157
[ 170.135820] [<c0438fc3>] warn_slowpath_fmt+0x2b/0x2f
[ 170.137168] [<c051a42a>] proc_register+0x126/0x157
[ 170.137700] [<c051a4dd>] proc_create_data+0x82/0x98
[ 170.138174] [<f94e6af6>] bond_create_proc_entry+0x3f/0x73 [bonding]
[ 170.138745] [<f94dd806>] bond_init+0x77/0xa5 [bonding]
[ 170.139278] [<c0721ac6>] register_netdevice+0x8c/0x1d3
[ 170.139828] [<f94e0e30>] bond_create+0x6c/0x90 [bonding]
[ 170.140361] [<f94f4763>] bonding_init+0x763/0x7b1 [bonding]
[ 170.140927] [<c0401240>] do_one_initcall+0x76/0x122
[ 170.141494] [<f94f4000>] ? 0xf94f3fff
[ 170.141975] [<c0463f1e>] sys_init_module+0x1286/0x140d
[ 170.142463] [<c07c6d9f>] sysenter_do_call+0x12/0x28
[ 170.142974] ---[ end trace 64e4fac3ae5fff1b ]---
[ 170.144949] bond%d: Warning: failed to register to debugfs
Signed-off-by: Pan Weiping <panweiping3@gmail.com>
---
net/core/dev.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 75898a3..f289117 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5412,6 +5412,10 @@ int register_netdevice(struct net_device *dev)
dev->iflink = -1;
+ ret = dev_get_valid_name(dev, dev->name);
+ if (ret < 0)
+ goto out;
+
/* Init, if this function is available */
if (dev->netdev_ops->ndo_init) {
ret = dev->netdev_ops->ndo_init(dev);
@@ -5422,10 +5426,6 @@ int register_netdevice(struct net_device *dev)
}
}
- ret = dev_get_valid_name(dev, dev->name);
- if (ret < 0)
- goto err_uninit;
-
dev->ifindex = dev_new_index(net);
if (dev->iflink == -1)
dev->iflink = dev->ifindex;
--
1.7.4
^ permalink raw reply related
* Re: ifreq flags or ioctl calls to turn an interface down?
From: Eric Dumazet @ 2011-05-12 13:32 UTC (permalink / raw)
To: Chin Shi Hong; +Cc: netdev
In-Reply-To: <BANLkTim=DPqz+pS3DDHw=C4MGMJmu6t=GA@mail.gmail.com>
Le jeudi 12 mai 2011 à 21:27 +0800, Chin Shi Hong a écrit :
> Dear all,
>
> I am developing an application that will turn a network interface down
> (something like command "ifconfig <network interface> down".
>
> What are the ifreq flags or ioctl calls to turn a network interface down?
>
> Or is there any other way to implement this feature into my application?
Quick answer :
strace ifconfig eth2 down
...
socket(PF_INET, SOCK_DGRAM, IPPROTO_IP) = 4
ioctl(4, SIOCGIFFLAGS, {ifr_name="eth2", ifr_flags=IFF_UP|IFF_BROADCAST|
IFF_RUNNING|IFF_MULTICAST}) = 0
ioctl(4, SIOCSIFFLAGS, {ifr_name="eth2", ifr_flags=IFF_BROADCAST|
IFF_RUNNING|IFF_MULTICAST}) = 0
^ permalink raw reply
* [PATCH net-next-2.6] garp: remove last synchronize_rcu() call
From: Eric Dumazet @ 2011-05-12 13:29 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, Ben Greear, netdev, Paul E. McKenney
In-Reply-To: <4DC831B0.80107@trash.net>
Le lundi 09 mai 2011 à 20:25 +0200, Patrick McHardy a écrit :
> Am 09.05.2011 15:35, schrieb Eric Dumazet:
> > Speedup vlan dismantling in CONFIG_VLAN_8021Q_GVRP=y cases,
> > by using a call_rcu() to free the memory instead of waiting with
> > expensive synchronize_rcu() [ while RTNL is held ]
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: Ben Greear <greearb@candelatech.com>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > ---
> > Note: I'll take care of using kfree_rcu() when available in net-next-2.6
> >
>
> Looks good to me.
>
Here is a followup on this patch, thanks !
[PATCH net-next-2.6] garp: remove last synchronize_rcu() call
When removing last vlan from a device, garp_uninit_applicant() calls
synchronize_rcu() to make sure no user can still manipulate struct
garp_applicant before we free it.
Use call_rcu() instead, as a step to further net_device dismantle
optimizations.
Add the temporary garp_cleanup_module() function to make sure no pending
call_rcu() are left at module unload time [ this will be removed when
kfree_rcu() is available ]
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ben Greear <greearb@candelatech.com>
---
include/net/garp.h | 1 +
net/802/garp.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/net/garp.h b/include/net/garp.h
index 8cabbf08..834d8ad 100644
--- a/include/net/garp.h
+++ b/include/net/garp.h
@@ -104,6 +104,7 @@ struct garp_applicant {
struct sk_buff_head queue;
struct sk_buff *pdu;
struct rb_root gid;
+ struct rcu_head rcu;
};
struct garp_port {
diff --git a/net/802/garp.c b/net/802/garp.c
index 5dbe896..f8300a8 100644
--- a/net/802/garp.c
+++ b/net/802/garp.c
@@ -603,6 +603,11 @@ err1:
}
EXPORT_SYMBOL_GPL(garp_init_applicant);
+static void garp_app_kfree_rcu(struct rcu_head *head)
+{
+ kfree(container_of(head, struct garp_applicant, rcu));
+}
+
void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl)
{
struct garp_port *port = rtnl_dereference(dev->garp_port);
@@ -611,7 +616,6 @@ void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl
ASSERT_RTNL();
rcu_assign_pointer(port->applicants[appl->type], NULL);
- synchronize_rcu();
/* Delete timer and generate a final TRANSMIT_PDU event to flush out
* all pending messages before the applicant is gone. */
@@ -621,7 +625,7 @@ void garp_uninit_applicant(struct net_device *dev, struct garp_application *appl
garp_queue_xmit(app);
dev_mc_del(dev, appl->proto.group_address);
- kfree(app);
+ call_rcu(&app->rcu, garp_app_kfree_rcu);
garp_release_port(dev);
}
EXPORT_SYMBOL_GPL(garp_uninit_applicant);
@@ -639,3 +643,9 @@ void garp_unregister_application(struct garp_application *appl)
stp_proto_unregister(&appl->proto);
}
EXPORT_SYMBOL_GPL(garp_unregister_application);
+
+static void __exit garp_cleanup_module(void)
+{
+ rcu_barrier(); /* Wait for completion of call_rcu()'s */
+}
+module_exit(garp_cleanup_module);
^ permalink raw reply related
* ifreq flags or ioctl calls to turn an interface down?
From: Chin Shi Hong @ 2011-05-12 13:27 UTC (permalink / raw)
To: netdev
Dear all,
I am developing an application that will turn a network interface down
(something like command "ifconfig <network interface> down".
What are the ifreq flags or ioctl calls to turn a network interface down?
Or is there any other way to implement this feature into my application?
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Marc Kleine-Budde @ 2011-05-12 13:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Subhasish Ghosh, nsekhar-l0cyMroinI0, open list,
CAN NETWORK DRIVERS, Wolfgang Grandegger,
Netdev-u79uwXL29TY76Z2rM5mHXA, m-watkins-l0cyMroinI0, Alan Cox
In-Reply-To: <201105121454.47781.arnd-r2nGTMty4D4@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1301 bytes --]
On 05/12/2011 02:54 PM, Arnd Bergmann wrote:
> On Thursday 12 May 2011, Wolfgang Grandegger wrote:
>> Well, that seems sophisticated resulting in a complex implementation
>> (may code line) also because hardware filters are very hardware
>> dependent. Usually just one global filter can be defined. I think that's
>> overkill. A simple interface using:
>>
>> ip link set can0 type can filter <id>:<mask> [<id>:<mask> ...]
>>
>> would just be fine.
>
> Ok, fair enough. Still I would suggest you first come up with
> a reasonable user interface (the one you posted may be just right,
> I don't know), and then let someone do the implementation in the
> pruss firmware that is the best match for the user interface, rather
> than the other way around.
I suggested some time ago to implement the more-or-less standard
<id>:<mask> filter instead of just a single <id> (per filter entry). The
cost is probably quite low, it's just a single AND operation per filter
entry more expensive.
regards, Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
[-- Attachment #2: Type: text/plain, Size: 188 bytes --]
_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core
^ permalink raw reply
* CONGRATS;
From: MICROSOFT @ 2011-05-12 12:57 UTC (permalink / raw)
To: microsoft_777
Dear Guaranteed Winner,
We are Pleased to inform you that you have won The Sum of Five Hundred Thousand Great British Pounds.
(500,000.00GBP) by Msn/Yahoo Lottery in conjunction with the MICROSOFT
WINDOWS.
we collects all the email addresses of the people that are active online,
among the millions that subscribed to Yahoo,Hotmail and various Microsoft
window users, we only select five people every year as
our winners, through electronic balloting system without the winner applying.
Ref Number: ...........XYL /26510460037/05
Batch Number: ................24/00319/IPD
Winning Number:.......................YM/09788
However you will have to fill and submit this form to the events manager
1.Full name..............
2.Contact Address........
3.Age....................
4.Telephone Number.......
5.Marital Status.........
6.Sex....................
7.Occupation.............
8.State:.................
9.Country................
10Winning E-mail Address...............
11.Amount Won.........
Your Reference and Batch number at the top of this mail:
(CONTACT EVENTS MANAGER)
Name: DR Don mike.
E-mail:clamunit55@hotmail.com
Tel:+447010047061
Congratulations from all members and staff of msn/yahoo lottery board.
^ permalink raw reply
* Re: [PATCH v4 1/1] can: add pruss CAN driver.
From: Arnd Bergmann @ 2011-05-12 12:54 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: sachi-EvXpCiN+lbve9wHmmfpqLFaTQe2KTcn/,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
Subhasish Ghosh, nsekhar-l0cyMroinI0, open list,
CAN NETWORK DRIVERS, Marc Kleine-Budde,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Netdev-u79uwXL29TY76Z2rM5mHXA, m-watkins-l0cyMroinI0, Alan Cox
In-Reply-To: <4DCB88A4.2010901-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
On Thursday 12 May 2011, Wolfgang Grandegger wrote:
> Well, that seems sophisticated resulting in a complex implementation
> (may code line) also because hardware filters are very hardware
> dependent. Usually just one global filter can be defined. I think that's
> overkill. A simple interface using:
>
> ip link set can0 type can filter <id>:<mask> [<id>:<mask> ...]
>
> would just be fine.
Ok, fair enough. Still I would suggest you first come up with
a reasonable user interface (the one you posted may be just right,
I don't know), and then let someone do the implementation in the
pruss firmware that is the best match for the user interface, rather
than the other way around.
Arnd
^ permalink raw reply
* Re: [PATCH net-next-2.6 2/2] be2net: fix mbox polling for signal reception
From: Ben Hutchings @ 2011-05-12 12:49 UTC (permalink / raw)
To: Sathya Perla; +Cc: netdev
In-Reply-To: <f816628b-a4b6-4a53-867e-12e75bc7b4de@exht1.ad.emulex.com>
On Thu, 2011-05-12 at 11:41 +0530, Sathya Perla wrote:
> Sending mbox cmds require multiple steps of writing to the DB register and polling
> for an ack. Gettting interrupted in the middle by a signal breaks the mbox protocol.
> So, set the task to UNINTERRUPTIBLE for mbox polling.
>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
> ---
> drivers/net/benet/be_cmds.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
> index bff41ed..55c8301 100644
> --- a/drivers/net/benet/be_cmds.c
> +++ b/drivers/net/benet/be_cmds.c
> @@ -297,7 +297,7 @@ static int be_mbox_db_ready_wait(struct be_adapter *adapter, void __iomem *db)
> return -1;
> }
>
> - set_current_state(TASK_INTERRUPTIBLE);
> + set_current_state(TASK_UNINTERRUPTIBLE);
> schedule_timeout(msecs_to_jiffies(1));
msleep(1) is a lot more readable.
Ben.
> msecs++;
> } while (true);
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Frank Blaschka @ 2011-05-12 11:59 UTC (permalink / raw)
To: Michał Mirosław; +Cc: davem, netdev, linux-s390
In-Reply-To: <BANLkTimUq-6E1F_ns_jTvXuYoPU-sLUmqw@mail.gmail.com>
On Thu, May 12, 2011 at 01:25:51PM +0200, Michał Mirosław wrote:
> W dniu 12 maja 2011 13:10 użytkownik Frank Blaschka
> <blaschka@linux.vnet.ibm.com> napisał:
> > On Thu, May 12, 2011 at 10:12:51AM +0200, Michał Mirosław wrote:
> >> W dniu 12 maja 2011 09:21 użytkownik Frank Blaschka
> >> <blaschka@linux.vnet.ibm.com> napisał:
> >> > On Thu, May 12, 2011 at 07:59:45AM +0200, Michał Mirosław wrote:
> >> >> 2011/5/12 <frank.blaschka@de.ibm.com>:
> >> >> > Set rx csum default to hw checksumming again.
> >> >> > Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
> >> >> > With the new hw_features it does not work to keep the old sysfs
> >> >> > interface in parallel. Convert options.checksum_type to new hw_features.
> >> >> [...]
> >> >> > + /* hw may have changed during offline or recovery */
> >> >> > + if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
> >> >> [...]
> >> >>
> >> >> This check should go to ndo_fix_features callback. If it fails then
> >> >> just return features &~NETIF_F_RXCSUM from there ...
> >> >>
> > Ok so all I have to do to complete this is to remove
> > card->dev->hw_features &= ~NETIF_F_RXCSUM;
> > from here (ndo_fix_feature has already the check)?
> >
> > Since we might be in recovery it is smarter to keep the support check here so we
> > do not have to use try and error approach to see if capabilities have
> > changed. Do you aggree?
>
> The key is that the driver should call netdev_update_features()
> whenever some conditions affecting available features might have
> changed. If you can keep the checks contained inside ndo_fix_features
> callback, you can avoid caring about the offloads in the recovery
> process.
Ok, we have this now.
>
> There's also the question what happens when packets are queued while
> there's ongoing recovery? Simplest way would be to drop them all until
> recovery completes.
Since this is RX processing there is nothing queued during recovery. But
you are right for tx csum or TSO we have to think about this. For now
I would like to complete this patch.
>
> Best Regards,
> Michał Mirosław
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" 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 net-next] ethtool: bring back missing comma in netdev_features_strings
From: Michał Mirosław @ 2011-05-12 11:33 UTC (permalink / raw)
To: Franco Fichtner; +Cc: Netdev, Mahesh Bandewar
In-Reply-To: <4DCBA4C5.5040308@lastsummer.de>
W dniu 12 maja 2011 11:13 użytkownik Franco Fichtner
<franco@lastsummer.de> napisał:
> On 12/05/11 10:55, Michał Mirosław wrote:
>> 2011/5/12 Franco Fichtner <franco@lastsummer.de>:
>>> I found this while skimming through recent net-next patches.
>>> I'm not quite sure if the padding in this struct is correct now.
>> Nice catch. But please send the patch inline instead of as an attachment.
> Thanks, but I have a short question. Are the empty strings in
> this struct really needed?
>
> /* NETIF_F_TSO6 */ "tx-tcp6-segmentation",
> /* NETIF_F_FSO */ "tx-fcoe-segmentation",
> "",
> "",
>
> /* NETIF_F_FCOE_CRC */ "tx-checksum-fcoe-crc",
>
> They seem to have caused this mess in the first place and I would
> think they could be removed entirely.
Mahesh Bandewar tried to do some cleanup in how feature bits are
defined using an enum, but it wasn't completed. If that cleanup was
completed, this array could be initialized using C99 style:
[NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
(This has more advantages than pure aesthetics.) For now those empty
strings have to stay, as the array positions are relevant.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Michał Mirosław @ 2011-05-12 11:25 UTC (permalink / raw)
To: Frank Blaschka; +Cc: davem, netdev, linux-s390
In-Reply-To: <20110512111009.GA4176@tuxmaker.boeblingen.de.ibm.com>
W dniu 12 maja 2011 13:10 użytkownik Frank Blaschka
<blaschka@linux.vnet.ibm.com> napisał:
> On Thu, May 12, 2011 at 10:12:51AM +0200, Michał Mirosław wrote:
>> W dniu 12 maja 2011 09:21 użytkownik Frank Blaschka
>> <blaschka@linux.vnet.ibm.com> napisał:
>> > On Thu, May 12, 2011 at 07:59:45AM +0200, Michał Mirosław wrote:
>> >> 2011/5/12 <frank.blaschka@de.ibm.com>:
>> >> > Set rx csum default to hw checksumming again.
>> >> > Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
>> >> > With the new hw_features it does not work to keep the old sysfs
>> >> > interface in parallel. Convert options.checksum_type to new hw_features.
>> >> [...]
>> >> > + /* hw may have changed during offline or recovery */
>> >> > + if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
>> >> [...]
>> >>
>> >> This check should go to ndo_fix_features callback. If it fails then
>> >> just return features &~NETIF_F_RXCSUM from there ...
>> >>
> Ok so all I have to do to complete this is to remove
> card->dev->hw_features &= ~NETIF_F_RXCSUM;
> from here (ndo_fix_feature has already the check)?
>
> Since we might be in recovery it is smarter to keep the support check here so we
> do not have to use try and error approach to see if capabilities have
> changed. Do you aggree?
The key is that the driver should call netdev_update_features()
whenever some conditions affecting available features might have
changed. If you can keep the checks contained inside ndo_fix_features
callback, you can avoid caring about the offloads in the recovery
process.
There's also the question what happens when packets are queued while
there's ongoing recovery? Simplest way would be to drop them all until
recovery completes.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH] net/irda/ircomm_tty.c: Use flip buffers to deliver data
From: Alan Cox @ 2011-05-12 11:17 UTC (permalink / raw)
To: Amit Virdi
Cc: samuel, alan, davem, eric.dumazet, netdev, linux-kernel,
shiraz.hashim, armando.visconti, viresh.kumar
In-Reply-To: <1305198280-14449-1-git-send-email-amit.virdi@st.com>
On Thu, 12 May 2011 16:34:40 +0530
Amit Virdi <amit.virdi@st.com> wrote:
> use tty_insert_flip_string and tty_flip_buffer_push to deliver incoming data
> packets from the IrDA device instead of delivering the packets directly to the
> line discipline. Following later approach resulted in warning "Sleeping function
> called from invalid context".
>
> Signed-off-by: Amit Virdi <amit.virdi@st.com>
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Re: [patch 1/9] [PATCH] qeth: convert to hw_features part 2
From: Frank Blaschka @ 2011-05-12 11:10 UTC (permalink / raw)
To: Michał Mirosław; +Cc: davem, netdev, linux-s390
In-Reply-To: <BANLkTimAisv82ZbFb8D1AkL8vuK1V5zAXw@mail.gmail.com>
On Thu, May 12, 2011 at 10:12:51AM +0200, Michał Mirosław wrote:
> W dniu 12 maja 2011 09:21 użytkownik Frank Blaschka
> <blaschka@linux.vnet.ibm.com> napisał:
> > On Thu, May 12, 2011 at 07:59:45AM +0200, Michał Mirosław wrote:
> >> 2011/5/12 <frank.blaschka@de.ibm.com>:
> >> > Set rx csum default to hw checksumming again.
> >> > Remove sysfs interface for rx csum (checksumming) and TSO (large_send).
> >> > With the new hw_features it does not work to keep the old sysfs
> >> > interface in parallel. Convert options.checksum_type to new hw_features.
> >> [...]
> >> > + /* hw may have changed during offline or recovery */
> >> > + if (!qeth_is_supported(card, IPA_INBOUND_CHECKSUM)) {
> >> [...]
> >>
> >> This check should go to ndo_fix_features callback. If it fails then
> >> just return features &~NETIF_F_RXCSUM from there ...
> >>
Ok so all I have to do to complete this is to remove
card->dev->hw_features &= ~NETIF_F_RXCSUM;
from here (ndo_fix_feature has already the check)?
Since we might be in recovery it is smarter to keep the support check here so we
do not have to use try and error approach to see if capabilities have
changed. Do you aggree?
> >> > +update_feature:
> >> > + rtnl_lock();
> >> > + card->dev->hw_features &= ~NETIF_F_RXCSUM;
> >> > + card->dev->features &= ~NETIF_F_RXCSUM;
> >> > + netdev_update_features(card->dev);
> >> > + rtnl_unlock();
> >>
> >> ... and then this should be just:
> >> rtnl_lock();
> >> netdev_update_features(card->dev);
> >> rtnl_unlock();
> >>
> > This code is part of the recovery, assuming the recovery detects the hw
> > has lost it's rx csum capability I still don't understand how ndo_fix_features
> > come into play?
>
> ndo_fix_features is always called before ndo_set_features to alter
> features that are to be enabled. So in this case, it should check if
> checksumming is available and remove NETIF_F_RXCSUM from the set if it
> isn't. The resulting set (after further cleanup by core code) is
> passed to ndo_set_features if the features had changed. hw_features
> only control what can be requested by user.
>
> I assume that if the device can loose the checksumming capability
> during runtime, it can also regain it. Above solution will
> automatically reenable checksumming if its requested and the
> capability comes back.
>
> Best Regards,
> Michał Mirosław
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" 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
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