* Re: [PATCH net-next] team: add ethtool support
From: Stephen Hemminger @ 2013-01-10 17:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Flavio Leitner, netdev, Jiri Pirko
In-Reply-To: <1356834716.21409.6258.camel@edumazet-glaptop>
On Sat, 29 Dec 2012 18:31:56 -0800
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2012-12-29 at 23:44 -0200, Flavio Leitner wrote:
>
> > Speaking as a support engineer, it's a lot easier to grab ethtool -S and
> > see everything than grab two or more outputs.
> >
>
> I agree its very convenient.
>
> I have a patch to add GRO statistics at the core layer, in the ethtool
> -S stats.
>
> I was about to ask netdev guys what they think of this idea ?
>
>
> net-gro: Add GRO counters to ethtool -S
>
> In order to get an idea of how effective is GRO aggregation on a machine,
> we need appropriate counters. Preferably use "ethtool -S" to display them
> on a per device basis, or even per RX queue.
>
> In this implementation, I chose to not change NIC drivers.
>
> Core network stack adds the gro counters at the end of the counters
> each NIC driver provides for ethtool -S
>
> There are 5 counters per RX queue :
>
> gro_complete: number of time the NAPI handler did not consume its budget
> (This force a flush of all GRO packets in the GRO queue)
> gro_overflows: number of time a segment could not be stored in GRO queue
> because current number or messages is too high
> gro_nogro: number of time a segment was not stored in GRO queue.
> (Because its not a TCP packet, or it includes a
> SYN/FIN/RST/PSH flag)
> gro_msgs: number of GRO messages (might contain 1 to 17 segments)
> gro_segs: number of GRO segments
>
> Example:
>
> On receiver machine, with 8 RX queues :
>
> ethtool -S eth4 | tail -n 10
> gro_complete[7]: 56635
> gro_overflows[7]: 0
> gro_nogro[7]: 212
> gro_msgs[7]: 129410
> gro_segs[7]: 1434925
> gro_complete: 699479
> gro_overflows: 0
> gro_nogro: 2455
> gro_msgs: 1626470
> gro_segs: 17876794
>
> In this example, we can compute average number of segments per GRO message :
>
> 17876794/17876794 = 10.99
>
> Or more precisely : 17876794/(17876794+2455) = 10.97
>
>
> --
> 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
Ethtool is awkward because it is only available through ioctl, no netlink or /proc.
If you use ethtool for GRO, please make it a separate ioctl not an add-on to
existing device statistics. ethtool --gro ??
^ permalink raw reply
* BUG: NULL pointer dereference in netif_carrier_off
From: Tom Parkin @ 2013-01-10 17:50 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
Hi Eric,
I've just had a chance to try your fix for the veth oops I previously
reported[1], but I still see some problems (please see oops below).
If I revert the original stats overhead fix (2681128f0ced8) and the
subsequent NULL deref patch (d0e2c55e7c94) this problem goes away for
me.
I've reproduced this bug using a script intended to stress test L2TP
network namespace code, which uses veth tunnels to bridge namespaces.
Apologies for the late report!
Thanks,
Tom
[1]
http://www.spinics.net/lists/netdev/msg221284.html
BUG: unable to handle kernel NULL pointer dereference at 0000002c
IP: [<c151f5f8>] netif_carrier_off+0x8/0x30
*pde = 00000000
Oops: 0002 [#1] SMP
Modules linked in: veth bridge stp llc l2tp_ip6 l2tp_ip l2tp_ppp pppox
l2tp_eth l2tp_netlink l2tp_core kvm_amd 9
Pid: 5778, comm: ip Tainted: G W 3.8.0-rc1-tpdev-33-lockdep+
#42 Gigabyte Technology Co., Ltd. GA-MA62
EIP: 0060:[<c151f5f8>] EFLAGS: 00010202 CPU: 0
EIP is at netif_carrier_off+0x8/0x30
EAX: 00000000 EBX: ee422800 ECX: 00000001 EDX: c18e4b80
ESI: efec7c44 EDI: ee422840 EBP: efec7b38 ESP: efec7b38
DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 80050033 CR2: 0000002c CR3: 2fdb2000 CR4: 000007d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process ip (pid: 5778, ti=efec6000 task=f1d4bf00 task.ti=efec6000)
Stack:
efec7b44 f810e23d ee422800 efec7b60 c14fc169 ffffffff ffffffff f594c210
ee422800 efec7c44 efec7b80 c14fc25e efec7b6c ee422040 ee422040 efec7c04
ee422800 efec7c44 efec7ba0 c14fc357 f180dd00 efec7c44 ee422000 efec7c44
Call Trace:
[<f810e23d>] veth_close+0x2d/0x60 [veth]
[<c14fc169>] __dev_close_many+0x69/0xb0
[<c14fc25e>] dev_close_many+0x6e/0xc0
[<c14fc357>] rollback_registered_many+0xa7/0x1c0
[<c14fc484>] unregister_netdevice_many+0x14/0x60
[<c1510e0e>] rtnl_dellink+0x9e/0x100
[<c10461a8>] ? ns_capable+0x38/0x70
[<c1510d70>] ? rtnetlink_put_metrics+0xf0/0xf0
[<c1512ae3>] rtnetlink_rcv_msg+0x153/0x2a0
[<c160db1a>] ? mutex_lock_nested+0x21a/0x2e0
[<c150f924>] ? rtnl_lock+0x14/0x20
[<c1512990>] ? __rtnl_unlock+0x20/0x20
[<c152a31e>] netlink_rcv_skb+0x8e/0xb0
[<c150f94c>] rtnetlink_rcv+0x1c/0x30
[<c1529cdd>] netlink_unicast+0x17d/0x1f0
[<c1529f74>] netlink_sendmsg+0x224/0x390
[<c14e8791>] sock_sendmsg+0xd1/0xf0
[<c1135b89>] ? might_fault+0x89/0x90
[<c12fa932>] ? _copy_from_user+0x42/0x60
[<c14f5c64>] ? verify_iovec+0x44/0xb0
[<c14e96a2>] __sys_sendmsg+0x262/0x270
[<c1073e4f>] ? sched_clock_cpu+0xcf/0x150
[<c109c02b>] ? trace_hardirqs_off+0xb/0x10
[<c1073f35>] ? local_clock+0x65/0x70
[<c109c69c>] ? lock_release_holdtime.part.23+0xbc/0xf0
[<c10a15ed>] ? lock_release_non_nested+0x29d/0x2e0
[<c1073f35>] ? local_clock+0x65/0x70
[<c1178ab1>] ? fget_light+0x371/0x450
[<c14ead9b>] sys_sendmsg+0x3b/0x60
[<c14eb443>] sys_socketcall+0x283/0x2e0
[<c1611310>] ? restore_all+0xf/0xf
[<c1614bc0>] ? __do_page_fault+0x4e0/0x4e0
[<c12fa548>] ? trace_hardirqs_on_thunk+0xc/0x10
[<c1618acd>] sysenter_do_call+0x12/0x38
Code: 00 00 00 e8 ab 2f fd ff 8b 75 f8 b8 01 00 00 00 83 83 9c 00 00
00 01 8b 5d f4 8b 7d fc 89 ec 5d c3 66 90 3
EIP: [<c151f5f8>] netif_carrier_off+0x8/0x30 SS:ESP 0068:efec7b38
CR2: 000000000000002c
---[ end trace e9f7b433da03329a ]---
--
Tom Parkin
www.thesecretdogproject.com
The competent programmer is fully aware of the strictly limited size
of his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the
plague. /E. W. Dijkstra/
^ permalink raw reply
* Fw: [Bug 52481] New: Occasional NULL pointer dereference during ftp networking since 3.7
From: Stephen Hemminger @ 2013-01-10 17:21 UTC (permalink / raw)
To: netdev
Begin forwarded message:
Date: Tue, 8 Jan 2013 14:29:18 +0000 (UTC)
From: bugzilla-daemon@bugzilla.kernel.org
To: shemminger@linux-foundation.org
Subject: [Bug 52481] New: Occasional NULL pointer dereference during ftp networking since 3.7
https://bugzilla.kernel.org/show_bug.cgi?id=52481
Summary: Occasional NULL pointer dereference during ftp
networking since 3.7
Product: Networking
Version: 2.5
Kernel Version: 3.7.0
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: IPV4
AssignedTo: shemminger@linux-foundation.org
ReportedBy: itumaykin@gmail.com
Regression: Yes
Created an attachment (id=90771)
--> (https://bugzilla.kernel.org/attachment.cgi?id=90771)
lspci -vvv
I am experiencing occasional panics during ftp networking after installing
kernel 3.7.0.
I have vsftpd-3.0.2 up and running on port 21 with configuration not changed
for months, I use passive FTP with nf_conntrack_ftp helper. My distro is
Hardened Gentoo amd64 and I use not vanilla kernel, but with some Gentoo
specific and grsec specific patches.
Anyway, with kernel 3.5.4 I had no problems with my FTP at all, but with 3.7.0
there are random panics during or after FTP sessions. It happens more often
after sessions with lots of data transferred, but sometimes only `ls` is enough
to hang my machine.
The nature of these errors is unknown to me, but the issue is definetely within
the kernel as I tried to downgrade solely kernel back to 3.5.4 and everything
returned back to normal.
Also, I don't have traces of these panics in my logs, but I took photos of one
of such cases. Here are links to them:
https://docs.google.com/folder/d/0B9NAm8_0mptgU2RrMTZWX0NucUk/edit
I understand there is probably not enough info to investigate the source of
this issue, but I am ready to provide any additional info or do some testing.
--
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
^ permalink raw reply
* Re: [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach
From: Florian Fainelli @ 2013-01-10 17:12 UTC (permalink / raw)
To: Kosta Zertsekel
Cc: netdev@vger.kernel.org, zertsekel@gmail.com, andrew@lunn.ch,
Eran Ben-Avi, linux-arm-kernel@lists.infradead.org, Lior Amsalem
In-Reply-To: <BC373419EB4337418B2B595BAEDC155F015896370062@IL-MB01.marvell.com>
Le 01/10/13 16:57, Kosta Zertsekel a écrit :
>> By the way, most, if not all of the phy_connect() users in drivers/net/ethernet/ also do not ensure they pass the phy device flags, so you might
> want to fix this globally and not just for Marvell driver.
> Indeed, phy_connect() mostly just pass zero intead of phy_dev->dev_flags.
> But, I think, the guy that calls phy_connect() in its driver should know what he does, and,
> probably, we should rely on him knowing his stuff.
> The only evidence of the bug is when phy_dev->dev_flags was actually changed by PHY fixup callback
> (see dns323-setup.c for example) and was *not* propagated to phy_connect() or phy_attach() as in pxa168_eth.c phy_init().
> The code conforming to the former should be fixed IMHO.
Actually, I wonder if we should not rather remove entirely the flags
argument, let the phy_connect() or phy_attach() callers modify the phy
device dev_flags like it does today (e.g: tg3) and modify phy_connect()
and phy_attach() to pass phy->dev_flags to phy_attach_direct().
--
Florian
^ permalink raw reply
* [PATCH] tcp: splice: fix an infinite loop in tcp_read_sock()
From: Eric Dumazet @ 2013-01-10 17:06 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Willy Tarreau
From: Eric Dumazet <edumazet@google.com>
commit 02275a2ee7c0 (tcp: don't abort splice() after small transfers)
added a regression.
[ 83.843570] INFO: rcu_sched self-detected stall on CPU
[ 83.844575] INFO: rcu_sched detected stalls on CPUs/tasks: { 6} (detected by 0, t=21002 jiffies, g=4457, c=4456, q=13132)
[ 83.844582] Task dump for CPU 6:
[ 83.844584] netperf R running task 0 8966 8952 0x0000000c
[ 83.844587] 0000000000000000 0000000000000006 0000000000006c6c 0000000000000000
[ 83.844589] 000000000000006c 0000000000000096 ffffffff819ce2bc ffffffffffffff10
[ 83.844592] ffffffff81088679 0000000000000010 0000000000000246 ffff880c4b9ddcd8
[ 83.844594] Call Trace:
[ 83.844596] [<ffffffff81088679>] ? vprintk_emit+0x1c9/0x4c0
[ 83.844601] [<ffffffff815ad449>] ? schedule+0x29/0x70
[ 83.844606] [<ffffffff81537bd2>] ? tcp_splice_data_recv+0x42/0x50
[ 83.844610] [<ffffffff8153beaa>] ? tcp_read_sock+0xda/0x260
[ 83.844613] [<ffffffff81537b90>] ? tcp_prequeue_process+0xb0/0xb0
[ 83.844615] [<ffffffff8153c0f0>] ? tcp_splice_read+0xc0/0x250
[ 83.844618] [<ffffffff814dc0c2>] ? sock_splice_read+0x22/0x30
[ 83.844622] [<ffffffff811b820b>] ? do_splice_to+0x7b/0xa0
[ 83.844627] [<ffffffff811ba4bc>] ? sys_splice+0x59c/0x5d0
[ 83.844630] [<ffffffff8119745b>] ? putname+0x2b/0x40
[ 83.844633] [<ffffffff8118bcb4>] ? do_sys_open+0x174/0x1e0
[ 83.844636] [<ffffffff815b6202>] ? system_call_fastpath+0x16/0x1b
if recv_actor() returns 0, we should stop immediately,
because looping wont give a chance to drain the pipe.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Willy Tarreau <w@1wt.eu>
---
net/ipv4/tcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1ca2536..5f173dc 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1482,7 +1482,7 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
break;
}
used = recv_actor(desc, skb, offset, len);
- if (used < 0) {
+ if (used <= 0) {
if (!copied)
copied = used;
break;
^ permalink raw reply related
* [PATCH 2/2] net: ethernet: xilinx: Do not use NO_IRQ in axienet
From: Michal Simek @ 2013-01-10 16:58 UTC (permalink / raw)
To: netdev
In-Reply-To: <1357837123-18996-1-git-send-email-michal.simek@xilinx.com>
This driver is used on Microblaze and will be used
on Arm Zynq.
Microblaze doesn't define NO_IRQ and no IRQ is 0.
Arm still uses NO_IRQ as -1 and there is no option
to connect IRQ to irq 0.
That's why <= 0 is only one option how to find out
undefined IRQ.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index d9f69b8..6f47100 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -1590,7 +1590,7 @@ static int axienet_of_probe(struct platform_device *op)
lp->rx_irq = irq_of_parse_and_map(np, 1);
lp->tx_irq = irq_of_parse_and_map(np, 0);
of_node_put(np);
- if ((lp->rx_irq == NO_IRQ) || (lp->tx_irq == NO_IRQ)) {
+ if ((lp->rx_irq <= 0) || (lp->tx_irq <= 0)) {
dev_err(&op->dev, "could not determine irqs\n");
ret = -ENOMEM;
goto err_iounmap_2;
--
1.7.0.4
^ permalink raw reply related
* [PATCH 1/2] net: ethernet: xilinx: Do not use axienet on PPC
From: Michal Simek @ 2013-01-10 16:58 UTC (permalink / raw)
To: netdev
Axi ethernet can't be used on PPC because it is
little endian IP and PPC is big endian.
This system can't be designed.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/net/ethernet/xilinx/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/Kconfig b/drivers/net/ethernet/xilinx/Kconfig
index 5778a4a..122d60c 100644
--- a/drivers/net/ethernet/xilinx/Kconfig
+++ b/drivers/net/ethernet/xilinx/Kconfig
@@ -27,7 +27,7 @@ config XILINX_EMACLITE
config XILINX_AXI_EMAC
tristate "Xilinx 10/100/1000 AXI Ethernet support"
- depends on (PPC32 || MICROBLAZE)
+ depends on MICROBLAZE
select PHYLIB
---help---
This driver supports the 10/100/1000 Ethernet from Xilinx for the
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 0/4] l2tp: fix up network namespace use in l2tp_core
From: Tom Parkin @ 2013-01-10 16:48 UTC (permalink / raw)
To: Tom Parkin; +Cc: netdev
In-Reply-To: <1357756583-22535-1-git-send-email-tparkin@katalix.com>
On 9 January 2013 18:36, Tom Parkin <tparkin@katalix.com> wrote:
> This patchset addresses network namespace issues in l2tp_core as previously
> raised on netdev[1].
Bah, please ignore this changeset for now. I have just discovered a
late change I made to support preemption-enabled kernels breaks the
non-preempt case. I need to reconsider tunnel deletion to properly
address this.
Apologies,
Tom
--
Tom Parkin
www.thesecretdogproject.com
The competent programmer is fully aware of the strictly limited size
of his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the
plague. /E. W. Dijkstra/
^ permalink raw reply
* R: Re: mtu issue with ipsec tunnel and netfilter snat
From: pupilla @ 2013-01-10 16:46 UTC (permalink / raw)
To: jengelh; +Cc: netdev
jengelh@inai.de wrote:
>On Wednesday 2013-01-09 10:01, pupilla@libero.it wrote:
>
>>As you can see there are incoming 1500 bytes packets (these are the
>>decrypted ipsec packets) with DF bit set. These packets are never
>>delivered to the final client 10.81.128.176 (the destination address
>>is 172.16.128.1 which is the ip used for SNATing the original ip
>>10.81.128.176).
>>
>>IMHO this is a mtu issue: 1500 bytes packets cannot be routed inside
>>the ipsec tunnel.
>>
>>But why linux_gw_snat is not sending icmp need to frag packets to
>>10.148.12.23?
>
>Perhaps because ICMP was blocked erroneously?
well, I don't see the icmp packets
because tcpdump 'see' only the incoming
ipsec clear packets. Is there a way to
see the outgoing clear ipsec packets
with tcpdump?
^ permalink raw reply
* Re: [PATCH 01/19] netfilter: move nf_conntrack initialize out of pernet operations
From: Pablo Neira Ayuso @ 2013-01-10 16:41 UTC (permalink / raw)
To: Gao feng; +Cc: netfilter-devel, netdev, canqunzhang, kaber, ebiederm
In-Reply-To: <1356662206-2260-1-git-send-email-gaofeng@cn.fujitsu.com>
[-- Attachment #1: Type: text/plain, Size: 937 bytes --]
Hi Gao,
On Fri, Dec 28, 2012 at 10:36:27AM +0800, Gao feng wrote:
> canqun zhang reported a panic BUG,kernel may panic when
> unloading nf_conntrack module.
>
> It's because we reset nf_ct_destroy to NULL when we deal
> with init_net,it's too early.Some packets belongs to other
> netns still refers to the conntrack.when these packets need
> to be freed, kfree_skb will call nf_ct_destroy which is
> NULL.
>
> fix this bug by moving the nf_conntrack initialize and cleanup
> codes out of the pernet operations,this job should be done
> in module_init/exit.We can't use init_net to identify if
> it's the right time.
First off, thanks for looking into this.
I want to get this fix into 3.8 and -stable but this patch includes a
rework whose scope is net-next (upcoming 3.9).
The attached patch aims to fix the issue according to your patch
description. Once this is in, we can revisit your code refactoring
proposal.
Let me know.
[-- Attachment #2: 0001-netfilter-nf_conntrack-fix-BUG_ON-while-removing-nf_.patch --]
[-- Type: text/x-diff, Size: 2758 bytes --]
>From a211bd666fbfe17ae7171a50ad92fedc7b9e19fa Mon Sep 17 00:00:00 2001
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu, 10 Jan 2013 16:12:01 +0100
Subject: [PATCH] netfilter: nf_conntrack: fix BUG_ON while removing
nf_conntrack with netns
canqun zhang reported that we're hitting BUG_ON in the
nf_conntrack_destroy path when calling kfree_skb while
rmmod'ing the nf_conntrack module.
Currently, the nf_ct_destroy hook is being set to NULL in the
destroy path of conntrack.init_net. However, this is a problem
since init_net may be destroyed before any other existing netns
(we cannot assume any specific ordering while releasing existing
netns according to what I read in recent emails).
Thanks to Gao feng for initial patch to address this issue.
Reported-by: canqun zhang <canqunzhang@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_conntrack_core.h | 2 ++
net/netfilter/nf_conntrack_core.c | 9 +++++----
net/netfilter/nf_conntrack_standalone.c | 1 +
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index d8f5b9f..e98aeb3 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -31,6 +31,8 @@ extern void nf_conntrack_cleanup(struct net *net);
extern int nf_conntrack_proto_init(struct net *net);
extern void nf_conntrack_proto_fini(struct net *net);
+extern void nf_conntrack_cleanup_end(void);
+
extern bool
nf_ct_get_tuple(const struct sk_buff *skb,
unsigned int nhoff,
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 016d95e..e4a0c4f 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1376,11 +1376,12 @@ void nf_conntrack_cleanup(struct net *net)
synchronize_net();
nf_conntrack_proto_fini(net);
nf_conntrack_cleanup_net(net);
+}
- if (net_eq(net, &init_net)) {
- RCU_INIT_POINTER(nf_ct_destroy, NULL);
- nf_conntrack_cleanup_init_net();
- }
+void nf_conntrack_cleanup_end(void)
+{
+ RCU_INIT_POINTER(nf_ct_destroy, NULL);
+ nf_conntrack_cleanup_init_net();
}
void *nf_ct_alloc_hashtable(unsigned int *sizep, int nulls)
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 363285d..e7185c6 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -575,6 +575,7 @@ static int __init nf_conntrack_standalone_init(void)
static void __exit nf_conntrack_standalone_fini(void)
{
unregister_pernet_subsys(&nf_conntrack_net_ops);
+ nf_conntrack_cleanup_end();
}
module_init(nf_conntrack_standalone_init);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] bfin_mac: Restore hardware time-stamping dependency on BF518
From: Richard Cochran @ 2013-01-10 16:40 UTC (permalink / raw)
To: Lars-Peter Clausen; +Cc: David S. Miller, uclinux-dist-devel, netdev
In-Reply-To: <1357828948-22682-1-git-send-email-lars@metafoo.de>
On Thu, Jan 10, 2013 at 03:42:28PM +0100, Lars-Peter Clausen wrote:
>
> This patch adds back the dependency on BF518, and since it does not make sense
> to expose this config option when the blackfin MAC driver is not enabled also
> restore the dependency on BFIN_MAC.
Thanks a lot for fixing this.
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next] team: add ethtool support
From: Ben Hutchings @ 2013-01-10 16:27 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Flavio Leitner, Stephen Hemminger, netdev, Jiri Pirko
In-Reply-To: <1356834716.21409.6258.camel@edumazet-glaptop>
On Sat, 2012-12-29 at 18:31 -0800, Eric Dumazet wrote:
> On Sat, 2012-12-29 at 23:44 -0200, Flavio Leitner wrote:
>
> > Speaking as a support engineer, it's a lot easier to grab ethtool -S and
> > see everything than grab two or more outputs.
> >
>
> I agree its very convenient.
>
> I have a patch to add GRO statistics at the core layer, in the ethtool
> -S stats.
>
> I was about to ask netdev guys what they think of this idea ?
[...]
I would like to make these statistics available *but* I would prefer to
see a plan for properly integrating the basic statistics
(net_device_stats/rtnl_link_stats64), additional core statistics (such
as these) and driver-specific statistics.
It's annoying that users have to use different tools to get these, and
adding core statistics to ethtool (currently driver-specific) is just
going to be more confusing.
Ben.
--
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] CDC_NCM adding support IFF_NOARP for buggy device
From: Sergei Shtylyov @ 2013-01-10 17:18 UTC (permalink / raw)
To: Wei Shuai; +Cc: gregkh, alexey.orishko, hans.petter.selasky, linux-usb, netdev
In-Reply-To: <1357826368-30543-1-git-send-email-cpuwolf@gmail.com>
Hello.
On 01/10/2013 04:59 PM, Wei Shuai wrote:
> Signed-off-by: Wei Shuai <cpuwolf@gmail.com>
> ---
> drivers/net/usb/cdc_ncm.c | 15 +++++++++++++++
> 1 files changed, 15 insertions(+), 0 deletions(-)
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 71b6e92..9903f79 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
[...]
> @@ -572,6 +573,10 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
> cdc_ncm_comm_intf_is_mbim(intf->cur_altsetting))
> return -ENODEV;
> #endif
> + /* this buggy device cannot do ARP */
> + if( (le16_to_cpu(udev->descriptor.idVendor) == 0xAAAA) ) {
Please run the patch thru scripts/checkpatch.pl and resubmit.
WBR, Sergei
^ permalink raw reply
* Re: tainted warnings with tcp splicing in 3.7.1
From: Eric Dumazet @ 2013-01-10 16:20 UTC (permalink / raw)
To: Willy Tarreau, Rick Jones
Cc: Christian Becker, David Miller, netdev@vger.kernel.org
In-Reply-To: <1357831787.27446.2168.camel@edumazet-glaptop>
On Thu, 2013-01-10 at 07:29 -0800, Eric Dumazet wrote:
> On Thu, 2013-01-10 at 08:21 +0100, Willy Tarreau wrote:
>
> > FWIW, I tested your patch here and did not notice any regression
> > compared to last week-end tests, at various MTU size combinations.
> >
>
> Thanks Willy
I also want to thanks Rick, as the latest netperf has splice() support.
Thanks Rick !
^ permalink raw reply
* RE: [PATCH 2/2] Fix phy_attach - forward dev_flags for phy_attach
From: Kosta Zertsekel @ 2013-01-10 15:57 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, zertsekel@gmail.com, andrew@lunn.ch,
Eran Ben-Avi, linux-arm-kernel@lists.infradead.org, Lior Amsalem
In-Reply-To: <50EED2E4.80008@openwrt.org>
> By the way, most, if not all of the phy_connect() users in drivers/net/ethernet/ also do not ensure they pass the phy device flags, so you might
want to fix this globally and not just for Marvell driver.
Indeed, phy_connect() mostly just pass zero intead of phy_dev->dev_flags.
But, I think, the guy that calls phy_connect() in its driver should know what he does, and,
probably, we should rely on him knowing his stuff.
The only evidence of the bug is when phy_dev->dev_flags was actually changed by PHY fixup callback
(see dns323-setup.c for example) and was *not* propagated to phy_connect() or phy_attach() as in pxa168_eth.c phy_init().
The code conforming to the former should be fixed IMHO.
Thanks,
--- KostaZ
^ permalink raw reply
* [PATCH net 2/2] bnx2x: Allow management traffic after boot from SAN
From: Yuval Mintz @ 2013-01-10 14:53 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, ariele, Barak Witkowski, Yuval Mintz
In-Reply-To: <1357829620-17890-1-git-send-email-yuvalmin@broadcom.com>
From: Barak Witkowski <barak@broadcom.com>
As part of the previous driver unload flow, whenever bnx2x is
loaded after the UNDI driver it closes all Rx traffic.
However, this leads to management traffic also being stopped until
the network interface associated with one of its functions gets loaded.
To remedy this, management traffic is re-opened once the 'cleaning'
after the previous driver ends.
Signed-off-by: Barak Witkowski <barak@broadcom.com>
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 60 +++++++++++++++++++---
1 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 940ef85..5523da3 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -127,6 +127,17 @@ MODULE_PARM_DESC(debug, " Default debug msglevel");
struct workqueue_struct *bnx2x_wq;
+struct bnx2x_mac_vals {
+ u32 xmac_addr;
+ u32 xmac_val;
+ u32 emac_addr;
+ u32 emac_val;
+ u32 umac_addr;
+ u32 umac_val;
+ u32 bmac_addr;
+ u32 bmac_val[2];
+};
+
enum bnx2x_board_type {
BCM57710 = 0,
BCM57711,
@@ -9420,12 +9431,19 @@ static inline void bnx2x_undi_int_disable(struct bnx2x *bp)
bnx2x_undi_int_disable_e1h(bp);
}
-static void bnx2x_prev_unload_close_mac(struct bnx2x *bp)
+static void bnx2x_prev_unload_close_mac(struct bnx2x *bp,
+ struct bnx2x_mac_vals *vals)
{
u32 val, base_addr, offset, mask, reset_reg;
bool mac_stopped = false;
u8 port = BP_PORT(bp);
+ /* reset addresses as they also mark which values were changed */
+ vals->bmac_addr = 0;
+ vals->umac_addr = 0;
+ vals->xmac_addr = 0;
+ vals->emac_addr = 0;
+
reset_reg = REG_RD(bp, MISC_REG_RESET_REG_2);
if (!CHIP_IS_E3(bp)) {
@@ -9447,14 +9465,18 @@ static void bnx2x_prev_unload_close_mac(struct bnx2x *bp)
*/
wb_data[0] = REG_RD(bp, base_addr + offset);
wb_data[1] = REG_RD(bp, base_addr + offset + 0x4);
+ vals->bmac_addr = base_addr + offset;
+ vals->bmac_val[0] = wb_data[0];
+ vals->bmac_val[1] = wb_data[1];
wb_data[0] &= ~BMAC_CONTROL_RX_ENABLE;
- REG_WR(bp, base_addr + offset, wb_data[0]);
- REG_WR(bp, base_addr + offset + 0x4, wb_data[1]);
+ REG_WR(bp, vals->bmac_addr, wb_data[0]);
+ REG_WR(bp, vals->bmac_addr + 0x4, wb_data[1]);
}
BNX2X_DEV_INFO("Disable emac Rx\n");
- REG_WR(bp, NIG_REG_NIG_EMAC0_EN + BP_PORT(bp)*4, 0);
-
+ vals->emac_addr = NIG_REG_NIG_EMAC0_EN + BP_PORT(bp)*4;
+ vals->emac_val = REG_RD(bp, vals->emac_addr);
+ REG_WR(bp, vals->emac_addr, 0);
mac_stopped = true;
} else {
if (reset_reg & MISC_REGISTERS_RESET_REG_2_XMAC) {
@@ -9465,14 +9487,18 @@ static void bnx2x_prev_unload_close_mac(struct bnx2x *bp)
val & ~(1 << 1));
REG_WR(bp, base_addr + XMAC_REG_PFC_CTRL_HI,
val | (1 << 1));
- REG_WR(bp, base_addr + XMAC_REG_CTRL, 0);
+ vals->xmac_addr = base_addr + XMAC_REG_CTRL;
+ vals->xmac_val = REG_RD(bp, vals->xmac_addr);
+ REG_WR(bp, vals->xmac_addr, 0);
mac_stopped = true;
}
mask = MISC_REGISTERS_RESET_REG_2_UMAC0 << port;
if (mask & reset_reg) {
BNX2X_DEV_INFO("Disable umac Rx\n");
base_addr = BP_PORT(bp) ? GRCBASE_UMAC1 : GRCBASE_UMAC0;
- REG_WR(bp, base_addr + UMAC_REG_COMMAND_CONFIG, 0);
+ vals->umac_addr = base_addr + UMAC_REG_COMMAND_CONFIG;
+ vals->umac_val = REG_RD(bp, vals->umac_addr);
+ REG_WR(bp, vals->umac_addr, 0);
mac_stopped = true;
}
}
@@ -9664,12 +9690,16 @@ static int bnx2x_prev_unload_common(struct bnx2x *bp)
{
u32 reset_reg, tmp_reg = 0, rc;
bool prev_undi = false;
+ struct bnx2x_mac_vals mac_vals;
+
/* It is possible a previous function received 'common' answer,
* but hasn't loaded yet, therefore creating a scenario of
* multiple functions receiving 'common' on the same path.
*/
BNX2X_DEV_INFO("Common unload Flow\n");
+ memset(&mac_vals, 0, sizeof(mac_vals));
+
if (bnx2x_prev_is_path_marked(bp))
return bnx2x_prev_mcp_done(bp);
@@ -9680,7 +9710,10 @@ static int bnx2x_prev_unload_common(struct bnx2x *bp)
u32 timer_count = 1000;
/* Close the MAC Rx to prevent BRB from filling up */
- bnx2x_prev_unload_close_mac(bp);
+ bnx2x_prev_unload_close_mac(bp, &mac_vals);
+
+ /* close LLH filters towards the BRB */
+ bnx2x_set_rx_filter(&bp->link_params, 0);
/* Check if the UNDI driver was previously loaded
* UNDI driver initializes CID offset for normal bell to 0x7
@@ -9727,6 +9760,17 @@ static int bnx2x_prev_unload_common(struct bnx2x *bp)
/* No packets are in the pipeline, path is ready for reset */
bnx2x_reset_common(bp);
+ if (mac_vals.xmac_addr)
+ REG_WR(bp, mac_vals.xmac_addr, mac_vals.xmac_val);
+ if (mac_vals.umac_addr)
+ REG_WR(bp, mac_vals.umac_addr, mac_vals.umac_val);
+ if (mac_vals.emac_addr)
+ REG_WR(bp, mac_vals.emac_addr, mac_vals.emac_val);
+ if (mac_vals.bmac_addr) {
+ REG_WR(bp, mac_vals.bmac_addr, mac_vals.bmac_val[0]);
+ REG_WR(bp, mac_vals.bmac_addr + 4, mac_vals.bmac_val[1]);
+ }
+
rc = bnx2x_prev_mark_path(bp, prev_undi);
if (rc) {
bnx2x_prev_mcp_done(bp);
--
1.7.1
^ permalink raw reply related
* [PATCH net 1/2] bnx2x: Fix fastpath structures when memory allocation fails
From: Yuval Mintz @ 2013-01-10 14:53 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, ariele, Yuval Mintz
In-Reply-To: <1357829620-17890-1-git-send-email-yuvalmin@broadcom.com>
When allocating Tx queues, if for some reason
(e.g., lack of memory) allocation fails, driver will incorrectly
calculate the pointers of the various queues.
This patch repositions all pointers in such a case to point at
sequential structures in memory, allowing the bnx2x macros to
be used correctly when accessing them.
Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ariel Elior <ariele@broadcom.com>
Signed-off-by: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 30 +++++++++++++++++++++-
1 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 01588b6..f771ddf 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -80,12 +80,37 @@ static inline void bnx2x_move_fp(struct bnx2x *bp, int from, int to)
new_txdata_index = new_max_eth_txqs + FCOE_TXQ_IDX_OFFSET;
}
- memcpy(&bp->bnx2x_txq[old_txdata_index],
- &bp->bnx2x_txq[new_txdata_index],
+ memcpy(&bp->bnx2x_txq[new_txdata_index],
+ &bp->bnx2x_txq[old_txdata_index],
sizeof(struct bnx2x_fp_txdata));
to_fp->txdata_ptr[0] = &bp->bnx2x_txq[new_txdata_index];
}
+/**
+ * bnx2x_shrink_eth_fp - guarantees fastpath structures stay intact
+ *
+ * @bp: driver handle
+ * @delta: number of eth queues which were not allocated
+ */
+static void bnx2x_shrink_eth_fp(struct bnx2x *bp, int delta)
+{
+ int i, cos, old_eth_num = BNX2X_NUM_ETH_QUEUES(bp);
+
+ /* Queue pointer cannot be re-set on an fp-basis, as moving pointer
+ * backward along the array could cause memory to be overriden
+ */
+ for (cos = 1; cos < bp->max_cos; cos++) {
+ for (i = 0; i < old_eth_num - delta; i++) {
+ struct bnx2x_fastpath *fp = &bp->fp[i];
+ int new_idx = cos * (old_eth_num - delta) + i;
+
+ memcpy(&bp->bnx2x_txq[new_idx], fp->txdata_ptr[cos],
+ sizeof(struct bnx2x_fp_txdata));
+ fp->txdata_ptr[cos] = &bp->bnx2x_txq[new_idx];
+ }
+ }
+}
+
int load_count[2][3] = { {0} }; /* per-path: 0-common, 1-port0, 2-port1 */
/* free skb in the packet ring at pos idx
@@ -3863,6 +3888,7 @@ int bnx2x_alloc_fp_mem(struct bnx2x *bp)
int delta = BNX2X_NUM_ETH_QUEUES(bp) - i;
WARN_ON(delta < 0);
+ bnx2x_shrink_eth_fp(bp, delta);
if (CNIC_SUPPORT(bp))
/* move non eth FPs next to last eth FP
* must be done in that order
--
1.7.1
^ permalink raw reply related
* [PATCH net 0/2] bnx2x: bug fix patch series
From: Yuval Mintz @ 2013-01-10 14:53 UTC (permalink / raw)
To: davem, netdev; +Cc: eilong, ariele, Yuval Mintz
Hi Dave,
This patch series contains 2 bug fixes:
1. Failed memory allocation during Tx queue allocation can
later cause the driver to dereference incorrect pointers,
causing a panic.
2. During probe, management traffic is stopped and resumes
only after some of its functions gets loaded.
Please consider applying this patch-series to 'net'.
Thanks,
Yuval Mintz
^ permalink raw reply
* Re: tainted warnings with tcp splicing in 3.7.1
From: Eric Dumazet @ 2013-01-10 15:29 UTC (permalink / raw)
To: Willy Tarreau; +Cc: Christian Becker, David Miller, netdev@vger.kernel.org
In-Reply-To: <20130110072136.GK9943@1wt.eu>
On Thu, 2013-01-10 at 08:21 +0100, Willy Tarreau wrote:
> FWIW, I tested your patch here and did not notice any regression
> compared to last week-end tests, at various MTU size combinations.
>
Thanks Willy
Tested-by: Willy Tarreau <w@1wt.eu>
I believe I need to fix net-next,
commit 9ca1b22d6d228177e6f929f6818a1cd3d5e30c4a
(net: splice: avoid high order page splitting)
missed the loopback case : the skb->head might need several
linear_to_page() calls.
I'll send a patch after full testing.
^ permalink raw reply
* Re: [PATCH] tun: avoid owner checks on IFF_ATTACH_QUEUE
From: Jason Wang @ 2013-01-10 15:27 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, David S. Miller, netdev, linux-kernel,
Paul Moore
In-Reply-To: <20130110151037.GD30731@redhat.com>
On 01/10/2013 11:10 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 10, 2013 at 10:47:49PM +0800, Jason Wang wrote:
>> On 01/10/2013 10:41 PM, Michael S. Tsirkin wrote:
>>> On Thu, Jan 10, 2013 at 10:27:20PM +0800, Jason Wang wrote:
>>>> On 01/10/2013 10:19 PM, Michael S. Tsirkin wrote:
>>>>> On Thu, Jan 10, 2013 at 10:08:03PM +0800, Jason Wang wrote:
>>>>>> On 01/10/2013 07:31 PM, Michael S. Tsirkin wrote:
>>>>>>> At the moment, we check owner when we enable queue in tun.
>>>>>>> This seems redundant and will break some valid uses
>>>>>>> where fd is passed around: I think TUNSETOWNER is there
>>>>>>> to prevent others from attaching to a persistent device not
>>>>>>> owned by them. Here the fd is already attached,
>>>>>>> enabling/disabling queue is more like read/write.
>>>>>> It also change the number of queues of the tuntap, maybe we should limit
>>>>>> this.
>>>>> Number of active queues? Why does it matter?
>>>>> Max number of queues is already limited by SETIFF.
>>>> Yes the number of active(real) queues in the kernel net device and this
>>>> changing may introduce other events such uevent.
>>> How can it trigger a uevent?
>> netif_set_real_num_{tx|rx}_queues() will update the queue kobjects which
>> may trigger an uevent.
> Look SETOWNER is a tool intended mostly for persistent taps,
> where you give a specific user the rights to attach to
> specific taps but not others.
True.
> The uevent issue is preventing a DOS by a uevent flood?
> Then it applies to persistent and non persistent as one.
> So if one cares about this one should use an LSM
> or we can add a separate capability to limit this if we
> care enough.
Ok.
>>>> With this patch, even
>>>> if a owner is set for tap, every user could change the number of real
>>>> queues which I don't think is not expected. Without this patch, we can
>>>> limit a user that just do read and write.
>>> In the end if you want very fine tuned security policy you have to
>>> use an LSM.
>>>
>>> Here we are talking about the expected usage without an LSM.
>>> There, enabling/disabling queues is just an optimization:
>>> if an application wants to process data from a single thread
>>> it's better off getting it through a single fd.
>>> Having to channel threading changes through a priveledged
>>> proxy would be very awkward.
>> Yes, but we have something similar like bridge-helper in qemu which
>> create devices through a privileged proxy.
> This only happens on startup. Threading changes can happen
> at any time.
Yes. So no objection from my side. Thanks for the explanation.
>>>>>> Note that if management layer does not call TUNSETOWNER, the check
>>>>>> is just a nop. So if management layer want to limit the behavior, it's
>>>>>> its duty to do this correctly.
>>>>> The point is that management limits tun to allow SETIFF from libvirt
>>>>> only, then passes the fds to qemu.
>>>> Yes, but looks like libvirt does not call TUNSETOWNER before passing it
>>>> to qemu, so we're ok even without this patch. And if libvirt want to do
>>>> this, it can just call TUNSETOWNER to the user of qemu.
>>> No, that would allow qemu to do SETIFF which we don't want.
>> True, I was wrong.
>>>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> Note: this is unrelated to Stefan's bugfix.
>>>>>>>
>>>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>>>>> index fbd106e..78e3225 100644
>>>>>>> --- a/drivers/net/tun.c
>>>>>>> +++ b/drivers/net/tun.c
>>>>>>> @@ -1789,10 +1792,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>>>>>> tun = tfile->detached;
>>>>>>> if (!tun)
>>>>>>> ret = -EINVAL;
>>>>>>> - else if (tun_not_capable(tun))
>>>>>>> - ret = -EPERM;
>>>>>>> else
>>>>>>> ret = tun_attach(tun, file);
>>>>>>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>>>>>> tun = rcu_dereference_protected(tfile->tun,
>>>>>>> lockdep_rtnl_is_held());
>>>>> --
>>>>> 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: recvmmsg() timeout behavior strangeness
From: Chris Friesen @ 2013-01-10 15:24 UTC (permalink / raw)
To: Steven Whitehouse
Cc: Michael Kerrisk, Arnaldo Carvalho de Melo, Caitlin Bestler,
David Miller, netdev, Chris Van Hoof, Clark Williams, Neil Horman,
Arnaldo Carvalho de Melo, Andrew Grover, Elie De Brauwer,
linux-man, Rémi Denis-Courmont
In-Reply-To: <1357812249.2760.6.camel@menhir>
On 01/10/2013 04:04 AM, Steven Whitehouse wrote:
> Which timeout are we talking about? I've been copied into the thread
> without seeing the start of it.
The discussion is about the timeout parameter for the recvmmsg() call.
Currently in the recvmmsg() code it only checks the timeout after
__sys_recvmsg() returns, so if __sys_recvmsg() blocks forever we could
end up essentially ignoring the timeout.
> If this is the rcvtimeo then afaik this is supposed to be the max time
> that the call waits for data, but is overridden by MSG_DONTWAIT, for
> example, on a per call basis. I'd assume that recvmmsg should work
> exactly like recvmsg in this case unless there is a good reason for it
> to differ,
recvmsg() doesn't have a timeout parameter, so it uses SO_RCVTIMEO.
recvmmsg() has an explicit timeout parameter but it doesn't look like it
works properly and the documentation doesn't mention how it is supposed
to interact with SO_RCVTIMEO.
Chris
^ permalink raw reply
* Re: [PATCH] tun: avoid owner checks on IFF_ATTACH_QUEUE
From: Michael S. Tsirkin @ 2013-01-10 15:10 UTC (permalink / raw)
To: Jason Wang
Cc: Stefan Hajnoczi, David S. Miller, netdev, linux-kernel,
Paul Moore
In-Reply-To: <50EED495.4040405@redhat.com>
On Thu, Jan 10, 2013 at 10:47:49PM +0800, Jason Wang wrote:
> On 01/10/2013 10:41 PM, Michael S. Tsirkin wrote:
> > On Thu, Jan 10, 2013 at 10:27:20PM +0800, Jason Wang wrote:
> >> On 01/10/2013 10:19 PM, Michael S. Tsirkin wrote:
> >>> On Thu, Jan 10, 2013 at 10:08:03PM +0800, Jason Wang wrote:
> >>>> On 01/10/2013 07:31 PM, Michael S. Tsirkin wrote:
> >>>>> At the moment, we check owner when we enable queue in tun.
> >>>>> This seems redundant and will break some valid uses
> >>>>> where fd is passed around: I think TUNSETOWNER is there
> >>>>> to prevent others from attaching to a persistent device not
> >>>>> owned by them. Here the fd is already attached,
> >>>>> enabling/disabling queue is more like read/write.
> >>>> It also change the number of queues of the tuntap, maybe we should limit
> >>>> this.
> >>> Number of active queues? Why does it matter?
> >>> Max number of queues is already limited by SETIFF.
> >> Yes the number of active(real) queues in the kernel net device and this
> >> changing may introduce other events such uevent.
> > How can it trigger a uevent?
>
> netif_set_real_num_{tx|rx}_queues() will update the queue kobjects which
> may trigger an uevent.
Look SETOWNER is a tool intended mostly for persistent taps,
where you give a specific user the rights to attach to
specific taps but not others.
The uevent issue is preventing a DOS by a uevent flood?
Then it applies to persistent and non persistent as one.
So if one cares about this one should use an LSM
or we can add a separate capability to limit this if we
care enough.
> >
> >> With this patch, even
> >> if a owner is set for tap, every user could change the number of real
> >> queues which I don't think is not expected. Without this patch, we can
> >> limit a user that just do read and write.
> > In the end if you want very fine tuned security policy you have to
> > use an LSM.
> >
> > Here we are talking about the expected usage without an LSM.
> > There, enabling/disabling queues is just an optimization:
> > if an application wants to process data from a single thread
> > it's better off getting it through a single fd.
> > Having to channel threading changes through a priveledged
> > proxy would be very awkward.
>
> Yes, but we have something similar like bridge-helper in qemu which
> create devices through a privileged proxy.
This only happens on startup. Threading changes can happen
at any time.
> >>>> Note that if management layer does not call TUNSETOWNER, the check
> >>>> is just a nop. So if management layer want to limit the behavior, it's
> >>>> its duty to do this correctly.
> >>> The point is that management limits tun to allow SETIFF from libvirt
> >>> only, then passes the fds to qemu.
> >> Yes, but looks like libvirt does not call TUNSETOWNER before passing it
> >> to qemu, so we're ok even without this patch. And if libvirt want to do
> >> this, it can just call TUNSETOWNER to the user of qemu.
> > No, that would allow qemu to do SETIFF which we don't want.
>
> True, I was wrong.
> >
> >>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>>>
> >>>>> ---
> >>>>>
> >>>>> Note: this is unrelated to Stefan's bugfix.
> >>>>>
> >>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >>>>> index fbd106e..78e3225 100644
> >>>>> --- a/drivers/net/tun.c
> >>>>> +++ b/drivers/net/tun.c
> >>>>> @@ -1789,10 +1792,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
> >>>>> tun = tfile->detached;
> >>>>> if (!tun)
> >>>>> ret = -EINVAL;
> >>>>> - else if (tun_not_capable(tun))
> >>>>> - ret = -EPERM;
> >>>>> else
> >>>>> ret = tun_attach(tun, file);
> >>>>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
> >>>>> tun = rcu_dereference_protected(tfile->tun,
> >>>>> lockdep_rtnl_is_held());
> >>> --
> >>> 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] tun: avoid owner checks on IFF_ATTACH_QUEUE
From: Jason Wang @ 2013-01-10 14:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Hajnoczi, David S. Miller, netdev, linux-kernel,
Paul Moore
In-Reply-To: <20130110144138.GA30731@redhat.com>
On 01/10/2013 10:41 PM, Michael S. Tsirkin wrote:
> On Thu, Jan 10, 2013 at 10:27:20PM +0800, Jason Wang wrote:
>> On 01/10/2013 10:19 PM, Michael S. Tsirkin wrote:
>>> On Thu, Jan 10, 2013 at 10:08:03PM +0800, Jason Wang wrote:
>>>> On 01/10/2013 07:31 PM, Michael S. Tsirkin wrote:
>>>>> At the moment, we check owner when we enable queue in tun.
>>>>> This seems redundant and will break some valid uses
>>>>> where fd is passed around: I think TUNSETOWNER is there
>>>>> to prevent others from attaching to a persistent device not
>>>>> owned by them. Here the fd is already attached,
>>>>> enabling/disabling queue is more like read/write.
>>>> It also change the number of queues of the tuntap, maybe we should limit
>>>> this.
>>> Number of active queues? Why does it matter?
>>> Max number of queues is already limited by SETIFF.
>> Yes the number of active(real) queues in the kernel net device and this
>> changing may introduce other events such uevent.
> How can it trigger a uevent?
netif_set_real_num_{tx|rx}_queues() will update the queue kobjects which
may trigger an uevent.
>
>> With this patch, even
>> if a owner is set for tap, every user could change the number of real
>> queues which I don't think is not expected. Without this patch, we can
>> limit a user that just do read and write.
> In the end if you want very fine tuned security policy you have to
> use an LSM.
>
> Here we are talking about the expected usage without an LSM.
> There, enabling/disabling queues is just an optimization:
> if an application wants to process data from a single thread
> it's better off getting it through a single fd.
> Having to channel threading changes through a priveledged
> proxy would be very awkward.
Yes, but we have something similar like bridge-helper in qemu which
create devices through a privileged proxy.
>>>> Note that if management layer does not call TUNSETOWNER, the check
>>>> is just a nop. So if management layer want to limit the behavior, it's
>>>> its duty to do this correctly.
>>> The point is that management limits tun to allow SETIFF from libvirt
>>> only, then passes the fds to qemu.
>> Yes, but looks like libvirt does not call TUNSETOWNER before passing it
>> to qemu, so we're ok even without this patch. And if libvirt want to do
>> this, it can just call TUNSETOWNER to the user of qemu.
> No, that would allow qemu to do SETIFF which we don't want.
True, I was wrong.
>
>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>
>>>>> ---
>>>>>
>>>>> Note: this is unrelated to Stefan's bugfix.
>>>>>
>>>>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>>>>> index fbd106e..78e3225 100644
>>>>> --- a/drivers/net/tun.c
>>>>> +++ b/drivers/net/tun.c
>>>>> @@ -1789,10 +1792,8 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>>>>> tun = tfile->detached;
>>>>> if (!tun)
>>>>> ret = -EINVAL;
>>>>> - else if (tun_not_capable(tun))
>>>>> - ret = -EPERM;
>>>>> else
>>>>> ret = tun_attach(tun, file);
>>>>> } else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>>>>> tun = rcu_dereference_protected(tfile->tun,
>>>>> lockdep_rtnl_is_held());
>>> --
>>> 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 2/2] Fix phy_attach - forward dev_flags for phy_attach
From: Florian Fainelli @ 2013-01-10 14:40 UTC (permalink / raw)
To: Kosta Zertsekel
Cc: netdev, zertsekel, andrew, benavi, linux-arm-kernel, alior
In-Reply-To: <1357819234-27752-3-git-send-email-konszert@marvell.com>
Le 01/10/13 13:00, Kosta Zertsekel a écrit :
> Change-Id: Ie3191f95c36eada6d0c673460de5393641128182
> ---
> drivers/net/ethernet/marvell/pxa168_eth.c | 2 +-
> net/dsa/slave.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
I think that you should have actually two patches, one for pxa168_eth
and one for net/dsa/slave.c.
Also, please prefix your patches with what was usually used on these
files before: "pxa168_eth:" and "net: dsa:" respectively.
By the way, most, if not all of the phy_connect() users in
drivers/net/ethernet/ also do not ensure they pass the phy device flags,
so you might want to fix this globally and not just for Marvell driver.
Thanks.
--
Florian
^ permalink raw reply
* [PATCH] bfin_mac: Restore hardware time-stamping dependency on BF518
From: Lars-Peter Clausen @ 2013-01-10 14:42 UTC (permalink / raw)
To: David S. Miller
Cc: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
Richard Cochran, netdev-u79uwXL29TY76Z2rM5mHXA
Commit 70ac618c07 ("ptp: fixup Kconfig for two PHC drivers.") removed all
dependencies for the blackfin hardware time-stamping Kconfig entry. Hardware
time-stamping is only available on BF518 though. Since the Kconfig entry is
'default y', just updateing your kernel source and running `make defconfig` will
result in the the following build errors:
drivers/net/ethernet/adi/bfin_mac.c:694: error: implicit declaration of function ‘bfin_read_EMAC_PTP_CTL’
drivers/net/ethernet/adi/bfin_mac.c:702: error: implicit declaration of function ‘bfin_write_EMAC_PTP_FV3’
drivers/net/ethernet/adi/bfin_mac.c:712: error: implicit declaration of function ‘bfin_write_EMAC_PTP_CTL’
drivers/net/ethernet/adi/bfin_mac.c:717: error: implicit declaration of function ‘bfin_write_EMAC_PTP_FOFF’
...
This patch adds back the dependency on BF518, and since it does not make sense
to expose this config option when the blackfin MAC driver is not enabled also
restore the dependency on BFIN_MAC.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
drivers/net/ethernet/adi/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
index e49c0ef..a948160 100644
--- a/drivers/net/ethernet/adi/Kconfig
+++ b/drivers/net/ethernet/adi/Kconfig
@@ -61,6 +61,7 @@ config BFIN_RX_DESC_NUM
config BFIN_MAC_USE_HWSTAMP
bool "Use IEEE 1588 hwstamp"
+ depends on BFIN_MAC && BF518
select PTP_1588_CLOCK
default y
---help---
--
1.8.0
_______________________________________________
Uclinux-dist-devel mailing list
Uclinux-dist-devel@blackfin.uclinux.org
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox