* [PATCH 3/6] netfilter: nf_nat_snmp_basic: fix duplicates in if/else branches
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: FX Le Bail <fx.lebail@yahoo.com>
The solution was found by Patrick in 2.4 kernel sources.
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
Acked-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/ipv4/netfilter/nf_nat_snmp_basic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index d551e31..7c67667 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -1198,8 +1198,8 @@ static int snmp_translate(struct nf_conn *ct,
map.to = NOCT1(&ct->tuplehash[!dir].tuple.dst.u3.ip);
} else {
/* DNAT replies */
- map.from = NOCT1(&ct->tuplehash[dir].tuple.src.u3.ip);
- map.to = NOCT1(&ct->tuplehash[!dir].tuple.dst.u3.ip);
+ map.from = NOCT1(&ct->tuplehash[!dir].tuple.src.u3.ip);
+ map.to = NOCT1(&ct->tuplehash[dir].tuple.dst.u3.ip);
}
if (map.from == map.to)
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/6] netfilter: nft_meta: fix typo "CONFIG_NET_CLS_ROUTE"
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: Paul Bolle <pebolle@tiscali.nl>
There are two checks for CONFIG_NET_CLS_ROUTE, but the corresponding
Kconfig symbol was dropped in v2.6.39. Since the code guards access to
dst_entry.tclassid it seems CONFIG_IP_ROUTE_CLASSID should be used
instead.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_meta.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index e8254ad..425cf39 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -116,7 +116,7 @@ static void nft_meta_get_eval(const struct nft_expr *expr,
skb->sk->sk_socket->file->f_cred->fsgid);
read_unlock_bh(&skb->sk->sk_callback_lock);
break;
-#ifdef CONFIG_NET_CLS_ROUTE
+#ifdef CONFIG_IP_ROUTE_CLASSID
case NFT_META_RTCLASSID: {
const struct dst_entry *dst = skb_dst(skb);
@@ -199,7 +199,7 @@ static int nft_meta_init_validate_get(uint32_t key)
case NFT_META_OIFTYPE:
case NFT_META_SKUID:
case NFT_META_SKGID:
-#ifdef CONFIG_NET_CLS_ROUTE
+#ifdef CONFIG_IP_ROUTE_CLASSID
case NFT_META_RTCLASSID:
#endif
#ifdef CONFIG_NETWORK_SECMARK
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/6] netfilter: nf_tables: fix nf_trace always-on with XT_TRACE=n
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: Florian Westphal <fw@strlen.de>
When using nftables with CONFIG_NETFILTER_XT_TARGET_TRACE=n, we get
lots of "TRACE: filter:output:policy:1 IN=..." warnings as several
places will leave skb->nf_trace uninitialised.
Unlike iptables tracing functionality is not conditional in nftables,
so always copy/zero nf_trace setting when nftables is enabled.
Move this into __nf_copy() helper.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/linux/skbuff.h | 5 ++++-
net/core/skbuff.c | 3 ---
net/ipv4/ip_output.c | 3 ---
net/ipv6/ip6_output.c | 3 ---
4 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f589c9a..d40d40b 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2725,7 +2725,7 @@ static inline void nf_reset(struct sk_buff *skb)
static inline void nf_reset_trace(struct sk_buff *skb)
{
-#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE)
+#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE) || defined(CONFIG_NF_TABLES)
skb->nf_trace = 0;
#endif
}
@@ -2742,6 +2742,9 @@ static inline void __nf_copy(struct sk_buff *dst, const struct sk_buff *src)
dst->nf_bridge = src->nf_bridge;
nf_bridge_get(src->nf_bridge);
#endif
+#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE) || defined(CONFIG_NF_TABLES)
+ dst->nf_trace = src->nf_trace;
+#endif
}
static inline void nf_copy(struct sk_buff *dst, const struct sk_buff *src)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 5976ef0..5d6236d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -707,9 +707,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
new->mark = old->mark;
new->skb_iif = old->skb_iif;
__nf_copy(new, old);
-#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE)
- new->nf_trace = old->nf_trace;
-#endif
#ifdef CONFIG_NET_SCHED
new->tc_index = old->tc_index;
#ifdef CONFIG_NET_CLS_ACT
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 8971780..73c6b63 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -422,9 +422,6 @@ static void ip_copy_metadata(struct sk_buff *to, struct sk_buff *from)
to->tc_index = from->tc_index;
#endif
nf_copy(to, from);
-#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE)
- to->nf_trace = from->nf_trace;
-#endif
#if defined(CONFIG_IP_VS) || defined(CONFIG_IP_VS_MODULE)
to->ipvs_property = from->ipvs_property;
#endif
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index ef02b26..4cfbe0f 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -517,9 +517,6 @@ static void ip6_copy_metadata(struct sk_buff *to, struct sk_buff *from)
to->tc_index = from->tc_index;
#endif
nf_copy(to, from);
-#if IS_ENABLED(CONFIG_NETFILTER_XT_TARGET_TRACE)
- to->nf_trace = from->nf_trace;
-#endif
skb_copy_secmark(to, from);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 6/6] netfilter: ctnetlink: force null nat binding on insert
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
Quoting Andrey Vagin:
When a conntrack is created by kernel, it is initialized (sets
IPS_{DST,SRC}_NAT_DONE_BIT bits in nf_nat_setup_info) and only then it
is added in hashes (__nf_conntrack_hash_insert), so one conntract
can't be initialized from a few threads concurrently.
ctnetlink can add an uninitialized conntrack (w/o
IPS_{DST,SRC}_NAT_DONE_BIT) in hashes, then a few threads can look up
this conntrack and start initialize it concurrently. It's dangerous,
because BUG can be triggered from nf_nat_setup_info.
Fix this race by always setting up nat, even if no CTA_NAT_ attribute
was requested before inserting the ct into the hash table. In absence
of CTA_NAT_ attribute, a null binding is created.
This alters current behaviour: Before this patch, the first packet
matching the newly injected conntrack would be run through the nat
table since nf_nat_initialized() returns false. IOW, this forces
ctnetlink users to specify the desired nat transformation on ct
creation time.
Thanks for Florian Westphal, this patch is based on his original
patch to address this problem, including this patch description.
Reported-By: Andrey Vagin <avagin@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_netlink.c | 35 +++++++++------------
net/netfilter/nf_nat_core.c | 56 +++++++++++++++++++++-------------
2 files changed, 49 insertions(+), 42 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index bb322d0..b9f0e03 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1310,27 +1310,22 @@ ctnetlink_change_status(struct nf_conn *ct, const struct nlattr * const cda[])
}
static int
-ctnetlink_change_nat(struct nf_conn *ct, const struct nlattr * const cda[])
+ctnetlink_setup_nat(struct nf_conn *ct, const struct nlattr * const cda[])
{
#ifdef CONFIG_NF_NAT_NEEDED
int ret;
- if (cda[CTA_NAT_DST]) {
- ret = ctnetlink_parse_nat_setup(ct,
- NF_NAT_MANIP_DST,
- cda[CTA_NAT_DST]);
- if (ret < 0)
- return ret;
- }
- if (cda[CTA_NAT_SRC]) {
- ret = ctnetlink_parse_nat_setup(ct,
- NF_NAT_MANIP_SRC,
- cda[CTA_NAT_SRC]);
- if (ret < 0)
- return ret;
- }
- return 0;
+ ret = ctnetlink_parse_nat_setup(ct, NF_NAT_MANIP_DST,
+ cda[CTA_NAT_DST]);
+ if (ret < 0)
+ return ret;
+
+ ret = ctnetlink_parse_nat_setup(ct, NF_NAT_MANIP_SRC,
+ cda[CTA_NAT_SRC]);
+ return ret;
#else
+ if (!cda[CTA_NAT_DST] && !cda[CTA_NAT_SRC])
+ return 0;
return -EOPNOTSUPP;
#endif
}
@@ -1659,11 +1654,9 @@ ctnetlink_create_conntrack(struct net *net, u16 zone,
goto err2;
}
- if (cda[CTA_NAT_SRC] || cda[CTA_NAT_DST]) {
- err = ctnetlink_change_nat(ct, cda);
- if (err < 0)
- goto err2;
- }
+ err = ctnetlink_setup_nat(ct, cda);
+ if (err < 0)
+ goto err2;
nf_ct_acct_ext_add(ct, GFP_ATOMIC);
nf_ct_tstamp_ext_add(ct, GFP_ATOMIC);
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index d3f5cd6..52ca952 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -432,15 +432,15 @@ nf_nat_setup_info(struct nf_conn *ct,
}
EXPORT_SYMBOL(nf_nat_setup_info);
-unsigned int
-nf_nat_alloc_null_binding(struct nf_conn *ct, unsigned int hooknum)
+static unsigned int
+__nf_nat_alloc_null_binding(struct nf_conn *ct, enum nf_nat_manip_type manip)
{
/* Force range to this IP; let proto decide mapping for
* per-proto parts (hence not IP_NAT_RANGE_PROTO_SPECIFIED).
* Use reply in case it's already been mangled (eg local packet).
*/
union nf_inet_addr ip =
- (HOOK2MANIP(hooknum) == NF_NAT_MANIP_SRC ?
+ (manip == NF_NAT_MANIP_SRC ?
ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.u3 :
ct->tuplehash[IP_CT_DIR_REPLY].tuple.src.u3);
struct nf_nat_range range = {
@@ -448,7 +448,13 @@ nf_nat_alloc_null_binding(struct nf_conn *ct, unsigned int hooknum)
.min_addr = ip,
.max_addr = ip,
};
- return nf_nat_setup_info(ct, &range, HOOK2MANIP(hooknum));
+ return nf_nat_setup_info(ct, &range, manip);
+}
+
+unsigned int
+nf_nat_alloc_null_binding(struct nf_conn *ct, unsigned int hooknum)
+{
+ return __nf_nat_alloc_null_binding(ct, HOOK2MANIP(hooknum));
}
EXPORT_SYMBOL_GPL(nf_nat_alloc_null_binding);
@@ -702,9 +708,9 @@ static const struct nla_policy nat_nla_policy[CTA_NAT_MAX+1] = {
static int
nfnetlink_parse_nat(const struct nlattr *nat,
- const struct nf_conn *ct, struct nf_nat_range *range)
+ const struct nf_conn *ct, struct nf_nat_range *range,
+ const struct nf_nat_l3proto *l3proto)
{
- const struct nf_nat_l3proto *l3proto;
struct nlattr *tb[CTA_NAT_MAX+1];
int err;
@@ -714,38 +720,46 @@ nfnetlink_parse_nat(const struct nlattr *nat,
if (err < 0)
return err;
- rcu_read_lock();
- l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct));
- if (l3proto == NULL) {
- err = -EAGAIN;
- goto out;
- }
err = l3proto->nlattr_to_range(tb, range);
if (err < 0)
- goto out;
+ return err;
if (!tb[CTA_NAT_PROTO])
- goto out;
+ return 0;
- err = nfnetlink_parse_nat_proto(tb[CTA_NAT_PROTO], ct, range);
-out:
- rcu_read_unlock();
- return err;
+ return nfnetlink_parse_nat_proto(tb[CTA_NAT_PROTO], ct, range);
}
+/* This function is called under rcu_read_lock() */
static int
nfnetlink_parse_nat_setup(struct nf_conn *ct,
enum nf_nat_manip_type manip,
const struct nlattr *attr)
{
struct nf_nat_range range;
+ const struct nf_nat_l3proto *l3proto;
int err;
- err = nfnetlink_parse_nat(attr, ct, &range);
+ /* Should not happen, restricted to creating new conntracks
+ * via ctnetlink.
+ */
+ if (WARN_ON_ONCE(nf_nat_initialized(ct, manip)))
+ return -EEXIST;
+
+ /* Make sure that L3 NAT is there by when we call nf_nat_setup_info to
+ * attach the null binding, otherwise this may oops.
+ */
+ l3proto = __nf_nat_l3proto_find(nf_ct_l3num(ct));
+ if (l3proto == NULL)
+ return -EAGAIN;
+
+ /* No NAT information has been passed, allocate the null-binding */
+ if (attr == NULL)
+ return __nf_nat_alloc_null_binding(ct, manip);
+
+ err = nfnetlink_parse_nat(attr, ct, &range, l3proto);
if (err < 0)
return err;
- if (nf_nat_initialized(ct, manip))
- return -EEXIST;
return nf_nat_setup_info(ct, &range, manip);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 5/6] netfilter: nf_tables: check if payload length is a power of 2
From: Pablo Neira Ayuso @ 2014-02-19 11:41 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1392810102-13877-1-git-send-email-pablo@netfilter.org>
From: Nikolay Aleksandrov <nikolay@redhat.com>
Add a check if payload's length is a power of 2 when selecting ops.
The fast ops were meant for well aligned loads, also this fixes a
small bug when using a length of 3 with some offsets which causes
only 1 byte to be loaded because the fast ops are chosen.
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Acked-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nft_payload.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nft_payload.c b/net/netfilter/nft_payload.c
index a2aeb31..85daa84 100644
--- a/net/netfilter/nft_payload.c
+++ b/net/netfilter/nft_payload.c
@@ -135,7 +135,8 @@ nft_payload_select_ops(const struct nft_ctx *ctx,
if (len == 0 || len > FIELD_SIZEOF(struct nft_data, data))
return ERR_PTR(-EINVAL);
- if (len <= 4 && IS_ALIGNED(offset, len) && base != NFT_PAYLOAD_LL_HEADER)
+ if (len <= 4 && is_power_of_2(len) && IS_ALIGNED(offset, len) &&
+ base != NFT_PAYLOAD_LL_HEADER)
return &nft_payload_fast_ops;
else
return &nft_payload_ops;
--
1.7.10.4
^ permalink raw reply related
* Re: Fw:[Bug 70471] xfrm policy node will double unlink.
From: Steffen Klassert @ 2014-02-19 11:43 UTC (permalink / raw)
To: Xianpeng Zhao; +Cc: netdev, alan
In-Reply-To: <tencent_4CE06FA903D8DCB54F951A30@qq.com>
On Wed, Feb 19, 2014 at 10:07:14AM +0800, Xianpeng Zhao wrote:
> Hi Steffen,
>
> This problem is happened when running stress test; Very little chance can get this case.
>
> As you say, add a long time sleep in function xfrm_policy_bysel_ctx between __xfrm_policy_unlink and
> xfrm_policy_kill, will reproduce this issue manually.
>
> About my patch, I am not sure it is OK, because after it patched, the issue had reproduced once, but after some days test recently, have not reproduced again.
>
> But I can make sure when __xfrm_policy_unlink find the node had been removed, return NULL instead of delete again will fix this problem.
>
> What's your suggestions?
>
Please test the patch I've sent with the last mail.
Thanks!
^ permalink raw reply
* [PATCH next v3] tcp: use zero-window when free_space is low
From: Florian Westphal @ 2014-02-19 11:51 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Neal Cardwell, Yuchung Cheng
Currently the kernel tries to announce a zero window when free_space
is below the current receiver mss estimate.
When a sender is transmitting small packets and reader consumes data
slowly (or not at all), receiver might be unable to shrink the receive
win because
a) we cannot withdraw already-commited receive window, and,
b) we have to round the current rwin up to a multiple of the wscale
factor, else we would shrink the current window.
This causes the receive buffer to fill up until the rmem limit is hit.
When this happens, we start dropping packets.
Moreover, tcp_clamp_window may continue to grow sk_rcvbuf towards rmem[2]
even if socket is not being read from.
As we cannot avoid the "current_win is rounded up to multiple of mss"
issue [we would violate a) above] at least try to prevent the receive buf
growth towards tcp_rmem[2] limit by attempting to move to zero-window
announcement when free_space becomes less than 1/16 of the current
allowed receive buffer maximum. If tcp_rmem[2] is large, this will
increase our chances to get a zero-window announcement out in time.
Reproducer:
On server:
$ nc -l -p 12345
<suspend it: CTRL-Z>
Client:
#!/usr/bin/env python
import socket
import time
sock = socket.socket()
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
sock.connect(("192.168.4.1", 12345));
while True:
sock.send('A' * 23)
time.sleep(0.005)
socket buffer on server-side will grow until tcp_rmem[2] is hit,
at which point the client rexmits data until -EDTIMEOUT:
tcp_data_queue invokes tcp_try_rmem_schedule which will call
tcp_prune_queue which calls tcp_clamp_window(). And that function will
grow sk->sk_rcvbuf up until it eventually hits tcp_rmem[2].
Thanks to Eric Dumazet for running regression tests.
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Tested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
no changes since v2; resend with Erics Ack/Tested-by tags
V1 of this patch was deferred, resending to get discussion going again.
Changes since v1:
- add reproducer to commit message
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 2a69f42..fd8d821 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2145,7 +2145,8 @@ u32 __tcp_select_window(struct sock *sk)
*/
int mss = icsk->icsk_ack.rcv_mss;
int free_space = tcp_space(sk);
- int full_space = min_t(int, tp->window_clamp, tcp_full_space(sk));
+ int allowed_space = tcp_full_space(sk);
+ int full_space = min_t(int, tp->window_clamp, allowed_space);
int window;
if (mss > full_space)
@@ -2158,7 +2159,19 @@ u32 __tcp_select_window(struct sock *sk)
tp->rcv_ssthresh = min(tp->rcv_ssthresh,
4U * tp->advmss);
- if (free_space < mss)
+ /* free_space might become our new window, make sure we don't
+ * increase it due to wscale.
+ */
+ free_space = round_down(free_space, 1 << tp->rx_opt.rcv_wscale);
+
+ /* if free space is less than mss estimate, or is below 1/16th
+ * of the maximum allowed, try to move to zero-window, else
+ * tcp_clamp_window() will grow rcv buf up to tcp_rmem[2], and
+ * new incoming data is dropped due to memory limits.
+ * With large window, mss test triggers way too late in order
+ * to announce zero window in time before rmem limit kicks in.
+ */
+ if (free_space < (allowed_space >> 4) || free_space < mss)
return 0;
}
--
1.8.1.5
^ permalink raw reply related
* Re: [Xen-devel] [PATCH net-next v5 2/9] xen-netback: Change TX path from grant copy to mapping
From: David Vrabel @ 2014-02-19 12:27 UTC (permalink / raw)
To: Ian Campbell
Cc: Zoltan Kiss, xen-devel, jonathan.davies, wei.liu2, linux-kernel,
netdev
In-Reply-To: <1392803670.23084.100.camel@kazak.uk.xensource.com>
On 19/02/14 09:54, Ian Campbell wrote:
> On Tue, 2014-02-18 at 18:46 +0000, David Vrabel wrote:
>> On 18/02/14 17:40, Ian Campbell wrote:
>>> On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
>>>>
>>>> @@ -344,8 +346,26 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
>>>> vif->pending_prod = MAX_PENDING_REQS;
>>>> for (i = 0; i < MAX_PENDING_REQS; i++)
>>>> vif->pending_ring[i] = i;
>>>> - for (i = 0; i < MAX_PENDING_REQS; i++)
>>>> - vif->mmap_pages[i] = NULL;
>>>> + spin_lock_init(&vif->dealloc_lock);
>>>> + spin_lock_init(&vif->response_lock);
>>>> + /* If ballooning is disabled, this will consume real memory, so you
>>>> + * better enable it.
>>>
>>> Almost no one who would be affected by this is going to read this
>>> comment. And it doesn't just require enabling ballooning, but actually
>>> booting with some maxmem "slack" to leave space.
>>>
>>> Classic-xen kernels used to add 8M of slop to the physical address space
>>> to leave a suitable pool for exactly this sort of thing. I never liked
>>> that but perhaps it should be reconsidered (or at least raised as a
>>> possibility with the core-Xen Linux guys).
>>
>> I plan to fix the balloon memory hotplug stuff to do the right thing
>
> Which is for alloc_xenballoon_pages to hotplug a new empty region,
> rather than inflating the balloon if it doesn't have enough pages to
> satisfy the allocation?
Yes.
David
^ permalink raw reply
* RE: [PATCH net-next 12/14] r8152: replace netif_rxwithnetif_receive_skb
From: hayeswang @ 2014-02-19 12:45 UTC (permalink / raw)
To: 'Francois Romieu'; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <20140219074633.GA26271@electric-eye.fr.zoreil.com>
Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Wednesday, February 19, 2014 3:47 PM
> To: hayeswang
> Cc: netdev@vger.kernel.org; nic_swsd@realtek.com;
> linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org
> Subject: Re: [PATCH net-next 12/14] r8152: replace
> netif_rxwithnetif_receive_skb
>
[...]
> The change in rx_bottom is fine. My point is about read_bulk_callback.
>
> rx_bottom races with read_bulk_callback. rx_bottom is issued in
> tasklet (softirq) context. read_bulk_callback is issued in irq
> context, with irq disabled. read_bulk_callback does not need to
> disable irq itself and could go with spin_lock in place of
> spin_lock_irqsave (rx_bottom can't, of course).
I think I misunderstand your meaning.
I would modify them. Thanks.
Best Regards,
Hayes
^ permalink raw reply
* [PATCH net-next V1 0/3] net/mlx4: Mellanox driver update 01-01-2014
From: Amir Vadai @ 2014-02-19 12:58 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Amir Vadai
Hi,
V0 of this patch was sent before previous net-next got closed, and now we would
like to resume it.
Yuval has reworked the affinity hint patch, according to Ben's comments. The
patch was actually rewritten.
After a discussion with Yuval Mintz, use of netif_get_num_default_rss_queues()
is not reverted, but done in the right place. Instead of limiting the number of
IRQ's for the driver it will limit the number of queues in RSS.
Patchset was applied and tested against commit: cb6e926 "ipv6:fix checkpatch
errors with assignment in if condition"
Thanks,
Amir
Ido Shamay (2):
net/mlx4: Set number of RX rings in a utility function
net/mlx4: Fix limiting number of IRQ's instead of RSS queues
Yuval Atias (1):
net/mlx4_en: Use affinity hint
drivers/infiniband/hw/mlx4/main.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 5 +-
drivers/net/ethernet/mellanox/mlx4/en_main.c | 15 +----
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 88 ++++++++++++++++++++++++--
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 32 +++++++++-
drivers/net/ethernet/mellanox/mlx4/eq.c | 14 +++-
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 5 +-
include/linux/mlx4/device.h | 2 +-
9 files changed, 138 insertions(+), 28 deletions(-)
--
1.8.3.4
^ permalink raw reply
* [PATCH net-next V1 2/3] net/mlx4: Set number of RX rings in a utility function
From: Amir Vadai @ 2014-02-19 12:58 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Ido Shamay, Amir Vadai
In-Reply-To: <1392814684-2129-1-git-send-email-amirv@mellanox.com>
From: Ido Shamay <idos@mellanox.com>
mlx4_en_add() is too long.
Moving set number of RX rings to a utiltity function to improve
readability and modulization of the code.
Signed-off-by: Ido Shamay <idos@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_main.c | 15 ++-------------
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 22 ++++++++++++++++++++++
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 2 +-
3 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c
index d357bf5..fa2f6e7 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c
@@ -274,19 +274,8 @@ static void *mlx4_en_add(struct mlx4_dev *dev)
if (mdev->dev->caps.flags2 & MLX4_DEV_CAP_FLAG2_TS)
mlx4_en_init_timestamp(mdev);
- mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_ETH) {
- if (!dev->caps.comp_pool) {
- mdev->profile.prof[i].rx_ring_num =
- rounddown_pow_of_two(max_t(int, MIN_RX_RINGS,
- min_t(int,
- dev->caps.num_comp_vectors,
- DEF_RX_RINGS)));
- } else {
- mdev->profile.prof[i].rx_ring_num = rounddown_pow_of_two(
- min_t(int, dev->caps.comp_pool/
- dev->caps.num_ports - 1 , MAX_MSIX_P_PORT - 1));
- }
- }
+ /* Set default number of RX rings*/
+ mlx4_en_set_num_rx_rings(mdev);
/* Create our own workqueue for reset/multicast tasks
* Note: we cannot use the shared workqueue because of deadlocks caused
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index b0eba6d..61e44ae 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -318,6 +318,28 @@ static void mlx4_en_free_rx_buf(struct mlx4_en_priv *priv,
}
}
+void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev)
+{
+ int i;
+ int num_of_eqs;
+ struct mlx4_dev *dev = mdev->dev;
+
+ mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_ETH) {
+ if (!dev->caps.comp_pool)
+ num_of_eqs = max_t(int, MIN_RX_RINGS,
+ min_t(int,
+ dev->caps.num_comp_vectors,
+ DEF_RX_RINGS));
+ else
+ num_of_eqs = min_t(int, MAX_MSIX_P_PORT,
+ dev->caps.comp_pool/
+ dev->caps.num_ports) - 1;
+
+ mdev->profile.prof[i].rx_ring_num =
+ rounddown_pow_of_two(num_of_eqs);
+ }
+}
+
int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
struct mlx4_en_rx_ring **pring,
u32 size, u16 stride, int node)
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 101d636..f97a0d3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -738,7 +738,7 @@ int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
int cq, int user_prio);
void mlx4_en_deactivate_tx_ring(struct mlx4_en_priv *priv,
struct mlx4_en_tx_ring *ring);
-
+void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev);
int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
struct mlx4_en_rx_ring **pring,
u32 size, u16 stride, int node);
--
1.8.3.4
^ permalink raw reply related
* [PATCH net-next V1 1/3] net/mlx4_en: Use affinity hint
From: Amir Vadai @ 2014-02-19 12:58 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Yuval Atias, Ben Hutchings,
Amir Vadai
In-Reply-To: <1392814684-2129-1-git-send-email-amirv@mellanox.com>
From: Yuval Atias <yuvala@mellanox.com>
The affinity hint mechanism is used by the user space
daemon, irqbalancer, to indicate a preferred CPU mask for irqs.
Irqbalancer can use this hint to balance the irqs between the
cpus indicated by the mask.
We wish the HCA to preferentially map the IRQs it uses to numa cores
close to it.
To accomplish this, we use affinity hint: first we map IRQs to close
numa cores.
If these are exhausted, the remaining IRQs are mapped to far numa
cores.
CC: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Yuval Atias <yuvala@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
Changes from V0:
Use numa bit mask to set affinity hint.
Move dynamic allocation to mlx4_en_start_port function.
Handle error when device numa node is not known.
Change rx_ring affinity_mask var type to cpumask_var_t.
drivers/infiniband/hw/mlx4/main.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 5 +-
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 88 ++++++++++++++++++++++++--
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 7 +-
drivers/net/ethernet/mellanox/mlx4/eq.c | 14 +++-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 3 +-
include/linux/mlx4/device.h | 2 +-
7 files changed, 109 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index c2702f5..a7dcfa7 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -1763,7 +1763,7 @@ static void mlx4_ib_alloc_eqs(struct mlx4_dev *dev, struct mlx4_ib_dev *ibdev)
i, j, dev->pdev->bus->name);
/* Set IRQ for specific name (per ring) */
if (mlx4_assign_eq(dev, name, NULL,
- &ibdev->eq_table[eq])) {
+ &ibdev->eq_table[eq], NULL)) {
/* Use legacy (same as mlx4_en driver) */
pr_warn("Can't allocate EQ %d; reverting to legacy\n", eq);
ibdev->eq_table[eq] =
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 70e9532..5ef3f7a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -96,7 +96,7 @@ err_cq:
}
int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
- int cq_idx)
+ int cq_idx, cpumask_var_t affinity_mask)
{
struct mlx4_en_dev *mdev = priv->mdev;
int err = 0;
@@ -123,7 +123,8 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
cq->ring);
/* Set IRQ for specific name (per ring) */
if (mlx4_assign_eq(mdev->dev, name, rmap,
- &cq->vector)) {
+ &cq->vector,
+ affinity_mask)) {
cq->vector = (cq->ring + 1 + priv->port)
% mdev->dev->caps.num_comp_vectors;
mlx4_warn(mdev, "Failed Assigning an EQ to "
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index fad4531..88f16c0 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1533,6 +1533,55 @@ static void mlx4_en_linkstate(struct work_struct *work)
mutex_unlock(&mdev->state_lock);
}
+static int mlx4_en_rings_affinity_hint(struct mlx4_en_priv *priv,
+ cpumask_var_t non_numa_cores_mask,
+ cpumask_var_t numa_cores_mask,
+ const struct cpumask **p_affinity_numa_mask,
+ int *affinity_cpu)
+{
+ if (priv->mdev->dev->numa_node == -1)
+ goto err;
+ *p_affinity_numa_mask = cpumask_of_node(priv->mdev->dev->numa_node);
+ if (!*p_affinity_numa_mask)
+ goto err;
+ cpumask_copy(numa_cores_mask, *p_affinity_numa_mask);
+ if (!cpumask_and(numa_cores_mask,
+ cpu_online_mask, numa_cores_mask)) {
+ en_warn(priv, "Failed to find online cores for numa\n");
+ goto err;
+ }
+ cpumask_xor(non_numa_cores_mask, cpu_online_mask,
+ numa_cores_mask);
+ *p_affinity_numa_mask = numa_cores_mask;
+ *affinity_cpu = cpumask_first(*p_affinity_numa_mask);
+
+ return 0;
+
+err:
+ *affinity_cpu = -1;
+ return -EINVAL;
+}
+
+static int mlx4_en_set_affinity_hint(struct mlx4_en_priv *priv,
+ cpumask_var_t non_numa_cores_mask,
+ cpumask_var_t numa_cores_mask,
+ const struct cpumask **p_affinity_numa_mask,
+ int *affinity_cpu, int ring)
+{
+ if (*affinity_cpu == -1)
+ return -EINVAL;
+ cpumask_set_cpu(*affinity_cpu,
+ priv->rx_ring[ring]->affinity_mask);
+ *affinity_cpu = cpumask_next(*affinity_cpu, *p_affinity_numa_mask);
+ if (*affinity_cpu >= nr_cpu_ids) {
+ *p_affinity_numa_mask =
+ *p_affinity_numa_mask == numa_cores_mask ?
+ non_numa_cores_mask : numa_cores_mask;
+ *affinity_cpu = cpumask_first(*p_affinity_numa_mask);
+ }
+
+ return 0;
+}
int mlx4_en_start_port(struct net_device *dev)
{
@@ -1540,6 +1589,10 @@ int mlx4_en_start_port(struct net_device *dev)
struct mlx4_en_dev *mdev = priv->mdev;
struct mlx4_en_cq *cq;
struct mlx4_en_tx_ring *tx_ring;
+ const struct cpumask *affinity_numa_mask;
+ cpumask_var_t numa_cores_mask = NULL;
+ cpumask_var_t non_numa_cores_mask = NULL;
+ int affinity_cpu;
int rx_index = 0;
int tx_index = 0;
int err = 0;
@@ -1551,7 +1604,12 @@ int mlx4_en_start_port(struct net_device *dev)
en_dbg(DRV, priv, "start port called while port already up\n");
return 0;
}
-
+ if (!zalloc_cpumask_var(&numa_cores_mask, GFP_KERNEL) ||
+ !zalloc_cpumask_var(&non_numa_cores_mask, GFP_KERNEL)) {
+ en_err(priv, "Failed to allocating core mask\n");
+ err = -EINVAL;
+ goto affinity_err;
+ }
INIT_LIST_HEAD(&priv->mc_list);
INIT_LIST_HEAD(&priv->curr_list);
INIT_LIST_HEAD(&priv->ethtool_list);
@@ -1569,12 +1627,28 @@ int mlx4_en_start_port(struct net_device *dev)
en_err(priv, "Failed to activate RX rings\n");
return err;
}
+ err = mlx4_en_rings_affinity_hint(priv,
+ non_numa_cores_mask,
+ numa_cores_mask,
+ &affinity_numa_mask,
+ &affinity_cpu);
+ if (err)
+ en_err(priv, "Failed to set affinity hints\n");
+
for (i = 0; i < priv->rx_ring_num; i++) {
cq = priv->rx_cq[i];
mlx4_en_cq_init_lock(cq);
-
- err = mlx4_en_activate_cq(priv, cq, i);
+ err = mlx4_en_set_affinity_hint(priv,
+ non_numa_cores_mask,
+ numa_cores_mask,
+ &affinity_numa_mask,
+ &affinity_cpu, i);
+ if (err)
+ en_err(priv, "Failed setting affinity hint\n");
+ err = mlx4_en_activate_cq(priv, cq, i,
+ affinity_cpu == -1 ? NULL :
+ priv->rx_ring[i]->affinity_mask);
if (err) {
en_err(priv, "Failed activating Rx CQ\n");
goto cq_err;
@@ -1615,7 +1689,7 @@ int mlx4_en_start_port(struct net_device *dev)
for (i = 0; i < priv->tx_ring_num; i++) {
/* Configure cq */
cq = priv->tx_cq[i];
- err = mlx4_en_activate_cq(priv, cq, i);
+ err = mlx4_en_activate_cq(priv, cq, i, NULL);
if (err) {
en_err(priv, "Failed allocating Tx CQ\n");
goto tx_err;
@@ -1704,7 +1778,8 @@ int mlx4_en_start_port(struct net_device *dev)
priv->port_up = true;
netif_tx_start_all_queues(dev);
netif_device_attach(dev);
-
+ free_cpumask_var(non_numa_cores_mask);
+ free_cpumask_var(numa_cores_mask);
return 0;
tx_err:
@@ -1722,6 +1797,9 @@ cq_err:
mlx4_en_deactivate_cq(priv, priv->rx_cq[rx_index]);
for (i = 0; i < priv->rx_ring_num; i++)
mlx4_en_deactivate_rx_ring(priv, priv->rx_ring[i]);
+affinity_err:
+ free_cpumask_var(non_numa_cores_mask);
+ free_cpumask_var(numa_cores_mask);
return err; /* need to close devices */
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 890922c..b0eba6d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -335,7 +335,11 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
return -ENOMEM;
}
}
-
+ if (!zalloc_cpumask_var(&ring->affinity_mask, GFP_KERNEL)) {
+ en_err(priv, "Failed to allocating core mask\n");
+ err = -ENOMEM;
+ goto err_ring;
+ }
ring->prod = 0;
ring->cons = 0;
ring->size = size;
@@ -470,6 +474,7 @@ void mlx4_en_destroy_rx_ring(struct mlx4_en_priv *priv,
mlx4_free_hwq_res(mdev->dev, &ring->wqres, size * stride + TXBB_SIZE);
vfree(ring->rx_info);
ring->rx_info = NULL;
+ free_cpumask_var(ring->affinity_mask);
kfree(ring);
*pring = NULL;
#ifdef CONFIG_RFS_ACCEL
diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
index 8992b38..a3d8502 100644
--- a/drivers/net/ethernet/mellanox/mlx4/eq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
@@ -1311,7 +1311,7 @@ int mlx4_test_interrupts(struct mlx4_dev *dev)
EXPORT_SYMBOL(mlx4_test_interrupts);
int mlx4_assign_eq(struct mlx4_dev *dev, char *name, struct cpu_rmap *rmap,
- int *vector)
+ int *vector, cpumask_var_t cpu_hint_mask)
{
struct mlx4_priv *priv = mlx4_priv(dev);
@@ -1344,6 +1344,16 @@ int mlx4_assign_eq(struct mlx4_dev *dev, char *name, struct cpu_rmap *rmap,
continue;
/*we dont want to break here*/
}
+ if (cpu_hint_mask) {
+ err = irq_set_affinity_hint(
+ priv->eq_table.eq[vec].irq,
+ cpu_hint_mask);
+ if (err) {
+ mlx4_warn(dev, "Failed setting affinity hint\n");
+ /*we dont want to break here*/
+ }
+ }
+
eq_set_ci(&priv->eq_table.eq[vec], 1);
}
}
@@ -1370,6 +1380,8 @@ void mlx4_release_eq(struct mlx4_dev *dev, int vec)
Belonging to a legacy EQ*/
mutex_lock(&priv->msix_ctl.pool_lock);
if (priv->msix_ctl.pool_bm & 1ULL << i) {
+ irq_set_affinity_hint(priv->eq_table.eq[vec].irq,
+ NULL);
free_irq(priv->eq_table.eq[vec].irq,
&priv->eq_table.eq[vec]);
priv->msix_ctl.pool_bm &= ~(1ULL << i);
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index 3af04c3..101d636 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -303,6 +303,7 @@ struct mlx4_en_rx_ring {
unsigned long csum_ok;
unsigned long csum_none;
int hwtstamp_rx_filter;
+ cpumask_var_t affinity_mask;
};
struct mlx4_en_cq {
@@ -716,7 +717,7 @@ int mlx4_en_create_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq **pcq,
int entries, int ring, enum cq_type mode, int node);
void mlx4_en_destroy_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq **pcq);
int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
- int cq_idx);
+ int cq_idx, cpumask_var_t affinity_mask);
void mlx4_en_deactivate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq);
int mlx4_en_set_cq_moder(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq);
int mlx4_en_arm_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq);
diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index 5edd2c6..f8c253f 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -1148,7 +1148,7 @@ int mlx4_fmr_free(struct mlx4_dev *dev, struct mlx4_fmr *fmr);
int mlx4_SYNC_TPT(struct mlx4_dev *dev);
int mlx4_test_interrupts(struct mlx4_dev *dev);
int mlx4_assign_eq(struct mlx4_dev *dev, char *name, struct cpu_rmap *rmap,
- int *vector);
+ int *vector, cpumask_t *cpu_hint_mask);
void mlx4_release_eq(struct mlx4_dev *dev, int vec);
int mlx4_get_phys_port_id(struct mlx4_dev *dev);
--
1.8.3.4
^ permalink raw reply related
* [PATCH net-next V1 3/3] net/mlx4: Fix limiting number of IRQ's instead of RSS queues
From: Amir Vadai @ 2014-02-19 12:58 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yevgeny Petrilin, Or Gerlitz, Ido Shamay, Yuval Mintz,
Amir Vadai
In-Reply-To: <1392814684-2129-1-git-send-email-amirv@mellanox.com>
From: Ido Shamay <idos@mellanox.com>
This fix a performance bug introduced by commit 90b1ebe "mlx4: set
maximal number of default RSS queues", which limits the numbers of IRQs
opened by core module.
The limit should be on the number of queues in the indirection table -
rx_rings, and not on the number of IRQ's. Also, limiting on mlx4_core
initialization instead of in mlx4_en, prevented using "ethtool -L" to
utilize all the CPU's, when performance mode is prefered, since limiting
this number to 8 reduces overall packet rate by 15%-50% in multiple TCP
streams applications.
For example, after running ethtool -L <ethx> rx 16
Packet rate
Before the fix 897799
After the fix 1142070
Results were obtained using netperf:
S=200 ; ( for i in $(seq 1 $S) ; do ( \
netperf -H 11.7.13.55 -t TCP_RR -l 30 &) ; \
wait ; done | grep "1 1" | awk '{SUM+=$6} END {print SUM}' )
CC: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Ido Shamay <idos@mellanox.com>
Signed-off-by: Amir Vadai <amirv@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 5 ++++-
drivers/net/ethernet/mellanox/mlx4/main.c | 3 +--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 61e44ae..630ec03 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -322,6 +322,7 @@ void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev)
{
int i;
int num_of_eqs;
+ int num_rx_rings;
struct mlx4_dev *dev = mdev->dev;
mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_ETH) {
@@ -335,8 +336,10 @@ void mlx4_en_set_num_rx_rings(struct mlx4_en_dev *mdev)
dev->caps.comp_pool/
dev->caps.num_ports) - 1;
+ num_rx_rings = min_t(int, num_of_eqs,
+ netif_get_num_default_rss_queues());
mdev->profile.prof[i].rx_ring_num =
- rounddown_pow_of_two(num_of_eqs);
+ rounddown_pow_of_two(num_rx_rings);
}
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index d711158..8c82a6b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -41,7 +41,6 @@
#include <linux/slab.h>
#include <linux/io-mapping.h>
#include <linux/delay.h>
-#include <linux/netdevice.h>
#include <linux/kmod.h>
#include <linux/mlx4/device.h>
@@ -1974,7 +1973,7 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
struct mlx4_priv *priv = mlx4_priv(dev);
struct msix_entry *entries;
int nreq = min_t(int, dev->caps.num_ports *
- min_t(int, netif_get_num_default_rss_queues() + 1,
+ min_t(int, num_online_cpus() + 1,
MAX_MSIX_P_PORT) + MSIX_LEGACY_SZ, MAX_MSIX);
int err;
int i;
--
1.8.3.4
^ permalink raw reply related
* [sctp] ef2820a735: -50% netperf Throughput_Mbps
From: Fengguang Wu @ 2014-02-19 13:20 UTC (permalink / raw)
To: Matija Glavinic Pecotic; +Cc: LKML, Linux Netdev List
Hi Matija,
We noticed the below changes on commit ef2820a735f74ea60335f8ba3801b844f0cb184d
(" net: sctp: Fix a_rwnd/rwnd management to reflect real state of the receiver's buffer")
in netperf SCTP_STREAM tests:
cd0f0b95fd2cd2b ef2820a735f74ea60335f8ba3
--------------- -------------------------
8 ~ 0% -50.0% 4 ~ 0% TOTAL netperf.Throughput_Mbps
54287 ~44% +338.1% 237842 ~48% TOTAL cpuidle.C1E-NHM.time
12008353 ~12% -56.0% 5281848 ~ 0% TOTAL proc-vmstat.pgalloc_normal
114861 ~ 0% -50.3% 57085 ~ 3% TOTAL softirqs.NET_RX
12964639 ~11% -55.1% 5818663 ~ 0% TOTAL proc-vmstat.pgfree
866489 ~ 0% -43.3% 491417 ~ 0% TOTAL proc-vmstat.pgalloc_dma32
119373 ~17% -39.1% 72661 ~ 1% TOTAL softirqs.SCHED
1985 ~13% -24.3% 1502 ~19% TOTAL slabinfo.kmalloc-128.active_objs
2139 ~20% -28.4% 1532 ~ 4% TOTAL proc-vmstat.nr_alloc_batch
124360 ~33% -31.0% 85748 ~ 2% TOTAL softirqs.RCU
1977 ~ 9% -18.5% 1610 ~ 9% TOTAL slabinfo.UDP.active_objs
1977 ~ 9% -18.5% 1610 ~ 9% TOTAL slabinfo.UDP.num_objs
2066 ~ 6% -12.9% 1800 ~ 7% TOTAL slabinfo.kmalloc-128.num_objs
1738 ~10% -18.4% 1418 ~ 9% TOTAL slabinfo.UDPv6.active_objs
1738 ~10% -18.4% 1418 ~ 9% TOTAL slabinfo.UDPv6.num_objs
923 ~10% -17.7% 760 ~ 8% TOTAL slabinfo.TCPv6.active_objs
923 ~10% -17.7% 760 ~ 8% TOTAL slabinfo.TCPv6.num_objs
989 ~ 9% -17.1% 820 ~ 7% TOTAL slabinfo.TCP.active_objs
989 ~ 9% -17.1% 820 ~ 7% TOTAL slabinfo.TCP.num_objs
398761 ~44% -32.6% 268792 ~ 3% TOTAL numa-vmstat.node2.numa_hit
389672 ~49% -32.9% 261443 ~ 3% TOTAL numa-vmstat.node0.numa_hit
447 ~ 1% -13.8% 385 ~ 0% TOTAL vmstat.system.cs
Note: the "~ XX%" numbers are stddev percent.
netperf.Throughput_Mbps
4 *+-*--*--*--*-*--*--*--*--*--*--*--*-*--*--*--*--*--*--*--*-*--*--*--*
| |
| |
3.5 ++ |
| |
| |
| |
3 ++ |
| |
| |
2.5 ++ |
| |
| |
| |
2 O+-O--O--O--O-O--O--O--O--O--O--O--O-O--O--O--O--O--O--O-------------+
vmstat.system.cs
460 ++------------------------*--*----------------*----------------------+
| .*.. .*.. : + + *.. .*
450 ++ *. .*. *.. : + .*..*.. + *..*..*.. + .*. |
440 *+ : * : *..* * * *. |
|: : * |
430 ++: : |
420 ++: : |
| * |
410 ++ |
400 ++ |
| |
390 ++ O O O O O |
380 O+ O O O O O O O O O O O O O |
| O |
370 ++-------------------------------------------------------------------+
^ permalink raw reply
* [PATCH net-next 0/5] Device reset and reconfig fixes
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
These patches end up fixing some notable device reset & reconfig
related problems. One issue is on-the-fly (Rx/Tx on) programming
of interrupt coalescing (IC) registers on the processing path,
against HW recommendation. This is an old issue that became visible
after BQL introduction, as under certain conditions (low traffic)
one TX interrupt gets lost and BQL fires Tx timeout as a result.
Another notable issue is a race on the Tx path (xmit, clean_tx)
during device reset (i.e. during Tx timeout watchdog firing)
that leads to NULL access.
Fixing the problematic on-thy-fly register writes (i.e. the IC regs)
required the implementation of a MAC soft reset procedure.
The race leading to NULL access was addressed by fixing the
stop_gfar()/startup_gfar() pair (disable/enable napi a.s.o.)
and adding the device state DOWN to sync with the TX path.
Thanks.
Claudiu Manoil (5):
gianfar: Implement MAC reset and reconfig procedure
gianfar: Fix on-the-fly vlan and mtu updates
gianfar: Don't free/request irqs on device reset
gianfar: Fix device reset races (oops) for Tx
gianfar: Fix Tx int miss, dont write IC on-the-fly
drivers/net/ethernet/freescale/gianfar.c | 599 ++++++++++-------------
drivers/net/ethernet/freescale/gianfar.h | 18 +-
drivers/net/ethernet/freescale/gianfar_ethtool.c | 104 ++--
3 files changed, 325 insertions(+), 396 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net-next 2/5] gianfar: Fix on-the-fly vlan and mtu updates
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <1392816138-31645-1-git-send-email-claudiu.manoil@freescale.com>
The RCTRL and TCTRL registers should not be changed
on-the-fly, while the controller is running, otherwise
unexpected behaviour occurs. But that's exactly what
gfar_vlan_mode() does, updating the VLAN acceleration
bits inside RCTRL/TCTRL. The attempt to lock these
operations doesn't help, but only adds to the confusion.
There's also a dependency for Rx FCB insertion (activating
/de-activating the TOE offload block on Rx) which might
change the required rx buffer size. This makes matters
worse as gfar_vlan_mode() ends up calling gfar_change_mtu(),
though the MTU size remains the same. Note that there are
other situations that may affect the required rx buffer size,
like changing RXCSUM or rx hw timestamping, but errorneously
the rx buffer size is not recomputed/ updated in the process.
To fix this, do the vlan updates properly inside the MAC
reset and reconfiguration procedure, which takes care of
the rx buffer size dependecy and the rx TOE block (PRSDEP)
activation/deactivation as well (in the correct order).
As a consequence, MTU/ rx buff size updates are done now
by the same MAC reset and reconfig procedure, so that out
of context updates to MAXFRM, MRBLR, and MACCFG inside
change_mtu() are no longer needed. The rx buffer size
dependecy to Rx FCB is now handled for the other cases too
(RXCSUM and rx hw timestamping).
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 165 +++++++----------------
drivers/net/ethernet/freescale/gianfar.h | 2 -
drivers/net/ethernet/freescale/gianfar_ethtool.c | 10 +-
3 files changed, 52 insertions(+), 125 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 446e9c9..728078f 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -329,14 +329,35 @@ static void gfar_init_tx_rx_base(struct gfar_private *priv)
}
}
-static void gfar_mac_rx_config(struct gfar_private *priv)
+static void gfar_rx_buff_size_config(struct gfar_private *priv)
{
- struct gfar __iomem *regs = priv->gfargrp[0].regs;
- u32 rctrl = 0;
+ int frame_size = priv->ndev->mtu + ETH_HLEN;
/* set this when rx hw offload (TOE) functions are being used */
priv->uses_rxfcb = 0;
+ if (priv->ndev->features & (NETIF_F_RXCSUM | NETIF_F_HW_VLAN_CTAG_RX))
+ priv->uses_rxfcb = 1;
+
+ if (priv->hwts_rx_en)
+ priv->uses_rxfcb = 1;
+
+ if (priv->uses_rxfcb)
+ frame_size += GMAC_FCB_LEN;
+
+ frame_size += priv->padding;
+
+ frame_size = (frame_size & ~(INCREMENTAL_BUFFER_SIZE - 1)) +
+ INCREMENTAL_BUFFER_SIZE;
+
+ priv->rx_buffer_size = frame_size;
+}
+
+static void gfar_mac_rx_config(struct gfar_private *priv)
+{
+ struct gfar __iomem *regs = priv->gfargrp[0].regs;
+ u32 rctrl = 0;
+
if (priv->rx_filer_enable) {
rctrl |= RCTRL_FILREN;
/* Program the RIR0 reg with the required distribution */
@@ -347,15 +368,11 @@ static void gfar_mac_rx_config(struct gfar_private *priv)
if (priv->ndev->flags & IFF_PROMISC)
rctrl |= RCTRL_PROM;
- if (priv->ndev->features & NETIF_F_RXCSUM) {
+ if (priv->ndev->features & NETIF_F_RXCSUM)
rctrl |= RCTRL_CHECKSUMMING;
- priv->uses_rxfcb = 1;
- }
- if (priv->extended_hash) {
- rctrl |= RCTRL_EXTHASH;
- rctrl |= RCTRL_EMEN;
- }
+ if (priv->extended_hash)
+ rctrl |= RCTRL_EXTHASH | RCTRL_EMEN;
if (priv->padding) {
rctrl &= ~RCTRL_PAL_MASK;
@@ -363,15 +380,11 @@ static void gfar_mac_rx_config(struct gfar_private *priv)
}
/* Enable HW time stamping if requested from user space */
- if (priv->hwts_rx_en) {
+ if (priv->hwts_rx_en)
rctrl |= RCTRL_PRSDEP_INIT | RCTRL_TS_ENABLE;
- priv->uses_rxfcb = 1;
- }
- if (priv->ndev->features & NETIF_F_HW_VLAN_CTAG_RX) {
+ if (priv->ndev->features & NETIF_F_HW_VLAN_CTAG_RX)
rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
- priv->uses_rxfcb = 1;
- }
/* Init rctrl based on our settings */
gfar_write(®s->rctrl, rctrl);
@@ -393,6 +406,9 @@ static void gfar_mac_tx_config(struct gfar_private *priv)
gfar_write(®s->tr47wt, DEFAULT_WRRS_WEIGHT);
}
+ if (priv->ndev->features & NETIF_F_HW_VLAN_CTAG_TX)
+ tctrl |= TCTRL_VLINS;
+
gfar_write(®s->tctrl, tctrl);
}
@@ -1029,7 +1045,11 @@ static void gfar_mac_reset(struct gfar_private *priv)
udelay(3);
- /* Initialize the max receive buffer length */
+ /* Compute rx_buff_size based on config flags */
+ gfar_rx_buff_size_config(priv);
+
+ /* Initialize the max receive frame/buffer lengths */
+ gfar_write(®s->maxfrm, priv->rx_buffer_size);
gfar_write(®s->mrblr, priv->rx_buffer_size);
/* Initialize the Minimum Frame Length Register */
@@ -1037,8 +1057,15 @@ static void gfar_mac_reset(struct gfar_private *priv)
/* Initialize MACCFG2. */
tempval = MACCFG2_INIT_SETTINGS;
- if (gfar_has_errata(priv, GFAR_ERRATA_74))
+
+ /* If the mtu is larger than the max size for standard
+ * ethernet frames (ie, a jumbo frame), then set maccfg2
+ * to allow huge frames, and to check the length
+ */
+ if (priv->rx_buffer_size > DEFAULT_RX_BUFFER_SIZE ||
+ gfar_has_errata(priv, GFAR_ERRATA_74))
tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
+
gfar_write(®s->maccfg2, tempval);
/* Clear mac addr hash registers */
@@ -2353,77 +2380,9 @@ static int gfar_set_mac_address(struct net_device *dev)
return 0;
}
-/* Check if rx parser should be activated */
-void gfar_check_rx_parser_mode(struct gfar_private *priv)
-{
- struct gfar __iomem *regs;
- u32 tempval;
-
- regs = priv->gfargrp[0].regs;
-
- tempval = gfar_read(®s->rctrl);
- /* If parse is no longer required, then disable parser */
- if (tempval & RCTRL_REQ_PARSER) {
- tempval |= RCTRL_PRSDEP_INIT;
- priv->uses_rxfcb = 1;
- } else {
- tempval &= ~RCTRL_PRSDEP_INIT;
- priv->uses_rxfcb = 0;
- }
- gfar_write(®s->rctrl, tempval);
-}
-
-/* Enables and disables VLAN insertion/extraction */
-void gfar_vlan_mode(struct net_device *dev, netdev_features_t features)
-{
- struct gfar_private *priv = netdev_priv(dev);
- struct gfar __iomem *regs = NULL;
- unsigned long flags;
- u32 tempval;
-
- regs = priv->gfargrp[0].regs;
- local_irq_save(flags);
- lock_rx_qs(priv);
-
- if (features & NETIF_F_HW_VLAN_CTAG_TX) {
- /* Enable VLAN tag insertion */
- tempval = gfar_read(®s->tctrl);
- tempval |= TCTRL_VLINS;
- gfar_write(®s->tctrl, tempval);
- } else {
- /* Disable VLAN tag insertion */
- tempval = gfar_read(®s->tctrl);
- tempval &= ~TCTRL_VLINS;
- gfar_write(®s->tctrl, tempval);
- }
-
- if (features & NETIF_F_HW_VLAN_CTAG_RX) {
- /* Enable VLAN tag extraction */
- tempval = gfar_read(®s->rctrl);
- tempval |= (RCTRL_VLEX | RCTRL_PRSDEP_INIT);
- gfar_write(®s->rctrl, tempval);
- priv->uses_rxfcb = 1;
- } else {
- /* Disable VLAN tag extraction */
- tempval = gfar_read(®s->rctrl);
- tempval &= ~RCTRL_VLEX;
- gfar_write(®s->rctrl, tempval);
-
- gfar_check_rx_parser_mode(priv);
- }
-
- gfar_change_mtu(dev, dev->mtu);
-
- unlock_rx_qs(priv);
- local_irq_restore(flags);
-}
-
static int gfar_change_mtu(struct net_device *dev, int new_mtu)
{
- int tempsize, tempval;
struct gfar_private *priv = netdev_priv(dev);
- struct gfar __iomem *regs = priv->gfargrp[0].regs;
- int oldsize = priv->rx_buffer_size;
int frame_size = new_mtu + ETH_HLEN;
if ((frame_size < 64) || (frame_size > JUMBO_FRAME_SIZE)) {
@@ -2431,42 +2390,12 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
return -EINVAL;
}
- if (priv->uses_rxfcb)
- frame_size += GMAC_FCB_LEN;
-
- frame_size += priv->padding;
-
- tempsize = (frame_size & ~(INCREMENTAL_BUFFER_SIZE - 1)) +
- INCREMENTAL_BUFFER_SIZE;
-
- /* Only stop and start the controller if it isn't already
- * stopped, and we changed something
- */
- if ((oldsize != tempsize) && (dev->flags & IFF_UP))
+ if (dev->flags & IFF_UP)
stop_gfar(dev);
- priv->rx_buffer_size = tempsize;
-
dev->mtu = new_mtu;
- gfar_write(®s->mrblr, priv->rx_buffer_size);
- gfar_write(®s->maxfrm, priv->rx_buffer_size);
-
- /* If the mtu is larger than the max size for standard
- * ethernet frames (ie, a jumbo frame), then set maccfg2
- * to allow huge frames, and to check the length
- */
- tempval = gfar_read(®s->maccfg2);
-
- if (priv->rx_buffer_size > DEFAULT_RX_BUFFER_SIZE ||
- gfar_has_errata(priv, GFAR_ERRATA_74))
- tempval |= (MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK);
- else
- tempval &= ~(MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK);
-
- gfar_write(®s->maccfg2, tempval);
-
- if ((oldsize != tempsize) && (dev->flags & IFF_UP))
+ if (dev->flags & IFF_UP)
startup_gfar(dev);
return 0;
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 2a59398..9db9556 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -1211,8 +1211,6 @@ void gfar_phy_test(struct mii_bus *bus, struct phy_device *phydev, int enable,
u32 regnum, u32 read);
void gfar_configure_coalescing_all(struct gfar_private *priv);
int gfar_set_features(struct net_device *dev, netdev_features_t features);
-void gfar_check_rx_parser_mode(struct gfar_private *priv);
-void gfar_vlan_mode(struct net_device *dev, netdev_features_t features);
extern const struct ethtool_ops gfar_ethtool_ops;
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 19557ec..0fbe3aa 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -582,18 +582,18 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
netdev_features_t changed = dev->features ^ features;
int err = 0;
- if (changed & (NETIF_F_HW_VLAN_CTAG_TX|NETIF_F_HW_VLAN_CTAG_RX))
- gfar_vlan_mode(dev, features);
+ if (changed & (NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX))
+ goto reset;
if (!(changed & NETIF_F_RXCSUM))
return 0;
+reset:
+ dev->features = features;
+
if (dev->flags & IFF_UP) {
/* Now we take down the rings to rebuild them */
stop_gfar(dev);
-
- dev->features = features;
-
err = startup_gfar(dev);
netif_tx_wake_all_queues(dev);
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 1/5] gianfar: Implement MAC reset and reconfig procedure
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <1392816138-31645-1-git-send-email-claudiu.manoil@freescale.com>
The main MAC config registers like: RCTRL/TCTRL, MRBLR,
MAXFRM, RXIC/TXIC, most fields of MACCFG1/2, should not
be changed on-the-fly, but at least after stopping the
DMA and disabling the Rx/Tx blocks and, for increased
reliability, after a MAC soft reset.
Impelement a complete MAC soft reset and reconfig procedure
following the latest HW advisories - gfar_mac_reset() - to
replace gfar_mac_init() and (the confusing) init_registers()
functions.
Factor out separate config functions for RCTRL and TCTRL,
insure programming order of the relevant config regs after
MAC soft reset.
Split gfar_hw_init() into gfar_mac_reset() and the remaining
global regs that don't need to be reconfigured after MAC soft
reset (FIFOCFG, ATTRELI, HW counters a.s.o).
As gfar_hw_init() now makes all the register writes @probe()
time, based on all the device flags and config options, it
must be moved further down, just before register_netdev(),
as the last config step when the config values are comitted
to HW. Also, move netif_carrier_off() after register_netdev(),
because it has no effect if called before.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 172 ++++++++++++++++---------------
1 file changed, 90 insertions(+), 82 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index a2977a8..446e9c9 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -121,7 +121,6 @@ static irqreturn_t gfar_error(int irq, void *dev_id);
static irqreturn_t gfar_transmit(int irq, void *dev_id);
static irqreturn_t gfar_interrupt(int irq, void *dev_id);
static void adjust_link(struct net_device *dev);
-static void init_registers(struct net_device *dev);
static int init_phy(struct net_device *dev);
static int gfar_probe(struct platform_device *ofdev);
static int gfar_remove(struct platform_device *ofdev);
@@ -330,18 +329,10 @@ static void gfar_init_tx_rx_base(struct gfar_private *priv)
}
}
-static void gfar_init_mac(struct net_device *ndev)
+static void gfar_mac_rx_config(struct gfar_private *priv)
{
- struct gfar_private *priv = netdev_priv(ndev);
struct gfar __iomem *regs = priv->gfargrp[0].regs;
u32 rctrl = 0;
- u32 tctrl = 0;
-
- /* write the tx/rx base registers */
- gfar_init_tx_rx_base(priv);
-
- /* Configure the coalescing support */
- gfar_configure_coalescing_all(priv);
/* set this when rx hw offload (TOE) functions are being used */
priv->uses_rxfcb = 0;
@@ -353,18 +344,16 @@ static void gfar_init_mac(struct net_device *ndev)
}
/* Restore PROMISC mode */
- if (ndev->flags & IFF_PROMISC)
+ if (priv->ndev->flags & IFF_PROMISC)
rctrl |= RCTRL_PROM;
- if (ndev->features & NETIF_F_RXCSUM) {
+ if (priv->ndev->features & NETIF_F_RXCSUM) {
rctrl |= RCTRL_CHECKSUMMING;
priv->uses_rxfcb = 1;
}
if (priv->extended_hash) {
rctrl |= RCTRL_EXTHASH;
-
- gfar_clear_exact_match(ndev);
rctrl |= RCTRL_EMEN;
}
@@ -379,15 +368,21 @@ static void gfar_init_mac(struct net_device *ndev)
priv->uses_rxfcb = 1;
}
- if (ndev->features & NETIF_F_HW_VLAN_CTAG_RX) {
+ if (priv->ndev->features & NETIF_F_HW_VLAN_CTAG_RX) {
rctrl |= RCTRL_VLEX | RCTRL_PRSDEP_INIT;
priv->uses_rxfcb = 1;
}
/* Init rctrl based on our settings */
gfar_write(®s->rctrl, rctrl);
+}
- if (ndev->features & NETIF_F_IP_CSUM)
+static void gfar_mac_tx_config(struct gfar_private *priv)
+{
+ struct gfar __iomem *regs = priv->gfargrp[0].regs;
+ u32 tctrl = 0;
+
+ if (priv->ndev->features & NETIF_F_IP_CSUM)
tctrl |= TCTRL_INIT_CSUM;
if (priv->prio_sched_en)
@@ -1016,28 +1011,94 @@ static void gfar_detect_errata(struct gfar_private *priv)
priv->errata);
}
-static void gfar_hw_init(struct gfar_private *priv)
+static void gfar_mac_reset(struct gfar_private *priv)
{
struct gfar __iomem *regs = priv->gfargrp[0].regs;
- u32 tempval, attrs;
+ u32 tempval;
/* Reset MAC layer */
gfar_write(®s->maccfg1, MACCFG1_SOFT_RESET);
/* We need to delay at least 3 TX clocks */
- udelay(2);
+ udelay(3);
/* the soft reset bit is not self-resetting, so we need to
* clear it before resuming normal operation
*/
gfar_write(®s->maccfg1, 0);
+ udelay(3);
+
+ /* Initialize the max receive buffer length */
+ gfar_write(®s->mrblr, priv->rx_buffer_size);
+
+ /* Initialize the Minimum Frame Length Register */
+ gfar_write(®s->minflr, MINFLR_INIT_SETTINGS);
+
/* Initialize MACCFG2. */
tempval = MACCFG2_INIT_SETTINGS;
if (gfar_has_errata(priv, GFAR_ERRATA_74))
tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
gfar_write(®s->maccfg2, tempval);
+ /* Clear mac addr hash registers */
+ gfar_write(®s->igaddr0, 0);
+ gfar_write(®s->igaddr1, 0);
+ gfar_write(®s->igaddr2, 0);
+ gfar_write(®s->igaddr3, 0);
+ gfar_write(®s->igaddr4, 0);
+ gfar_write(®s->igaddr5, 0);
+ gfar_write(®s->igaddr6, 0);
+ gfar_write(®s->igaddr7, 0);
+
+ gfar_write(®s->gaddr0, 0);
+ gfar_write(®s->gaddr1, 0);
+ gfar_write(®s->gaddr2, 0);
+ gfar_write(®s->gaddr3, 0);
+ gfar_write(®s->gaddr4, 0);
+ gfar_write(®s->gaddr5, 0);
+ gfar_write(®s->gaddr6, 0);
+ gfar_write(®s->gaddr7, 0);
+
+ if (priv->extended_hash)
+ gfar_clear_exact_match(priv->ndev);
+
+ gfar_mac_rx_config(priv);
+
+ gfar_mac_tx_config(priv);
+
+ gfar_set_mac_address(priv->ndev);
+
+ gfar_set_multi(priv->ndev);
+
+ /* clear ievent and imask before configuring coalescing */
+ gfar_ints_disable(priv);
+
+ /* Configure the coalescing support */
+ gfar_configure_coalescing_all(priv);
+}
+
+static void gfar_hw_init(struct gfar_private *priv)
+{
+ struct gfar __iomem *regs = priv->gfargrp[0].regs;
+ u32 attrs;
+
+ /* Stop the DMA engine now, in case it was running before
+ * (The firmware could have used it, and left it running).
+ */
+ gfar_halt(priv);
+
+ gfar_mac_reset(priv);
+
+ /* Zero out the rmon mib registers if it has them */
+ if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
+ memset_io(&(regs->rmon), 0, sizeof(struct rmon_mib));
+
+ /* Mask off the CAM interrupts */
+ gfar_write(®s->rmon.cam1, 0xffffffff);
+ gfar_write(®s->rmon.cam2, 0xffffffff);
+ }
+
/* Initialize ECNTRL */
gfar_write(®s->ecntrl, ECNTRL_INIT_SETTINGS);
@@ -1137,13 +1198,6 @@ static int gfar_probe(struct platform_device *ofdev)
gfar_detect_errata(priv);
- /* Stop the DMA engine now, in case it was running before
- * (The firmware could have used it, and left it running).
- */
- gfar_halt(priv);
-
- gfar_hw_init(priv);
-
/* Set the dev->base_addr to the gfar reg region */
dev->base_addr = (unsigned long) priv->gfargrp[0].regs;
@@ -1209,8 +1263,7 @@ static int gfar_probe(struct platform_device *ofdev)
if (priv->num_tx_queues == 1)
priv->prio_sched_en = 1;
- /* Carrier starts down, phylib will bring it up */
- netif_carrier_off(dev);
+ gfar_hw_init(priv);
err = register_netdev(dev);
@@ -1219,6 +1272,9 @@ static int gfar_probe(struct platform_device *ofdev)
goto register_fail;
}
+ /* Carrier starts down, phylib will bring it up */
+ netif_carrier_off(dev);
+
device_init_wakeup(&dev->dev,
priv->device_flags &
FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);
@@ -1401,9 +1457,10 @@ static int gfar_restore(struct device *dev)
return -ENOMEM;
}
- init_registers(ndev);
- gfar_set_mac_address(ndev);
- gfar_init_mac(ndev);
+ gfar_mac_reset(priv);
+
+ gfar_init_tx_rx_base(priv);
+
gfar_start(priv);
priv->oldlink = 0;
@@ -1562,48 +1619,6 @@ static void gfar_configure_serdes(struct net_device *dev)
BMCR_SPEED1000);
}
-static void init_registers(struct net_device *dev)
-{
- struct gfar_private *priv = netdev_priv(dev);
- struct gfar __iomem *regs = priv->gfargrp[0].regs;
-
- gfar_ints_disable(priv);
-
- /* Init hash registers to zero */
- gfar_write(®s->igaddr0, 0);
- gfar_write(®s->igaddr1, 0);
- gfar_write(®s->igaddr2, 0);
- gfar_write(®s->igaddr3, 0);
- gfar_write(®s->igaddr4, 0);
- gfar_write(®s->igaddr5, 0);
- gfar_write(®s->igaddr6, 0);
- gfar_write(®s->igaddr7, 0);
-
- gfar_write(®s->gaddr0, 0);
- gfar_write(®s->gaddr1, 0);
- gfar_write(®s->gaddr2, 0);
- gfar_write(®s->gaddr3, 0);
- gfar_write(®s->gaddr4, 0);
- gfar_write(®s->gaddr5, 0);
- gfar_write(®s->gaddr6, 0);
- gfar_write(®s->gaddr7, 0);
-
- /* Zero out the rmon mib registers if it has them */
- if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
- memset_io(&(regs->rmon), 0, sizeof (struct rmon_mib));
-
- /* Mask off the CAM interrupts */
- gfar_write(®s->rmon.cam1, 0xffffffff);
- gfar_write(®s->rmon.cam2, 0xffffffff);
- }
-
- /* Initialize the max receive buffer length */
- gfar_write(®s->mrblr, priv->rx_buffer_size);
-
- /* Initialize the Minimum Frame Length Register */
- gfar_write(®s->minflr, MINFLR_INIT_SETTINGS);
-}
-
static int __gfar_is_rx_idle(struct gfar_private *priv)
{
u32 res;
@@ -1939,13 +1954,13 @@ int startup_gfar(struct net_device *ndev)
struct gfar_private *priv = netdev_priv(ndev);
int err, i, j;
- gfar_ints_disable(priv);
+ gfar_mac_reset(priv);
err = gfar_alloc_skb_resources(ndev);
if (err)
return err;
- gfar_init_mac(ndev);
+ gfar_init_tx_rx_base(priv);
for (i = 0; i < priv->num_grps; i++) {
err = register_grp_irqs(&priv->gfargrp[i]);
@@ -1961,8 +1976,6 @@ int startup_gfar(struct net_device *ndev)
phy_start(priv->phydev);
- gfar_configure_coalescing_all(priv);
-
return 0;
irq_fail:
@@ -1980,11 +1993,6 @@ static int gfar_enet_open(struct net_device *dev)
enable_napi(priv);
- /* Initialize a bunch of registers */
- init_registers(dev);
-
- gfar_set_mac_address(dev);
-
err = init_phy(dev);
if (err) {
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 3/5] gianfar: Don't free/request irqs on device reset
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <1392816138-31645-1-git-send-email-claudiu.manoil@freescale.com>
Resetting the device (stop_gfar()/startup_gfar()) should
be fast and to the point, in order to timely recover
from an error condition (like Tx timeout) or during
device reconfig. The irq free/ request routines are just
redundant here, and they should be part of the device
close/ open routines instead.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 77 +++++++++++++++++++-------------
1 file changed, 45 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 728078f..6c054b5 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -1715,18 +1715,10 @@ void gfar_halt(struct gfar_private *priv)
gfar_write(®s->maccfg1, tempval);
}
-static void free_grp_irqs(struct gfar_priv_grp *grp)
-{
- free_irq(gfar_irq(grp, TX)->irq, grp);
- free_irq(gfar_irq(grp, RX)->irq, grp);
- free_irq(gfar_irq(grp, ER)->irq, grp);
-}
-
void stop_gfar(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
unsigned long flags;
- int i;
phy_stop(priv->phydev);
@@ -1742,16 +1734,6 @@ void stop_gfar(struct net_device *dev)
unlock_tx_qs(priv);
local_irq_restore(flags);
- /* Free the IRQs */
- if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
- for (i = 0; i < priv->num_grps; i++)
- free_grp_irqs(&priv->gfargrp[i]);
- } else {
- for (i = 0; i < priv->num_grps; i++)
- free_irq(gfar_irq(&priv->gfargrp[i], TX)->irq,
- &priv->gfargrp[i]);
- }
-
free_skb_resources(priv);
}
@@ -1919,6 +1901,13 @@ void gfar_configure_coalescing_all(struct gfar_private *priv)
gfar_configure_coalescing(priv, 0xFF, 0xFF);
}
+static void free_grp_irqs(struct gfar_priv_grp *grp)
+{
+ free_irq(gfar_irq(grp, TX)->irq, grp);
+ free_irq(gfar_irq(grp, RX)->irq, grp);
+ free_irq(gfar_irq(grp, ER)->irq, grp);
+}
+
static int register_grp_irqs(struct gfar_priv_grp *grp)
{
struct gfar_private *priv = grp->priv;
@@ -1975,11 +1964,42 @@ err_irq_fail:
}
+static void gfar_free_irq(struct gfar_private *priv)
+{
+ int i;
+
+ /* Free the IRQs */
+ if (priv->device_flags & FSL_GIANFAR_DEV_HAS_MULTI_INTR) {
+ for (i = 0; i < priv->num_grps; i++)
+ free_grp_irqs(&priv->gfargrp[i]);
+ } else {
+ for (i = 0; i < priv->num_grps; i++)
+ free_irq(gfar_irq(&priv->gfargrp[i], TX)->irq,
+ &priv->gfargrp[i]);
+ }
+}
+
+static int gfar_request_irq(struct gfar_private *priv)
+{
+ int err, i, j;
+
+ for (i = 0; i < priv->num_grps; i++) {
+ err = register_grp_irqs(&priv->gfargrp[i]);
+ if (err) {
+ for (j = 0; j < i; j++)
+ free_grp_irqs(&priv->gfargrp[j]);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
/* Bring the controller up and running */
int startup_gfar(struct net_device *ndev)
{
struct gfar_private *priv = netdev_priv(ndev);
- int err, i, j;
+ int err;
gfar_mac_reset(priv);
@@ -1989,25 +2009,12 @@ int startup_gfar(struct net_device *ndev)
gfar_init_tx_rx_base(priv);
- for (i = 0; i < priv->num_grps; i++) {
- err = register_grp_irqs(&priv->gfargrp[i]);
- if (err) {
- for (j = 0; j < i; j++)
- free_grp_irqs(&priv->gfargrp[j]);
- goto irq_fail;
- }
- }
-
/* Start the controller */
gfar_start(priv);
phy_start(priv->phydev);
return 0;
-
-irq_fail:
- free_skb_resources(priv);
- return err;
}
/* Called when something needs to use the ethernet device
@@ -2027,6 +2034,10 @@ static int gfar_enet_open(struct net_device *dev)
return err;
}
+ err = gfar_request_irq(priv);
+ if (err)
+ return err;
+
err = startup_gfar(dev);
if (err) {
disable_napi(priv);
@@ -2369,6 +2380,8 @@ static int gfar_close(struct net_device *dev)
netif_tx_stop_all_queues(dev);
+ gfar_free_irq(priv);
+
return 0;
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 4/5] gianfar: Fix device reset races (oops) for Tx
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <1392816138-31645-1-git-send-email-claudiu.manoil@freescale.com>
The device reset procedure, stop_gfar()/startup_gfar(), has
concurrency issues.
"Kernel access of bad area" oopses show up during Tx timeout
device reset or other reset cases (like changing MTU) that
happen while the interface still has traffic. The oopses
happen in start_xmit and clean_tx_ring when accessing tx_queue->
tx_skbuff which is NULL. The race comes from de-allocating the
tx_skbuff while transmission and napi processing are still
active. Though the Tx queues get temoprarily stopped when Tx
timeout occurs, they get re-enabled as a result of Tx congestion
handling inside the napi context (see clean_tx_ring()). Not
disabling the napi during reset is also a bug, because
clean_tx_ring() will try to access tx_skbuff while it is being
de-alloc'ed and re-alloc'ed.
To fix this, stop_gfar() needs to disable napi processing
after stopping the Tx queues. However, in order to prevent
clean_tx_ring() to re-enable the Tx queue before the napi
gets disabled, the device state DOWN has been introduced.
It prevents the Tx congestion management from re-enabling the
de-congested Tx queue while the device is brought down.
An additional locking state, RESETTING, has been introduced
to prevent simultaneous resets or to prevent configuring the
device while it is resetting.
The bogus 'rxlock's (for each Rx queue) have been removed since
their purpose is not justified, as they don't prevent nor are
suited to prevent device reset/reconfig races (such as this one).
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 116 ++++++++++-------------
drivers/net/ethernet/freescale/gianfar.h | 16 ++--
drivers/net/ethernet/freescale/gianfar_ethtool.c | 28 ++++--
3 files changed, 76 insertions(+), 84 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 6c054b5..4eac25f 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -480,14 +480,6 @@ static void gfar_ints_enable(struct gfar_private *priv)
}
}
-void lock_rx_qs(struct gfar_private *priv)
-{
- int i;
-
- for (i = 0; i < priv->num_rx_queues; i++)
- spin_lock(&priv->rx_queue[i]->rxlock);
-}
-
void lock_tx_qs(struct gfar_private *priv)
{
int i;
@@ -496,14 +488,6 @@ void lock_tx_qs(struct gfar_private *priv)
spin_lock(&priv->tx_queue[i]->txlock);
}
-void unlock_rx_qs(struct gfar_private *priv)
-{
- int i;
-
- for (i = 0; i < priv->num_rx_queues; i++)
- spin_unlock(&priv->rx_queue[i]->rxlock);
-}
-
void unlock_tx_qs(struct gfar_private *priv)
{
int i;
@@ -543,7 +527,6 @@ static int gfar_alloc_rx_queues(struct gfar_private *priv)
priv->rx_queue[i]->rx_skbuff = NULL;
priv->rx_queue[i]->qindex = i;
priv->rx_queue[i]->dev = priv->ndev;
- spin_lock_init(&(priv->rx_queue[i]->rxlock));
}
return 0;
}
@@ -857,18 +840,16 @@ static int gfar_hwtstamp_set(struct net_device *netdev, struct ifreq *ifr)
switch (config.rx_filter) {
case HWTSTAMP_FILTER_NONE:
if (priv->hwts_rx_en) {
- stop_gfar(netdev);
priv->hwts_rx_en = 0;
- startup_gfar(netdev);
+ reset_gfar(netdev);
}
break;
default:
if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_TIMER))
return -ERANGE;
if (!priv->hwts_rx_en) {
- stop_gfar(netdev);
priv->hwts_rx_en = 1;
- startup_gfar(netdev);
+ reset_gfar(netdev);
}
config.rx_filter = HWTSTAMP_FILTER_ALL;
break;
@@ -1027,7 +1008,7 @@ static void gfar_detect_errata(struct gfar_private *priv)
priv->errata);
}
-static void gfar_mac_reset(struct gfar_private *priv)
+void gfar_mac_reset(struct gfar_private *priv)
{
struct gfar __iomem *regs = priv->gfargrp[0].regs;
u32 tempval;
@@ -1290,6 +1271,8 @@ static int gfar_probe(struct platform_device *ofdev)
if (priv->num_tx_queues == 1)
priv->prio_sched_en = 1;
+ set_bit(GFAR_DOWN, &priv->state);
+
gfar_hw_init(priv);
err = register_netdev(dev);
@@ -1389,7 +1372,6 @@ static int gfar_suspend(struct device *dev)
local_irq_save(flags);
lock_tx_qs(priv);
- lock_rx_qs(priv);
gfar_halt_nodisable(priv);
@@ -1403,7 +1385,6 @@ static int gfar_suspend(struct device *dev)
gfar_write(®s->maccfg1, tempval);
- unlock_rx_qs(priv);
unlock_tx_qs(priv);
local_irq_restore(flags);
@@ -1449,7 +1430,6 @@ static int gfar_resume(struct device *dev)
*/
local_irq_save(flags);
lock_tx_qs(priv);
- lock_rx_qs(priv);
tempval = gfar_read(®s->maccfg2);
tempval &= ~MACCFG2_MPEN;
@@ -1457,7 +1437,6 @@ static int gfar_resume(struct device *dev)
gfar_start(priv);
- unlock_rx_qs(priv);
unlock_tx_qs(priv);
local_irq_restore(flags);
@@ -1718,21 +1697,19 @@ void gfar_halt(struct gfar_private *priv)
void stop_gfar(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
- unsigned long flags;
- phy_stop(priv->phydev);
+ netif_tx_stop_all_queues(dev);
+ smp_mb__before_clear_bit();
+ set_bit(GFAR_DOWN, &priv->state);
+ smp_mb__after_clear_bit();
- /* Lock it down */
- local_irq_save(flags);
- lock_tx_qs(priv);
- lock_rx_qs(priv);
+ disable_napi(priv);
+ /* disable ints and gracefully shut down Rx/Tx DMA */
gfar_halt(priv);
- unlock_rx_qs(priv);
- unlock_tx_qs(priv);
- local_irq_restore(flags);
+ phy_stop(priv->phydev);
free_skb_resources(priv);
}
@@ -2009,11 +1986,19 @@ int startup_gfar(struct net_device *ndev)
gfar_init_tx_rx_base(priv);
- /* Start the controller */
+ smp_mb__before_clear_bit();
+ clear_bit(GFAR_DOWN, &priv->state);
+ smp_mb__after_clear_bit();
+
+ /* Start Rx/Tx DMA and enable the interrupts */
gfar_start(priv);
phy_start(priv->phydev);
+ enable_napi(priv);
+
+ netif_tx_wake_all_queues(ndev);
+
return 0;
}
@@ -2025,26 +2010,17 @@ static int gfar_enet_open(struct net_device *dev)
struct gfar_private *priv = netdev_priv(dev);
int err;
- enable_napi(priv);
-
err = init_phy(dev);
-
- if (err) {
- disable_napi(priv);
+ if (err)
return err;
- }
err = gfar_request_irq(priv);
if (err)
return err;
err = startup_gfar(dev);
- if (err) {
- disable_napi(priv);
+ if (err)
return err;
- }
-
- netif_tx_start_all_queues(dev);
device_set_wakeup_enable(&dev->dev, priv->wol_en);
@@ -2369,8 +2345,6 @@ static int gfar_close(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
- disable_napi(priv);
-
cancel_work_sync(&priv->reset_task);
stop_gfar(dev);
@@ -2378,8 +2352,6 @@ static int gfar_close(struct net_device *dev)
phy_disconnect(priv->phydev);
priv->phydev = NULL;
- netif_tx_stop_all_queues(dev);
-
gfar_free_irq(priv);
return 0;
@@ -2403,6 +2375,9 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
return -EINVAL;
}
+ while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+ cpu_relax();
+
if (dev->flags & IFF_UP)
stop_gfar(dev);
@@ -2411,9 +2386,24 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
if (dev->flags & IFF_UP)
startup_gfar(dev);
+ clear_bit_unlock(GFAR_RESETTING, &priv->state);
+
return 0;
}
+void reset_gfar(struct net_device *ndev)
+{
+ struct gfar_private *priv = netdev_priv(ndev);
+
+ while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+ cpu_relax();
+
+ stop_gfar(ndev);
+ startup_gfar(ndev);
+
+ clear_bit_unlock(GFAR_RESETTING, &priv->state);
+}
+
/* gfar_reset_task gets scheduled when a packet has not been
* transmitted after a set amount of time.
* For now, assume that clearing out all the structures, and
@@ -2423,16 +2413,7 @@ static void gfar_reset_task(struct work_struct *work)
{
struct gfar_private *priv = container_of(work, struct gfar_private,
reset_task);
- struct net_device *dev = priv->ndev;
-
- if (dev->flags & IFF_UP) {
- netif_tx_stop_all_queues(dev);
- stop_gfar(dev);
- startup_gfar(dev);
- netif_tx_start_all_queues(dev);
- }
-
- netif_tx_schedule_all(dev);
+ reset_gfar(priv->ndev);
}
static void gfar_timeout(struct net_device *dev)
@@ -2545,8 +2526,10 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
}
/* If we freed a buffer, we can restart transmission, if necessary */
- if (netif_tx_queue_stopped(txq) && tx_queue->num_txbdfree)
- netif_wake_subqueue(dev, tqi);
+ if (tx_queue->num_txbdfree &&
+ netif_tx_queue_stopped(txq) &&
+ !(test_bit(GFAR_DOWN, &priv->state)))
+ netif_wake_subqueue(priv->ndev, tqi);
/* Update dirty indicators */
tx_queue->skb_dirtytx = skb_dirtytx;
@@ -3023,12 +3006,11 @@ static void adjust_link(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
struct gfar __iomem *regs = priv->gfargrp[0].regs;
- unsigned long flags;
struct phy_device *phydev = priv->phydev;
int new_state = 0;
- local_irq_save(flags);
- lock_tx_qs(priv);
+ if (test_bit(GFAR_RESETTING, &priv->state))
+ return;
if (phydev->link) {
u32 tempval1 = gfar_read(®s->maccfg1);
@@ -3100,8 +3082,6 @@ static void adjust_link(struct net_device *dev)
if (new_state && netif_msg_link(priv))
phy_print_status(phydev);
- unlock_tx_qs(priv);
- local_irq_restore(flags);
}
/* Update the hash table based on the current list of multicast
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 9db9556..1e16216 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -965,7 +965,6 @@ struct rx_q_stats {
/**
* struct gfar_priv_rx_q - per rx queue structure
- * @rxlock: per queue rx spin lock
* @rx_skbuff: skb pointers
* @skb_currx: currently use skb pointer
* @rx_bd_base: First rx buffer descriptor
@@ -978,8 +977,7 @@ struct rx_q_stats {
*/
struct gfar_priv_rx_q {
- spinlock_t rxlock __attribute__ ((aligned (SMP_CACHE_BYTES)));
- struct sk_buff ** rx_skbuff;
+ struct sk_buff **rx_skbuff __aligned(SMP_CACHE_BYTES);
dma_addr_t rx_bd_dma_base;
struct rxbd8 *rx_bd_base;
struct rxbd8 *cur_rx;
@@ -1040,6 +1038,11 @@ enum gfar_errata {
GFAR_ERRATA_12 = 0x08, /* a.k.a errata eTSEC49 */
};
+enum gfar_dev_state {
+ GFAR_DOWN = 1,
+ GFAR_RESETTING
+};
+
/* Struct stolen almost completely (and shamelessly) from the FCC enet source
* (Ok, that's not so true anymore, but there is a family resemblance)
* The GFAR buffer descriptors track the ring buffers. The rx_bd_base
@@ -1068,6 +1071,7 @@ struct gfar_private {
struct gfar_priv_rx_q *rx_queue[MAX_RX_QS];
struct gfar_priv_grp gfargrp[MAXGROUPS];
+ unsigned long state;
u32 device_flags;
unsigned int mode;
@@ -1198,13 +1202,11 @@ static inline void gfar_write_isrg(struct gfar_private *priv)
}
}
-void lock_rx_qs(struct gfar_private *priv);
-void lock_tx_qs(struct gfar_private *priv);
-void unlock_rx_qs(struct gfar_private *priv);
-void unlock_tx_qs(struct gfar_private *priv);
irqreturn_t gfar_receive(int irq, void *dev_id);
int startup_gfar(struct net_device *dev);
void stop_gfar(struct net_device *dev);
+void reset_gfar(struct net_device *dev);
+void gfar_mac_reset(struct gfar_private *priv);
void gfar_halt(struct gfar_private *priv);
void gfar_start(struct gfar_private *priv);
void gfar_phy_test(struct mii_bus *bus, struct phy_device *phydev, int enable,
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 0fbe3aa..b711236 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -487,6 +487,9 @@ static int gfar_sringparam(struct net_device *dev,
return -EINVAL;
}
+ while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+ cpu_relax();
+
if (dev->flags & IFF_UP)
stop_gfar(dev);
@@ -498,10 +501,11 @@ static int gfar_sringparam(struct net_device *dev,
priv->tx_queue[i]->tx_ring_size = rvals->tx_pending;
/* Rebuild the rings with the new size */
- if (dev->flags & IFF_UP) {
+ if (dev->flags & IFF_UP)
err = startup_gfar(dev);
- netif_tx_wake_all_queues(dev);
- }
+
+ clear_bit_unlock(GFAR_RESETTING, &priv->state);
+
return err;
}
@@ -580,6 +584,7 @@ static int gfar_spauseparam(struct net_device *dev,
int gfar_set_features(struct net_device *dev, netdev_features_t features)
{
netdev_features_t changed = dev->features ^ features;
+ struct gfar_private *priv = netdev_priv(dev);
int err = 0;
if (changed & (NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX))
@@ -589,14 +594,21 @@ int gfar_set_features(struct net_device *dev, netdev_features_t features)
return 0;
reset:
+ while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+ cpu_relax();
+
dev->features = features;
if (dev->flags & IFF_UP) {
/* Now we take down the rings to rebuild them */
stop_gfar(dev);
err = startup_gfar(dev);
- netif_tx_wake_all_queues(dev);
+ } else {
+ gfar_mac_reset(priv);
}
+
+ clear_bit_unlock(GFAR_RESETTING, &priv->state);
+
return err;
}
@@ -1562,9 +1574,6 @@ static int gfar_write_filer_table(struct gfar_private *priv,
if (tab->index > MAX_FILER_IDX - 1)
return -EBUSY;
- /* Avoid inconsistent filer table to be processed */
- lock_rx_qs(priv);
-
/* Fill regular entries */
for (; i < MAX_FILER_IDX - 1 && (tab->fe[i].ctrl | tab->fe[i].ctrl);
i++)
@@ -1577,8 +1586,6 @@ static int gfar_write_filer_table(struct gfar_private *priv,
*/
gfar_write_filer(priv, i, 0x20, 0x0);
- unlock_rx_qs(priv);
-
return 0;
}
@@ -1783,6 +1790,9 @@ static int gfar_set_nfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
struct gfar_private *priv = netdev_priv(dev);
int ret = 0;
+ if (test_bit(GFAR_RESETTING, &priv->state))
+ return -EBUSY;
+
mutex_lock(&priv->rx_queue_access);
switch (cmd->cmd) {
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 5/5] gianfar: Fix Tx int miss, dont write IC on-the-fly
From: Claudiu Manoil @ 2014-02-19 13:22 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <1392816138-31645-1-git-send-email-claudiu.manoil@freescale.com>
Programming the interrupt coalescing (IC) registers while
the controller/DMA is on may incur the loss of one Tx
confirmation interrupt, under certain conditions. This is
a subtle hw race because it does not occur during a burst
of Tx packets. It has been observed on p2020 devices that,
if just one packet is being xmit'ed, the Tx confirmation
doesn't trigger and BQL evetually blocks the Tx queues,
followed by Tx timeout and an un-responsive device.
This issue was not apparent prior to introducing BQL
support, as a late Tx confirmation was not an issue back then
and the next burst of Tx frames would have triggered the
Tx confirmation/ Tx ring cleanup anyway.
Bottom line, the hw specifications state that the IC registers
should not be programmed while the Rx/Tx blocks (the DMA) are
enabled. Further more, these registers are currently re-written
with the same values on the processing path, over and over again.
To fix this, rewriting the IC registers has been removed from
the processing path (napi poll). A complete MAC reset procedure
has been implemented for the ethtool -c option instead, to
reliably update these registers while the controller is stopped.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 99 ++++++++++--------------
drivers/net/ethernet/freescale/gianfar_ethtool.c | 66 +++++++++-------
2 files changed, 77 insertions(+), 88 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 4eac25f..c5b9320 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -412,6 +412,47 @@ static void gfar_mac_tx_config(struct gfar_private *priv)
gfar_write(®s->tctrl, tctrl);
}
+static void gfar_configure_coalescing(struct gfar_private *priv,
+ unsigned long tx_mask, unsigned long rx_mask)
+{
+ struct gfar __iomem *regs = priv->gfargrp[0].regs;
+ u32 __iomem *baddr;
+
+ if (priv->mode == MQ_MG_MODE) {
+ int i = 0;
+
+ baddr = ®s->txic0;
+ for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
+ gfar_write(baddr + i, 0);
+ if (likely(priv->tx_queue[i]->txcoalescing))
+ gfar_write(baddr + i, priv->tx_queue[i]->txic);
+ }
+
+ baddr = ®s->rxic0;
+ for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
+ gfar_write(baddr + i, 0);
+ if (likely(priv->rx_queue[i]->rxcoalescing))
+ gfar_write(baddr + i, priv->rx_queue[i]->rxic);
+ }
+ } else {
+ /* Backward compatible case -- even if we enable
+ * multiple queues, there's only single reg to program
+ */
+ gfar_write(®s->txic, 0);
+ if (likely(priv->tx_queue[0]->txcoalescing))
+ gfar_write(®s->txic, priv->tx_queue[0]->txic);
+
+ gfar_write(®s->rxic, 0);
+ if (unlikely(priv->rx_queue[0]->rxcoalescing))
+ gfar_write(®s->rxic, priv->rx_queue[0]->rxic);
+ }
+}
+
+void gfar_configure_coalescing_all(struct gfar_private *priv)
+{
+ gfar_configure_coalescing(priv, 0xFF, 0xFF);
+}
+
static struct net_device_stats *gfar_get_stats(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
@@ -1837,47 +1878,6 @@ void gfar_start(struct gfar_private *priv)
priv->ndev->trans_start = jiffies; /* prevent tx timeout */
}
-static void gfar_configure_coalescing(struct gfar_private *priv,
- unsigned long tx_mask, unsigned long rx_mask)
-{
- struct gfar __iomem *regs = priv->gfargrp[0].regs;
- u32 __iomem *baddr;
-
- if (priv->mode == MQ_MG_MODE) {
- int i = 0;
-
- baddr = ®s->txic0;
- for_each_set_bit(i, &tx_mask, priv->num_tx_queues) {
- gfar_write(baddr + i, 0);
- if (likely(priv->tx_queue[i]->txcoalescing))
- gfar_write(baddr + i, priv->tx_queue[i]->txic);
- }
-
- baddr = ®s->rxic0;
- for_each_set_bit(i, &rx_mask, priv->num_rx_queues) {
- gfar_write(baddr + i, 0);
- if (likely(priv->rx_queue[i]->rxcoalescing))
- gfar_write(baddr + i, priv->rx_queue[i]->rxic);
- }
- } else {
- /* Backward compatible case -- even if we enable
- * multiple queues, there's only single reg to program
- */
- gfar_write(®s->txic, 0);
- if (likely(priv->tx_queue[0]->txcoalescing))
- gfar_write(®s->txic, priv->tx_queue[0]->txic);
-
- gfar_write(®s->rxic, 0);
- if (unlikely(priv->rx_queue[0]->rxcoalescing))
- gfar_write(®s->rxic, priv->rx_queue[0]->rxic);
- }
-}
-
-void gfar_configure_coalescing_all(struct gfar_private *priv)
-{
- gfar_configure_coalescing(priv, 0xFF, 0xFF);
-}
-
static void free_grp_irqs(struct gfar_priv_grp *grp)
{
free_irq(gfar_irq(grp, TX)->irq, grp);
@@ -2812,17 +2812,6 @@ static int gfar_poll_sq(struct napi_struct *napi, int budget)
gfar_write(®s->rstat, gfargrp->rstat);
gfar_write(®s->imask, IMASK_DEFAULT);
-
- /* If we are coalescing interrupts, update the timer
- * Otherwise, clear it
- */
- gfar_write(®s->txic, 0);
- if (likely(tx_queue->txcoalescing))
- gfar_write(®s->txic, tx_queue->txic);
-
- gfar_write(®s->rxic, 0);
- if (unlikely(rx_queue->rxcoalescing))
- gfar_write(®s->rxic, rx_queue->rxic);
}
return work_done;
@@ -2892,12 +2881,6 @@ static int gfar_poll(struct napi_struct *napi, int budget)
gfar_write(®s->rstat, gfargrp->rstat);
gfar_write(®s->imask, IMASK_DEFAULT);
-
- /* If we are coalescing interrupts, update the timer
- * Otherwise, clear it
- */
- gfar_configure_coalescing(priv, gfargrp->rx_bit_map,
- gfargrp->tx_bit_map);
}
return work_done;
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index b711236..94d7d20 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -360,25 +360,11 @@ static int gfar_scoalesce(struct net_device *dev,
struct ethtool_coalesce *cvals)
{
struct gfar_private *priv = netdev_priv(dev);
- int i = 0;
+ int i, err = 0;
if (!(priv->device_flags & FSL_GIANFAR_DEV_HAS_COALESCE))
return -EOPNOTSUPP;
- /* Set up rx coalescing */
- /* As of now, we will enable/disable coalescing for all
- * queues together in case of eTSEC2, this will be modified
- * along with the ethtool interface
- */
- if ((cvals->rx_coalesce_usecs == 0) ||
- (cvals->rx_max_coalesced_frames == 0)) {
- for (i = 0; i < priv->num_rx_queues; i++)
- priv->rx_queue[i]->rxcoalescing = 0;
- } else {
- for (i = 0; i < priv->num_rx_queues; i++)
- priv->rx_queue[i]->rxcoalescing = 1;
- }
-
if (NULL == priv->phydev)
return -ENODEV;
@@ -395,6 +381,32 @@ static int gfar_scoalesce(struct net_device *dev,
return -EINVAL;
}
+ /* Check the bounds of the values */
+ if (cvals->tx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
+ netdev_info(dev, "Coalescing is limited to %d microseconds\n",
+ GFAR_MAX_COAL_USECS);
+ return -EINVAL;
+ }
+
+ if (cvals->tx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
+ netdev_info(dev, "Coalescing is limited to %d frames\n",
+ GFAR_MAX_COAL_FRAMES);
+ return -EINVAL;
+ }
+
+ while (test_and_set_bit_lock(GFAR_RESETTING, &priv->state))
+ cpu_relax();
+
+ /* Set up rx coalescing */
+ if ((cvals->rx_coalesce_usecs == 0) ||
+ (cvals->rx_max_coalesced_frames == 0)) {
+ for (i = 0; i < priv->num_rx_queues; i++)
+ priv->rx_queue[i]->rxcoalescing = 0;
+ } else {
+ for (i = 0; i < priv->num_rx_queues; i++)
+ priv->rx_queue[i]->rxcoalescing = 1;
+ }
+
for (i = 0; i < priv->num_rx_queues; i++) {
priv->rx_queue[i]->rxic = mk_ic_value(
cvals->rx_max_coalesced_frames,
@@ -411,28 +423,22 @@ static int gfar_scoalesce(struct net_device *dev,
priv->tx_queue[i]->txcoalescing = 1;
}
- /* Check the bounds of the values */
- if (cvals->tx_coalesce_usecs > GFAR_MAX_COAL_USECS) {
- netdev_info(dev, "Coalescing is limited to %d microseconds\n",
- GFAR_MAX_COAL_USECS);
- return -EINVAL;
- }
-
- if (cvals->tx_max_coalesced_frames > GFAR_MAX_COAL_FRAMES) {
- netdev_info(dev, "Coalescing is limited to %d frames\n",
- GFAR_MAX_COAL_FRAMES);
- return -EINVAL;
- }
-
for (i = 0; i < priv->num_tx_queues; i++) {
priv->tx_queue[i]->txic = mk_ic_value(
cvals->tx_max_coalesced_frames,
gfar_usecs2ticks(priv, cvals->tx_coalesce_usecs));
}
- gfar_configure_coalescing_all(priv);
+ if (dev->flags & IFF_UP) {
+ stop_gfar(dev);
+ err = startup_gfar(dev);
+ } else {
+ gfar_mac_reset(priv);
+ }
- return 0;
+ clear_bit_unlock(GFAR_RESETTING, &priv->state);
+
+ return err;
}
/* Fills in rvals with the current ring parameters. Currently,
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v2] of_mdio: fix phy interrupt passing
From: Sergei Shtylyov @ 2014-02-19 13:27 UTC (permalink / raw)
To: David Miller, grant.likely
Cc: ben.dooks, linux-kernel, devicetree, linux-kernel, netdev,
linux-sh
In-Reply-To: <20140218.181317.1712776234149210938.davem@davemloft.net>
Hello.
On 19-02-2014 3:13, David Miller wrote:
>>> The of_mdiobus_register_phy() is not setting phy->irq thus causing
>>> some drivers to incorrectly assume that the PHY does not have an
>>> IRQ associated with it. Not only do some drivers report no IRQ
>>> they do not install an interrupt handler for the PHY.
>>> Simplify the code setting irq and set the phy->irq at the same
>>> time so that we cover the following issues, which should cover
>>> all the cases the code will find:
>>> - Set phy->irq if node has irq property and mdio->irq is NULL
>>> - Set phy->irq if node has no irq and mdio->irq is not NULL
>>> - Leave phy->irq as PHY_POLL default if none of the above
>>> This fixes the issue:
>>> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
>>> to the correct:
>>> net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> Looks okay to me
>> Reviewed-by: Grant Likely <grant.likely@linaro.org>
> Applied, thanks.
So, you decided to ignore my comment about unneeded *else* branch?
WBR, Sergei
^ permalink raw reply
* Re: [Xen-devel] [RFC v2 1/4] bridge: enable interfaces to opt out from becoming the root bridge
From: Zoltan Kiss @ 2014-02-19 14:35 UTC (permalink / raw)
To: Ian Campbell, Luis R. Rodriguez
Cc: kvm, netdev@vger.kernel.org, bridge, linux-kernel@vger.kernel.org,
Stephen Hemminger, xen-devel
In-Reply-To: <1392803559.23084.99.camel@kazak.uk.xensource.com>
On 19/02/14 09:52, Ian Campbell wrote:
> On Tue, 2014-02-18 at 13:02 -0800, Luis R. Rodriguez wrote:
>> On Sun, Feb 16, 2014 at 10:57 AM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>>> On Fri, 14 Feb 2014 18:59:37 -0800
>>> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> wrote:
>>>
>>>> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>>>>
>>>> It doesn't make sense for some interfaces to become a root bridge
>>>> at any point in time. One example is virtual backend interfaces
>>>> which rely on other entities on the bridge for actual physical
>>>> connectivity. They only provide virtual access.
>>>>
>>>> Device drivers that know they should never become part of the
>>>> root bridge have been using a trick of setting their MAC address
>>>> to a high broadcast MAC address such as FE:FF:FF:FF:FF:FF. Instead
>>>> of using these hacks lets the interfaces annotate its intent and
>>>> generalizes a solution for multiple drivers, while letting the
>>>> drivers use a random MAC address or one prefixed with a proper OUI.
>>>> This sort of hack is used by both qemu and xen for their backend
>>>> interfaces.
>>>>
>>>> Cc: Stephen Hemminger <stephen@networkplumber.org>
>>>> Cc: bridge@lists.linux-foundation.org
>>>> Cc: netdev@vger.kernel.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>>>
>>> This is already supported in a more standard way via the root
>>> block flag.
>>
>> Great! For documentation purposes the root_block flag is a sysfs
>> attribute, added via 3.8 through commit 1007dd1a. The respective
>> interface flag is IFLA_BRPORT_PROTECT and can be set via the iproute2
>> bridge utility or through sysfs:
>>
>> mcgrof@garbanzo ~/linux (git::master)$ find /sys/ -name root_block
>> /sys/devices/pci0000:00/0000:00:04.0/0000:02:00.0/net/eth0/brport/root_block
>> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
>> /sys/devices/virtual/net/vif3.0-emu/brport/root_block
>>
>> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ cat
>> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
>> 0
>> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ sudo bridge link set
>> dev vif3.0 root_block on
>> mcgrof@garbanzo ~/devel/iproute2 (git::master)$ cat
>> /sys/devices/vif-3-0/net/vif3.0/brport/root_block
>> 1
>>
>> So if we'd want to avoid using the MAC address hack alternative to
>> skip a root port userspace would need to be updated to simply set this
>> attribute after adding the device to the bridge. Based on Zoltan's
>> feedback there seems to be use cases to not enable this always for all
>> xen-netback interfaces though as such we can just punt this to
>> userspace for the topologies that require this.
>>
>> The original motivation for this series was to avoid the IPv6
>> duplicate address incurred by the MAC address hack for avoiding the
>> root bridge. Given that Zoltan also noted a use case whereby IPv4 and
>> IPv6 addresses can be assigned to the backend interfaces we should be
>> able to avoid the duplicate address situation for IPv6 by using a
>> proper random MAC address *once* userspace has been updated also to
>> use IFLA_BRPORT_PROTECT. New userspace can't and won't need to set
>> this flag for older kernels (older than 3.8) as root_block is not
>> implemented on those kernels and the MAC address hack would still be
>> used there. This strategy however does put a requirement on new
>> kernels to use new userspace as otherwise the MAC address workaround
>> would not be in place and root_block would not take effect.
>
> Can't we arrange things in the Xen hotplug scripts such that if the
> root_block stuff isn't available/doesn't work we fallback to the
> existing fe:ff:ff:ff:ff usage?
>
> That would avoid concerns about forward/backwards compat I think. It
> wouldn't solve the issue you are targeting on old systems, but it also
> doesn't regress them any further.
I agree, I think this problem could be better handled from userspace: if
it can set root_block then change the default MAC to a random one, if it
can't, then stay with the default one. Or if someone doesn't care about
STP but DAD is still important, userspace can have a force_random_mac
option somewhere to change to a random MAC regardless of root_block
presence.
Zoli
^ permalink raw reply
* Re: [PATCH] atm: ambassador: use NULL instead of 0 for pointer
From: chas williams - CONTRACTOR @ 2014-02-19 14:52 UTC (permalink / raw)
To: Daeseok Youn; +Cc: linux-atm-general, netdev, linux-kernel
In-Reply-To: <4982315.WAlBt88ElR@daeseok-laptop.cloud.net>
On Wed, 19 Feb 2014 14:11:15 +0900
Daeseok Youn <daeseok.youn@gmail.com> wrote:
> >From 932e928d53b1e588dc17019e7f9fa7a61b8b7468 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <daeseok.youn@gmail.com>
> Date: Wed, 19 Feb 2014 10:35:41 +0900
> Subject: [PATCH] atm: ambassador: use NULL instead of 0 for pointer
>
> sparse says:
>
> drivers/atm/ambassador.c:1928:24: warning:
> Using plain integer as NULL pointer
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> drivers/atm/ambassador.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/atm/ambassador.c b/drivers/atm/ambassador.c
> index 62a7607..f1a9198 100644
> --- a/drivers/atm/ambassador.c
> +++ b/drivers/atm/ambassador.c
> @@ -1925,7 +1925,7 @@ static int ucode_init(loader_block *lb, amb_dev *dev)
> const struct firmware *fw;
> unsigned long start_address;
> const struct ihex_binrec *rec;
> - const char *errmsg = 0;
> + const char *errmsg = NULL;
> int res;
>
> res = request_ihex_firmware(&fw, "atmsar11.fw", &dev->pci_dev->dev);
Acked-by: Chas Williams <chas@cmf.nrl.navy.mil>
^ permalink raw reply
* Re: [PATCH] atm: nicstar: use NULL instead of 0 for pointer
From: chas williams - CONTRACTOR @ 2014-02-19 14:53 UTC (permalink / raw)
To: Daeseok Youn; +Cc: linux-atm-general, netdev, linux-kernel
In-Reply-To: <1738246.bUPoQjSxLC@daeseok-laptop.cloud.net>
On Wed, 19 Feb 2014 14:12:46 +0900
Daeseok Youn <daeseok.youn@gmail.com> wrote:
> >From c320d2ea1ed51c88255c33a50c74fa3598ab7be6 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <daeseok.youn@gmail.com>
> Date: Wed, 19 Feb 2014 10:10:11 +0900
> Subject: [PATCH] atm: nicstar: use NULL instead of 0 for pointer
>
> sparse says:
>
> drivers/atm/nicstar.c:642:27: warning:
> Using plain integer as NULL pointer
> drivers/atm/nicstar.c:644:27:
> warning: Using plain integer as NULL pointer
> drivers/atm/nicstar.c:982:51:
> warning: Using plain integer as NULL pointer
> drivers/atm/nicstar.c:996:51:
> warning: Using plain integer as NULL pointer
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> drivers/atm/nicstar.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
> index 9587e95..13ed54c 100644
> --- a/drivers/atm/nicstar.c
> +++ b/drivers/atm/nicstar.c
> @@ -639,9 +639,9 @@ static int ns_init_card(int i, struct pci_dev *pcidev)
> card->hbnr.init = NUM_HB;
> card->hbnr.max = MAX_HB;
>
> - card->sm_handle = 0x00000000;
> + card->sm_handle = NULL;
> card->sm_addr = 0x00000000;
> - card->lg_handle = 0x00000000;
> + card->lg_handle = NULL;
> card->lg_addr = 0x00000000;
>
> card->efbie = 1; /* To prevent push_rxbufs from enabling the interrupt */
> @@ -979,7 +979,7 @@ static void push_rxbufs(ns_dev * card, struct sk_buff *skb)
> addr2 = card->sm_addr;
> handle2 = card->sm_handle;
> card->sm_addr = 0x00000000;
> - card->sm_handle = 0x00000000;
> + card->sm_handle = NULL;
> } else { /* (!sm_addr) */
>
> card->sm_addr = addr1;
> @@ -993,7 +993,7 @@ static void push_rxbufs(ns_dev * card, struct sk_buff *skb)
> addr2 = card->lg_addr;
> handle2 = card->lg_handle;
> card->lg_addr = 0x00000000;
> - card->lg_handle = 0x00000000;
> + card->lg_handle = NULL;
> } else { /* (!lg_addr) */
>
> card->lg_addr = addr1;
Acked-by: Chas Williams <chas@cmf.nrl.navy.mil>
^ permalink raw reply
* Re: [PATCH] atm: solos-pci: make solos_bh() as static
From: chas williams - CONTRACTOR @ 2014-02-19 14:54 UTC (permalink / raw)
To: Daeseok Youn; +Cc: linux-atm-general, netdev, linux-kernel
In-Reply-To: <1547067.MtNWZjBp8J@daeseok-laptop.cloud.net>
On Wed, 19 Feb 2014 14:13:54 +0900
Daeseok Youn <daeseok.youn@gmail.com> wrote:
> >From 6297aabeff748777b520cc0ee835af0a3ddc79e2 Mon Sep 17 00:00:00 2001
> From: Daeseok Youn <daeseok.youn@gmail.com>
> Date: Wed, 19 Feb 2014 10:49:12 +0900
> Subject: [PATCH] atm: solos-pci: make solos_bh() as static
>
> sparse says:
>
> drivers/atm/solos-pci.c:763:6: warning:
> symbol 'solos_bh' was not declared. Should it be static?
>
> Signed-off-by: Daeseok Youn <daeseok.youn@gmail.com>
> ---
> drivers/atm/solos-pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> index e3fb496..943cf0d 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -760,7 +760,7 @@ static irqreturn_t solos_irq(int irq, void *dev_id)
> return IRQ_RETVAL(handled);
> }
>
> -void solos_bh(unsigned long card_arg)
> +static void solos_bh(unsigned long card_arg)
> {
> struct solos_card *card = (void *)card_arg;
> uint32_t card_flags;
Acked-by: Chas Williams <chas@cmf.nrl.navy.mil>
^ 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