* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: Maciej Żenczykowski @ 2011-10-20 4:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev, bazsi
In-Reply-To: <20111020.001939.548341110762997206.davem@davemloft.net>
> I don't see any real benefit.
>
> If it has been decided that you can't create a new capability for
> tproxy, so that tasks can be segregated out of these more powerful
> networking capability levels, I simply don't see the point.
It indeed seems that transparent may be a little too specific a capability.
Ultimately linux permissions are just not fine grained enough...
unless you start using LSMs
> A process with CAP_NET_RAW can spit whatever crap they want onto the
> network, and receive all packets with impunity.
Agreed. But it can do so via raw sockets, it cannot do so via normal
udp/tcp/ip sockets.
That's why I'd like to relax the permissions check on being able to
switch a socket
into transparent mode. A process with CAP_NET_RAW can already pretty
much emulate
that behaviour by using raw sockets - it just can't do that using the
higher level, often more
usable/useful socket/protocol apis.
> I can't see what this buys us at all, sorry.
See above.
^ permalink raw reply
* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: David Miller @ 2011-10-20 4:19 UTC (permalink / raw)
To: zenczykowski; +Cc: netdev, bazsi
In-Reply-To: <CANP3RGf1ZyNEiADXMsD1C=4GMyOMvxEZQ93nYpghhXhCJKNF6Q@mail.gmail.com>
From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Wed, 19 Oct 2011 20:32:31 -0700
> Are you okay with the patch without any warnings or deprecation markings?
> Or are you against opening up CAP_NET_RAW to this in general?
I don't see any real benefit.
If it has been decided that you can't create a new capability for
tproxy, so that tasks can be segregated out of these more powerful
networking capability levels, I simply don't see the point.
A process with CAP_NET_RAW can spit whatever crap they want onto the
network, and receive all packets with impunity.
I can't see what this buys us at all, sorry.
^ permalink raw reply
* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: Maciej Żenczykowski @ 2011-10-20 3:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev, bazsi
In-Reply-To: <20111019.193435.1214580639401316303.davem@davemloft.net>
Are you okay with the patch without any warnings or deprecation markings?
Or are you against opening up CAP_NET_RAW to this in general?
On Wed, Oct 19, 2011 at 4:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Maciej Żenczykowski <zenczykowski@gmail.com>
> Date: Mon, 17 Oct 2011 15:16:23 -0700
>
>> From: Maciej Żenczykowski <maze@google.com>
>>
>> Up till now the IP{,V6}_TRANSPARENT socket options (which actually set
>> the same bit in the socket struct) have required CAP_NET_ADMIN
>> privileges to set or clear the option.
>>
>> - we make clearing the bit not require any privileges.
>> - we deprecate using CAP_NET_ADMIN for this purpose.
>> - we allow CAP_NET_RAW to set this bit, because raw
>> sockets already effectively allow you to emulate socket
>> transparency.
>> - we print a warning (but allow it) if you try to set the socket
>> option with CAP_NET_ADMIN privs, but without CAP_NET_RAW.
>>
>> Signed-off-by: Maciej Żenczykowski <maze@google.com>
>
> Warnings for something that has worked ever since the feature was
> added, and in fact was the only way to make use of the feature, is
> terrible.
>
> You must support the status quo forever or else you risk breaking
> existing setups. So the warning is pointless, you'll never be
> able to remove CAP_NET_ADMIN from these code paths, so there is
> zero value in warning about it because we'll never change this.
>
> I'm disliking these changes more and more. I refuse to apply this
> patch.
>
--
Maciej A. Żenczykowski
Kernel Networking Developer @ Google
1600 Amphitheatre Parkway, Mountain View, CA 94043
tel: +1 (650) 253-0062
^ permalink raw reply
* IPsec performance bug
From: Kim Phillips @ 2011-10-20 2:22 UTC (permalink / raw)
To: netdev
Hi,
I'm trying to debug an IPSec forwarding performance slowdown on a
p2020 dual-core powerpc linux box using s/w crypto (no crypto h/w
offload enabled) between vanilla kernel versions 2.6.35 and 3.0.
Using a h/w packet generator set to 64-byte packets, I get the
following results:
v2.6.35: 48.5kpps
v3.0.0: 0.2kpps
v3.0.7: 0.2kpps
v3.1.0-rc9-01707-gf7ba35d (a recent net-next): 13.6kpps
I was able to bisect the problem down to the following commit:
7e1dc7b6f709dfc1a9ab4b320dbe723f45992693 is the first bad commit
commit 7e1dc7b6f709dfc1a9ab4b320dbe723f45992693
Author: David S. Miller <davem@davemloft.net>
Date: Sat Mar 12 02:42:11 2011 -0500
net: Use flowi4 and flowi6 in xfrm layer.
Signed-off-by: David S. Miller <davem@davemloft.net>
And, indeed, going back one commit (i.e., v2.6.38-rc8-1468-g2032656
and manually applying commit 7313714: "xfrm: fix
__xfrm_route_forward ()"), brings performance back to ~50kpps from
0.2kpps.
Tracing shows that the commit breaks the route cache [1], and I
understand there is major surgery going on in the area [2], so I
suppose my question is twofold:
(a) was such a large performance drop to be expected for v3.0?
(b) any ideas how to fix? I don't know much about routing
internals, but in ip_route_input_common(), if I remove the input
interface comparison (rth->rt_route_iif ^ iif), I get some
performance back, but the system becomes unstable (it's booted over
nfs).
Thanks,
Kim
[1]
(a) ip_rcv() to xfrm4_output() WITHOUT "net: Use flowi4 and flowi6
in xfrm layer.":
0) | ip_rcv() {
0) | ip_route_input_common() {
0) 0.832 us | skb_dst_set_noref();
0) 2.672 us | }
0) | ip_forward() {
0) | __xfrm_policy_check() {
0) | __xfrm_decode_session() {
0) | xfrm_policy_get_afinfo() {
0) 0.864 us | _raw_read_lock();
0) 2.496 us | }
0) 1.008 us | _decode_session4();
0) 0.848 us | xfrm_policy_put_afinfo.clone.26();
0) 7.696 us | }
0) | flow_cache_lookup() {
0) 0.864 us | local_bh_disable();
0) 0.848 us | local_bh_enable();
0) 4.560 us | }
0) + 14.864 us | }
0) | __xfrm_route_forward() {
0) | __xfrm_decode_session() {
0) | xfrm_policy_get_afinfo() {
0) 0.816 us | _raw_read_lock();
0) 2.496 us | }
0) 0.912 us | _decode_session4();
0) 0.848 us | xfrm_policy_put_afinfo.clone.26();
0) 7.488 us | }
0) | xfrm_lookup() {
0) | flow_cache_lookup() {
0) 0.800 us | local_bh_disable();
0) | xfrm_bundle_flo_get() {
0) | xfrm_bundle_ok.clone.34() {
0) 0.944 us | ipv4_dst_check();
0) 0.800 us | ipv4_default_mtu();
0) 0.832 us | ipv4_dst_check();
0) 0.816 us | ipv4_default_mtu();
0) 7.664 us | }
0) 9.344 us | }
0) 0.832 us | local_bh_enable();
0) + 14.416 us | }
0) 0.832 us | get_seconds();
0) 0.864 us | dst_release();
0) + 19.552 us | }
0) + 29.664 us | }
0) | xfrm4_output() {
(b) ip_rcv() to xfrm4_output() WITH "net: Use flowi4 and flowi6 in
xfrm layer.":
0) | ip_rcv() {
0) | ip_route_input_common() {
0) | fib_lookup() {
0) | fib_rules_lookup() {
0) 0.912 us | fib4_rule_match();
0) | fib4_rule_action() {
0) 0.992 us | fib_get_table();
0) | fib_table_lookup() {
0) 0.976 us | check_leaf.clone.10();
0) 0.928 us | check_leaf.clone.10();
0) 5.872 us | }
0) 9.536 us | }
0) 0.832 us | fib4_rule_match();
0) | fib4_rule_action() {
0) 0.816 us | fib_get_table();
0) | fib_table_lookup() {
0) 1.280 us | check_leaf.clone.10();
0) 3.440 us | }
0) 6.720 us | }
0) + 22.944 us | }
0) + 25.344 us | }
0) | fib_validate_source() {
0) | fib_lookup() {
0) | fib_rules_lookup() {
0) 0.800 us | fib4_rule_match();
0) | fib4_rule_action() {
0) 0.880 us | fib_get_table();
0) | fib_table_lookup() {
0) 0.832 us | check_leaf.clone.10();
0) 2.688 us | }
0) 5.952 us | }
0) 0.816 us | fib4_rule_match();
0) | fib4_rule_action() {
0) 0.800 us | fib_get_table();
0) | fib_table_lookup() {
0) 1.168 us | check_leaf.clone.10();
0) 2.960 us | }
0) 6.160 us | }
0) + 17.808 us | }
0) + 19.392 us | }
0) + 21.424 us | }
0) | rt_dst_alloc() {
0) | dst_alloc() {
0) 1.200 us | kmem_cache_alloc();
0) 0.816 us | local_bh_disable();
0) 0.912 us | local_bh_enable();
0) 6.816 us | }
0) 8.496 us | }
0) | rt_set_nexthop.clone.37() {
0) 1.392 us | inet_getpeer();
0) 0.928 us | ipv4_default_mtu();
0) 5.424 us | }
0) | rt_intern_hash() {
0) | _raw_spin_lock_bh() {
0) 0.880 us | local_bh_disable();
0) 2.528 us | }
0) | call_rcu_bh() {
0) | __call_rcu() {
0) | force_quiescent_state() {
0) 0.816 us | _raw_spin_trylock();
0) 0.928 us | _raw_spin_lock();
0) | force_qs_rnp() {
0) 0.848 us | _raw_spin_lock_irqsave();
0) 0.960 us | dyntick_save_progress_counter();
0) 0.896 us | dyntick_save_progress_counter();
0) | rcu_report_qs_rnp() {
0) 0.816 us | _raw_spin_unlock_irqrestore();
0) 2.576 us | }
0) + 10.016 us | }
0) 0.816 us | _raw_spin_lock();
0) 0.832 us | _raw_spin_unlock_irqrestore();
0) + 19.088 us | }
0) + 21.216 us | }
0) + 23.824 us | }
0) | arp_bind_neighbour() {
0) | neigh_lookup() {
0) 0.784 us | local_bh_disable();
0) 0.992 us | arp_hash();
0) 0.928 us | local_bh_enable();
0) 7.568 us | }
0) 9.552 us | }
0) | _raw_spin_unlock_bh() {
0) 0.848 us | local_bh_enable_ip();
0) 2.496 us | }
0) ! 521.056 us | }
0) ! 971.952 us | }
0) | ip_forward() {
0) | __xfrm_policy_check() {
0) | __xfrm_decode_session() {
0) | xfrm_policy_get_afinfo() {
0) 0.864 us | _raw_read_lock();
0) 2.576 us | }
0) 1.248 us | _decode_session4();
0) 0.896 us | xfrm_policy_put_afinfo.clone.26();
0) 8.384 us | }
0) | flow_cache_lookup() {
0) 0.800 us | local_bh_disable();
0) 0.816 us | local_bh_enable();
0) 4.976 us | }
0) + 16.240 us | }
0) | __xfrm_route_forward() {
0) | __xfrm_decode_session() {
0) | xfrm_policy_get_afinfo() {
0) 0.912 us | _raw_read_lock();
0) 2.496 us | }
0) 0.928 us | _decode_session4();
0) 0.848 us | xfrm_policy_put_afinfo.clone.26();
0) 7.408 us | }
0) | xfrm_lookup() {
0) | flow_cache_lookup() {
0) 0.784 us | local_bh_disable();
0) 1.104 us | kmem_cache_alloc();
0) | xfrm_bundle_lookup() {
0) | xfrm_policy_lookup_bytype.clone.37() {
0) | _raw_read_lock_bh() {
0) 0.784 us | local_bh_disable();
0) 2.464 us | }
0) 1.312 us | xfrm_selector_match();
0) | _raw_read_unlock_bh() {
0) 0.880 us | local_bh_enable_ip();
0) 2.576 us | }
0) + 10.800 us | }
0) | xfrm_resolve_and_create_bundle() {
0) | xfrm_state_find() {
0) | _raw_spin_lock_bh() {
0) 0.832 us | local_bh_disable();
0) 2.544 us | }
0) | xfrm_state_look_at.clone.18() {
0) 1.136 us | xfrm_selector_match();
0) 2.912 us | }
0) | _raw_spin_unlock_bh() {
0) 0.800 us | local_bh_enable_ip();
0) 2.416 us | }
0) + 12.336 us | }
0) | xfrm_policy_get_afinfo() {
0) 0.816 us | _raw_read_lock();
0) 2.528 us | }
0) 0.816 us | xfrm4_get_tos();
0) 0.832 us | xfrm_policy_put_afinfo.clone.26();
0) | xfrm_policy_get_afinfo() {
0) 0.832 us | _raw_read_lock();
0) 2.496 us | }
0) | dst_alloc() {
0) 1.200 us | kmem_cache_alloc();
0) 0.800 us | local_bh_disable();
0) 0.816 us | local_bh_enable();
0) 6.400 us | }
0) 0.816 us | xfrm_policy_put_afinfo.clone.26();
0) | dst_cow_metrics_generic() {
0) 1.056 us | kmem_cache_alloc_trace();
0) 3.168 us | }
0) | xfrm_policy_get_afinfo() {
0) 0.816 us | _raw_read_lock();
0) 2.416 us | }
0) | xfrm4_dst_lookup() {
0) | __ip_route_output_key() {
0) 0.800 us | local_bh_disable();
0) 0.896 us | local_bh_enable();
0) 5.616 us | }
0) 7.264 us | }
0) 0.832 us | xfrm_policy_put_afinfo.clone.26();
0) | xfrm_policy_get_afinfo() {
0) 0.816 us | _raw_read_lock();
0) 2.496 us | }
0) 0.800 us | xfrm4_init_path();
0) 0.832 us | xfrm_policy_put_afinfo.clone.26();
0) 0.832 us | ipv4_default_mtu();
0) | xfrm_state_mtu() {
0) | _raw_spin_lock_bh() {
0) 0.800 us | local_bh_disable();
0) 2.448 us | }
0) 1.056 us | esp4_get_mtu();
0) | _raw_spin_unlock_bh() {
0) 0.832 us | local_bh_enable_ip();
0) 2.416 us | }
0) 9.312 us | }
0) 0.816 us | ipv4_default_mtu();
0) | xfrm_policy_get_afinfo() {
0) 0.816 us | _raw_read_lock();
0) 2.496 us | }
0) 1.088 us | xfrm4_fill_dst();
0) 0.832 us | xfrm_policy_put_afinfo.clone.26();
0) + 79.488 us | }
0) + 93.104 us | }
0) 0.816 us | local_bh_enable();
0) ! 101.120 us | }
0) 0.816 us | get_seconds();
0) 0.976 us | dst_release();
0) ! 106.640 us | }
0) ! 116.656 us | }
0) | xfrm4_output() {
[2] http://vger.kernel.org/netconf2011_slides/davem_netconf2011.pdf
^ permalink raw reply
* [PATCH] ipv4: avoid useless call of the function check_peer_pmtu
From: Gao feng @ 2011-10-20 1:34 UTC (permalink / raw)
To: davem, eric.dumazet; +Cc: netdev, Gao feng
In func ipv4_dst_check,check_peer_pmtu should be called only when peer is updated.
So,if the peer is not updated in ip_rt_frag_needed,we can not inc __rt_peer_genid.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
net/ipv4/route.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 075212e..04a14db 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1568,11 +1568,10 @@ unsigned short ip_rt_frag_needed(struct net *net, const struct iphdr *iph,
est_mtu = mtu;
peer->pmtu_learned = mtu;
peer->pmtu_expires = pmtu_expires;
+ atomic_inc(&__rt_peer_genid);
}
inet_putpeer(peer);
-
- atomic_inc(&__rt_peer_genid);
}
return est_mtu ? : new_mtu;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCBN net-next] mlx4_en: fix skb truesize underestimation
From: David Miller @ 2011-10-19 23:53 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, yevgenyp
In-Reply-To: <1318932782.2657.36.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 18 Oct 2011 12:13:02 +0200
> skb->truesize must account for allocated memory, not the used part of
> it. Doing this work is important to avoid unexpected OOM situations.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
Eric please respin this against current net-next, it doesn't apply
after the mlx4 series I applied today.
Thanks!
^ permalink raw reply
* Re: [PATCH] Fix guest memory leak and panic
From: David Miller @ 2011-10-19 23:42 UTC (permalink / raw)
To: eric.dumazet
Cc: Ian.Campbell, krkumar2, rusty, mst, netdev, linux-kernel,
virtualization
In-Reply-To: <1319015259.3103.3.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 19 Oct 2011 11:07:39 +0200
>> From 369022220db31efb9c261cbabcb890a4d216a176 Mon Sep 17 00:00:00 2001
>> From: Ian Campbell <ian.campbell@citrix.com>
>> Date: Tue, 18 Oct 2011 09:59:37 +0100
>> Subject: [PATCH] net: do not take an additional reference in skb_frag_set_page
>>
>> I audited all of the callers in the tree and only one of them (pktgen) expects
>> it to do so. Taking this reference is pretty obviously confusing and error
>> prone.
>>
>> In particular I looked at the following commits which switched callers of
>> (__)skb_frag_set_page to the skb paged fragment api:
...
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
...
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH net-next] neigh: fix rcu splat in neigh_update()
From: David Miller @ 2011-10-19 23:39 UTC (permalink / raw)
To: eric.dumazet; +Cc: roy.qing.li, ari.m.savolainen, netdev
In-Reply-To: <1318929095.2657.19.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 18 Oct 2011 11:11:35 +0200
> Le mardi 18 octobre 2011 à 16:32 +0800, roy.qing.li@gmail.com a écrit :
>> when use dst_get_neighbour to get neighbour, we need
>> rcu_read_lock to protect, since dst_get_neighbour uses
>> rcu_dereference.
>>
>> The bug was reported by Ari Savolainen <ari.m.savolainen@gmail.com>
>>
> ...
>>
>> Reported-by: Ari Savolainen <ari.m.savolainen@gmail.com>
>> Signed-off-by: RongQing <roy.qing.li@gmail.com>
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] Fix guest memory leak and panic
From: David Miller @ 2011-10-19 23:38 UTC (permalink / raw)
To: krkumar2; +Cc: Ian.Campbell, linux-kernel, mst, netdev, rusty, virtualization
In-Reply-To: <OFF565DE9F.92BF6FE5-ON6525792D.00488336-6525792D.00494D56@in.ibm.com>
Krishna could you please respin your original patch, fixing the subject
and adjusting the commit message to indicate this is a cleanup.
Assume I have applied Ian's patch, because that's what I'm about to
do.
Thanks!
^ permalink raw reply
* Re: [patch] filter: use unsigned int to silence static checker warning
From: David Miller @ 2011-10-19 23:36 UTC (permalink / raw)
To: dan.carpenter; +Cc: netdev, eric.dumazet, xiaosuo, kernel-janitors
In-Reply-To: <20111018070420.GR27732@elgon.mountain>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Tue, 18 Oct 2011 10:04:20 +0300
> This is just a cleanup.
>
> My testing version of Smatch warns about this:
> net/core/filter.c +380 check_load_and_stores(6)
> warn: check 'flen' for negative values
>
> flen comes from the user. We try to clamp the values here between 1
> and BPF_MAXINSNS but the clamp doesn't work because it could be
> negative. This is a bug, but it's not exploitable.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied to net-next, thanks Dan.
^ permalink raw reply
* Re: [PATCH] net: change capability used by socket options IP{,V6}_TRANSPARENT
From: David Miller @ 2011-10-19 23:34 UTC (permalink / raw)
To: zenczykowski; +Cc: maze, netdev, bazsi
In-Reply-To: <1318889783-23183-1-git-send-email-zenczykowski@gmail.com>
From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Mon, 17 Oct 2011 15:16:23 -0700
> From: Maciej Żenczykowski <maze@google.com>
>
> Up till now the IP{,V6}_TRANSPARENT socket options (which actually set
> the same bit in the socket struct) have required CAP_NET_ADMIN
> privileges to set or clear the option.
>
> - we make clearing the bit not require any privileges.
> - we deprecate using CAP_NET_ADMIN for this purpose.
> - we allow CAP_NET_RAW to set this bit, because raw
> sockets already effectively allow you to emulate socket
> transparency.
> - we print a warning (but allow it) if you try to set the socket
> option with CAP_NET_ADMIN privs, but without CAP_NET_RAW.
>
> Signed-off-by: Maciej Żenczykowski <maze@google.com>
Warnings for something that has worked ever since the feature was
added, and in fact was the only way to make use of the feature, is
terrible.
You must support the status quo forever or else you risk breaking
existing setups. So the warning is pointless, you'll never be
able to remove CAP_NET_ADMIN from these code paths, so there is
zero value in warning about it because we'll never change this.
I'm disliking these changes more and more. I refuse to apply this
patch.
^ permalink raw reply
* Re: [PATCH] NET: asix: fix ethtool -e for AX88178 USB dongle
From: David Miller @ 2011-10-19 23:31 UTC (permalink / raw)
To: grundler; +Cc: netdev, linux-kernel, allan, freddy
In-Reply-To: <1318866666-956-1-git-send-email-grundler@chromium.org>
From: Grant Grundler <grundler@chromium.org>
Date: Mon, 17 Oct 2011 08:51:06 -0700
> "ethtool -e ethX" dumps EEPROM data. Patch sets EEPROM length for device.
> Ethtool works alot better when the kernel believes the length is > 0.
>
> From: Allan Chou <allan@asix.com.tw>
> Signed-off-by: Grant Grundler <grundler@chromium.org>
Applied to net-next, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] [IPV6] cleanup: remove unnecessary include.
From: David Miller @ 2011-10-19 23:27 UTC (permalink / raw)
To: wkevils; +Cc: netdev
In-Reply-To: <CAGXs5wWNwy-dWaXMVp62FiaXxiKnDNNE3PX3DtqPVs9VCY+kMA@mail.gmail.com>
From: Kevin Wilson <wkevils@gmail.com>
Date: Sun, 16 Oct 2011 17:21:57 +0200
> This cleanup patch removes unnecessary include from net/ipv6/ip6_fib.c.
>
> Signed-off-by: Kevin Wilson <wkevils@gmail.com>
Applied.
^ permalink raw reply
* Re: net-next [PATCH 1/1] ipv4: compat_ioctl is local to af_inet.c, make it static
From: David Miller @ 2011-10-19 23:24 UTC (permalink / raw)
To: gerrit; +Cc: netdev
In-Reply-To: <20111015192656.GA4066@gerrit.erg.abdn.ac.uk>
From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Date: Sat, 15 Oct 2011 13:26:56 -0600
> ipv4: compat_ioctl is local to af_inet.c, make it static
>
> Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] fib_rules: fix unresolved_rules counting
From: David Miller @ 2011-10-19 23:18 UTC (permalink / raw)
To: eric.dumazet; +Cc: zheng.z.yan, netdev
In-Reply-To: <1318909651.2571.41.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 18 Oct 2011 05:47:31 +0200
> Le mardi 18 octobre 2011 à 09:20 +0800, Yan, Zheng a écrit :
>> we should decrease ops->unresolved_rules when deleting a unresolved rule.
>>
>> Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
...
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [patch net-next]alx: Atheros AR8131/AR8151/AR8152/AR8161 Ethernet driver
From: Joe Perches @ 2011-10-19 22:59 UTC (permalink / raw)
To: Francois Romieu; +Cc: cloud.ren, davem, Luis.Rodriguez, netdev, linux-kernel
In-Reply-To: <20111019222140.GA9937@electric-eye.fr.zoreil.com>
On Thu, 2011-10-20 at 00:21 +0200, Francois Romieu wrote:
> cloud.ren@atheros.com <cloud.ren@atheros.com> :
> > diff --git a/drivers/net/ethernet/atheros/alx/alc_cb.c b/drivers/net/ethernet/atheros/alx/alc_cb.c
> [...]
A bunch of good style comments.
> > + hw->autoneg_advertised = (ALX_LINK_SPEED_1GB_FULL |
> > + ALX_LINK_SPEED_10_HALF |
> > + ALX_LINK_SPEED_10_FULL |
> > + ALX_LINK_SPEED_100_HALF |
> > + ALX_LINK_SPEED_100_FULL);
> Parenthesis abuse.
Maybe. I use parenthesis too, but not the trailing | alignment.
emacs does leading alignment nicely when you use an open paren.
^ permalink raw reply
* Re: [net-next 0/8] stmmac: update to Oct 2011 version (v4)
From: David Miller @ 2011-10-19 22:53 UTC (permalink / raw)
To: peppe.cavallaro; +Cc: netdev, eric.dumazet
In-Reply-To: <1318932085-14927-1-git-send-email-peppe.cavallaro@st.com>
From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
Date: Tue, 18 Oct 2011 12:01:17 +0200
> This patches update the driver adding the chained
> descriptor mode and some new useful fixes.
>
> I've reviewed some patches after the V1/2/3 and added new ones:
All applied, with the updated version of patch #8.
^ permalink raw reply
* Re: [PATCH 0/5] Better namespace handling for /sys/class/net/bonding_masters
From: David Miller @ 2011-10-19 22:49 UTC (permalink / raw)
To: ebiederm; +Cc: gregkh, linux-kernel, netdev, htejun, fubar, andy
In-Reply-To: <m18voh158i.fsf@fess.ebiederm.org>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Wed, 19 Oct 2011 06:27:25 -0700
> David Miller <davem@davemloft.net> writes:
>
>> From: ebiederm@xmission.com (Eric W. Biederman)
>> Date: Thu, 13 Oct 2011 00:47:46 -0700
>>
>>> Greg, Dave I'm don't know whose tree to merge this through as this code
>>> is equally device-core and networking. I am hoping that we can get this
>>> improvement merged for 3.2.
>>
>> I'm happy to take this series into my net-next tree.
>>
>> Greg? Any objections?
>
> on the 13th Greg said:
>
>> These all look fine to me, and I have no problem with them going through
>> the network tree as that will most likely be the easiest way due to any
>> potential merge issues.
>>
>> David, feel free to add an:
>> Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
Awesome, thanks for the reminder, I'll apply these right now.
^ permalink raw reply
* Re: [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: David Miller @ 2011-10-19 22:46 UTC (permalink / raw)
To: romieu; +Cc: hayeswang, netdev, linux-kernel
In-Reply-To: <20111019224312.GA10574@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Thu, 20 Oct 2011 00:43:12 +0200
> David Miller <davem@davemloft.net> :
>> > Correct the wrong parameter for setting EEE for RTL8111E-VL.
>> >
>> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>>
>> Francois, want me to apply this directly or will you handle it?
>
> Please apply it directly. I must trust Hayes on this one.
Ok, will do, thanks!
^ permalink raw reply
* Re: [PATCH] r8169: fix wrong eee setting for rlt8111evl
From: Francois Romieu @ 2011-10-19 22:43 UTC (permalink / raw)
To: David Miller; +Cc: hayeswang, netdev, linux-kernel
In-Reply-To: <20111019.165742.1621277627528277376.davem@davemloft.net>
David Miller <davem@davemloft.net> :
> > Correct the wrong parameter for setting EEE for RTL8111E-VL.
> >
> > Signed-off-by: Hayes Wang <hayeswang@realtek.com>
>
> Francois, want me to apply this directly or will you handle it?
Please apply it directly. I must trust Hayes on this one.
--
Ueimor
^ permalink raw reply
* Re: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering support for passthru mode
From: Roopa Prabhu @ 2011-10-19 22:30 UTC (permalink / raw)
To: Rose, Gregory V, netdev@vger.kernel.org
Cc: sri@us.ibm.com, dragos.tatulea@gmail.com, arnd@arndb.de,
kvm@vger.kernel.org, mst@redhat.com, davem@davemloft.net,
mchan@broadcom.com, dwang2@cisco.com, shemminger@vyatta.com,
eric.dumazet@gmail.com, kaber@trash.net, benve@cisco.com
In-Reply-To: <43F901BD926A4E43B106BF17856F075501A177719C@orsmsx508.amr.corp.intel.com>
On 10/19/11 2:06 PM, "Rose, Gregory V" <gregory.v.rose@intel.com> wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>> On Behalf Of Roopa Prabhu
>> Sent: Tuesday, October 18, 2011 11:26 PM
>> To: netdev@vger.kernel.org
>> Cc: sri@us.ibm.com; dragos.tatulea@gmail.com; arnd@arndb.de;
>> kvm@vger.kernel.org; mst@redhat.com; davem@davemloft.net;
>> mchan@broadcom.com; dwang2@cisco.com; shemminger@vyatta.com;
>> eric.dumazet@gmail.com; kaber@trash.net; benve@cisco.com
>> Subject: [net-next-2.6 PATCH 0/8 RFC v2] macvlan: MAC Address filtering
>> support for passthru mode
>>
>
> [snip...]
>
>>
>>
>> Note: The choice of rtnl_link_ops was because I saw the use case for
>> this in virtual devices that need to do filtering in sw like macvlan
>> and tun. Hw devices usually have filtering in hw with netdev->uc and
>> mc lists to indicate active filters. But I can move from rtnl_link_ops
>> to netdev_ops if that is the preferred way to go and if there is a
>> need to support this interface on all kinds of interfaces.
>> Please suggest.
>
> I'm still digesting the rest of the RFC patches but I did want to quickly jump
> in and push for adding this support in netdev_ops. I would like to see these
> features available in more devices than just macvtap and macvlan. I can
> conceive
> of use cases for multiple HW MAC and VLAN filters for a VF device that isn't
> owned by a macvlan/macvtap interface and only has netdev_ops support. In this
> case it would be necessary to program the filters directly to the VF device
> interface or PF interface (or lowerdev as you refer to it) instead of going
> through macvlan/macvtap.
>
> This work dovetails nicely with some work I've been doing and I'd be very
> interested
> in helping move this forward if we could work out the details that would allow
> support
> of the features we (and the community) require.
Great. Thanks. I will definitely be interested to get this patch working for
any other use case you have.
Moving the ops to netdev should be trivial. You probably want the ops to
work on the VF via the PF, like the existing ndo_set_vf_mac etc.
Yes, lets work out the details and I can move this to netdev->ops. Let me
know.
Thanks,
Roopa
^ permalink raw reply
* Re: [patch net-next]alx: Atheros AR8131/AR8151/AR8152/AR8161 Ethernet driver
From: Francois Romieu @ 2011-10-19 22:21 UTC (permalink / raw)
To: cloud.ren; +Cc: davem, Luis.Rodriguez, netdev, linux-kernel
In-Reply-To: <1319009213-20627-1-git-send-email-cloud.ren@atheros.com>
cloud.ren@atheros.com <cloud.ren@atheros.com> :
> diff --git a/drivers/net/ethernet/atheros/alx/alc_cb.c b/drivers/net/ethernet/atheros/alx/alc_cb.c
[...]
> +int alc_read_phy_reg(struct alx_hw *hw, u32 device_type,
> + u16 reg_addr, u16 *phy_data)
> +{
> + bool fast = false;
> + bool ext = false;
> + u16 error;
> + int retval = 0;
> +
> + ALX_MDIO_LOCK(&hw->mdio_lock);
Frowned upon / useless wrapper.
> +
> + if (device_type != ALX_MDIO_NORM_DEV)
> + ext = true;
This method seems to be always called with device_type = ALX_MDIO_NORM_DEV.
> +
> + error = l1c_read_phy(hw, ext, device_type, fast,
> + reg_addr, phy_data);
Imho the driver wants something like l1c_read_phy_{fast/slow}.
> + if (error) {
> + HW_PRINT(ERR, "Error when reading phy reg (%d).", error);
> + retval = ALX_ERR_PHY_READ_REG;
I am a bit sceptical that private error codes are really useful
in a mundane ethernet driver.
[...]
> +int alc_write_phy_reg(struct alx_hw *hw, u32 device_type,
> + u16 reg_addr, u16 phy_data)
> +{
> + bool fast = false;
> + bool ext = false;
> + u16 error;
> + int retval = 0;
> +
> + ALX_MDIO_LOCK(&hw->mdio_lock);
> +
> + if (device_type != ALX_MDIO_NORM_DEV)
> + ext = true;
This method seems to be always called with device_type = ALX_MDIO_NORM_DEV.
> +
> + error = l1c_write_phy(hw, ext, device_type, fast,
> + reg_addr, phy_data);
It fits in a single 80 columns line.
[...]
> +int alc_init_phy(struct alx_hw *hw)
> +{
> + u16 phy_id[2];
> + int retval = 0;
Useless init.
> +
> + HW_PRINT(DEBUG, "ENTER\n");
Old school.
> +
> + /* 1. init mdio spin lock */
> + ALX_MDIO_LOCK_INIT(&hw->mdio_lock);
The comment adds no value.
> +
> + /* 2. read phy id */
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_PHYSID1, &phy_id[0]);
Sic.
> + if (retval)
> + return retval;
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_PHYSID1, &phy_id[1]);
> + if (retval)
> + return retval;
> +
> + memcpy(&hw->phy_id, phy_id, sizeof(hw->phy_id));
> +
> + hw->autoneg_advertised = (ALX_LINK_SPEED_1GB_FULL |
> + ALX_LINK_SPEED_10_HALF |
> + ALX_LINK_SPEED_10_FULL |
> + ALX_LINK_SPEED_100_HALF |
> + ALX_LINK_SPEED_100_FULL);
Parenthesis abuse.
[...]
> +int alc_ack_phy_intr(struct alx_hw *hw)
> +{
> + int retval = 0;
Useless init.
> + u16 isr = 0;
> +
> + HW_PRINT(DEBUG, "ENTER\n");
> +
> + retval = alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV,
> + L1C_MII_ISR, &isr);
> + if (retval)
> + return retval;
> +
> + return retval;
The style is terrible. What about:
return alc_read_phy_reg(hw, ALX_MDIO_NORM_DEV, L1C_MII_ISR, &isr);
[...]
> +int alc_clear_mac_addr(struct alx_hw *hw)
> +{
> + int retval = 0;
> +
> + HW_PRINT(DEBUG, "ENTER\n");
> +
> + return retval;
> +}
{alc/alf}_clear_mac_addr always returns 0 and it does not seem to do
anything else.
[...]
> +int alc_config_rx(struct alx_hw *hw)
> +{
> + int retval = 0;
> + return retval;
> +}
Useless. {alc/alf}_config_rx always returns 0 and it does not anything else.
[...]
> +int alc_check_nvram(struct alx_hw *hw, bool *exist)
> +{
> + *exist = false;
> + return 0;
> +}
Strictly identical to alf_check_nvram.
[...]
> +int alc_get_ethtool_regs(struct alx_hw *hw, void *buff)
> +{
> + u32 *regs = (u32 *)buff;
Useless cast from void.
> + int retval = 0;
> +
> + MEM_R32(hw, L1C_LNK_CAP, ®s[0]);
> + MEM_R32(hw, L1C_PMCTRL, ®s[1]);
> + MEM_R32(hw, L1C_HALFD, ®s[2]);
> + MEM_R32(hw, L1C_SLD, ®s[3]);
> + MEM_R32(hw, L1C_MASTER, ®s[4]);
> + MEM_R32(hw, L1C_MANU_TIMER, ®s[5]);
> + MEM_R32(hw, L1C_IRQ_MODU_TIMER, ®s[6]);
> + MEM_R32(hw, L1C_PHY_CTRL, ®s[7]);
> + MEM_R32(hw, L1C_LNK_CTRL, ®s[8]);
> + MEM_R32(hw, L1C_MAC_STS, ®s[9]);
> +
> + MEM_R32(hw, L1C_MDIO, ®s[10]);
> + MEM_R32(hw, L1C_SERDES, ®s[11]);
> + MEM_R32(hw, L1C_MAC_CTRL, ®s[12]);
> + MEM_R32(hw, L1C_GAP, ®s[13]);
> + MEM_R32(hw, L1C_STAD0, ®s[14]);
> + MEM_R32(hw, L1C_STAD1, ®s[15]);
> + MEM_R32(hw, L1C_HASH_TBL0, ®s[16]);
> + MEM_R32(hw, L1C_HASH_TBL1, ®s[17]);
> + MEM_R32(hw, L1C_RXQ0, ®s[18]);
> + MEM_R32(hw, L1C_RXQ1, ®s[19]);
> +
> + MEM_R32(hw, L1C_RXQ2, ®s[20]);
> + MEM_R32(hw, L1C_RXQ3, ®s[21]);
> + MEM_R32(hw, L1C_TXQ0, ®s[22]);
> + MEM_R32(hw, L1C_TXQ1, ®s[23]);
> + MEM_R32(hw, L1C_TXQ2, ®s[24]);
> + MEM_R32(hw, L1C_MTU, ®s[25]);
> + MEM_R32(hw, L1C_WOL0, ®s[26]);
> + MEM_R32(hw, L1C_WOL1, ®s[27]);
> + MEM_R32(hw, L1C_WOL2, ®s[28]);
Tabulate and save code ?
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alc_hw.h b/drivers/net/ethernet/atheros/alx/alc_hw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alc_hw.h
[...]
> +#ifdef __cplusplus
> +extern "C" {
> +#endif/*__cplusplus*/
Please remove this stuff before it gets noticed.
[...]
> +#define L1C_PCI_CMD 0x0004 /* 16bit */
> +#define L1C_PCI_CMD_DISINT BIT_10S
> +#define L1C_PCI_CMD_MASTEREN BIT_2S
> +#define L1C_PCI_CMD_MEMEN BIT_1S
> +#define L1C_PCI_CMD_IOEN BIT_0S
Duplicates from <linux/pci_regs.h>
[...]
> +/********************* PHY regs definition ***************************/
> +
> +/* PHY Control Register */
> +#define L1C_MII_BMCR 0x00
> +#define L1C_BMCR_SPEED_SELECT_MSB 0x0040
> +#define L1C_BMCR_COLL_TEST_ENABLE 0x0080
> +#define L1C_BMCR_FULL_DUPLEX 0x0100
> +#define L1C_BMCR_RESTART_AUTO_NEG 0x0200
> +#define L1C_BMCR_ISOLATE 0x0400
> +#define L1C_BMCR_POWER_DOWN 0x0800
> +#define L1C_BMCR_AUTO_NEG_EN 0x1000
> +#define L1C_BMCR_SPEED_SELECT_LSB 0x2000
> +#define L1C_BMCR_LOOPBACK 0x4000
> +#define L1C_BMCR_RESET 0x8000
> +#define L1C_BMCR_SPEED_MASK 0x2040
> +#define L1C_BMCR_SPEED_1000 0x0040
> +#define L1C_BMCR_SPEED_100 0x2000
> +#define L1C_BMCR_SPEED_10 0x0000
Please use existing #define from <linux/mii.h>.
> +
> +/* PHY Status Register */
> +#define L1C_MII_BMSR 0x01
> +#define L1C_BMSR_EXTENDED_CAPS 0x0001
> +#define L1C_BMSR_JABBER_DETECT 0x0002
> +#define L1C_BMSR_LINK_STATUS 0x0004
> +#define L1C_BMSR_AUTONEG_CAPS 0x0008
> +#define L1C_BMSR_REMOTE_FAULT 0x0010
> +#define L1C_BMSR_AUTONEG_COMPLETE 0x0020
> +#define L1C_BMSR_PREAMBLE_SUPPRESS 0x0040
> +#define L1C_BMSR_EXTENDED_STATUS 0x0100
> +#define L1C_BMSR_100T2_HD_CAPS 0x0200
> +#define L1C_BMSR_100T2_FD_CAPS 0x0400
> +#define L1C_BMSR_10T_HD_CAPS 0x0800
> +#define L1C_BMSR_10T_FD_CAPS 0x1000
> +#define L1C_BMSR_100X_HD_CAPS 0x2000
> +#define L1C_BMMII_SR_100X_FD_CAPS 0x4000
> +#define L1C_BMMII_SR_100T4_CAPS 0x8000
Sic.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_cb.c b/drivers/net/ethernet/atheros/alx/alf_cb.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_cb.c
[...]
> +int alf_get_ethtool_regs(struct alx_hw *hw, void *buff)
> +{
> + u32 *regs = (u32 *)buff;
> + int i;
> + int retval = 0;
> +
> + MEM_R32(hw, L1F_PM_CSR, ®s[0]);
> + MEM_R32(hw, L1F_DEV_CAP, ®s[1]);
> + MEM_R32(hw, L1F_DEV_CTRL, ®s[2]);
> + MEM_R32(hw, L1F_LNK_CAP, ®s[3]);
> + MEM_R32(hw, L1F_LNK_CTRL, ®s[4]);
> + MEM_R32(hw, L1F_UE_SVRT, ®s[5]);
> + MEM_R32(hw, L1F_EFLD, ®s[6]);
> + MEM_R32(hw, L1F_SLD, ®s[7]);
> + MEM_R32(hw, L1F_PPHY_MISC1, ®s[8]);
> + MEM_R32(hw, L1F_PPHY_MISC2, ®s[9]);
> +
> + MEM_R32(hw, L1F_PDLL_TRNS1, ®s[10]);
> + MEM_R32(hw, L1F_TLEXTN_STATS, ®s[11]);
> + MEM_R32(hw, L1F_EFUSE_CTRL, ®s[12]);
> + MEM_R32(hw, L1F_EFUSE_DATA, ®s[13]);
> + MEM_R32(hw, L1F_SPI_OP1, ®s[14]);
> + MEM_R32(hw, L1F_SPI_OP2, ®s[15]);
> + MEM_R32(hw, L1F_SPI_OP3, ®s[16]);
> + MEM_R32(hw, L1F_EF_CTRL, ®s[17]);
> + MEM_R32(hw, L1F_EF_ADDR, ®s[18]);
> + MEM_R32(hw, L1F_EF_DATA, ®s[19]);
> +
> + MEM_R32(hw, L1F_SPI_ID, ®s[20]);
> + MEM_R32(hw, L1F_SPI_CFG_START, ®s[21]);
> + MEM_R32(hw, L1F_PMCTRL, ®s[22]);
> + MEM_R32(hw, L1F_LTSSM_CTRL, ®s[23]);
> + MEM_R32(hw, L1F_MASTER, ®s[24]);
> + MEM_R32(hw, L1F_MANU_TIMER, ®s[25]);
> + MEM_R32(hw, L1F_IRQ_MODU_TIMER, ®s[26]);
> + MEM_R32(hw, L1F_PHY_CTRL, ®s[27]);
> + MEM_R32(hw, L1F_MAC_STS, ®s[28]);
> + MEM_R32(hw, L1F_MDIO, ®s[29]);
> +
> + MEM_R32(hw, L1F_MDIO_EXTN, ®s[30]);
> + MEM_R32(hw, L1F_PHY_STS, ®s[31]);
> + MEM_R32(hw, L1F_BIST0, ®s[32]);
> + MEM_R32(hw, L1F_BIST1, ®s[33]);
> + MEM_R32(hw, L1F_SERDES, ®s[34]);
> + MEM_R32(hw, L1F_LED_CTRL, ®s[35]);
> + MEM_R32(hw, L1F_LED_PATN, ®s[36]);
> + MEM_R32(hw, L1F_LED_PATN2, ®s[37]);
> + MEM_R32(hw, L1F_SYSALV, ®s[38]);
> + MEM_R32(hw, L1F_PCIERR_INST, ®s[39]);
> +
> + MEM_R32(hw, L1F_LPI_DECISN_TIMER, ®s[40]);
> + MEM_R32(hw, L1F_LPI_CTRL, ®s[41]);
> + MEM_R32(hw, L1F_LPI_WAIT, ®s[42]);
> + MEM_R32(hw, L1F_HRTBT_VLAN, ®s[43]);
> + MEM_R32(hw, L1F_HRTBT_CTRL, ®s[44]);
> + MEM_R32(hw, L1F_RXPARSE, ®s[45]);
> + MEM_R32(hw, L1F_MAC_CTRL, ®s[46]);
> + MEM_R32(hw, L1F_GAP, ®s[47]);
> + MEM_R32(hw, L1F_STAD1, ®s[48]);
> + MEM_R32(hw, L1F_LED_CTRL, ®s[49]);
> +
> + MEM_R32(hw, L1F_HASH_TBL0, ®s[50]);
> + MEM_R32(hw, L1F_HASH_TBL1, ®s[51]);
> + MEM_R32(hw, L1F_HALFD, ®s[52]);
> + MEM_R32(hw, L1F_DMA, ®s[53]);
> + MEM_R32(hw, L1F_WOL0, ®s[54]);
> + MEM_R32(hw, L1F_WOL1, ®s[55]);
> + MEM_R32(hw, L1F_WOL2, ®s[56]);
> + MEM_R32(hw, L1F_WRR, ®s[57]);
> + MEM_R32(hw, L1F_HQTPD, ®s[58]);
> + MEM_R32(hw, L1F_CPUMAP1, ®s[59]);
> + MEM_R32(hw, L1F_CPUMAP2, ®s[60]);
> + MEM_R32(hw, L1F_MISC, ®s[61]);
You ought to iterate with a well chosen data structure so as to avoid
this huge code pollution.
[...]
> +int alf_init_hw_callbacks(struct alx_hw *hw)
> +{
> + int retval = 0;
> +
> + /* NIC */
> + hw->cbs.identify_nic = &alf_identify_nic;
> + /* MAC */
> + hw->cbs.reset_mac = &alf_reset_mac;
> + hw->cbs.start_mac = &alf_start_mac;
> + hw->cbs.stop_mac = &alf_stop_mac;
> + hw->cbs.config_mac = &alf_config_mac;
> + hw->cbs.get_mac_addr = &alf_get_mac_addr;
> + hw->cbs.set_mac_addr = &alf_set_mac_addr;
> + hw->cbs.clear_mac_addr = &alf_clear_mac_addr;
> + hw->cbs.set_mc_addr = &alf_set_mc_addr;
> + hw->cbs.clear_mc_addr = &alf_clear_mc_addr;
> +
> + /* PHY */
> + hw->cbs.init_phy = &alf_init_phy;
> + hw->cbs.reset_phy = &alf_reset_phy;
> + hw->cbs.read_phy_reg = &alf_read_phy_reg;
> + hw->cbs.write_phy_reg = &alf_write_phy_reg;
> + hw->cbs.check_phy_link = &alf_check_phy_link;
> + hw->cbs.setup_phy_link = &alf_setup_phy_link;
> + hw->cbs.setup_phy_link_speed = &alf_setup_phy_link_speed;
> +
> + /* Interrupt */
> + hw->cbs.ack_phy_intr = &alf_ack_phy_intr;
> + hw->cbs.enable_legacy_intr = &alf_enable_legacy_intr;
> + hw->cbs.disable_legacy_intr = &alf_disable_legacy_intr;
> + hw->cbs.enable_msix_intr = &alf_enable_msix_intr;
> + hw->cbs.disable_msix_intr = &alf_disable_msix_intr;
> +
> + /* Configure */
> + hw->cbs.config_rx = &alf_config_rx;
> + hw->cbs.config_tx = &alf_config_tx;
> + hw->cbs.config_fc = &alf_config_fc;
> + hw->cbs.config_rss = &alf_config_rss;
> + hw->cbs.config_msix = &alf_config_msix;
> + hw->cbs.config_wol = &alf_config_wol;
> + hw->cbs.config_aspm = &alf_config_aspm;
> + hw->cbs.config_mac_ctrl = &alf_config_mac_ctrl;
> + hw->cbs.config_pow_save = &alf_config_pow_save;
> + hw->cbs.reset_pcie = &alf_reset_pcie;
> +
> + /* NVRam */
> + hw->cbs.check_nvram = &alf_check_nvram;
> +
> + /* Others */
> + hw->cbs.get_ethtool_regs = alf_get_ethtool_regs;
> +
> + retval = alf_set_hw_capabilities(hw);
> +
> + retval = alf_set_hw_infos(hw);
> +
> + /* print hw flags */
> + HW_PRINT(INFO, "HW Flags = 0x%x\n", hw->flags);
> + return retval;
Almost every method in this file ought to be static.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_hw.c b/drivers/net/ethernet/atheros/alx/alf_hw.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_hw.c
[...]
> +/* disable/enable MAC/RXQ/TXQ
> + * en
> + * true:enable
> + * false:disable
> + * return
> + * 0:success
> + * non-0-fail
> + */
> +u16 l1f_enable_mac(PETHCONTEXT ctx, bool en, u16 en_ctrl)
The name of this method is rather poor.
> +{
> + u32 rxq, txq, mac, val;
> + u16 i;
> +
> + MEM_R32(ctx, L1F_RXQ0, &rxq);
> + MEM_R32(ctx, L1F_TXQ0, &txq);
> + MEM_R32(ctx, L1F_MAC_CTRL, &mac);
> +
> + if (en) { /* enable */
> + MEM_W32(ctx, L1F_RXQ0, rxq | L1F_RXQ0_EN);
> + MEM_W32(ctx, L1F_TXQ0, txq | L1F_TXQ0_EN);
> + if (0 != (en_ctrl & LX_MACSPEED_1000)) {
> + FIELD_SETL(mac, L1F_MAC_CTRL_SPEED,
> + L1F_MAC_CTRL_SPEED_1000);
> + } else {
> + FIELD_SETL(mac, L1F_MAC_CTRL_SPEED,
> + L1F_MAC_CTRL_SPEED_10_100);
> + }
> + if (0 != (en_ctrl & LX_MACDUPLEX_FULL)) {
> + mac |= L1F_MAC_CTRL_FULLD;
> + } else {
> + mac &= ~L1F_MAC_CTRL_FULLD;
> + }
> + /* rx filter */
> + if (0 != (en_ctrl & LX_FLT_PROMISC)) {
> + mac |= L1F_MAC_CTRL_PROMISC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_PROMISC_EN;
> + }
> + if (0 != (en_ctrl & LX_FLT_MULTI_ALL)) {
> + mac |= L1F_MAC_CTRL_MULTIALL_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_MULTIALL_EN;
> + }
> + if (0 != (en_ctrl & LX_FLT_BROADCAST)) {
> + mac |= L1F_MAC_CTRL_BRD_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_BRD_EN;
> + }
Code duplication. Who cares ?
> + if (0 != (en_ctrl & LX_FLT_DIRECT)) {
> + mac |= L1F_MAC_CTRL_RX_EN;
> + } else { /* disable all recv if direct not enable */
> + mac &= ~L1F_MAC_CTRL_RX_EN;
> + }
> + if (0 != (en_ctrl & LX_FC_TXEN)) {
> + mac |= L1F_MAC_CTRL_TXFC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_TXFC_EN;
> + }
<blink>do it</blink>
> + if (0 != (en_ctrl & LX_FC_RXEN)) {
> + mac |= L1F_MAC_CTRL_RXFC_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_RXFC_EN;
> + }
> + if (0 != (en_ctrl & LX_VLAN_STRIP)) {
> + mac |= L1F_MAC_CTRL_VLANSTRIP;
> + } else {
> + mac &= ~L1F_MAC_CTRL_VLANSTRIP;
> + }
Nevermind.
> + if (0 != (en_ctrl & LX_LOOPBACK)) {
> + mac |= (L1F_MAC_CTRL_LPBACK_EN);
> + } else {
> + mac &= ~L1F_MAC_CTRL_LPBACK_EN;
> + }
> + if (0 != (en_ctrl & LX_SINGLE_PAUSE)) {
> + mac |= L1F_MAC_CTRL_SPAUSE_EN;
> + } else {
> + mac &= ~L1F_MAC_CTRL_SPAUSE_EN;
> + }
> + if (0 != (en_ctrl & LX_ADD_FCS)) {
> + mac |= (L1F_MAC_CTRL_PCRCE | L1F_MAC_CTRL_CRCE);
> + } else {
> + mac &= ~(L1F_MAC_CTRL_PCRCE | L1F_MAC_CTRL_CRCE);
> + }
> + MEM_W32(ctx, L1F_MAC_CTRL, mac | L1F_MAC_CTRL_TX_EN);
It may make some sense to move these ~60 loc in a xyz_enable_something
method...
> + } else { /* disable mac */
... and this would be the xyz_disable_something counterpart.
[...]
> +u16 l1f_enable_aspm(PETHCONTEXT ctx, bool l0s_en, bool l1_en, u8 lnk_stat)
> +{
> + u32 pmctrl;
> + u8 rev = (u8)(FIELD_GETX(ctx->pci_revid, L1F_PCI_REVID));
> +
> + lnk_stat = lnk_stat;
> +
> +
> +#if 0 /* let sys bios control the L0S/L1 enable/disable */
Don't #if 0. Just remove it.
[...]
> +u16 l1f_write_phy(PETHCONTEXT ctx, bool ext, u8 dev, bool fast,
> + u16 reg, u16 data)
> +{
> + u32 val;
> + u16 clk_sel, i, ret = 0;
> +
> +#if MAC_TYPE_FPGA == MAC_TYPE
> + MEM_W32(ctx, L1F_MDIO, 0);
> + US_DELAY(ctx, 30);
> + for (i = 0; i < L1F_MDIO_MAX_AC_TO; i++) {
> + MEM_R32(ctx, L1F_MDIO, &val);
> + if (0 == (val & L1F_MDIO_BUSY)) {
> + break;
> + }
> + US_DELAY(ctx, 10);
> + }
I wonder how many times this 'for' loop is duplicated (4x ?).
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alf_hw.h b/drivers/net/ethernet/atheros/alx/alf_hw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alf_hw.h
[...]
> +#ifdef __cplusplus
> +extern "C" {
> +#endif/*__cplusplus*/
Oops.
[...]
> +#define L1F_PCI_CMD 0x0004 /* 16bit */
> +#define L1F_PCI_CMD_DISINT BIT_10S
> +#define L1F_PCI_CMD_MASTEREN BIT_2S
> +#define L1F_PCI_CMD_MEMEN BIT_1S
> +#define L1F_PCI_CMD_IOEN BIT_0S
Duplicates <linux/pci_regs.h>
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_ethtool.c b/drivers/net/ethernet/atheros/alx/alx_ethtool.c
> +++ b/drivers/net/ethernet/atheros/alx/alx_ethtool.c
[...]
> +static struct ethtool_ops alx_ethtool_ops = {
> + .get_settings = alx_get_settings,
> + .set_settings = alx_set_settings,
> + .get_pauseparam = alx_get_pauseparam,
> + .set_pauseparam = alx_set_pauseparam,
> + .get_drvinfo = alx_get_drvinfo,
> + .get_regs_len = alx_get_regs_len,
> + .get_regs = alx_get_regs,
> + .get_wol = alx_get_wol,
> + .set_wol = alx_set_wol,
> + .get_msglevel = alx_get_msglevel,
> + .set_msglevel = alx_set_msglevel,
> + .nway_reset = alx_nway_reset,
> + .get_link = ethtool_op_get_link,
> + .get_eeprom_len = alx_get_eeprom_len,
> + .get_eeprom = alx_get_eeprom,
> + .set_eeprom = alx_set_eeprom,
> + .get_tx_csum = alx_get_tx_csum,
> + .get_sg = ethtool_op_get_sg,
> + .set_sg = ethtool_op_set_sg,
> +#ifdef NETIF_F_TSO
> + .get_tso = ethtool_op_get_tso,
> +#endif
Please switch to ndo_{fix/set}_features.
> diff --git a/drivers/net/ethernet/atheros/alx/alx_hwcom.h b/drivers/net/ethernet/atheros/alx/alx_hwcom.h
> +++ b/drivers/net/ethernet/atheros/alx/alx_hwcom.h
[...]
> +#define LX_SWAP_DW(_x) (\
> + (((_x) << 24) & 0xFF000000UL) |\
> + (((_x) << 8) & 0x00FF0000UL) |\
> + (((_x) >> 8) & 0x0000FF00UL) |\
> + (((_x) >> 24) & 0x000000FFUL))
> +
> +#define LX_SWAP_W(_x) (\
> + (((_x) >> 8) & 0x00FFU) |\
> + (((_x) << 8) & 0xFF00U))
Duplicates swabXY.
[...]
> +/* interop between drivers */
> +#define LX_DRV_TYPE_MASK SHIFT27(0x1FUL)
> +#define LX_DRV_TYPE_SHIFT 27
> +#define LX_DRV_TYPE_UNKNOWN 0
> +#define LX_DRV_TYPE_BIOS 1
> +#define LX_DRV_TYPE_BTROM 2
> +#define LX_DRV_TYPE_PKT 3
> +#define LX_DRV_TYPE_NDS2 4
> +#define LX_DRV_TYPE_UEFI 5
> +#define LX_DRV_TYPE_NDS5 6
> +#define LX_DRV_TYPE_NDS62 7
> +#define LX_DRV_TYPE_NDS63 8
> +#define LX_DRV_TYPE_LNX 9
> +#define LX_DRV_TYPE_ODI16 10
> +#define LX_DRV_TYPE_ODI32 11
> +#define LX_DRV_TYPE_FRBSD 12
> +#define LX_DRV_TYPE_NTBSD 13
> +#define LX_DRV_TYPE_WCE 14
No.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_main.c b/drivers/net/ethernet/atheros/alx/alx_main.c
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alx_main.c
[...]
> +static int alx_validate_mac_addr(u8 *mac_addr)
> +{
> + int retval = 0;
> +
> + if (mac_addr[0] & 0x01) {
> + printk(KERN_DEBUG "MAC address is multicast\n");
> + retval = ALX_ERR_MAC_ADDR;
> + } else if (mac_addr[0] == 0xff && mac_addr[1] == 0xff) {
> + printk(KERN_DEBUG "MAC address is broadcast\n");
> + retval = ALX_ERR_MAC_ADDR;
> + } else if (mac_addr[0] == 0 && mac_addr[1] == 0 &&
> + mac_addr[2] == 0 && mac_addr[3] == 0 &&
> + mac_addr[4] == 0 && mac_addr[5] == 0) {
> + printk(KERN_DEBUG "MAC address is all zeros\n");
> + retval = ALX_ERR_MAC_ADDR;
> + }
> + return retval;
> +}
Please see is_valid_ether_addr, is_broadcast_ether_addr.
[...]
> +static void alx_receive_skb(struct alx_msix_param *msix,
> + struct sk_buff *skb,
> + u32 vlan_tag, bool vlan_flag)
> +{
> + struct alx_adapter *adpt = msix->adpt;
> +
> + if (adpt->vlgrp && vlan_flag) {
This kind of vlan support code is outdated. Please see ndo_{fix/set}_features.
[...]
> +/* alx_alloc_tx_descriptor - allocate Tx Descriptors */
> +static int alx_alloc_tx_descriptor(struct alx_adapter *adpt,
> + struct alx_tx_queue *txque)
> +{
> + struct alx_ring_header *ring_header = &adpt->ring_header;
> + int size;
> +
> + DRV_PRINT(IF, INFO, "tpq.count = %d\n", txque->tpq.count);
> +
> + size = sizeof(struct alx_buffer) * txque->tpq.count;
> + txque->tpq.tpbuff = kzalloc(size, GFP_KERNEL);
> + if (!txque->tpq.tpbuff)
> + goto err_alloc_tpq_buffer;
> + memset(txque->tpq.tpbuff, 0, size);
Useless memset (kZalloc).
> +
> + /* round up to nearest 4K */
> + txque->tpq.size = txque->tpq.count * sizeof(struct alx_tpdesc);
> +
> + txque->tpq.tpdma = ring_header->dma + ring_header->used;
> + txque->tpq.tpdesc = ring_header->desc + ring_header->used;
> + adpt->hw.tpdma[txque->que_idx] = (u64)txque->tpq.tpdma;
> + ring_header->used += ALIGN(txque->tpq.size, 8);
> +
> + txque->tpq.produce_idx = 0;
> + txque->tpq.consume_idx = 0;
> + txque->max_packets = txque->tpq.count;
> + return 0;
> +
> +err_alloc_tpq_buffer:
> + kfree(txque->tpq.tpbuff);
> + txque->tpq.tpbuff = NULL;
txque->tpq.tpbuff is already NULL before the kfree.
[...]
> +static int alx_alloc_rx_descriptor(struct alx_adapter *adpt,
> + struct alx_rx_queue *rxque)
> +{
[...]
> + rxque->rfq.rfbuff = kzalloc(size, GFP_KERNEL);
> + if (!rxque->rfq.rfbuff)
> + goto err_alloc_rfq_buffer;
> + memset(rxque->rfq.rfbuff, 0, size);
kzalloc + useless memset.
[...]
> + if (CHK_RX_FLAG(SW_QUE)) {
> + size = sizeof(struct alx_sw_buffer) * rxque->swq.count;
> + rxque->swq.swbuff = kzalloc(size, GFP_KERNEL);
> + if (!rxque->swq.swbuff)
> + goto err_alloc_swq_buffer;
> + memset(rxque->swq.swbuff, 0, size);
> +
> + rxque->swq.consume_idx = 0;
> + rxque->swq.produce_idx = 0;
> + }
> +
> + rxque->max_packets = rxque->rrq.count / 2;
> + return 0;
> +
> +err_alloc_swq_buffer:
> + kfree(rxque->swq.swbuff);
> + rxque->swq.swbuff = NULL;
rxque->swq.swbuff is already NULL before the kfree.
[...]
> +static int alx_alloc_all_rtx_descriptor(struct alx_adapter *adpt)
> +{
[...]
> + ring_header->desc = pci_alloc_consistent(pdev, ring_header->size,
> + &ring_header->dma);
> +
> + if (!ring_header->desc) {
> + DRV_PRINT(IF, ERR, "pci_alloc_consistend failed\n");
> + retval = -ENOMEM;
> + goto err_alloc_dma;
> + }
> + memset(ring_header->desc, 0, ring_header->size);
- pci_alloc_consistent returns a zeroed area.
- obsolescent. Ought to be dma_alloc_coherent.
[...]
> +#ifdef CONFIG_PM
> +int alx_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + int retval;
> +
> + retval = alx_shutdown_internal(pdev, state);
> + if (retval)
> + return retval;
> +
> + return 0;
int alx_suspend(struct pci_dev *pdev, pm_message_t state)
{
return alx_shutdown_internal(pdev, state);
}
You may consider using device_driver.pm.
[...]
> +netdev_tx_t alx_start_xmit_frames(struct sk_buff *skb,
> + struct alx_adapter *adpt,
> + struct alx_tx_queue *txque)
> +{
[...]
> + if (!spin_trylock_irqsave(&adpt->tx_lock, flags)) {
> + DRV_PRINT(TX, EMERG, "tx locked!\n");
> + return NETDEV_TX_LOCKED;
> + }
I am not sure that NETDEV_TX_LOCKED is welcome in new drivers.
> +
> + if (!alx_check_num_tpdescs(txque, skb)) {
> + /* no enough descriptor, just stop queue */
> + netif_stop_queue(adpt->netdev);
> + spin_unlock_irqrestore(&adpt->tx_lock, flags);
> + return NETDEV_TX_BUSY;
> + }
The driver has gone a bridge too far : it should disable the tx queue when
it detects that it may not be able to honor the next xmit request.
[...]
> +static int alx_mac_ioctl(struct net_device *netdev,
> + struct ifreq *ifr, int cmd)
> +{
[...]
> + switch (cmd) {
> + case SIOCDEVGMACREG:
SIOCDEVPRIVATE abuse.
[...]
> +int __devinit alx_init(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
[...]
> + netdev->base_addr = (unsigned long)adpt->hw.hw_addr;
Use of base_addr ought to be avoided. It's old-fashioned.
[...]
> + adpt->bd_number = cards_found;
bd_number is never used. Please remove it as well as cards_found.
[...]
> +static void __devexit alx_remove(struct pci_dev *pdev)
> +{
[...]
> + for (que_idx = 0; que_idx < adpt->num_txques; que_idx++) {
> + kfree(adpt->tx_queue[que_idx]);
> + adpt->tx_queue[que_idx] = NULL;
> + }
> + for (que_idx = 0; que_idx < adpt->num_rxques; que_idx++) {
> + kfree(adpt->rx_queue[que_idx]);
> + adpt->rx_queue[que_idx] = NULL;
> + }
This is duplicating alx_free_all_rtx_queue.
[...]
> diff --git a/drivers/net/ethernet/atheros/alx/alx_sw.h b/drivers/net/ethernet/atheros/alx/alx_sw.h
> --- /dev/null
> +++ b/drivers/net/ethernet/atheros/alx/alx_sw.h
[...]
> +struct alx_hw {
> + struct alx_adapter *adpt;
> + struct alx_hw_callbacks cbs;
> + u8 __iomem *hw_addr; /* inner register address */
> + u16 pci_venid;
> + u16 pci_devid;
> + u16 pci_sub_devid;
> + u16 pci_sub_venid;
> + u8 pci_revid;
Gross duplication of fields available through adpt->pdev.
[...]
> + spinlock_t mdio_lock;
> + unsigned long mdio_flags;
Please leave it on the stack, thanks.
[...]
> +typedef struct alx_hw ETHCONTEXT;
> +typedef ETHCONTEXT * PETHCONTEXT;
Please remove the obfuscating typedefs.
[...]
> +/* Error Codes */
> +#define ALX_SUCCESS 0
> +
> +#define ALX_ERR_MAC -1
Unused.
> +#define ALX_ERR_MAC_INIT -2
> +#define ALX_ERR_MAC_RESET -3
> +#define ALX_ERR_MAC_START -4
> +#define ALX_ERR_MAC_STOP -5
> +#define ALX_ERR_MAC_CONFIGURE -6
> +#define ALX_ERR_MAC_ADDR -7
> +
> +#define ALX_ERR_PHY -20
> +#define ALX_ERR_PHY_INIT -21
Unused.
> +#define ALX_ERR_PHY_RESET -22
> +#define ALX_ERR_PHY_SETUP_LNK -23
> +#define ALX_ERR_PHY_CHECK_LNK -24
> +#define ALX_ERR_PHY_READ_REG -25
> +#define ALX_ERR_PHY_WRITE_REG -26
> +#define ALX_ERR_PHY_RESOLVED -27
> +
> +#define ALX_ERR_PCIE -40
Unused.
> +#define ALX_ERR_PCIE_RESET -41
> +#define ALX_ERR_PWR_SAVING -42
> +#define ALX_ERR_ASPM -43
> +#define ALX_ERR_EEPROM -44
Unused.
> +#define ALX_ERR_DISABLE_DRV -45
> +
> +#define ALX_ERR_FLOW_CONTROL -50
> +#define ALX_ERR_FC_NOT_NEGOTIATED -51
Unused.
> +#define ALX_ERR_FC_NOT_SUPPORTED -52
Unused.
> +
> +#define ALX_ERR_INVALID_ARGUMENT 0x7FFFFFFD
Unused.
> +#define ALX_ERR_NOT_SUPPORTED 0x7FFFFFFE
> +#define ALX_ERR_NOT_IMPLEMENTED 0x7FFFFFFF
Unused.
--
Ueimor
^ permalink raw reply
* Re: [PATCH 07/10] RDMA/cxgb4: DB Drop Recovery for RDMA and LLD queues.
From: Roland Dreier @ 2011-10-19 22:12 UTC (permalink / raw)
To: Vipul Pandya
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, divy-ut6Up61K2wZBDgjK7y7TUQ,
dm-ut6Up61K2wZBDgjK7y7TUQ, kumaras-ut6Up61K2wZBDgjK7y7TUQ,
swise-7bPotxP6k4+P2YhJcF5u+vpXobYPEAuW
In-Reply-To: <1319044264-779-8-git-send-email-vipul-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
This looks like the only drivers/infiniband patch that depends on the
drivers/net changes?
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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: BUG: All network processes hang (brcmsmac/wpa_supplicant)
From: Nico Schottelius @ 2011-10-19 21:42 UTC (permalink / raw)
To: Arend van Spriel
Cc: Nico Schottelius, Eric Dumazet, LKML,
linux-wireless@vger.kernel.org, netdev
In-Reply-To: <4E9EFF51.6000104@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
Arend van Spriel [Wed, Oct 19, 2011 at 06:48:17PM +0200]:
> On 10/19/2011 04:55 PM, Nico Schottelius wrote:
> commit bee709ab1d390afe69e4407bc86bb706c6fb2965
> Merge: ad1c761 1f2c7e9
> Author: Nico Schottelius <nico@kr.ethz.ch>
> Date: Tue Oct 18 00:04:05 2011 +0200
>
> Merge branch 'fix-edp-vdd-power' of ../keithp/linux
>
> As it drops receive packets it may be your problem. Is your AP on 5GHz?
I've to confess I've no clue, but I guess the answer is "some of them":
Here in ETH Zurich (www.ethz.ch) I'm in an environment with a lot of
surrounding APs (probably 7 or more seen with the same ssid).
> From 7d14bd6cbfbf26369c5958e56a468fd8429841d7 Mon Sep 17 00:00:00 2001
> From: Arend van Spriel <arend@broadcom.com>
> Date: Wed, 19 Oct 2011 18:42:45 +0200
> Subject: [PATCH] staging: brcm80211: fix for rate index in receive status
Applied, recompiling, will test tomorrow.
I'll soon (Friday) leave the environment permanently,
thus testing will not be easily possible anymore.
If this issue is 5Ghz related only, can I somehow trigger it without an AP
running in that band / i.e. make the card try to use the 5 Ghz band although
there is no ap nearby?
Cheers,
Nico
--
PGP key: 7ED9 F7D3 6B10 81D7 0EC5 5C09 D7DC C8E4 3187 7DF0
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/1] net: validate HWTSTAMP ioctl parameters
From: Ben Hutchings @ 2011-10-19 21:16 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller
In-Reply-To: <eca6d279cd96da44d9ad26bdda8fc8c2130c03c1.1318584677.git.richard.cochran@omicron.at>
On Fri, 2011-10-14 at 11:37 +0200, Richard Cochran wrote:
> This patch adds a sanity check on the values provided by user space for
> the hardware time stamping configuration. If the values lie outside of
> the absolute limits, then the ioctl request will be denied.
[...]
What does this validation buy us? The driver still has to copy the
values into kernel space again, at which point they may have been
changed to be invalid. Depending on how the driver uses them (perhaps
as array indices), it may have to validate them again to avoid a
security vulnerability.
I think that either SIOCHWTSTAMP should be handled through a discrete
device operation (not ndo_ioctl) which receives a pointer to the
validated structure in kernel memory, or a validation function should be
exported to drivers so that they can call it from their ndo_ioctl
implementations after copying the structure into kernel memory.
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
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