* Re: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
From: Steve Glendinning @ 2012-12-14 13:35 UTC (permalink / raw)
To: Ming Lei
Cc: Steve Glendinning, netdev, linux-usb, Oliver Neukum,
Greg Kroah-Hartman
In-Reply-To: <CACVXFVNf1EWfnQmDBDoF991cqJOb8g_Ws+tuyBJdE34rbO6GSw@mail.gmail.com>
On 11 December 2012 16:13, Ming Lei <ming.lei@canonical.com> wrote:
> On Tue, Dec 11, 2012 at 11:26 PM, Steve Glendinning
> <steve.glendinning@shawell.net> wrote:
>> +
>> + if (on)
>> + usb_autopm_get_interface_no_resume(dev->intf);
>> + else
>> + usb_autopm_put_interface_no_suspend(dev->intf);
>
> The above line should be
>
> usb_autopm_put_interface(dev->intf);
Thanks Ming, I've updated this.
> IMO, it is better to keep smsc95xx_info.manage_power as NULL
> for devices without FEATURE_AUTOSUSPEND, so that fewer code
> and follow the current .mange_power usage(see its comment).
>
> Currently, if the function pointer of manage_power is set, it means that
> the device supports USB autosuspend, but your trick will make the
> assumption not true for some smsc devices.
Oliver?
--
Steve Glendinning
^ permalink raw reply
* Re: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
From: Steve Glendinning @ 2012-12-14 13:38 UTC (permalink / raw)
To: Joe Perches
Cc: Steve Glendinning, netdev, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Ming Lei, Oliver Neukum, Greg Kroah-Hartman
In-Reply-To: <1355243276.24219.3.camel@joe-AO722>
On 11 December 2012 16:27, Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
> On Tue, 2012-12-11 at 15:26 +0000, Steve Glendinning wrote:
> []> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> []
>> + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
>> + if (ret < 0) {
>> + netdev_warn(dev->net, "Error reading RX_FIFO_INF");
>
> Please remember terminating newlines.
Oops, I've fixed this now in my tree.
> If you are always going to warn bad reads or writes,
> it'd be good to change smsc95xx_read_reg_nopm
> and write to emit the message.
>
> It saves ~3% of code, 1K
> $ size drivers/net/usb/smsc95xx.o*
> text data bss dec hex filename
> 27064 2724 6072 35860 8c14 drivers/net/usb/smsc95xx.o.new
> 27921 2724 6256 36901 9025 drivers/net/usb/smsc95xx.o.old
>
> Signed-off-by: Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
Thanks for this Joe, actually as the register access functions already
print a warning with the register number I don't think we need to log
this at the calling site at all. I'll rip out all the warnings that
don't add any significant new information (i.e. *why* the call may
have failed, or what its failure means).
--
Steve Glendinning
--
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: [PATCH V2] ndisc: Use more standard logging styles
From: Eric Dumazet @ 2012-12-14 13:58 UTC (permalink / raw)
To: Joe Perches
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Duan Jiong, Steffen Klassert,
netdev, linux-kernel
In-Reply-To: <5f9b97d8b6e1a5e1a4e1d7a3731017bae47fb31d.1355458714.git.joe@perches.com>
On Thu, 2012-12-13 at 20:23 -0800, Joe Perches wrote:
> The logging style in this file is "baroque".
>
> Convert indirect uses of net_<level>_ratelimited to
> simply use net_<level>_ratelimited.
>
> Add a nd_dbg macro and #define ND_DEBUG for the other
> generally inactivated logging messages.
>
> Make those inactivated messages emit only at KERN_DEBUG
> instead of other levels. Add "%s: " __func__ to all
> these nd_dbg macros and remove the embedded function
> names and prefixes.
>
> Signed-off-by: Joe Perches <joe@perches.com>
It seems its not a bug fix, so why are you sending this now ?
Is it so hard to understand the merge window rule ?
net-next is currently closed.
^ permalink raw reply
* [PATCH v2] inet: Fix kmemleak in tcp_v4/6_syn_recv_sock and dccp_v4/6_request_recv_sock
From: Christoph Paasch @ 2012-12-14 14:07 UTC (permalink / raw)
To: David Miller
Cc: Gerrit Renker, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev, dccp, Eric Dumazet
If in either of the above functions inet_csk_route_child_sock() or
__inet_inherit_port() fails, the newsk will not be freed:
unreferenced object 0xffff88022e8a92c0 (size 1592):
comm "softirq", pid 0, jiffies 4294946244 (age 726.160s)
hex dump (first 32 bytes):
0a 01 01 01 0a 01 01 02 00 00 00 00 a7 cc 16 00 ................
02 00 03 01 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<ffffffff8153d190>] kmemleak_alloc+0x21/0x3e
[<ffffffff810ab3e7>] kmem_cache_alloc+0xb5/0xc5
[<ffffffff8149b65b>] sk_prot_alloc.isra.53+0x2b/0xcd
[<ffffffff8149b784>] sk_clone_lock+0x16/0x21e
[<ffffffff814d711a>] inet_csk_clone_lock+0x10/0x7b
[<ffffffff814ebbc3>] tcp_create_openreq_child+0x21/0x481
[<ffffffff814e8fa5>] tcp_v4_syn_recv_sock+0x3a/0x23b
[<ffffffff814ec5ba>] tcp_check_req+0x29f/0x416
[<ffffffff814e8e10>] tcp_v4_do_rcv+0x161/0x2bc
[<ffffffff814eb917>] tcp_v4_rcv+0x6c9/0x701
[<ffffffff814cea9f>] ip_local_deliver_finish+0x70/0xc4
[<ffffffff814cec20>] ip_local_deliver+0x4e/0x7f
[<ffffffff814ce9f8>] ip_rcv_finish+0x1fc/0x233
[<ffffffff814cee68>] ip_rcv+0x217/0x267
[<ffffffff814a7bbe>] __netif_receive_skb+0x49e/0x553
[<ffffffff814a7cc3>] netif_receive_skb+0x50/0x82
This happens, because sk_clone_lock initializes sk_refcnt to 2, and thus
a single sock_put() is not enough to free the memory. Additionally, things
like xfrm, memcg, cookie_values,... may have been initialized.
We have to free them properly.
This is fixed by forcing a call to tcp_done(), ending up in
inet_csk_destroy_sock, doing the final sock_put(). tcp_done() is necessary,
because it ends up doing all the cleanup on xfrm, memcg, cookie_values,
xfrm,...
Before calling tcp_done, we have to set the socket to SOCK_DEAD, to
force it entering inet_csk_destroy_sock. To avoid the warning in
inet_csk_destroy_sock, inet_num has to be set to 0.
As inet_csk_destroy_sock does a dec on orphan_count, we first have to
increase it.
Calling tcp_done() allows us to remove the calls to
tcp_clear_xmit_timer() and tcp_cleanup_congestion_control().
A similar approach is taken for dccp by calling dccp_done().
This is in the kernel since 093d282321 (tproxy: fix hash locking issue
when using port redirection in __inet_inherit_port()), thus since
version >= 2.6.37.
Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
include/net/inet_connection_sock.h | 1 +
net/dccp/ipv4.c | 4 ++--
net/dccp/ipv6.c | 3 ++-
net/ipv4/inet_connection_sock.c | 16 ++++++++++++++++
net/ipv4/tcp_ipv4.c | 6 ++----
net/ipv6/tcp_ipv6.c | 3 ++-
6 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index ba1d361..1832927 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -318,6 +318,7 @@ extern void inet_csk_reqsk_queue_prune(struct sock *parent,
const unsigned long max_rto);
extern void inet_csk_destroy_sock(struct sock *sk);
+extern void inet_csk_prepare_forced_close(struct sock *sk);
/*
* LISTEN is a special case for poll..
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 176ecdb..4f9f5eb 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -439,8 +439,8 @@ exit:
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS);
return NULL;
put_and_exit:
- bh_unlock_sock(newsk);
- sock_put(newsk);
+ inet_csk_prepare_forced_close(newsk);
+ dccp_done(newsk);
goto exit;
}
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 56840b2..6e05981 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -585,7 +585,8 @@ static struct sock *dccp_v6_request_recv_sock(struct sock *sk,
newinet->inet_rcv_saddr = LOOPBACK4_IPV6;
if (__inet_inherit_port(sk, newsk) < 0) {
- sock_put(newsk);
+ inet_csk_prepare_forced_close(newsk);
+ dccp_done(newsk);
goto out;
}
__inet6_hash(newsk, NULL);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 2026542..d0670f0 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -710,6 +710,22 @@ void inet_csk_destroy_sock(struct sock *sk)
}
EXPORT_SYMBOL(inet_csk_destroy_sock);
+/* This function allows to force a closure of a socket after the call to
+ * tcp/dccp_create_openreq_child().
+ */
+void inet_csk_prepare_forced_close(struct sock *sk)
+{
+ /* sk_clone_lock locked the socket and set refcnt to 2 */
+ bh_unlock_sock(sk);
+ sock_put(sk);
+
+ /* The below has to be done to allow calling inet_csk_destroy_sock */
+ sock_set_flag(sk, SOCK_DEAD);
+ percpu_counter_inc(sk->sk_prot->orphan_count);
+ inet_sk(sk)->inet_num = 0;
+}
+EXPORT_SYMBOL(inet_csk_prepare_forced_close);
+
int inet_csk_listen_start(struct sock *sk, const int nr_table_entries)
{
struct inet_sock *inet = inet_sk(sk);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1ed2307..54139fa 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1767,10 +1767,8 @@ exit:
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_LISTENDROPS);
return NULL;
put_and_exit:
- tcp_clear_xmit_timers(newsk);
- tcp_cleanup_congestion_control(newsk);
- bh_unlock_sock(newsk);
- sock_put(newsk);
+ inet_csk_prepare_forced_close(newsk);
+ tcp_done(newsk);
goto exit;
}
EXPORT_SYMBOL(tcp_v4_syn_recv_sock);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6565cf5..93825dd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1288,7 +1288,8 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
#endif
if (__inet_inherit_port(sk, newsk) < 0) {
- sock_put(newsk);
+ inet_csk_prepare_forced_close(newsk);
+ tcp_done(newsk);
goto out;
}
__inet6_hash(newsk, NULL);
--
1.7.10.4
^ permalink raw reply related
* Re: netconsole fun
From: Neil Horman @ 2012-12-14 14:20 UTC (permalink / raw)
To: Peter Hurley; +Cc: Cong Wang, netdev
In-Reply-To: <1355437480.2612.33.camel@thor>
On Thu, Dec 13, 2012 at 05:24:40PM -0500, Peter Hurley wrote:
> On Thu, 2012-12-13 at 16:17 -0500, Neil Horman wrote:
> > On Thu, Dec 13, 2012 at 02:27:01PM -0500, Peter Hurley wrote:
> > > On Thu, 2012-12-13 at 13:08 -0500, Neil Horman wrote:
> > > > On Thu, Dec 13, 2012 at 09:49:31AM -0500, Peter Hurley wrote:
> > > > > On Thu, 2012-12-13 at 07:36 -0500, Neil Horman wrote:
> > > > > > On Wed, Dec 12, 2012 at 03:59:17PM -0500, Peter Hurley wrote:
> > > > > > > On Tue, 2012-12-11 at 11:45 -0500, Neil Horman wrote:
> > > > > > > > On Tue, Dec 11, 2012 at 10:16:51AM -0500, Peter Hurley wrote:
> > > > > > > > > On Tue, 2012-12-11 at 09:30 -0500, Neil Horman wrote:
> > > > > > > > > > On Tue, Dec 11, 2012 at 09:19:52AM -0500, Peter Hurley wrote:
> > > > > > > > > > > On Tue, 2012-12-11 at 04:51 +0000, Cong Wang wrote:
> > > > > > > > > > > > On Mon, 10 Dec 2012 at 14:17 GMT, Peter Hurley <peter@hurleysoftware.com> wrote:
> > > > > > > > > > > > > Now that netpoll has been disabled for slaved devices, is there a
> > > > > > > > > > > > > recommended method of running netconsole on a machine that has a slaved
> > > > > > > > > > > > > device?
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Yes, running it on the master device instead.
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the suggestion, but:
> > > > > > > > > > >
> > > > > > > > > > > [ 0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-3.7.0-rc8-xeon ...... netconsole=@192.168.10.99/br0,30000@192.168.10.100/xx:xx:xx:xx:xx:xx
> > > > > > > > > > > ...
> > > > > > > > > > > [ 5.289869] netpoll: netconsole: local port 6665
> > > > > > > > > > > [ 5.289885] netpoll: netconsole: local IP 192.168.10.99
> > > > > > > > > > > [ 5.289892] netpoll: netconsole: interface 'br0'
> > > > > > > > > > > [ 5.289898] netpoll: netconsole: remote port 30000
> > > > > > > > > > > [ 5.289907] netpoll: netconsole: remote IP 192.168.10.100
> > > > > > > > > > > [ 5.289914] netpoll: netconsole: remote ethernet address xx:xx:xx:xx:xx:xx
> > > > > > > > > > > [ 5.289922] netpoll: netconsole: br0 doesn't exist, aborting
> > > > > > > > > > > [ 5.289929] netconsole: cleaning up
> > > > > > > > > > > ...
> > > > > > > > > > > [ 9.392291] Bridge firewalling registered
> > > > > > > > > > > [ 9.396805] device eth1 entered promiscuous mode
> > > > > > > > > > > [ 9.418350] eth1: setting full-duplex.
> > > > > > > > > > > [ 9.421268] br0: port 1(eth1) entered forwarding state
> > > > > > > > > > > [ 9.423354] br0: port 1(eth1) entered forwarding state
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Is there a way to control or associate network device names prior to
> > > > > > > > > > > udev renaming?
> > > > > > > > > > >
> > > > > > > > > > That looks like a systemd problem (or more specifically a boot dependency
> > > > > > > > > > problem). You need to modify your netconsole unit/service file to start after
> > > > > > > > > > all your networking is up. NetworkManager provides a dummy service file for
> > > > > > > > > > this purpose, called networkmanager-wait-online.service
> > > > > > > > >
> > > > > > > > > Ok. So with a single physical network interface that will be bridged,
> > > > > > > > > netconsole cannot used for kernel boot messages.
> > > > > > > > >
> > > > > > > > > With a machine with multiple nics, is there a way to control device
> > > > > > > > > naming so that the interface name to be used by netconsole specified on
> > > > > > > > > the boot command line will actually corresponding to the intended
> > > > > > > > > device. For example,
> > > > > > > > >
> > > > > > > > > [ 0.000000] Kernel command line: BOOT_IMAGE=/boot/vmlinuz-3.7.0-rc8-xeon ...... netconsole=@192.168.1.123/eth0,30000@192.168.1.139/xx:xx:xx:xx:xx:xx
> > > > > > > > > ....
> > > > > > > > > [ 4.092184] 3c59x: Donald Becker and others.
> > > > > > > > > [ 4.092204] 0000:07:05.0: 3Com PCI 3c905C Tornado at ffffc9000186cf80.
> > > > > > > > > [ 4.094035] tg3.c:v3.125 (September 26, 2012)
> > > > > > > > > ....
> > > > > > > > > [ 4.125038] tg3 0000:08:00.0 eth1: Tigon3 [partno(BCM95754) rev b002] (PCI Express) MAC address xx:xx:xx:xx:xx:xx
> > > > > > > > > [ 4.125055] tg3 0000:08:00.0 eth1: attached PHY is 5787 (10/100/1000Base-T Ethernet) (WireSpeed[1], EEE[0])
> > > > > > > > > [ 4.125062] tg3 0000:08:00.0 eth1: RXcsums[1] LinkChgREG[0] MIirq[0] ASF[0] TSOcap[1]
> > > > > > > > > [ 4.125068] tg3 0000:08:00.0 eth1: dma_rwctrl[76180000] dma_mask[64-bit]
> > > > > > > > >
> > > > > > > > > This is attaching netconsole to the wrong device because bus
> > > > > > > > > enumeration, and therefore load order, is not consistent from boot to
> > > > > > > > > boot.
> > > > > > > > >
> > > > > > > > No, theres no way to do that. As you note device ennumeration isn't consistent
> > > > > > > > accross boots, thats why udev creates rules to rename devices based on immutable
> > > > > > > > (or semi-immutable) data, like mac addresses, or pci bus locations). Once that
> > > > > > > > happens, you'll have consistent names for your interfaces, and that work will be
> > > > > > > > guaranteed to be done after networkmanager has finished opening all the
> > > > > > > > interfaces that it needs (hence my suggestion to make netconsole service
> > > > > > > > dependent on networkmanager service startup completing).
> > > > > > >
> > > > > > > Just wondering if you think something like the patch below is
> > > > > > > suitable/acceptable for insulating netconsole from inconsistent device
> > > > > > > name scenarios without changing the existing semantics. The basic idea
> > > > > > > is to allow an ethernet MAC address in the <dev> field of the
> > > > > > > netconsole= options, and if a MAC address was specified rather than a
> > > > > > > device name, to do the dev lookup from the MAC address instead.
> > > > > > >
> > > > > > > This doesn't extend to, but also doesn't interfere with, the dynamic
> > > > > > > config of netconsole via configfs.
> > > > > > >
> > > > > > > Would you mind reviewing it?
> > > > > > >
> > > > > > > Regards,
> > > > > > > Peter
> > > > > > >
> > > > > > This looks like a pretty good idea to me. That said, something occured to me
> > > > > > when you wrote your summary above. Have you looked at the netconsole service
> > > > > > scripts that most distros provide in their packaging? I'm almost positive Red
> > > > > > Hat/Fedora (and also like Suse and Ubuntu), already implement this functionality
> > > > > > from user space. Basically, instead of people just modprobing netconsole, they
> > > > > > create a service script that parses a config file that has contains all the
> > > > > > options needed to load the netconsole module, and it has the intellegence to see
> > > > > > if you specified a mac address rather than a device. If you did that it finds
> > > > > > the corresponding device mac address and uses that as the device. I'm sorry, I
> > > > > > don't know why I didn't think of that before. Check that out though, that will
> > > > > > likey give you exactly what you need
> > > > >
> > > > > Even with a udev rule to load netconsole that runs immediately after
> > > > > device renaming (so before scripting), most of the dynamic module
> > > > > loading has already happened so netconsole misses it. At least with the
> > > > > patch, netconsole will load and attach to the proper interface much
> > > > > earlier in the boot so that module-load-time messages will be caught.
> > > > >
> > > > I'm not sure what you mean by this.
> > >
> > > This is the beginning of my netconsole log if I use userspace scripts to
> > > start it.
> > >
> > > [ 19.125314] ip_tables: (C) 2000-2006 Netfilter Core Team
> > > [ 20.060925] nf_conntrack version 0.5.0 (16384 buckets, 65536 max)
> > > [ 21.829331] ip6_tables: (C) 2000-2006 Netfilter Core Team
> > > [ 25.728370] at-spi-registry[1862]: segfault at 18 ip 00007f6dd1dd45f1 sp 00007fff49bcd760 error 4 in libgconf-2.so.4.1.5[7f6dd1dbd000+2d000]
> > > [ 26.778848] EXT4-fs (dm-3): re-mounted. Opts: errors=remount-ro,commit=0
> > > [ 30.643469] Bluetooth: RFCOMM TTY layer initialized
> > > [ 30.643509] Bluetooth: RFCOMM socket layer initialized
> > > [ 30.643512] Bluetooth: RFCOMM ver 1.11
> > > [ 30.784550] Bluetooth: BNEP (Ethernet Emulation) ver 1.3
> > > [ 30.784567] Bluetooth: BNEP filters: protocol multicast
> > > [ 30.784584] Bluetooth: BNEP socket layer initialized
> > > [ 34.010813] init: plymouth-stop pre-start process (2205) terminated with status 1
> > >
> > > This is the beginning of my netconsole log if I am able to specify
> > > netconsole= options on the boot command line. Netconsole starts logging
> > > much earlier because it is much loaded earlier.
> > >
> > > [ 8.764336] EXT4-fs (dm-4): mounted filesystem with ordered data mode. Opts: (null)
> > > [ 9.409379] firewire_core 0000:07:06.0: created device fw1: GUID 0800460301c2d69e, S400
> > > [ 9.567395] init: ureadahead main process (500) terminated with status 5
> > > [ 10.400338] Adding 10996456k swap on /dev/mapper/isw_cbdbfhdjad_Raid0p5. Priority:-1 extents:1 across:10996456k
> > > [ 10.496974] udevd[541]: starting version 173
> > > [ 10.725906] EXT4-fs (dm-4): re-mounted. Opts: errors=remount-ro
> > > [ 11.288352] lp: driver loaded but no devices found
> > > [ 12.240058] parport_pc 00:05: reported by Plug and Play ACPI
> > > [ 12.240145] parport0: PC-style at 0x378 (0x778), irq 7, using FIFO [PCSPP,TRISTATE,COMPAT,ECP]
> > > [ 12.336161] lp0: using parport0 (interrupt-driven).
> > > [ 12.342867] microcode: CPU0 sig=0x10676, pf=0x40, revision=0x60f
> > > [ 12.436657] shpchp: Standard Hot Plug PCI Controller Driver version: 0.4
> > > [ 12.442245] ppdev: user-space parallel port driver
> > > [ 12.451592] net firewire0: IPv4 over IEEE 1394 on card 0000:07:06.0
> > >
> > > Does that make more sense now?
> > >
> > No, actually, what exactly are you trying to show me here? I don't see any
> > indication of netconsole doing anything in either of these log snippets.
> ^^^^^^^^^^^^^^^^^^^^^^
> except that it was netconsole that wrote these logs
>
> Note the log times.
>
> In the first log, which is from a netconsole loaded by userspace
> scripts, the first printk it logs isn't until 19.125314 secs into boot.
> Most kernel + module init has already happened by this time.
>
> In the second log, which is from a netconsole (still built as a module)
> loaded as a result of using the boot command line. It starts logging at
> 8.764336 secs into boot -- almost 10 secs earlier than using userspace
> scripting to load netconsole.
>
> > I'm
> > also not sure why you're specifying netconsole options on the kernel command
> > line at all.
> > Can you elaborate?
>
> Specifying netconsole= on the boot command line loads the netconsole
> module at the earliest possible time (and much earlier than scripting
> will do). And also leaves it as an optional configuration (at least for
> me it does since I use grub2 and can edit the boot command line before
> booting).
>
> AFAIK, there are 5 ways to load netconsole:
> 1. On the boot command line with netconsole=
> If netconsole is built-in, this is the only way to initialize it.
> If netconsole is a module, this forces netconsole to load at the
> earliest possible time.
> 2. Via /etc/modules
> This happens before network device renaming, so suffers from the
> problems we've been discussing.
> 3. Via a custom udev rule in /etc/udev/rules.d/
> Earliest userspace method to modprobe netconsole and can be used
> after device renaming, but is still fairly late in the boot process.
> 4. Via init/service scripting
> 5. At the user shell
>
> AFAIK, there are 4 ways to specify the necessary options to netconsole:
> a. On the boot command line with netconsole=
> Also loads netconsole
> b. In a .conf file in /etc/modprobe.d
> c. On the modprobe command line
> d. Via configfs
>
Ah! I'm sorry, I didn't realize this was really about getting netconsole up
early in the boot, rather than just getting it up robustly using the startup
script. If thats the case, then I would recommend that you modify the initramfs
to do something simmilar to the startup script (since thats where the netconsole
module will get loaded anyway). You can write a script there that will let you
specify the destination ip address and figure out the output dev based on the
routing tables. If you're using dracut to build your initramfs, then this
should be pretty straightforward.
Neil
> Regards,
> Peter
>
>
>
> --
> 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
* Re: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
From: Oliver Neukum @ 2012-12-14 14:22 UTC (permalink / raw)
To: Steve Glendinning
Cc: Ming Lei, Steve Glendinning, netdev,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <CAKh2mn7C3r_bvKAv4p3AHco=9JpmewOAJ8JwhtFPw50yDy5cUA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Friday 14 December 2012 13:35:04 Steve Glendinning wrote:
> Thanks Ming, I've updated this.
Very good. Good catch Ming.
> > IMO, it is better to keep smsc95xx_info.manage_power as NULL
> > for devices without FEATURE_AUTOSUSPEND, so that fewer code
> > and follow the current .mange_power usage(see its comment).
> >
> > Currently, if the function pointer of manage_power is set, it means that
> > the device supports USB autosuspend, but your trick will make the
manage_power() can return errors. So the assumption was always
strictly speaking invalid.
> > assumption not true for some smsc devices.
>
> Oliver?
On second thought, I think that if a driver can do manage_power(), even
only for a subset of devices, it should implement it. Doctoring the table of methods
is very, very ugly, especially as this not protected by a lock.
But this is not nice. We need a better way.
Regards
Oliver
--
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
* [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference
From: Daniel Borkmann @ 2012-12-14 14:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Vlad Yasevich
In-Reply-To: <cover.1355494956.git.dborkman@redhat.com>
During debugging a sctp problem, I hit a kernel NULL pointer
dereference in the jprobes callback jsctp_sf_eat_sack(). This
small patch fixes the panic.
Cc: Vlad Yasevich <vyasevich@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/probe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/probe.c b/net/sctp/probe.c
index bc6cd75..0a4e9d6 100644
--- a/net/sctp/probe.c
+++ b/net/sctp/probe.c
@@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep,
sp = asoc->peer.primary_path;
- if ((full || sp->cwnd != lcwnd) &&
+ if (sp && (full || sp->cwnd != lcwnd) &&
(!port || asoc->peer.port == port ||
ep->base.bind_addr.port == port)) {
lcwnd = sp->cwnd;
--
1.7.11.7
^ permalink raw reply related
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Jiri Pirko @ 2012-12-14 14:41 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Flavio Leitner, John Fastabend, netdev, davem, edumazet,
bhutchings, mirqus, greearb
In-Reply-To: <20121213102051.72ad69aa@nehalam.linuxnetplumber.net>
Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
>On Thu, 13 Dec 2012 16:17:33 -0200
>Flavio Leitner <fbl@redhat.com> wrote:
>
>> On Thu, 13 Dec 2012 10:09:33 -0800
>> Stephen Hemminger <shemminger@vyatta.com> wrote:
>>
>> > On Thu, 13 Dec 2012 15:54:23 -0200
>> > Flavio Leitner <fbl@redhat.com> wrote:
>> >
>> > > I am saying this because people are used to and there are scripts out
>> > > there using something like:
>> > > # ethtool <iface> | grep 'Link'
>> > > to react an interface failure.
>> >
>> > Then the script is broken. It is asking about hardware state.
>>
>> I was talking about the team master interface, so it makes sense
>> to check its 'hardware' state. Just think on 'bond0' interface
>> with no slaves. It should report Link detected: no.
>>
>> See bond_release(), what happens if bond->slave_cnt == 0, for instance.
>>
>
>I was thinking more that ethtool operation for reporting link on
>the team device should use the proper check rather than just using netif_carrier_ok(),
>the team ethtool operation for get_link should be check IFF_RUNNING flag
>in dev->flags which is controlled by operstate transistions.
I admit I'm bit confused now.
For example in bridge code:
in br_add_if() - netif_carrier_ok() is checked and by the value it is
decided if br_stp_enable_port() is called or not. Wouldn't it make more
sense to check IFF_RUNNING (or netif_oper_up()) here?
The reason I'm asing is that if team device is in bridge, carrier is
always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
userspace, in current code, bridge wouldn't know the difference...
There are more exmaples of similar usage of netif_carrier_ok() in
bridge (called on ports), bonding (called on slaves), team code (called on ports).
^ permalink raw reply
* Dealing nicely with PM failure
From: Oliver Neukum @ 2012-12-14 14:56 UTC (permalink / raw)
To: Ming Lei, Steve Glendinning, netdev
Hi,
this question about the smsc devices gave me an idea.
How about this?
>From f1bdff89eea4c10102f867f9f22cb2b60519e425 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver@neukum.org>
Date: Fri, 14 Dec 2012 15:51:36 +0100
Subject: [PATCH] usbnet: handle PM failure gracefully
If a device fails to do remote wakeup, this is no reason
to abort an open totally. This patch just continues without
runtime PM.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
drivers/net/usb/usbnet.c | 17 +++++++++--------
include/linux/usb/usbnet.h | 1 +
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index c04110b..2010eef 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -719,7 +719,8 @@ int usbnet_stop (struct net_device *net)
dev->flags = 0;
del_timer_sync (&dev->delay);
tasklet_kill (&dev->bh);
- if (info->manage_power)
+ if (info->manage_power &&
+ !test_and_clear_bit(EVENT_NO_RUNTIME_PM, &dev->flags))
info->manage_power(dev, 0);
else
usb_autopm_put_interface(dev->intf);
@@ -793,15 +794,15 @@ int usbnet_open (struct net_device *net)
// delay posting reads until we're fully open
tasklet_schedule (&dev->bh);
if (info->manage_power) {
- retval = info->manage_power(dev, 1);
- if (retval < 0)
- goto done_manage_power_error;
- usb_autopm_put_interface(dev->intf);
+ int r;
+
+ r = info->manage_power(dev, 1);
+ if (r < 0)
+ set_bit(EVENT_NO_RUNTIME_PM, &dev->flags);
+ else
+ usb_autopm_put_interface(dev->intf);
}
return retval;
-
-done_manage_power_error:
- clear_bit(EVENT_DEV_OPEN, &dev->flags);
done:
usb_autopm_put_interface(dev->intf);
done_nopm:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9bbeabf..288b32a 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -69,6 +69,7 @@ struct usbnet {
# define EVENT_DEV_ASLEEP 6
# define EVENT_DEV_OPEN 7
# define EVENT_DEVICE_REPORT_IDLE 8
+# define EVENT_NO_RUNTIME_PM 9
};
static inline struct usb_driver *driver_of(struct usb_interface *intf)
--
1.7.7
^ permalink raw reply related
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference
From: Vlad Yasevich @ 2012-12-14 15:12 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, netdev
In-Reply-To: <2813cf01509e495fee13ff1fab309f1186c0e57a.1355494956.git.dborkman@redhat.com>
On 12/14/2012 09:27 AM, Daniel Borkmann wrote:
> During debugging a sctp problem, I hit a kernel NULL pointer
> dereference in the jprobes callback jsctp_sf_eat_sack(). This
> small patch fixes the panic.
>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> net/sctp/probe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
> index bc6cd75..0a4e9d6 100644
> --- a/net/sctp/probe.c
> +++ b/net/sctp/probe.c
> @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep,
>
> sp = asoc->peer.primary_path;
>
> - if ((full || sp->cwnd != lcwnd) &&
> + if (sp && (full || sp->cwnd != lcwnd) &&
> (!port || asoc->peer.port == port ||
> ep->base.bind_addr.port == port)) {
> lcwnd = sp->cwnd;
>
Sorry, but this patch isn't making much sense. @sp points to the
primary path of the association and that can not be null if we receiving
SACKs on this association.
sctp_sf_eat_sack_6_2(), which we are probing, is only called while the
association is valid and all the transports still exist. It is also
called under lock, so the transports can not go away while processing of
the SACK. So unless there is some kind of jprobe issue, the pointer
you are looking at should always be valid.
-vlad
^ permalink raw reply
* Re: [PATCH] netlink: align attributes on 64-bits
From: Ben Hutchings @ 2012-12-14 15:49 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: tgraf, netdev, davem, David.Laight
In-Reply-To: <1355491002-3931-1-git-send-email-nicolas.dichtel@6wind.com>
On Fri, 2012-12-14 at 14:16 +0100, Nicolas Dichtel wrote:
> On 64 bits arch, we must ensure that attributes are always aligned on 64-bits
> boundary. We do that by adding attributes of type 0, size 4 (alignment on
> 32-bits is already done) when needed. Attribute type 0 should be available and
> unused in all netlink families.
[...]
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -450,9 +450,18 @@ EXPORT_SYMBOL(__nla_put_nohdr);
> */
> int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
> {
> - if (unlikely(skb_tailroom(skb) < nla_total_size(attrlen)))
> + int align = IS_ALIGNED((unsigned long)skb_tail_pointer(skb), sizeof(void *)) ? 0 : 4;
The assumption here is that nothing needs to be aligned to a greater
width than that of a pointer. However, for most 32-bit architectures
(i386 being an exception) the C ABI requires 64-bit alignment for 64-bit
types. There may be cases where a mostly 32-bit processor really
requires 64-bit alignment, e.g. to load or save a pair of registers.
Ben.
> + if (unlikely(skb_tailroom(skb) < nla_total_size(attrlen) + align))
> return -EMSGSIZE;
>
> + if (align) {
> + /* Goal is to add an attribute with size 4. We know that
> + * NLA_HDRLEN is 4, hence payload is 0.
> + */
> + __nla_reserve(skb, 0, 0);
> + }
> +
> __nla_put(skb, attrtype, attrlen, data);
> return 0;
> }
--
Ben Hutchings, Staff 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] netlink: align attributes on 64-bits
From: Nicolas Dichtel @ 2012-12-14 16:04 UTC (permalink / raw)
To: Ben Hutchings; +Cc: tgraf, netdev, davem, David.Laight
In-Reply-To: <1355500160.2626.9.camel@bwh-desktop.uk.solarflarecom.com>
Le 14/12/2012 16:49, Ben Hutchings a écrit :
> On Fri, 2012-12-14 at 14:16 +0100, Nicolas Dichtel wrote:
>> On 64 bits arch, we must ensure that attributes are always aligned on 64-bits
>> boundary. We do that by adding attributes of type 0, size 4 (alignment on
>> 32-bits is already done) when needed. Attribute type 0 should be available and
>> unused in all netlink families.
> [...]
>> --- a/lib/nlattr.c
>> +++ b/lib/nlattr.c
>> @@ -450,9 +450,18 @@ EXPORT_SYMBOL(__nla_put_nohdr);
>> */
>> int nla_put(struct sk_buff *skb, int attrtype, int attrlen, const void *data)
>> {
>> - if (unlikely(skb_tailroom(skb) < nla_total_size(attrlen)))
>> + int align = IS_ALIGNED((unsigned long)skb_tail_pointer(skb), sizeof(void *)) ? 0 : 4;
>
> The assumption here is that nothing needs to be aligned to a greater
> width than that of a pointer. However, for most 32-bit architectures
> (i386 being an exception) the C ABI requires 64-bit alignment for 64-bit
> types. There may be cases where a mostly 32-bit processor really
> requires 64-bit alignment, e.g. to load or save a pair of registers.
Ok, I will wait other comments to send a v2 (which will align these attributes
for all arch).
Nicolas
^ permalink raw reply
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Stephen Hemminger @ 2012-12-14 16:12 UTC (permalink / raw)
To: Jiri Pirko
Cc: Flavio Leitner, John Fastabend, netdev, davem, edumazet,
bhutchings, mirqus, greearb
In-Reply-To: <20121214144134.GA1652@minipsycho.brq.redhat.com>
On Fri, 14 Dec 2012 15:41:34 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
> >On Thu, 13 Dec 2012 16:17:33 -0200
> >Flavio Leitner <fbl@redhat.com> wrote:
> >
> >> On Thu, 13 Dec 2012 10:09:33 -0800
> >> Stephen Hemminger <shemminger@vyatta.com> wrote:
> >>
> >> > On Thu, 13 Dec 2012 15:54:23 -0200
> >> > Flavio Leitner <fbl@redhat.com> wrote:
> >> >
> >> > > I am saying this because people are used to and there are scripts out
> >> > > there using something like:
> >> > > # ethtool <iface> | grep 'Link'
> >> > > to react an interface failure.
> >> >
> >> > Then the script is broken. It is asking about hardware state.
> >>
> >> I was talking about the team master interface, so it makes sense
> >> to check its 'hardware' state. Just think on 'bond0' interface
> >> with no slaves. It should report Link detected: no.
> >>
> >> See bond_release(), what happens if bond->slave_cnt == 0, for instance.
> >>
> >
> >I was thinking more that ethtool operation for reporting link on
> >the team device should use the proper check rather than just using netif_carrier_ok(),
> >the team ethtool operation for get_link should be check IFF_RUNNING flag
> >in dev->flags which is controlled by operstate transistions.
>
> I admit I'm bit confused now.
>
> For example in bridge code:
> in br_add_if() - netif_carrier_ok() is checked and by the value it is
> decided if br_stp_enable_port() is called or not. Wouldn't it make more
> sense to check IFF_RUNNING (or netif_oper_up()) here?
>
> The reason I'm asing is that if team device is in bridge, carrier is
> always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
> userspace, in current code, bridge wouldn't know the difference...
>
> There are more exmaples of similar usage of netif_carrier_ok() in
> bridge (called on ports), bonding (called on slaves), team code (called on ports).
Yes the bridge should be fixed to work with user controlled devices.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5] Ease netns management for userland
From: Nicolas Dichtel @ 2012-12-14 16:13 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev, davem, aatteka
In-Reply-To: <87mwxh6a8y.fsf@xmission.com>
Le 13/12/2012 20:08, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> Le 12/12/2012 22:48, Eric W. Biederman a écrit :
>>> ebiederm@xmission.com (Eric W. Biederman) writes:
>>>
>>>> It is very wrong to presume that without context you know the reason for
>>>> the exsitence of any network namespace and that you should or even that
>>>> you can manage it. Think of running your multi-network namespace
>>>> managing application in a container.
>>>
>>> A good example of a network namespace you don't want to mess with are
>>> the network namespaces created by vsftp and chrome for security purposes
>>> to remove any possibility of creating new connections to the network.
>>>
>> Ok, I get the point.
>>
>> A last question: from an administration point of view, is it intended to
>> not be able to monitor which netns are currently used? Like it can be done
>> for sockets, files, ...
>
> No. The difficulty monitoring which network namespaces are being used
> is an unintended side effect.
Why is netlink a bad idea? Having a way to know all existing netns is a start
point to monitor netns, isn't it?
>
> My pending changes to /proc/<pid>/ns/net and friends that allow you to
> stat those files and compare if two network are the same network
> namespace should make that monitoring much easier. It isn't perfect as
> there currently isn't a way to take a socket and say which network
> namespace is this socket in. But the current solution should tell you
> what is happening most of the time.
Yes, this will give interessing infos.
> struct net allocates it's own slab type so /proc/slabinfo on a good day
> can tell you how many network namespace structures have been allocated
> and are in use.
Ok.
Nicolas
^ permalink raw reply
* Re: [PATCH] bridge: remove temporary variable for MLDv2 maximum response code computation
From: Stephen Hemminger @ 2012-12-14 16:19 UTC (permalink / raw)
To: Ang Way Chuang; +Cc: netdev
In-Reply-To: <50CAEC97.2090609@sfc.wide.ad.jp>
On Fri, 14 Dec 2012 17:08:39 +0800
Ang Way Chuang <wcang@sfc.wide.ad.jp> wrote:
> As suggested by Stephen Hemminger, this remove the temporary variable introduced in commit
> eca2a43bb0d2c6ebd528be6acb30a88435abe307
>
> Signed-off-by: Ang Way Chuang <wcang@sfc.wide.ad.jp>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
^ permalink raw reply
* Re: [PATCHv2][RFC] smsc95xx: enable dynamic autosuspend
From: Ming Lei @ 2012-12-14 16:24 UTC (permalink / raw)
To: Oliver Neukum
Cc: Steve Glendinning, Steve Glendinning, netdev,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <4228102.c0z2lbILKD-ugxBuEnWX9yG/4A2pS7c2Q@public.gmane.org>
On Fri, Dec 14, 2012 at 10:22 PM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>
> On second thought, I think that if a driver can do manage_power(), even
> only for a subset of devices, it should implement it. Doctoring the table of methods
> is very, very ugly,
Sorry, why is it very ugly? netdev_ops/ethtool_ops is still set dynamically
inside bind().
> especially as this not protected by a lock.
Looks lock isn't needed since probe/remove and open/close path
can't be concurrent.
Thanks,
--
Ming Lei
--
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: [patch net-next 0/4] net: allow to change carrier from userspace
From: Jiri Pirko @ 2012-12-14 16:35 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Flavio Leitner, John Fastabend, netdev, davem, edumazet,
bhutchings, mirqus, greearb
In-Reply-To: <20121214081201.1205b613@nehalam.linuxnetplumber.net>
Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
>On Fri, 14 Dec 2012 15:41:34 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
>> >On Thu, 13 Dec 2012 16:17:33 -0200
>> >Flavio Leitner <fbl@redhat.com> wrote:
>> >
>> >> On Thu, 13 Dec 2012 10:09:33 -0800
>> >> Stephen Hemminger <shemminger@vyatta.com> wrote:
>> >>
>> >> > On Thu, 13 Dec 2012 15:54:23 -0200
>> >> > Flavio Leitner <fbl@redhat.com> wrote:
>> >> >
>> >> > > I am saying this because people are used to and there are scripts out
>> >> > > there using something like:
>> >> > > # ethtool <iface> | grep 'Link'
>> >> > > to react an interface failure.
>> >> >
>> >> > Then the script is broken. It is asking about hardware state.
>> >>
>> >> I was talking about the team master interface, so it makes sense
>> >> to check its 'hardware' state. Just think on 'bond0' interface
>> >> with no slaves. It should report Link detected: no.
>> >>
>> >> See bond_release(), what happens if bond->slave_cnt == 0, for instance.
>> >>
>> >
>> >I was thinking more that ethtool operation for reporting link on
>> >the team device should use the proper check rather than just using netif_carrier_ok(),
>> >the team ethtool operation for get_link should be check IFF_RUNNING flag
>> >in dev->flags which is controlled by operstate transistions.
>>
>> I admit I'm bit confused now.
>>
>> For example in bridge code:
>> in br_add_if() - netif_carrier_ok() is checked and by the value it is
>> decided if br_stp_enable_port() is called or not. Wouldn't it make more
>> sense to check IFF_RUNNING (or netif_oper_up()) here?
>>
>> The reason I'm asing is that if team device is in bridge, carrier is
>> always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
>> userspace, in current code, bridge wouldn't know the difference...
>>
>> There are more exmaples of similar usage of netif_carrier_ok() in
>> bridge (called on ports), bonding (called on slaves), team code (called on ports).
>
>Yes the bridge should be fixed to work with user controlled devices.
Okay. I'll try to figure out some patchset over the weekend.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5] Ease netns management for userland
From: Eric W. Biederman @ 2012-12-14 16:50 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: netdev, davem, aatteka
In-Reply-To: <50CB5047.8060804@6wind.com>
Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
> Le 13/12/2012 20:08, Eric W. Biederman a écrit :
>> No. The difficulty monitoring which network namespaces are being used
>> is an unintended side effect.
> Why is netlink a bad idea? Having a way to know all existing netns is a start
> point to monitor netns, isn't it?
In the same way that having a neighbour table that contains all existing
ip address to mac addresses mappings is a starting point to monitor all
existing hosts.
All does not scale.
All removes a lot of perfectly valid use cases like checkpoint-restart,
and nesting containers.
All as different from what is already implemented requires implementing
yet another namespace to put the names of all into it. We have enough
namespaces now thank you very much.
An unfiltered global list is about as interesting to use as putting
all files in /. Sure you know which directory you put your file in but
which file is it?
What has already been implemented should be roughly as good for
monitoring as what is available with lsof.
And of course there is the fact that a global list of anything that is
the same from every perspective violates the principle of relativity,
and is in contradiction with the phsical reality in which we exist.
So there is no way that having a global all inclusive list of network
namespaces makes the least lick of sense and I really don't want to
think about it.
Eric
^ permalink raw reply
* Re: [PATCH 00/11] Add basic VLAN support to bridges
From: Vlad Yasevich @ 2012-12-14 16:50 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Stephen Hemminger, David Miller, or.gerlitz, netdev, mst,
john.r.fastabend
In-Reply-To: <50CA5D39.5060003@mojatatu.com>
On 12/13/2012 05:56 PM, Jamal Hadi Salim wrote:
> On 12-12-13 05:37 PM, Stephen Hemminger wrote:
>
>>
>> You can, run any action before it hits the bridge.
>
> I think you and I have had this discussion before ;->
> It works just fine on ingress.
>
>
> #Add ingress qdisc on br0
> tc qdisc add dev br0 ingress
> #Add a filter to accept all and count
> tc filter add dev br0 parent ffff: protocol ip prio 6 u32 match ip dst
> 0/0 flowid 1:16 action ok
> #show the stats
> root@jhs12:~# tc -s filter show parent ffff: dev br0
> filter protocol ip pref 6 u32
> filter protocol ip pref 6 u32 fh 800: ht divisor 1
> filter protocol ip pref 6 u32 fh 800::800 order 2048 key ht 800 bkt 0
> flowid 1:16
> match 00000000/00000000 at 16
> action order 1: gact action pass
> random type none pass val 0
> index 2 ref 1 bind 1 installed 269 sec used 74 sec
> Action statistics:
> Sent 1210 bytes 15 pkt (dropped 0, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
> ------
>
> Look at those packets ...
Interesting. But, but how complex would be be to configure a vlan
filter for say 10 different vlans, each one of them only permitted
to be forwarded to their respective VM. Oh, and Vlan tags should
be stripped when they are being forwarded.
config:
+- eth0
|
br0-+- vnet0 (vlan10) - VM1
|
+- vnet1 (vlan20) - VM2
|
+- vnet3 (vlan30) - VM3
... etc...
-vlad
>
> cheers,
> jamal
>
> cheers,
> jamal
> --
> 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
* Re: [PATCH net] sctp: jsctp_sf_eat_sack: fix kernel NULL ptr dereference
From: Daniel Borkmann @ 2012-12-14 16:57 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: David Miller, netdev
In-Reply-To: <50CB41F2.7040403@gmail.com>
On 12/14/2012 04:12 PM, Vlad Yasevich wrote:
> On 12/14/2012 09:27 AM, Daniel Borkmann wrote:
>> During debugging a sctp problem, I hit a kernel NULL pointer
>> dereference in the jprobes callback jsctp_sf_eat_sack(). This
>> small patch fixes the panic.
>>
>> Cc: Vlad Yasevich <vyasevich@gmail.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>> net/sctp/probe.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/probe.c b/net/sctp/probe.c
>> index bc6cd75..0a4e9d6 100644
>> --- a/net/sctp/probe.c
>> +++ b/net/sctp/probe.c
>> @@ -134,7 +134,7 @@ sctp_disposition_t jsctp_sf_eat_sack(const struct sctp_endpoint *ep,
>>
>> sp = asoc->peer.primary_path;
>>
>> - if ((full || sp->cwnd != lcwnd) &&
>> + if (sp && (full || sp->cwnd != lcwnd) &&
>> (!port || asoc->peer.port == port ||
>> ep->base.bind_addr.port == port)) {
>> lcwnd = sp->cwnd;
>
> Sorry, but this patch isn't making much sense. @sp points to the primary path of the association and that can not be null if we receiving
> SACKs on this association.
>
> sctp_sf_eat_sack_6_2(), which we are probing, is only called while the association is valid and all the transports still exist. It is also called under lock, so the transports can not go away while processing of the SACK. So unless there is some kind of jprobe issue, the pointer
> you are looking at should always be valid.
Okay, I'll dig a bit deeper into that over the weekend.
^ permalink raw reply
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Stephen Hemminger @ 2012-12-14 16:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: Flavio Leitner, John Fastabend, netdev, davem, edumazet,
bhutchings, mirqus, greearb
In-Reply-To: <20121214163532.GB1652@minipsycho.brq.redhat.com>
On Fri, 14 Dec 2012 17:35:32 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
> >On Fri, 14 Dec 2012 15:41:34 +0100
> >Jiri Pirko <jiri@resnulli.us> wrote:
> >
> >> Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
> >> >On Thu, 13 Dec 2012 16:17:33 -0200
> >> >Flavio Leitner <fbl@redhat.com> wrote:
> >> >
> >> >> On Thu, 13 Dec 2012 10:09:33 -0800
> >> >> Stephen Hemminger <shemminger@vyatta.com> wrote:
> >> >>
> >> >> > On Thu, 13 Dec 2012 15:54:23 -0200
> >> >> > Flavio Leitner <fbl@redhat.com> wrote:
> >> >> >
> >> >> > > I am saying this because people are used to and there are scripts out
> >> >> > > there using something like:
> >> >> > > # ethtool <iface> | grep 'Link'
> >> >> > > to react an interface failure.
> >> >> >
> >> >> > Then the script is broken. It is asking about hardware state.
> >> >>
> >> >> I was talking about the team master interface, so it makes sense
> >> >> to check its 'hardware' state. Just think on 'bond0' interface
> >> >> with no slaves. It should report Link detected: no.
> >> >>
> >> >> See bond_release(), what happens if bond->slave_cnt == 0, for instance.
> >> >>
> >> >
> >> >I was thinking more that ethtool operation for reporting link on
> >> >the team device should use the proper check rather than just using netif_carrier_ok(),
> >> >the team ethtool operation for get_link should be check IFF_RUNNING flag
> >> >in dev->flags which is controlled by operstate transistions.
> >>
> >> I admit I'm bit confused now.
> >>
> >> For example in bridge code:
> >> in br_add_if() - netif_carrier_ok() is checked and by the value it is
> >> decided if br_stp_enable_port() is called or not. Wouldn't it make more
> >> sense to check IFF_RUNNING (or netif_oper_up()) here?
> >>
> >> The reason I'm asing is that if team device is in bridge, carrier is
> >> always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
> >> userspace, in current code, bridge wouldn't know the difference...
> >>
> >> There are more exmaples of similar usage of netif_carrier_ok() in
> >> bridge (called on ports), bonding (called on slaves), team code (called on ports).
> >
> >Yes the bridge should be fixed to work with user controlled devices.
>
> Okay. I'll try to figure out some patchset over the weekend.
>
> Thanks.
>
Something like this seems needed.
--- a/net/bridge/br_if.c 2012-10-25 09:11:15.627272524 -0700
+++ b/net/bridge/br_if.c 2012-12-14 08:58:14.329847361 -0800
@@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
struct net_device *dev = p->dev;
struct net_bridge *br = p->br;
- if (netif_running(dev) && netif_carrier_ok(dev))
+ if (netif_running(dev) && netif_oper_up(dev))
p->path_cost = port_cost(dev);
if (!netif_running(br->dev))
return;
spin_lock_bh(&br->lock);
- if (netif_running(dev) && netif_carrier_ok(dev)) {
+ if (netif_running(dev) && netif_oper_up(dev))
if (p->state == BR_STATE_DISABLED)
br_stp_enable_port(p);
} else {
--- a/net/bridge/br_notify.c 2012-10-25 09:11:15.631272484 -0700
+++ b/net/bridge/br_notify.c 2012-12-14 08:57:36.954222724 -0800
@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
break;
case NETDEV_UP:
- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
+ if (netif_running(br->dev) && netif_oper_up(dev)) {
spin_lock_bh(&br->lock);
br_stp_enable_port(p);
spin_unlock_bh(&br->lock);
^ permalink raw reply
* Re: [PATCH] add a `make dist` helper
From: Stephen Hemminger @ 2012-12-14 17:09 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Mike Frysinger, netdev
In-Reply-To: <drbl5w8utpw8p960mxvy37j1.1355469370120@email.android.com>
On Thu, 13 Dec 2012 23:16:10 -0800
Stephen Hemminger <stephen.hemminger@vyatta.com> wrote:
>
> I appreciate the effort but there are a number of more steps to doing a release and I need to script them all together.
The tarball's have been rebased, and I built a iproute2-release script for next time.
^ permalink raw reply
* Re: [PATCH] iproute2: update usage info of bridge monitor
From: Stephen Hemminger @ 2012-12-14 17:11 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, bridge
In-Reply-To: <1355464772-17712-1-git-send-email-amwang@redhat.com>
On Fri, 14 Dec 2012 13:59:32 +0800
Cong Wang <amwang@redhat.com> wrote:
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> ---
> diff --git a/bridge/monitor.c b/bridge/monitor.c
> index 44e14d8..29bb931 100644
> --- a/bridge/monitor.c
> +++ b/bridge/monitor.c
> @@ -31,7 +31,7 @@ int prefix_banner;
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: bridge monitor\n");
> + fprintf(stderr, "Usage: bridge monitor [file | link | fdb | mdb | all]\n");
> exit(-1);
> }
>
Applied.
^ permalink raw reply
* Re: [patch net-next 0/4] net: allow to change carrier from userspace
From: Jiri Pirko @ 2012-12-14 17:13 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Flavio Leitner, John Fastabend, netdev, davem, edumazet,
bhutchings, mirqus, greearb
In-Reply-To: <20121214085918.6a2f3535@nehalam.linuxnetplumber.net>
Fri, Dec 14, 2012 at 05:59:18PM CET, shemminger@vyatta.com wrote:
>On Fri, 14 Dec 2012 17:35:32 +0100
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Dec 14, 2012 at 05:12:01PM CET, shemminger@vyatta.com wrote:
>> >On Fri, 14 Dec 2012 15:41:34 +0100
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> >> Thu, Dec 13, 2012 at 07:20:51PM CET, shemminger@vyatta.com wrote:
>> >> >On Thu, 13 Dec 2012 16:17:33 -0200
>> >> >Flavio Leitner <fbl@redhat.com> wrote:
>> >> >
>> >> >> On Thu, 13 Dec 2012 10:09:33 -0800
>> >> >> Stephen Hemminger <shemminger@vyatta.com> wrote:
>> >> >>
>> >> >> > On Thu, 13 Dec 2012 15:54:23 -0200
>> >> >> > Flavio Leitner <fbl@redhat.com> wrote:
>> >> >> >
>> >> >> > > I am saying this because people are used to and there are scripts out
>> >> >> > > there using something like:
>> >> >> > > # ethtool <iface> | grep 'Link'
>> >> >> > > to react an interface failure.
>> >> >> >
>> >> >> > Then the script is broken. It is asking about hardware state.
>> >> >>
>> >> >> I was talking about the team master interface, so it makes sense
>> >> >> to check its 'hardware' state. Just think on 'bond0' interface
>> >> >> with no slaves. It should report Link detected: no.
>> >> >>
>> >> >> See bond_release(), what happens if bond->slave_cnt == 0, for instance.
>> >> >>
>> >> >
>> >> >I was thinking more that ethtool operation for reporting link on
>> >> >the team device should use the proper check rather than just using netif_carrier_ok(),
>> >> >the team ethtool operation for get_link should be check IFF_RUNNING flag
>> >> >in dev->flags which is controlled by operstate transistions.
>> >>
>> >> I admit I'm bit confused now.
>> >>
>> >> For example in bridge code:
>> >> in br_add_if() - netif_carrier_ok() is checked and by the value it is
>> >> decided if br_stp_enable_port() is called or not. Wouldn't it make more
>> >> sense to check IFF_RUNNING (or netif_oper_up()) here?
>> >>
>> >> The reason I'm asing is that if team device is in bridge, carrier is
>> >> always ON and I'm fiddling with IF_OPER_UP and IF_OPER_DORMANT from
>> >> userspace, in current code, bridge wouldn't know the difference...
>> >>
>> >> There are more exmaples of similar usage of netif_carrier_ok() in
>> >> bridge (called on ports), bonding (called on slaves), team code (called on ports).
>> >
>> >Yes the bridge should be fixed to work with user controlled devices.
>>
>> Okay. I'll try to figure out some patchset over the weekend.
>>
>> Thanks.
>>
>
>Something like this seems needed.
>
>--- a/net/bridge/br_if.c 2012-10-25 09:11:15.627272524 -0700
>+++ b/net/bridge/br_if.c 2012-12-14 08:58:14.329847361 -0800
>@@ -66,14 +66,14 @@ void br_port_carrier_check(struct net_br
> struct net_device *dev = p->dev;
> struct net_bridge *br = p->br;
>
>- if (netif_running(dev) && netif_carrier_ok(dev))
>+ if (netif_running(dev) && netif_oper_up(dev))
> p->path_cost = port_cost(dev);
>
> if (!netif_running(br->dev))
> return;
>
> spin_lock_bh(&br->lock);
>- if (netif_running(dev) && netif_carrier_ok(dev)) {
>+ if (netif_running(dev) && netif_oper_up(dev))
> if (p->state == BR_STATE_DISABLED)
> br_stp_enable_port(p);
> } else {
>--- a/net/bridge/br_notify.c 2012-10-25 09:11:15.631272484 -0700
>+++ b/net/bridge/br_notify.c 2012-12-14 08:57:36.954222724 -0800
>@@ -82,7 +82,7 @@ static int br_device_event(struct notifi
> break;
>
> case NETDEV_UP:
>- if (netif_carrier_ok(dev) && (br->dev->flags & IFF_UP)) {
>+ if (netif_running(br->dev) && netif_oper_up(dev)) {
> spin_lock_bh(&br->lock);
> br_stp_enable_port(p);
> spin_unlock_bh(&br->lock);
Yes. I have this already in my queue. I just spotted a problem though.
Lets say teamd sets operstate of the team device by values IF_OPER_UP
and IF_OPER_DORMANT depending on teamd states of ports.
What if one would like to use 802.1X supplicant on the same device?
That would not be possible.
This proves that the layering would not be correct. It look like the
carrier userspace set would be the correct thing to do after all...
What do you think?
Jiri
^ permalink raw reply
* Re: [Patch] iproute2: clean up genl/genl.c::usage()
From: Stephen Hemminger @ 2012-12-14 17:15 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev
In-Reply-To: <1355465438-20586-1-git-send-email-amwang@redhat.com>
On Fri, 14 Dec 2012 14:10:38 +0800
Cong Wang <amwang@redhat.com> wrote:
> gcc attribute can be used in definition, no need to
> redeclare it.
>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Cong Wang <amwang@redhat.com>
>
> ---
> diff --git a/genl/genl.c b/genl/genl.c
> index 49b6596..a1e55e0 100644
> --- a/genl/genl.c
> +++ b/genl/genl.c
> @@ -97,9 +97,7 @@ noexist:
> return f;
> }
>
> -static void usage(void) __attribute__((noreturn));
> -
> -static void usage(void)
> +static void __attribute__((noreturn)) usage(void)
> {
> fprintf(stderr, "Usage: genl [ OPTIONS ] OBJECT | help }\n"
> "where OBJECT := { ctrl etc }\n"
> --
> 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
Why bother, all the other older code uses the prototype version,
besides the optimizing of usage() function is ridiculous anyway.
I am just going to remove all the
static void usage(void) __attribute__((noreturn));
^ 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