* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Andy Gospodarek @ 2011-03-01 20:38 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, fubar, eric.dumazet, nicolas.2p.debian, andy
In-Reply-To: <20110301092907.GG2855@psychotron.redhat.com>
On Tue, Mar 01, 2011 at 10:29:07AM +0100, Jiri Pirko wrote:
> Unapplicable, sorry (wrong branch :(). Here's corrected patch:
>
> Subject: [PATCH net-next-2.6 v2] bonding: remove skb_share_check in handle_frame
>
> No need to do share check here.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 3 ---
> 1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 584f97b..367ea60 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1498,9 +1498,6 @@ static struct sk_buff *bond_handle_frame(struct sk_buff *skb)
> struct net_device *slave_dev;
> struct net_device *bond_dev;
>
> - skb = skb_share_check(skb, GFP_ATOMIC);
> - if (unlikely(!skb))
> - return NULL;
> slave_dev = skb->dev;
> bond_dev = ACCESS_ONCE(slave_dev->master);
> if (unlikely(!bond_dev))
> --
> 1.7.3.4
>
Why did you decide to get rid of it here rather than the 3 places in the
bonding driver where it is currently needed? I think this can cover
those cases since bond_handle_frame will be called after the ptype_all
handlers before any of the ptype handlers.
^ permalink raw reply
* Re: SO_REUSEPORT - can it be done in kernel?
From: David Miller @ 2011-03-01 20:36 UTC (permalink / raw)
To: eric.dumazet
Cc: herbert, rick.jones2, therbert, wsommerfeld, daniel.baluta,
netdev
In-Reply-To: <1298997787.3284.123.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 01 Mar 2011 17:43:07 +0100
> Le mardi 01 mars 2011 à 20:35 +0800, Herbert Xu a écrit :
>> On Mon, Feb 28, 2011 at 07:36:59PM +0800, Herbert Xu wrote:
>> > Here are the patches I used. Please don't them yet as I intend
>> > to clean them up quite a bit.
>>
>> OK here is the version ready for merging (please retest them
>> though as I have changed things substantially).
>>
>> The main change is that the legacy UDP code path is now gone
>> so we use the same UDP header generation whether corking is on
>> or off.
>>
>> I will add IPv6 support in a later patch set.
>>
>> Thanks!
>
> For the whole patchset :
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, great work everyone!
^ permalink raw reply
* [PATCH] ipv6: Consolidate route lookup sequences.
From: David Miller @ 2011-03-01 20:36 UTC (permalink / raw)
To: netdev
Route lookups follow a general pattern in the ipv6 code wherein
we first find the non-IPSEC route, potentially override the
flow destination address due to ipv6 options settings, and then
finally make an IPSEC search using either xfrm_lookup() or
__xfrm_lookup().
__xfrm_lookup() is used when we want to generate a blackhole route
if the key manager needs to resolve the IPSEC rules (in this case
-EREMOTE is returned and the original 'dst' is left unchanged).
Otherwise plain xfrm_lookup() is used and when asynchronous IPSEC
resolution is necessary, we simply fail the lookup completely.
All of these cases are encapsulated into two routines,
ip6_dst_lookup_flow and ip6_sk_dst_lookup_flow. The latter of which
handles unconnected UDP datagram sockets.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ipv6.h | 11 ++++-
net/ipv6/af_inet6.c | 17 ++------
net/ipv6/datagram.c | 15 ++-----
net/ipv6/inet6_connection_sock.c | 9 +---
net/ipv6/ip6_output.c | 80 ++++++++++++++++++++++++++++++++-----
net/ipv6/raw.c | 15 +------
net/ipv6/syncookies.c | 7 +--
net/ipv6/tcp_ipv6.c | 57 ++++++++++-----------------
net/ipv6/udp.c | 15 ++-----
9 files changed, 118 insertions(+), 108 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 4a3cd2c..1fc5631 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -512,12 +512,17 @@ extern void ip6_flush_pending_frames(struct sock *sk);
extern int ip6_dst_lookup(struct sock *sk,
struct dst_entry **dst,
struct flowi *fl);
+extern struct dst_entry * ip6_dst_lookup_flow(struct sock *sk,
+ struct flowi *fl,
+ const struct in6_addr *final_dst,
+ bool want_blackhole);
+extern struct dst_entry * ip6_sk_dst_lookup_flow(struct sock *sk,
+ struct flowi *fl,
+ const struct in6_addr *final_dst,
+ bool want_blackhole);
extern int ip6_dst_blackhole(struct sock *sk,
struct dst_entry **dst,
struct flowi *fl);
-extern int ip6_sk_dst_lookup(struct sock *sk,
- struct dst_entry **dst,
- struct flowi *fl);
/*
* skb processing functions
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 3194aa9..a88b2e9 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -644,9 +644,8 @@ EXPORT_SYMBOL(inet6_unregister_protosw);
int inet6_sk_rebuild_header(struct sock *sk)
{
- int err;
- struct dst_entry *dst;
struct ipv6_pinfo *np = inet6_sk(sk);
+ struct dst_entry *dst;
dst = __sk_dst_check(sk, np->dst_cookie);
@@ -668,17 +667,11 @@ int inet6_sk_rebuild_header(struct sock *sk)
final_p = fl6_update_dst(&fl, np->opt, &final);
- err = ip6_dst_lookup(sk, &dst, &fl);
- if (err) {
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, false);
+ if (IS_ERR(dst)) {
sk->sk_route_caps = 0;
- return err;
- }
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0) {
- sk->sk_err_soft = -err;
- return err;
+ sk->sk_err_soft = -PTR_ERR(dst);
+ return PTR_ERR(dst);
}
__ip6_dst_store(sk, dst, NULL, NULL);
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 320bdb8..be3a781 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -162,18 +162,11 @@ ipv4_connected:
opt = flowlabel ? flowlabel->opt : np->opt;
final_p = fl6_update_dst(&fl, opt, &final);
- err = ip6_dst_lookup(sk, &dst, &fl);
- if (err)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, true);
+ err = 0;
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
goto out;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- err = __xfrm_lookup(sock_net(sk), &dst, &fl, sk, XFRM_LOOKUP_WAIT);
- if (err < 0) {
- if (err == -EREMOTE)
- err = ip6_dst_blackhole(sk, &dst, &fl);
- if (err < 0)
- goto out;
}
/* source address lookup done in ip6_dst_lookup */
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index d144e62..235ac19 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -74,13 +74,8 @@ struct dst_entry *inet6_csk_route_req(struct sock *sk,
fl.fl_ip_sport = inet_rsk(req)->loc_port;
security_req_classify_flow(req, &fl);
- if (ip6_dst_lookup(sk, &dst, &fl))
- return NULL;
-
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- if ((xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, false);
+ if (IS_ERR(dst))
return NULL;
return dst;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 065b3f7..13d3cb0 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1002,29 +1002,87 @@ int ip6_dst_lookup(struct sock *sk, struct dst_entry **dst, struct flowi *fl)
EXPORT_SYMBOL_GPL(ip6_dst_lookup);
/**
- * ip6_sk_dst_lookup - perform socket cached route lookup on flow
+ * ip6_dst_lookup_flow - perform route lookup on flow with ipsec
+ * @sk: socket which provides route info
+ * @fl: flow to lookup
+ * @final_dst: final destination address for ipsec lookup
+ * @want_blackhole: IPSEC blackhole handling desired
+ *
+ * This function performs a route lookup on the given flow.
+ *
+ * It returns a valid dst pointer on success, or a pointer encoded
+ * error code.
+ */
+struct dst_entry *ip6_dst_lookup_flow(struct sock *sk, struct flowi *fl,
+ const struct in6_addr *final_dst,
+ bool want_blackhole)
+{
+ struct dst_entry *dst = NULL;
+ int err;
+
+ err = ip6_dst_lookup_tail(sk, &dst, fl);
+ if (err)
+ return ERR_PTR(err);
+ if (final_dst)
+ ipv6_addr_copy(&fl->fl6_dst, final_dst);
+ if (want_blackhole) {
+ err = __xfrm_lookup(sock_net(sk), &dst, fl, sk, XFRM_LOOKUP_WAIT);
+ if (err == -EREMOTE)
+ err = ip6_dst_blackhole(sk, &dst, fl);
+ if (err)
+ return ERR_PTR(err);
+ } else {
+ err = xfrm_lookup(sock_net(sk), &dst, fl, sk, 0);
+ if (err)
+ return ERR_PTR(err);
+ }
+ return dst;
+}
+EXPORT_SYMBOL_GPL(ip6_dst_lookup_flow);
+
+/**
+ * ip6_sk_dst_lookup_flow - perform socket cached route lookup on flow
* @sk: socket which provides the dst cache and route info
- * @dst: pointer to dst_entry * for result
* @fl: flow to lookup
+ * @final_dst: final destination address for ipsec lookup
+ * @want_blackhole: IPSEC blackhole handling desired
*
* This function performs a route lookup on the given flow with the
* possibility of using the cached route in the socket if it is valid.
* It will take the socket dst lock when operating on the dst cache.
* As a result, this function can only be used in process context.
*
- * It returns zero on success, or a standard errno code on error.
+ * It returns a valid dst pointer on success, or a pointer encoded
+ * error code.
*/
-int ip6_sk_dst_lookup(struct sock *sk, struct dst_entry **dst, struct flowi *fl)
+struct dst_entry *ip6_sk_dst_lookup_flow(struct sock *sk, struct flowi *fl,
+ const struct in6_addr *final_dst,
+ bool want_blackhole)
{
- *dst = NULL;
- if (sk) {
- *dst = sk_dst_check(sk, inet6_sk(sk)->dst_cookie);
- *dst = ip6_sk_dst_check(sk, *dst, fl);
- }
+ struct dst_entry *dst = sk_dst_check(sk, inet6_sk(sk)->dst_cookie);
+ int err;
- return ip6_dst_lookup_tail(sk, dst, fl);
+ dst = ip6_sk_dst_check(sk, dst, fl);
+
+ err = ip6_dst_lookup_tail(sk, &dst, fl);
+ if (err)
+ return ERR_PTR(err);
+ if (final_dst)
+ ipv6_addr_copy(&fl->fl6_dst, final_dst);
+ if (want_blackhole) {
+ err = __xfrm_lookup(sock_net(sk), &dst, fl, sk, XFRM_LOOKUP_WAIT);
+ if (err == -EREMOTE)
+ err = ip6_dst_blackhole(sk, &dst, fl);
+ if (err)
+ return ERR_PTR(err);
+ } else {
+ err = xfrm_lookup(sock_net(sk), &dst, fl, sk, 0);
+ if (err)
+ return ERR_PTR(err);
+ }
+ return dst;
}
-EXPORT_SYMBOL_GPL(ip6_sk_dst_lookup);
+EXPORT_SYMBOL_GPL(ip6_sk_dst_lookup_flow);
static inline int ip6_ufo_append_data(struct sock *sk,
int getfrag(void *from, char *to, int offset, int len,
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 364e866..dc29b07 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -856,20 +856,11 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
fl.oif = np->mcast_oif;
security_sk_classify_flow(sk, &fl);
- err = ip6_dst_lookup(sk, &dst, &fl);
- if (err)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, true);
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
goto out;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- err = __xfrm_lookup(sock_net(sk), &dst, &fl, sk, XFRM_LOOKUP_WAIT);
- if (err < 0) {
- if (err == -EREMOTE)
- err = ip6_dst_blackhole(sk, &dst, &fl);
- if (err < 0)
- goto out;
}
-
if (hlimit < 0) {
if (ipv6_addr_is_multicast(&fl.fl6_dst))
hlimit = np->mcast_hops;
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index 09fd34f..0b4cf35 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -243,12 +243,9 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
fl.fl_ip_dport = inet_rsk(req)->rmt_port;
fl.fl_ip_sport = inet_sk(sk)->inet_sport;
security_req_classify_flow(req, &fl);
- if (ip6_dst_lookup(sk, &dst, &fl))
- goto out_free;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, false);
+ if (IS_ERR(dst))
goto out_free;
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 1d0ab55..e59a31c 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -255,18 +255,10 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
security_sk_classify_flow(sk, &fl);
- err = ip6_dst_lookup(sk, &dst, &fl);
- if (err)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, true);
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
goto failure;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- err = __xfrm_lookup(sock_net(sk), &dst, &fl, sk, XFRM_LOOKUP_WAIT);
- if (err < 0) {
- if (err == -EREMOTE)
- err = ip6_dst_blackhole(sk, &dst, &fl);
- if (err < 0)
- goto failure;
}
if (saddr == NULL) {
@@ -385,7 +377,7 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
np = inet6_sk(sk);
if (type == ICMPV6_PKT_TOOBIG) {
- struct dst_entry *dst = NULL;
+ struct dst_entry *dst;
if (sock_owned_by_user(sk))
goto out;
@@ -413,13 +405,9 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
fl.fl_ip_sport = inet->inet_sport;
security_skb_classify_flow(skb, &fl);
- if ((err = ip6_dst_lookup(sk, &dst, &fl))) {
- sk->sk_err_soft = -err;
- goto out;
- }
-
- if ((err = xfrm_lookup(net, &dst, &fl, sk, 0)) < 0) {
- sk->sk_err_soft = -err;
+ dst = ip6_dst_lookup_flow(sk, &fl, NULL, false);
+ if (IS_ERR(dst)) {
+ sk->sk_err_soft = -PTR_ERR(dst);
goto out;
}
@@ -496,7 +484,7 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
struct in6_addr * final_p, final;
struct flowi fl;
struct dst_entry *dst;
- int err = -1;
+ int err;
memset(&fl, 0, sizeof(fl));
fl.proto = IPPROTO_TCP;
@@ -512,15 +500,13 @@ static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
opt = np->opt;
final_p = fl6_update_dst(&fl, opt, &final);
- err = ip6_dst_lookup(sk, &dst, &fl);
- if (err)
+ dst = ip6_dst_lookup_flow(sk, &fl, final_p, false);
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
goto done;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
- goto done;
-
+ }
skb = tcp_make_synack(sk, dst, req, rvp);
+ err = -ENOMEM;
if (skb) {
__tcp_v6_send_check(skb, &treq->loc_addr, &treq->rmt_addr);
@@ -1079,15 +1065,14 @@ static void tcp_v6_send_response(struct sk_buff *skb, u32 seq, u32 ack, u32 win,
* Underlying function will use this to retrieve the network
* namespace
*/
- if (!ip6_dst_lookup(ctl_sk, &dst, &fl)) {
- if (xfrm_lookup(net, &dst, &fl, NULL, 0) >= 0) {
- skb_dst_set(buff, dst);
- ip6_xmit(ctl_sk, buff, &fl, NULL);
- TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
- if (rst)
- TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
- return;
- }
+ dst = ip6_dst_lookup_flow(ctl_sk, &fl, NULL, false);
+ if (!IS_ERR(dst)) {
+ skb_dst_set(buff, dst);
+ ip6_xmit(ctl_sk, buff, &fl, NULL);
+ TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
+ if (rst)
+ TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
+ return;
}
kfree_skb(buff);
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index a419a78..d86d7f6 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1125,18 +1125,11 @@ do_udp_sendmsg:
security_sk_classify_flow(sk, &fl);
- err = ip6_sk_dst_lookup(sk, &dst, &fl);
- if (err)
+ dst = ip6_sk_dst_lookup_flow(sk, &fl, final_p, true);
+ if (IS_ERR(dst)) {
+ err = PTR_ERR(dst);
+ dst = NULL;
goto out;
- if (final_p)
- ipv6_addr_copy(&fl.fl6_dst, final_p);
-
- err = __xfrm_lookup(sock_net(sk), &dst, &fl, sk, XFRM_LOOKUP_WAIT);
- if (err < 0) {
- if (err == -EREMOTE)
- err = ip6_dst_blackhole(sk, &dst, &fl);
- if (err < 0)
- goto out;
}
if (hlimit < 0) {
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure
From: Russell King - ARM Linux @ 2011-03-01 20:26 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Stuart Menefy, Peppe CAVALLARO, linux-sh@vger.kernel.org,
netdev@vger.kernel.org, John Stultz, Thomas Gleixner,
linux-kernel@vger.kernel.org
In-Reply-To: <201103011743.19847.arnd@arndb.de>
On Tue, Mar 01, 2011 at 05:43:19PM +0100, Arnd Bergmann wrote:
> On Tuesday 01 March 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > Also, what is the difference between this and clkdev?
> >
> > clkdev can be used to find a struct clk, which is fine if you just want to
> > read the time. In this instance we want to get interrupts from the timer
> > hardware, which isn't supported by the clk infrastructure.
>
> (adding Russell to Cc)
>
> Is this something that could sensibly be added to clk/clkdev?
I don't understand - why would anyone want to use clk/clkdev for timers.
clk/clkdev is all about those signals on the SoC which wiggle at regular
intervals between logic 0 and logic 1. It's not about things which count,
which seems to be an entirely separate problem, and hence why there's
nothing to deal with interrupts or setting timeouts etc.
^ permalink raw reply
* Re: pull request: sfc-next-2.6 2011-03-01
From: David Miller @ 2011-03-01 20:25 UTC (permalink / raw)
To: bhutchings; +Cc: linux-net-drivers, netdev
In-Reply-To: <1298942169.3069.179.camel@localhost>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 01 Mar 2011 01:16:09 +0000
> The following changes since commit 5b2c4dd2ec12cf0e53b2bd2926f0fe2d1fbb4eda:
> Jiri Pirko (1):
> net: convert bonding to use rx_handler
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem
>
Pulled, thanks Ben.
^ permalink raw reply
* Re: [PATCH net-next-2.6] sfc: Reduce size of efx_rx_buffer by unionising skb and page
From: David Miller @ 2011-03-01 20:24 UTC (permalink / raw)
To: bhutchings; +Cc: linux-net-drivers, netdev
In-Reply-To: <1298942490.3069.180.camel@localhost>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 01 Mar 2011 01:21:30 +0000
> From: Steve Hodgson <shodgson@solarflare.com>
>
> [bwh: Forward-ported to net-next-2.6.]
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
I'm going to apply this, but you can take this further by making
the buffer an "unsigned long" and encode the pointer plus a one-bit
binary state there to indicate what it is.
^ permalink raw reply
* Re: txqueuelen has wrong units; should be time
From: Eric Dumazet @ 2011-03-01 20:16 UTC (permalink / raw)
To: Albert Cahalan
Cc: David Miller, johnwheffner, linville, jussi.kivilinna, swmike,
linux-kernel, netdev
In-Reply-To: <1299010479.2930.1.camel@edumazet-laptop>
Le mardi 01 mars 2011 à 21:14 +0100, Eric Dumazet a écrit :
> Le mardi 01 mars 2011 à 14:37 -0500, Albert Cahalan a écrit :
> >
> > We certainly should allow some traffic spikes. 1 to 10 ms of
> > traffic ought to do nicely. Hundreds or thousands of ms is
> > getting way beyond "spike".
>
> OK.
Hmm, user error, hit wrong button, sorry.
^ permalink raw reply
* Re: txqueuelen has wrong units; should be time
From: Eric Dumazet @ 2011-03-01 20:14 UTC (permalink / raw)
To: Albert Cahalan
Cc: David Miller, johnwheffner, linville, jussi.kivilinna, swmike,
linux-kernel, netdev
In-Reply-To: <AANLkTimWMZHVQSHEUPuQgFXxNg85dEOesqhZE=e6x8kb@mail.gmail.com>
Le mardi 01 mars 2011 à 14:37 -0500, Albert Cahalan a écrit :
> On Tue, Mar 1, 2011 at 2:26 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le mardi 01 mars 2011 à 01:54 -0500, Albert Cahalan a écrit :
> >> On Mon, Feb 28, 2011 at 11:18 PM, David Miller <davem@davemloft.net> wrote:
> >> > From: Albert Cahalan <acahalan@gmail.com>
>
> >> >> It sounds like you need a callback or similar, so that TCP can be
> >> >> informed later that the drop has occurred.
> >> >
> >> > By that point we could have already sent an entire RTT's worth
> >> > of data, or more.
> >> >
> >> > It needs to be synchronous, otherwise performance suffers.
> >>
> >> Ouch. OTOH, the current situation: performance suffers.
> >>
> >> In case it makes you feel any better, consider two cases
> >> where synchronous feedback is already impossible.
> >> One is when you're routing packets that merely pass through.
> >> The other is when some other box is doing that to you.
> >> Either way, packets go bye-bye and nobody tells TCP.
> >
> > So in a hurry we decide to drop packets blindly because kernel took the
> > cpu to perform an urgent task ?
>
> Yes. If the system can't handle the load, it needs to fess up.
>
> > Bufferbloat is a configuration/tuning problem, not a "everything must be
> > redone" problem. We add new qdiscs (CHOKe, SFB, QFQ, ...) and let admins
> > do their job. Problem is most admins are unaware of the problems, and
> > only buy more bandwidth.
>
> We could at least do as well as Windows. >:-)
>
> You can not expect some random Linux user to tune things
> every time the link changes speed or the app mix changes.
> What person NOT ON THIS MAILING LIST is going to mess
> with their qdisc when they connect to a new access point
> or switch from running Skype to running Netflix? Heck, how
> many have any awareness of what a qdisk even is? Linux
> networking needs to be excellent for people with no clue.
>
> > We might need some changes (including new APIs).
>
> If an app can't specify latency, adding the ability could
> be nice. Still, stuff needs to JUST WORK more of the time.
>
> > ECN is a forward step. Blindly dropping packets before ever sending them
> > is a step backward.
>
> Last I knew, ECN defaulted to a setting of "2" which means
> it is only used in response. Perhaps it's time to change that.
> It's been a while, with defective firewalls being replaced
> by faster hardware.
>
> > We should allow some trafic spikes, or many applications will stop
> > working. Unless all applications are fixed, we are stuck.
>
> Such applications would stop working...
>
> 1. across a switch
> 2. across an older router
>
> We certainly should allow some traffic spikes. 1 to 10 ms of
> traffic ought to do nicely. Hundreds or thousands of ms is
> getting way beyond "spike".
OK.
^ permalink raw reply
* Re: [PATCH] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Ben Hutchings @ 2011-03-01 20:13 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: linux-kernel, mjt, arnd, mirqus, netdev, David Miller, kuznet,
pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo,
jesse, kees.cook, eugene, dan.j.rosenberg, akpm
In-Reply-To: <20110301194845.GA3533@albatros>
On Tue, 2011-03-01 at 22:48 +0300, Vasiliy Kulikov wrote:
> Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with
> CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean
> that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are
> limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't
> allow anybody load any module not related to networking.
>
> This patch restricts an ability of autoloading modules to netdev modules
> with explicit aliases. This fixes CVE-2011-1019.
>
> Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior
> of loading netdev modules by name (without any prefix) for processes
> with CAP_SYS_MODULE to maintain the compatibility with network scripts
> that use autoloading netdev modules by aliases like "eth0", "wlan0".
[...]
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8ae6631..fc6f037 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1119,8 +1119,16 @@ void dev_load(struct net *net, const char *name)
> dev = dev_get_by_name_rcu(net, name);
> rcu_read_unlock();
>
> - if (!dev && capable(CAP_NET_ADMIN))
> - request_module("%s", name);
> + if (!dev) {
> + if (capable(CAP_NET_ADMIN))
> + request_module("netdev-%s", name);
If this succeeds then the second request_module() should be skipped.
> + if (capable(CAP_SYS_MODULE)) {
> + if (!request_module("%s", name))
> + WARN_ONCE(1, "Loading kernel module for a "
> +"network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias "
> +"netdev-%s instead\n", name);
[...]
If this feature is to be deprecated, there should be an error message
for each interface that depends on it. However, use of the feature is
not a bug so WARN is not appropriate. I think pr_err() would be fine.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] fcoe: correct checking for bonding
From: Jiri Pirko @ 2011-03-01 20:12 UTC (permalink / raw)
To: Jay Vosburgh
Cc: James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk,
netdev-u79uwXL29TY76Z2rM5mHXA, devel-s9riP+hp16TNLxjTenLetw,
linux-scsi-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <9554.1299002762@death>
Tue, Mar 01, 2011 at 07:06:02PM CET, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org wrote:
>Joe Eykholt <joe.eykholt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>>On 2/28/11 10:37 PM, Jiri Pirko wrote:
>>> Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
>>>> On 2/28/11 9:15 AM, Jay Vosburgh wrote:
>>>>> Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>
>>>>>> Check for IFF_BONDING as this flag is set-up for all bonding devices.
>>>>>>
>>>>>> Signed-off-by: Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>> drivers/scsi/fcoe/fcoe.c | 4 +---
>>>>>> 1 files changed, 1 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>>>>> index 9f9600b..67714a4 100644
>>>>>> --- a/drivers/scsi/fcoe/fcoe.c
>>>>>> +++ b/drivers/scsi/fcoe/fcoe.c
>>>>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>>>>>> }
>>>>>>
>>>>>> /* Do not support for bonding device */
>>>>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) ||
>>>>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) ||
>>>>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) {
>>>>>> + if (netdev->priv_flags& IFF_BONDING) {
>>>>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>>>>>> return -EOPNOTSUPP;
>>>>>> }
>>>>>
>>>>> Based on past discussions, I believe the intent of the code is
>>>>> to permit FCOE over bonding only for active-backup mode, and possibly
>>>>> for -xor/-rr as well.
>>>>>
>>>>> I'm not sure if the slave or the master is what's being tested
>>>>> here, so I'm not sure what the right thing to do is. I suspect it's the
>>>>> master, as I recall discussion of one configuration involving
>>>>> active-backup mode balancing FCOE traffic over both the active and
>>>>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb
>>>>> to have the packets delivered even on the nominally inactive slave.
>>>>>
>>>>> -J
>>>>>
>>>>> ---
>>>>> -Jay Vosburgh, IBM Linux Technology Center, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
>>>>
>>>> Right. That was the intent. It should work on the physical dev, but probably
>>>> not on the master of the bond.
>>>>
>>>> If you have a master/slave bond for IPv4 between eth1 and eth2, say,
>>>> and they are going to two different DCE (FCoE) switches, presumably on
>>>> different VSANs but with ultimate access to the same disks,
>>>> then you want to split the FCoE traffic in active/active
>>>> mode using separate FCoE instances on eth1 and eth2 even though IP
>>>> is using active/standby on bond0. This should work. But, putting fcoe
>>>> on bond0 isn't going to do what you want.
>>>>
>>>> However, it seems like the check above shouldn't be checking
>>>> IFF_SLAVE_INACTIVE. I can't test this.
>>>
>>> OK. So I guess the right check should be for:
>>> (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER)
>>
>>I think that's OK. How about just checking for MASTER?
>>When is MASTER going to be set without BONDING?
>
> One or two other things besides bonding use IFF_MASTER, but
>IFF_BONDING is only for bonding.
>
>>Otherwise I'd add some parens or I might code this as:
>>
>> if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) ==
>> (IFF_BONDING | IFF_MASTER))
>
> This doesn't work because the flags are kept in different
>places, IFF_MASTER is in flags and IFF_BONDING in priv_flags.
Exactly - I'm going to send corrected patch very soon.
Jirka
>
> -J
>
>>Which is less clear, I know, but used to generate better code.
>>The compiler might generate the same code these days.
>>Not that this is performance-critical or anything.
>>
>>> This would disable adding all bond devices (like bond0 etc) and allows
>>> to use enslaved physdevs.
>>>
>>> Note that checking for mode is irrelevant here. Mode could be easily
>>> changed later without fcoe knowing that.
>
> This is also true.
>
> -J
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: remove skb_share_check in handle_frame
From: Nicolas de Pesloüan @ 2011-03-01 20:11 UTC (permalink / raw)
To: Changli Gao; +Cc: Jiri Pirko, netdev, davem, fubar, eric.dumazet, andy
In-Reply-To: <AANLkTimk_jyTYO8=8LRgY3C1un38qb1q9C-DbKWHirPS@mail.gmail.com>
Le 01/03/2011 16:12, Changli Gao a écrit :
> On Tue, Mar 1, 2011 at 5:29 PM, Jiri Pirko<jpirko@redhat.com> wrote:
>> Unapplicable, sorry (wrong branch :(). Here's corrected patch:
>>
>> Subject: [PATCH net-next-2.6 v2] bonding: remove skb_share_check in handle_frame
>>
>> No need to do share check here.
>>
>
> I don't think so. Although you avoid netif_rx(), you can't avoid
> ptype_all handlers. In fact, all the RX handlers should has this
> check(), if they may modify the skb.
Can you please develop your explanation?
In current __netif_receive_skb() (after the recent patch from Jiri), we deliver the skb to ptype_all
handlers inside a loop, while possibly changing skb->dev inside this loop.
Then, at the end of __netif_receive_skb(), we loop on ptype_base, without changing anything in skb.
Should we consider ptype_*->func() to be called in a pure sequential way? Should we consider that
when a ptype_*->func() returns, nothing from this handler will use the skb in anyway later, in a
parallel way?
Or should we, instead, consider that special precautions must be taken, because protocol handlers
may run in parallel for the same skb? Which kind of precautions?
Thanks.
Nicolas.
^ permalink raw reply
* Re: [Lxc-users] Bad checksums and lost packets with macvlan on dummy
From: Daniel Lezcano @ 2011-03-01 20:04 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Eric Dumazet, Andrian Nord, lxc-users, Linux Netdev List
In-Reply-To: <4D6D2402.6020705@trash.net>
On 03/01/2011 05:51 PM, Patrick McHardy wrote:
> On 01.03.2011 14:29, Daniel Lezcano wrote:
>> On 02/28/2011 08:45 AM, Eric Dumazet wrote:
>>>> In the normal case, dummy0 is supposed to drop the packets. But with
>>>> macvlan these packets are broadcasted to the other macvlan ports, so no
>>>> checksum is computed when the packets are transmitted between macvlan1
>>>> and macvlan2.
>>> So where frames get bad checksums ?
>>>
>>> In this "bridge" mode, I suspect the broadcast is done _before_ sending
>>> frame to dummy, so maybe macvlan should not inherit from lowerdev in
>>> this particular case ?
>> Hi Eric,
>>
>> yes, you are right, the packets are sent before.
>>
>> In the 'macvlan_queue_xmit', the code checks the dev is in 'bridge'
>> mode. If so, it looks if there is a destination port for the packet and
>> then calls the 'forward' callback which is 'dev_forward_skb'.
>>
>> I was able to reproduce the same problem with qemu and an emulated
>> 'e1000' card instead of dummy0. The packets are dropped too.
>>
>> Patrick, do you have any suggestions to fix this ?
> Since the frames are only looped back locally, I suppose the easiest
> fix would be to mark them with CHECKSUM_UNNECESSARY. Alternatively
> we need to complete the checksum manually, similar to what
> dev_hard_start_xmit() does.
That sounds very simple to fix, maybe too much simple :)
I did the following change:
---
drivers/net/macvlan.c | 1 +
1 file changed, 1 insertion(+)
Index: linux-next/drivers/net/macvlan.c
===================================================================
--- linux-next.orig/drivers/net/macvlan.c
+++ linux-next/drivers/net/macvlan.c
@@ -222,6 +222,7 @@ static int macvlan_queue_xmit(struct sk_
if (vlan->mode == MACVLAN_MODE_BRIDGE) {
const struct ethhdr *eth = (void *)skb->data;
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
/* send to other bridge ports directly */
if (is_multicast_ether_addr(eth->h_dest)) {
and that fixed the problem. Do you think it is acceptable ?
Thanks
-- Daniel
^ permalink raw reply
* Re: 2.6.37 regression: adding main interface to a bridge breaks vlan interface RX
From: Jesse Gross @ 2011-03-01 20:04 UTC (permalink / raw)
To: Francois Romieu; +Cc: chriss, netdev
In-Reply-To: <20110301101609.GA13261@electric-eye.fr.zoreil.com>
On Tue, Mar 1, 2011 at 2:16 AM, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Jesse Gross <jesse@nicira.com> :
> [...]
>> Putting a vlan device on eth1 actually affects the behavior of the
>> driver, which is why that works. I suspect that if you put the vlan
>> device on both the physical interface and the bridge you would see
>> packets with tags on the bridge.
>>
>> Regardless, the solution is to remove the dependency on vlan devices
>> by converting over to the new vlan model. Francois, any chance that
>> you might have some time to look at this?
>
> The completely untested patch below is available for the (really) desperate
> souls.
>
> I should do a -next testing session this afternoon for this patch +
> Oliver's Neukum advertisement control + Hayes 8105e stuff (in reverse order).
Thanks Francois. When you are ready this looks good to me.
Reviewed-by: Jesse Gross <jesse@nicira.com>
^ permalink raw reply
* Re: [PATCH] Fix possible null pointer dereference in ariadne.c
From: David Miller @ 2011-03-01 19:53 UTC (permalink / raw)
To: huzaifas; +Cc: netdev, geert, tj, eric.dumazet
In-Reply-To: <1298979207-11237-1-git-send-email-huzaifas@redhat.com>
From: huzaifas@redhat.com
Date: Tue, 1 Mar 2011 17:03:27 +0530
> + *lance = (struct Am79C960*)dev->base_addr;
If you had actually compiled this, the compiler would have given
you a warning shoing that the leading "*" in "*lance" is not
appropriate for this assignment.
^ permalink raw reply
* [PATCH] net: don't allow CAP_NET_ADMIN to load non-netdev kernel modules
From: Vasiliy Kulikov @ 2011-03-01 19:48 UTC (permalink / raw)
To: linux-kernel
Cc: mjt, arnd, mirqus, bhutchings, netdev, David Miller, kuznet,
pekkas, jmorris, yoshfuji, kaber, eric.dumazet, therbert, xiaosuo,
jesse, kees.cook, eugene, dan.j.rosenberg, akpm
In-Reply-To: <20110228.112349.104067277.davem@davemloft.net>
Since a8f80e8ff94ecba629542d9b4b5f5a8ee3eb565c any process with
CAP_NET_ADMIN may load any module from /lib/modules/. This doesn't mean
that CAP_NET_ADMIN is a superset of CAP_SYS_MODULE as modules are
limited to /lib/modules/**. However, CAP_NET_ADMIN capability shouldn't
allow anybody load any module not related to networking.
This patch restricts an ability of autoloading modules to netdev modules
with explicit aliases. This fixes CVE-2011-1019.
Arnd Bergmann suggested to leave untouched the old pre-v2.6.32 behavior
of loading netdev modules by name (without any prefix) for processes
with CAP_SYS_MODULE to maintain the compatibility with network scripts
that use autoloading netdev modules by aliases like "eth0", "wlan0".
Currently there are only three users of the feature in the upstream
kernel: ipip, ip_gre and sit.
root@albatros:~# capsh --drop=$(seq -s, 0 11),$(seq -s, 13 34) --
root@albatros:~# grep Cap /proc/$$/status
CapInh: 0000000000000000
CapPrm: fffffff800001000
CapEff: fffffff800001000
CapBnd: fffffff800001000
root@albatros:~# modprobe xfs
FATAL: Error inserting xfs
(/lib/modules/2.6.38-rc6-00001-g2bf4ca3/kernel/fs/xfs/xfs.ko): Operation not permitted
root@albatros:~# lsmod | grep xfs
root@albatros:~# ifconfig xfs
xfs: error fetching interface information: Device not found
root@albatros:~# lsmod | grep xfs
root@albatros:~# lsmod | grep sit
root@albatros:~# ifconfig sit
sit: error fetching interface information: Device not found
root@albatros:~# lsmod | grep sit
root@albatros:~# ifconfig sit0
sit0 Link encap:IPv6-in-IPv4
NOARP MTU:1480 Metric:1
root@albatros:~# lsmod | grep sit
sit 10457 0
tunnel4 2957 1 sit
For CAP_SYS_MODULE module loading is still relaxed:
root@albatros:~# grep Cap /proc/$$/status
CapInh: 0000000000000000
CapPrm: ffffffffffffffff
CapEff: ffffffffffffffff
CapBnd: ffffffffffffffff
root@albatros:~# ifconfig xfs
xfs: error fetching interface information: Device not found
root@albatros:~# lsmod | grep xfs
xfs 745319 0
Reference: https://lkml.org/lkml/2011/2/24/203
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
include/linux/netdevice.h | 3 +++
net/core/dev.c | 12 ++++++++++--
net/ipv4/ip_gre.c | 2 +-
net/ipv4/ipip.c | 2 +-
net/ipv6/sit.c | 2 +-
5 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d971346..71caf7a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2392,6 +2392,9 @@ extern int netdev_notice(const struct net_device *dev, const char *format, ...)
extern int netdev_info(const struct net_device *dev, const char *format, ...)
__attribute__ ((format (printf, 2, 3)));
+#define MODULE_ALIAS_NETDEV(device) \
+ MODULE_ALIAS("netdev-" device)
+
#if defined(DEBUG)
#define netdev_dbg(__dev, format, args...) \
netdev_printk(KERN_DEBUG, __dev, format, ##args)
diff --git a/net/core/dev.c b/net/core/dev.c
index 8ae6631..fc6f037 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1119,8 +1119,16 @@ void dev_load(struct net *net, const char *name)
dev = dev_get_by_name_rcu(net, name);
rcu_read_unlock();
- if (!dev && capable(CAP_NET_ADMIN))
- request_module("%s", name);
+ if (!dev) {
+ if (capable(CAP_NET_ADMIN))
+ request_module("netdev-%s", name);
+ if (capable(CAP_SYS_MODULE)) {
+ if (!request_module("%s", name))
+ WARN_ONCE(1, "Loading kernel module for a "
+"network device with CAP_SYS_MODULE (deprecated). Use CAP_NET_ADMIN and alias "
+"netdev-%s instead\n", name);
+ }
+ }
}
EXPORT_SYMBOL(dev_load);
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 6613edf..d1d0e2c 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -1765,4 +1765,4 @@ module_exit(ipgre_fini);
MODULE_LICENSE("GPL");
MODULE_ALIAS_RTNL_LINK("gre");
MODULE_ALIAS_RTNL_LINK("gretap");
-MODULE_ALIAS("gre0");
+MODULE_ALIAS_NETDEV("gre0");
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 988f52f..a5f58e7 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -913,4 +913,4 @@ static void __exit ipip_fini(void)
module_init(ipip_init);
module_exit(ipip_fini);
MODULE_LICENSE("GPL");
-MODULE_ALIAS("tunl0");
+MODULE_ALIAS_NETDEV("tunl0");
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 8ce38f1..d2c16e1 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1290,4 +1290,4 @@ static int __init sit_init(void)
module_init(sit_init);
module_exit(sit_cleanup);
MODULE_LICENSE("GPL");
-MODULE_ALIAS("sit0");
+MODULE_ALIAS_NETDEV("sit0");
--
1.7.0.4
^ permalink raw reply related
* Re: source route ignored in favor of local interface
From: Joe Buehler @ 2011-03-01 19:31 UTC (permalink / raw)
To: Chris Friesen; +Cc: netdev
In-Reply-To: <4D6D4370.5040308@genband.com>
> If you really want to modify things, have you looked at your ip rules?
> "ip ru" will dump them. Typically it looks something like
>
> 0: from all lookup local
> 32766: from all lookup main
> 32767: from all lookup default
>
> The last item in each row is the name of the routing table. As you can
> see, the first rule is for local interfaces, and a later rule deals with
> the main and default routing tables.
>
> You could move the local lookup to rule 1 and add in a new rule 0
> specifically dealing with your situation.
We have tried some variations on modifications to ip rules, but the
kernel appears to treat rule 0 specially -- it cannot be deleted, for
example. This results in the enforcing of local interfaces before
source routes -- surely something that should be a policy decision, not
hard-coded.
Let me rephrase and ask: Does anyone know how to replace the priority 0
rule?
Joe Buehler
^ permalink raw reply
* Re: txqueuelen has wrong units; should be time
From: Albert Cahalan @ 2011-03-01 19:37 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, johnwheffner, linville, jussi.kivilinna, swmike,
linux-kernel, netdev
In-Reply-To: <1298964381.2676.58.camel@edumazet-laptop>
On Tue, Mar 1, 2011 at 2:26 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 01 mars 2011 à 01:54 -0500, Albert Cahalan a écrit :
>> On Mon, Feb 28, 2011 at 11:18 PM, David Miller <davem@davemloft.net> wrote:
>> > From: Albert Cahalan <acahalan@gmail.com>
>> >> It sounds like you need a callback or similar, so that TCP can be
>> >> informed later that the drop has occurred.
>> >
>> > By that point we could have already sent an entire RTT's worth
>> > of data, or more.
>> >
>> > It needs to be synchronous, otherwise performance suffers.
>>
>> Ouch. OTOH, the current situation: performance suffers.
>>
>> In case it makes you feel any better, consider two cases
>> where synchronous feedback is already impossible.
>> One is when you're routing packets that merely pass through.
>> The other is when some other box is doing that to you.
>> Either way, packets go bye-bye and nobody tells TCP.
>
> So in a hurry we decide to drop packets blindly because kernel took the
> cpu to perform an urgent task ?
Yes. If the system can't handle the load, it needs to fess up.
> Bufferbloat is a configuration/tuning problem, not a "everything must be
> redone" problem. We add new qdiscs (CHOKe, SFB, QFQ, ...) and let admins
> do their job. Problem is most admins are unaware of the problems, and
> only buy more bandwidth.
We could at least do as well as Windows. >:-)
You can not expect some random Linux user to tune things
every time the link changes speed or the app mix changes.
What person NOT ON THIS MAILING LIST is going to mess
with their qdisc when they connect to a new access point
or switch from running Skype to running Netflix? Heck, how
many have any awareness of what a qdisk even is? Linux
networking needs to be excellent for people with no clue.
> We might need some changes (including new APIs).
If an app can't specify latency, adding the ability could
be nice. Still, stuff needs to JUST WORK more of the time.
> ECN is a forward step. Blindly dropping packets before ever sending them
> is a step backward.
Last I knew, ECN defaulted to a setting of "2" which means
it is only used in response. Perhaps it's time to change that.
It's been a while, with defective firewalls being replaced
by faster hardware.
> We should allow some trafic spikes, or many applications will stop
> working. Unless all applications are fixed, we are stuck.
Such applications would stop working...
1. across a switch
2. across an older router
We certainly should allow some traffic spikes. 1 to 10 ms of
traffic ought to do nicely. Hundreds or thousands of ms is
getting way beyond "spike".
^ permalink raw reply
* Re: source route ignored in favor of local interface
From: Chris Friesen @ 2011-03-01 19:05 UTC (permalink / raw)
To: Joe Buehler; +Cc: netdev
In-Reply-To: <loom.20110301T154947-689@post.gmane.org>
On 03/01/2011 08:57 AM, Joe Buehler wrote:
> I have a LINUX box talking on many different networks at the same time. Since
> IP addresses on the networks can overlap (they are completely different
> networks) we use source routing and NAT to get packets going in and out of the
> right interfaces.
>
> Everything works great, with one exception. If I try to talk to a remote host
> that happens to have the same IP address as one of my interfaces, the kernel
> routes the packet to the local interface.
>
> It looks to me as though the problem is that the source routes are lower
> priority than the local interfaces. As soon as the kernel sees a destination
> address that matches a local interface it routes to the local interface and pays
> no attention to the source route.
>
> I consider this a bug. Is there any way to change this behavior?
How exactly do you expect it to handle this case? You've explicitly
told your host that its address is X, so why would it expect to find
that address assigned to another machine on the network? Suppose you
have an app listening on INADDR_ANY, and it gets a packet from that
adddres--how does it know whether the packet is destined to the local
machine or the remote one?
If you really want to modify things, have you looked at your ip rules?
"ip ru" will dump them. Typically it looks something like
0: from all lookup local
32766: from all lookup main
32767: from all lookup default
The last item in each row is the name of the routing table. As you can
see, the first rule is for local interfaces, and a later rule deals with
the main and default routing tables.
You could move the local lookup to rule 1 and add in a new rule 0
specifically dealing with your situation.
Chris
--
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com
^ permalink raw reply
* pull request: wireless-2.6 2011-03-01
From: John W. Linville @ 2011-03-01 18:46 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
Dave,
Here is another handful of fixes intended for 2.6.38. Included are a
couple of device IDs, a one-liner to allow iwlagn to load a bug-fixed
firmware for iwl5000, an endian fix for ath9k, another ath9k fix to
correct a regression involving interrupt enabling, and a fix to disable
power save mode for BCM4320b in rndis_wlan.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit e3fa3aff0cb198e7c53d894f52146121d9592872:
net: fix nla_policy_len to actually _iterate_ over the policy (2011-02-28 12:38:25 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
Christian Lamparter (1):
p54usb: add Senao NUB-350 usbid
Fry, Donald H (1):
iwlagn: Support new 5000 microcode.
Jan Puk (1):
carl9170: add Airlive X.USB a/b/g/n USBID
John W. Linville (1):
Merge branch 'wireless-2.6' of git://git.kernel.org/.../iwlwifi/iwlwifi-2.6
Jussi Kivilinna (1):
rndis_wlan: use power save only for BCM4320b
Stanislaw Gruszka (1):
ath9k: correct ath9k_hw_set_interrupts
Sujith Manoharan (1):
ath9k_htc: Fix an endian issue
drivers/net/wireless/ath/ath9k/hif_usb.c | 9 +++++----
drivers/net/wireless/ath/ath9k/mac.c | 5 +++--
drivers/net/wireless/ath/carl9170/usb.c | 2 ++
drivers/net/wireless/iwlwifi/iwl-5000.c | 2 +-
drivers/net/wireless/p54/p54usb.c | 1 +
drivers/net/wireless/rndis_wlan.c | 3 +++
6 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 5ab3084..07b1633 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -219,8 +219,9 @@ static int __hif_usb_tx(struct hif_device_usb *hif_dev)
struct tx_buf *tx_buf = NULL;
struct sk_buff *nskb = NULL;
int ret = 0, i;
- u16 *hdr, tx_skb_cnt = 0;
+ u16 tx_skb_cnt = 0;
u8 *buf;
+ __le16 *hdr;
if (hif_dev->tx.tx_skb_cnt == 0)
return 0;
@@ -245,9 +246,9 @@ static int __hif_usb_tx(struct hif_device_usb *hif_dev)
buf = tx_buf->buf;
buf += tx_buf->offset;
- hdr = (u16 *)buf;
- *hdr++ = nskb->len;
- *hdr++ = ATH_USB_TX_STREAM_MODE_TAG;
+ hdr = (__le16 *)buf;
+ *hdr++ = cpu_to_le16(nskb->len);
+ *hdr++ = cpu_to_le16(ATH_USB_TX_STREAM_MODE_TAG);
buf += 4;
memcpy(buf, nskb->data, nskb->len);
tx_buf->len = nskb->len + 4;
diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index 180170d..2915b11 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -885,7 +885,7 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints)
struct ath_common *common = ath9k_hw_common(ah);
if (!(ints & ATH9K_INT_GLOBAL))
- ath9k_hw_enable_interrupts(ah);
+ ath9k_hw_disable_interrupts(ah);
ath_dbg(common, ATH_DBG_INTERRUPT, "0x%x => 0x%x\n", omask, ints);
@@ -963,7 +963,8 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah, enum ath9k_int ints)
REG_CLR_BIT(ah, AR_IMR_S5, AR_IMR_S5_TIM_TIMER);
}
- ath9k_hw_enable_interrupts(ah);
+ if (ints & ATH9K_INT_GLOBAL)
+ ath9k_hw_enable_interrupts(ah);
return;
}
diff --git a/drivers/net/wireless/ath/carl9170/usb.c b/drivers/net/wireless/ath/carl9170/usb.c
index 537732e..f82c400 100644
--- a/drivers/net/wireless/ath/carl9170/usb.c
+++ b/drivers/net/wireless/ath/carl9170/usb.c
@@ -118,6 +118,8 @@ static struct usb_device_id carl9170_usb_ids[] = {
{ USB_DEVICE(0x057c, 0x8402) },
/* Qwest/Actiontec 802AIN Wireless N USB Network Adapter */
{ USB_DEVICE(0x1668, 0x1200) },
+ /* Airlive X.USB a/b/g/n */
+ { USB_DEVICE(0x1b75, 0x9170) },
/* terminate */
{}
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index 79ab0a6..537fb8c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -51,7 +51,7 @@
#include "iwl-agn-debugfs.h"
/* Highest firmware API version supported */
-#define IWL5000_UCODE_API_MAX 2
+#define IWL5000_UCODE_API_MAX 5
#define IWL5150_UCODE_API_MAX 2
/* Lowest firmware API version supported */
diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 21713a7..9b344a9 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -98,6 +98,7 @@ static struct usb_device_id p54u_table[] __devinitdata = {
{USB_DEVICE(0x1413, 0x5400)}, /* Telsey 802.11g USB2.0 Adapter */
{USB_DEVICE(0x1435, 0x0427)}, /* Inventel UR054G */
{USB_DEVICE(0x1668, 0x1050)}, /* Actiontec 802UIG-1 */
+ {USB_DEVICE(0x1740, 0x1000)}, /* Senao NUB-350 */
{USB_DEVICE(0x2001, 0x3704)}, /* DLink DWL-G122 rev A2 */
{USB_DEVICE(0x2001, 0x3705)}, /* D-Link DWL-G120 rev C1 */
{USB_DEVICE(0x413c, 0x5513)}, /* Dell WLA3310 USB Wireless Adapter */
diff --git a/drivers/net/wireless/rndis_wlan.c b/drivers/net/wireless/rndis_wlan.c
index 848cc2c..518542b 100644
--- a/drivers/net/wireless/rndis_wlan.c
+++ b/drivers/net/wireless/rndis_wlan.c
@@ -2597,6 +2597,9 @@ static int rndis_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
__le32 mode;
int ret;
+ if (priv->device_type != RNDIS_BCM4320B)
+ return -ENOTSUPP;
+
netdev_dbg(usbdev->net, "%s(): %s, %d\n", __func__,
enabled ? "enabled" : "disabled",
timeout);
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply related
* [PATCH] arch/tile: fix deadlock bugs in rwlock implementation
From: Chris Metcalf @ 2011-03-01 18:30 UTC (permalink / raw)
To: linux-kernel; +Cc: David Miller, xiyou.wangcong, eric.dumazet, netdev
In-Reply-To: <AANLkTim1F679mmnbHnYkvi6ZDojxD-tPGer4F+61C+di@mail.gmail.com>
The first issue fixed in this patch is that pending rwlock write locks
could lock out new readers; this could cause a deadlock if a read lock was
held on cpu 1, a write lock was then attempted on cpu 2 and was pending,
and cpu 1 was interrupted and attempted to re-acquire a read lock.
The write lock code was modified to not lock out new readers.
The second issue fixed is that there was a narrow race window where a tns
instruction had been issued (setting the lock value to "1") and the store
instruction to reset the lock value correctly had not yet been issued.
In this case, if an interrupt occurred and the same cpu then tried to
manipulate the lock, it would find the lock value set to "1" and spin
forever, assuming some other cpu was partway through updating it. The fix
is to enforce an interrupt critical section around the tns/store pair.
Since these changes make the rwlock "fast path" code heavier weight,
I decided to move all the rwlock code all out of line, leaving only the
conventional spinlock code with fastpath inlines.
As part of this change I also eliminate support for the now-obsolete
tns_atomic mode.
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
arch/tile/include/asm/spinlock_32.h | 84 ++----------------
arch/tile/lib/spinlock_32.c | 163 +++++++++++++++++++++--------------
2 files changed, 108 insertions(+), 139 deletions(-)
diff --git a/arch/tile/include/asm/spinlock_32.h b/arch/tile/include/asm/spinlock_32.h
index 88efdde..32ba1fe 100644
--- a/arch/tile/include/asm/spinlock_32.h
+++ b/arch/tile/include/asm/spinlock_32.h
@@ -21,6 +21,7 @@
#include <asm/page.h>
#include <asm/system.h>
#include <linux/compiler.h>
+#include <arch/spr_def.h>
/*
* We only use even ticket numbers so the '1' inserted by a tns is
@@ -78,13 +79,6 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock);
#define _RD_COUNT_SHIFT 24
#define _RD_COUNT_WIDTH 8
-/* Internal functions; do not use. */
-void arch_read_lock_slow(arch_rwlock_t *, u32);
-int arch_read_trylock_slow(arch_rwlock_t *);
-void arch_read_unlock_slow(arch_rwlock_t *);
-void arch_write_lock_slow(arch_rwlock_t *, u32);
-void arch_write_unlock_slow(arch_rwlock_t *, u32);
-
/**
* arch_read_can_lock() - would read_trylock() succeed?
*/
@@ -104,94 +98,32 @@ static inline int arch_write_can_lock(arch_rwlock_t *rwlock)
/**
* arch_read_lock() - acquire a read lock.
*/
-static inline void arch_read_lock(arch_rwlock_t *rwlock)
-{
- u32 val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val << _RD_COUNT_WIDTH)) {
- arch_read_lock_slow(rwlock, val);
- return;
- }
- rwlock->lock = val + (1 << _RD_COUNT_SHIFT);
-}
+void arch_read_lock(arch_rwlock_t *rwlock);
/**
- * arch_read_lock() - acquire a write lock.
+ * arch_write_lock() - acquire a write lock.
*/
-static inline void arch_write_lock(arch_rwlock_t *rwlock)
-{
- u32 val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val != 0)) {
- arch_write_lock_slow(rwlock, val);
- return;
- }
- rwlock->lock = 1 << _WR_NEXT_SHIFT;
-}
+void arch_write_lock(arch_rwlock_t *rwlock);
/**
* arch_read_trylock() - try to acquire a read lock.
*/
-static inline int arch_read_trylock(arch_rwlock_t *rwlock)
-{
- int locked;
- u32 val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val & 1))
- return arch_read_trylock_slow(rwlock);
- locked = (val << _RD_COUNT_WIDTH) == 0;
- rwlock->lock = val + (locked << _RD_COUNT_SHIFT);
- return locked;
-}
+int arch_read_trylock(arch_rwlock_t *rwlock);
/**
* arch_write_trylock() - try to acquire a write lock.
*/
-static inline int arch_write_trylock(arch_rwlock_t *rwlock)
-{
- u32 val = __insn_tns((int *)&rwlock->lock);
-
- /*
- * If a tns is in progress, or there's a waiting or active locker,
- * or active readers, we can't take the lock, so give up.
- */
- if (unlikely(val != 0)) {
- if (!(val & 1))
- rwlock->lock = val;
- return 0;
- }
-
- /* Set the "next" field to mark it locked. */
- rwlock->lock = 1 << _WR_NEXT_SHIFT;
- return 1;
-}
+int arch_write_trylock(arch_rwlock_t *rwlock);
/**
* arch_read_unlock() - release a read lock.
*/
-static inline void arch_read_unlock(arch_rwlock_t *rwlock)
-{
- u32 val;
- mb(); /* guarantee anything modified under the lock is visible */
- val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val & 1)) {
- arch_read_unlock_slow(rwlock);
- return;
- }
- rwlock->lock = val - (1 << _RD_COUNT_SHIFT);
-}
+void arch_read_unlock(arch_rwlock_t *rwlock);
/**
* arch_write_unlock() - release a write lock.
*/
-static inline void arch_write_unlock(arch_rwlock_t *rwlock)
-{
- u32 val;
- mb(); /* guarantee anything modified under the lock is visible */
- val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val != (1 << _WR_NEXT_SHIFT))) {
- arch_write_unlock_slow(rwlock, val);
- return;
- }
- rwlock->lock = 0;
-}
+void arch_write_unlock(arch_rwlock_t *rwlock);
#define arch_read_lock_flags(lock, flags) arch_read_lock(lock)
#define arch_write_lock_flags(lock, flags) arch_write_lock(lock)
diff --git a/arch/tile/lib/spinlock_32.c b/arch/tile/lib/spinlock_32.c
index 5cd1c40..723e789 100644
--- a/arch/tile/lib/spinlock_32.c
+++ b/arch/tile/lib/spinlock_32.c
@@ -91,75 +91,82 @@ EXPORT_SYMBOL(arch_spin_unlock_wait);
#define RD_COUNT_MASK ((1 << RD_COUNT_WIDTH) - 1)
-/* Lock the word, spinning until there are no tns-ers. */
-static inline u32 get_rwlock(arch_rwlock_t *rwlock)
+/*
+ * We spin until everything but the reader bits (which are in the high
+ * part of the word) are zero, i.e. no active or waiting writers, no tns.
+ *
+ * We guard the tns/store-back with an interrupt critical section to
+ * preserve the semantic that the same read lock can be acquired in an
+ * interrupt context.
+ *
+ * ISSUE: This approach can permanently starve readers. A reader who sees
+ * a writer could instead take a ticket lock (just like a writer would),
+ * and atomically enter read mode (with 1 reader) when it gets the ticket.
+ * This way both readers and writers will always make forward progress
+ * in a finite time.
+ */
+void arch_read_lock(arch_rwlock_t *rwlock)
{
- u32 iterations = 0;
+ u32 val, iterations = 0;
+
for (;;) {
- u32 val = __insn_tns((int *)&rwlock->lock);
- if (unlikely(val & 1)) {
- delay_backoff(iterations++);
- continue;
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 1);
+ val = __insn_tns((int *)&rwlock->lock);
+ if (likely((val << _RD_COUNT_WIDTH) == 0)) {
+ rwlock->lock = val + (1 << RD_COUNT_SHIFT);
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
+ break;
}
- return val;
+ if ((val & 1) == 0)
+ rwlock->lock = val;
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
+ delay_backoff(iterations++);
}
}
+EXPORT_SYMBOL(arch_read_lock);
-int arch_read_trylock_slow(arch_rwlock_t *rwlock)
+int arch_read_trylock(arch_rwlock_t *rwlock)
{
- u32 val = get_rwlock(rwlock);
- int locked = (val << RD_COUNT_WIDTH) == 0;
- rwlock->lock = val + (locked << RD_COUNT_SHIFT);
+ u32 val;
+ int locked;
+
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 1);
+ val = __insn_tns((int *)&rwlock->lock);
+ locked = ((val << _RD_COUNT_WIDTH) == 0);
+ if (likely(locked))
+ rwlock->lock = val + (1 << RD_COUNT_SHIFT);
+ else if ((val & 1) == 0)
+ rwlock->lock = val;
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
return locked;
}
-EXPORT_SYMBOL(arch_read_trylock_slow);
+EXPORT_SYMBOL(arch_read_trylock);
-void arch_read_unlock_slow(arch_rwlock_t *rwlock)
+void arch_read_unlock(arch_rwlock_t *rwlock)
{
- u32 val = get_rwlock(rwlock);
- rwlock->lock = val - (1 << RD_COUNT_SHIFT);
-}
-EXPORT_SYMBOL(arch_read_unlock_slow);
+ u32 val, iterations = 0;
-void arch_write_unlock_slow(arch_rwlock_t *rwlock, u32 val)
-{
- u32 eq, mask = 1 << WR_CURR_SHIFT;
- while (unlikely(val & 1)) {
- /* Limited backoff since we are the highest-priority task. */
- relax(4);
+ mb(); /* guarantee anything modified under the lock is visible */
+ for (;;) {
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 1);
val = __insn_tns((int *)&rwlock->lock);
+ if (likely(val & 1) == 0) {
+ rwlock->lock = val - (1 << _RD_COUNT_SHIFT);
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
+ break;
+ }
+ __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
+ delay_backoff(iterations++);
}
- val = __insn_addb(val, mask);
- eq = __insn_seqb(val, val << (WR_CURR_SHIFT - WR_NEXT_SHIFT));
- val = __insn_mz(eq & mask, val);
- rwlock->lock = val;
}
-EXPORT_SYMBOL(arch_write_unlock_slow);
+EXPORT_SYMBOL(arch_read_unlock);
/*
- * We spin until everything but the reader bits (which are in the high
- * part of the word) are zero, i.e. no active or waiting writers, no tns.
- *
- * ISSUE: This approach can permanently starve readers. A reader who sees
- * a writer could instead take a ticket lock (just like a writer would),
- * and atomically enter read mode (with 1 reader) when it gets the ticket.
- * This way both readers and writers will always make forward progress
- * in a finite time.
+ * We don't need an interrupt critical section here (unlike for
+ * arch_read_lock) since we should never use a bare write lock where
+ * it could be interrupted by code that could try to re-acquire it.
*/
-void arch_read_lock_slow(arch_rwlock_t *rwlock, u32 val)
-{
- u32 iterations = 0;
- do {
- if (!(val & 1))
- rwlock->lock = val;
- delay_backoff(iterations++);
- val = __insn_tns((int *)&rwlock->lock);
- } while ((val << RD_COUNT_WIDTH) != 0);
- rwlock->lock = val + (1 << RD_COUNT_SHIFT);
-}
-EXPORT_SYMBOL(arch_read_lock_slow);
-
-void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
+void arch_write_lock(arch_rwlock_t *rwlock)
{
/*
* The trailing underscore on this variable (and curr_ below)
@@ -168,6 +175,12 @@ void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
*/
u32 my_ticket_;
u32 iterations = 0;
+ u32 val = __insn_tns((int *)&rwlock->lock);
+
+ if (likely(val == 0)) {
+ rwlock->lock = 1 << _WR_NEXT_SHIFT;
+ return;
+ }
/*
* Wait until there are no readers, then bump up the next
@@ -206,23 +219,47 @@ void arch_write_lock_slow(arch_rwlock_t *rwlock, u32 val)
relax(4);
}
}
-EXPORT_SYMBOL(arch_write_lock_slow);
+EXPORT_SYMBOL(arch_write_lock);
-int __tns_atomic_acquire(atomic_t *lock)
+int arch_write_trylock(arch_rwlock_t *rwlock)
{
- int ret;
- u32 iterations = 0;
+ u32 val = __insn_tns((int *)&rwlock->lock);
- BUG_ON(__insn_mfspr(SPR_INTERRUPT_CRITICAL_SECTION));
- __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 1);
+ /*
+ * If a tns is in progress, or there's a waiting or active locker,
+ * or active readers, we can't take the lock, so give up.
+ */
+ if (unlikely(val != 0)) {
+ if (!(val & 1))
+ rwlock->lock = val;
+ return 0;
+ }
- while ((ret = __insn_tns((void *)&lock->counter)) == 1)
- delay_backoff(iterations++);
- return ret;
+ /* Set the "next" field to mark it locked. */
+ rwlock->lock = 1 << _WR_NEXT_SHIFT;
+ return 1;
}
+EXPORT_SYMBOL(arch_write_trylock);
-void __tns_atomic_release(atomic_t *p, int v)
+void arch_write_unlock(arch_rwlock_t *rwlock)
{
- p->counter = v;
- __insn_mtspr(SPR_INTERRUPT_CRITICAL_SECTION, 0);
+ u32 val, eq, mask;
+
+ mb(); /* guarantee anything modified under the lock is visible */
+ val = __insn_tns((int *)&rwlock->lock);
+ if (likely(val == (1 << _WR_NEXT_SHIFT))) {
+ rwlock->lock = 0;
+ return;
+ }
+ while (unlikely(val & 1)) {
+ /* Limited backoff since we are the highest-priority task. */
+ relax(4);
+ val = __insn_tns((int *)&rwlock->lock);
+ }
+ mask = 1 << WR_CURR_SHIFT;
+ val = __insn_addb(val, mask);
+ eq = __insn_seqb(val, val << (WR_CURR_SHIFT - WR_NEXT_SHIFT));
+ val = __insn_mz(eq & mask, val);
+ rwlock->lock = val;
}
+EXPORT_SYMBOL(arch_write_unlock);
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH net-2.6] bonding: drop frames received with master's source MAC
From: Andy Gospodarek @ 2011-03-01 18:16 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Andy Gospodarek, Nicolas de =?iso-8859-1?Q?Peslo=FCan?=, netdev,
David Miller, Herbert Xu, Jiri Pirko
In-Reply-To: <9882.1298958366@death>
On Mon, Feb 28, 2011 at 09:46:06PM -0800, Jay Vosburgh wrote:
> Andy Gospodarek <andy@greyhouse.net> wrote:
>
> >On Mon, Feb 28, 2011 at 10:45:08PM +0100, Nicolas de Pesloüan wrote:
> >> Le 28/02/2011 17:32, Andy Gospodarek a écrit :
> >>> On Sat, Feb 26, 2011 at 12:08:03AM +0100, Nicolas de Pesloüan wrote:
> >>>> Le 25/02/2011 23:24, Andy Gospodarek a écrit :
> >>> [...]
> >>>>>
> >>>>> I confirmed your suspicion, this breaks ARP monitoring. I would still
> >>>>> welcome other opinions though as I think it would be nice to fix this as
> >>>>> low as possible.
> >>>>
> >>>> Why do you want to fix it earlier that in ndisc_recv_ns drop? Your
> >>>> original idea of silently dropping the frame there seems perfect to me.
> >>>>
> >>>
> >>> Maybe it's just me, but I cannot understand why we want a bunch of extra
> >>> packets floating up into the stack when they may only create issues for
> >>> the recipients of these duplicate frames.
> >>>
> >>> Clearly my original patch needs to be refined so ARP monitoring still
> >>> works, but I would rather fix the issue there than in a higher layer.
> >>
> >> Jay explained that the current implementation should already trap those
> >> frames, on inactive slaves, in modes where inactive slaves exist. I agree
> >> with him.
> >>
> >> What mode are you seeing this problem in? If the current "should drop"
> >> logic is leaking, then yes, we should fix it. But we currently don't see
> >> where it is leaking.
> >>
> >
> >Use round-robin. To reproduce just take the interface down and bring it
> >back up. You should see the messages right away.
>
> What is the bond connected to? The -rr and -xor modes are meant
> to interoperate with switch ports configured for Etherchannel (or an
> equivalent). In that case, I wouldn't expect the switch to turn the
> broadcasts / multicasts around and send them back out to a member of the
> channel group they originated from.
>
> If the switch isn't configured properly, then I'd expect it to
> complain about MAC flapping or the like. Unless it's an unmanaged
> switch that doesn't do Etherchannel (etc), or you're setting up an
> unusual topology.
>
My system is connected to an unmanaged switch, but I guess I don't
generally consider that to be an unual topology. While balance-rr does
work well when the switch is configured for etherchannel, it can also
work just fine without.
> >I've been doing some more testing on a new patch and should have
> >something ready tomorrow. My latest patch actually replaces the final
> >'return 0' with a call to a function that will return true if the sender
> >was the device that received it. This will hopefully prevent some of
> >the failures with the first patch I posted. I'll know a bit more
> >tomorrow if this approach seems reasonable.
>
> How do you figure that out? In -rr mode, all of the slaves
> should have the same MAC address, and the slaves shouldn't be running
> IPv6 addrconf separately from the master anyway. Something magic just
> for NA/NS packets?
>
Knowing that I'm using an unmanaged switch with balance-rr probably
helps understand how this is happening. I'll clarify this however, so
we are all on the same page.
In my situation, eth2 and eth3 are in bond0. When bond0 transmits the
NS, let's say it goes out eth3. Since it is a multicast frame my switch
will broadcast this to all ports and eth2 will receive the frame with
the source MAC address being the same as bond0's MAC address. This
frame is passed up the stack to the ipv6 layer and appears to be a
response to the NS from another host and is dropped.
^ permalink raw reply
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding
From: Vasu Dev @ 2011-03-01 18:09 UTC (permalink / raw)
To: Joe Eykholt
Cc: linux-scsi, Jiri Pirko, netdev, Jay Vosburgh, James.Bottomley,
devel
In-Reply-To: <1299002494.7692.202.camel@vi2.jf.intel.com>
On Tue, 2011-03-01 at 10:01 -0800, Vasu Dev wrote:
> On Tue, 2011-03-01 at 09:49 -0800, Joe Eykholt wrote:
> >
> > Otherwise I'd add some parens or I might code this as:
> >
> > if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) ==
> > (IFF_BONDING | IFF_MASTER))
>
> or also skip == in code as:-
>
> if (netdev->priv_flags & (IFF_BONDING | IFF_MASTER))
>
> Would least save few chars in source code.
Never mind, you wanted to check both bits set instead checking only one
of them, my mistake, anycase these are in different fields as another
most recent response mentioned.
Vasu
^ permalink raw reply
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding
From: Jay Vosburgh @ 2011-03-01 18:06 UTC (permalink / raw)
To: Joe Eykholt; +Cc: Jiri Pirko, James.Bottomley, netdev, devel, linux-scsi
In-Reply-To: <4D6D31BA.3000105@gmail.com>
Joe Eykholt <joe.eykholt@gmail.com> wrote:
>On 2/28/11 10:37 PM, Jiri Pirko wrote:
>> Mon, Feb 28, 2011 at 06:54:29PM CET, joe.eykholt@gmail.com wrote:
>>> On 2/28/11 9:15 AM, Jay Vosburgh wrote:
>>>> Jiri Pirko<jpirko@redhat.com> wrote:
>>>>
>>>>> Check for IFF_BONDING as this flag is set-up for all bonding devices.
>>>>>
>>>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>>>>> ---
>>>>> drivers/scsi/fcoe/fcoe.c | 4 +---
>>>>> 1 files changed, 1 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
>>>>> index 9f9600b..67714a4 100644
>>>>> --- a/drivers/scsi/fcoe/fcoe.c
>>>>> +++ b/drivers/scsi/fcoe/fcoe.c
>>>>> @@ -285,9 +285,7 @@ static int fcoe_interface_setup(struct fcoe_interface *fcoe,
>>>>> }
>>>>>
>>>>> /* Do not support for bonding device */
>>>>> - if ((netdev->priv_flags& IFF_MASTER_ALB) ||
>>>>> - (netdev->priv_flags& IFF_SLAVE_INACTIVE) ||
>>>>> - (netdev->priv_flags& IFF_MASTER_8023AD)) {
>>>>> + if (netdev->priv_flags& IFF_BONDING) {
>>>>> FCOE_NETDEV_DBG(netdev, "Bonded interfaces not supported\n");
>>>>> return -EOPNOTSUPP;
>>>>> }
>>>>
>>>> Based on past discussions, I believe the intent of the code is
>>>> to permit FCOE over bonding only for active-backup mode, and possibly
>>>> for -xor/-rr as well.
>>>>
>>>> I'm not sure if the slave or the master is what's being tested
>>>> here, so I'm not sure what the right thing to do is. I suspect it's the
>>>> master, as I recall discussion of one configuration involving
>>>> active-backup mode balancing FCOE traffic over both the active and
>>>> inactive slaves. FCOE uses the "orig_dev" logic in __netif_receive_skb
>>>> to have the packets delivered even on the nominally inactive slave.
>>>>
>>>> -J
>>>>
>>>> ---
>>>> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>>>
>>> Right. That was the intent. It should work on the physical dev, but probably
>>> not on the master of the bond.
>>>
>>> If you have a master/slave bond for IPv4 between eth1 and eth2, say,
>>> and they are going to two different DCE (FCoE) switches, presumably on
>>> different VSANs but with ultimate access to the same disks,
>>> then you want to split the FCoE traffic in active/active
>>> mode using separate FCoE instances on eth1 and eth2 even though IP
>>> is using active/standby on bond0. This should work. But, putting fcoe
>>> on bond0 isn't going to do what you want.
>>>
>>> However, it seems like the check above shouldn't be checking
>>> IFF_SLAVE_INACTIVE. I can't test this.
>>
>> OK. So I guess the right check should be for:
>> (netdev->priv_flags& IFF_BONDING&& netdev->flags& IFF_MASTER)
>
>I think that's OK. How about just checking for MASTER?
>When is MASTER going to be set without BONDING?
One or two other things besides bonding use IFF_MASTER, but
IFF_BONDING is only for bonding.
>Otherwise I'd add some parens or I might code this as:
>
> if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) ==
> (IFF_BONDING | IFF_MASTER))
This doesn't work because the flags are kept in different
places, IFF_MASTER is in flags and IFF_BONDING in priv_flags.
-J
>Which is less clear, I know, but used to generate better code.
>The compiler might generate the same code these days.
>Not that this is performance-critical or anything.
>
>> This would disable adding all bond devices (like bond0 etc) and allows
>> to use enslaved physdevs.
>>
>> Note that checking for mode is irrelevant here. Mode could be easily
>> changed later without fcoe knowing that.
This is also true.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [Open-FCoE] [PATCH] fcoe: correct checking for bonding
From: Vasu Dev @ 2011-03-01 18:01 UTC (permalink / raw)
To: Joe Eykholt
Cc: Jiri Pirko, James.Bottomley, netdev, Jay Vosburgh, devel,
linux-scsi
In-Reply-To: <4D6D31BA.3000105@gmail.com>
On Tue, 2011-03-01 at 09:49 -0800, Joe Eykholt wrote:
>
> Otherwise I'd add some parens or I might code this as:
>
> if ((netdev->priv_flags & (IFF_BONDING | IFF_MASTER)) ==
> (IFF_BONDING | IFF_MASTER))
or also skip == in code as:-
if (netdev->priv_flags & (IFF_BONDING | IFF_MASTER))
Would least save few chars in source code.
Vasu
^ permalink raw reply
* [PATCH] tile on-chip network driver: sync up with latest fixes
From: Chris Metcalf @ 2011-03-01 17:49 UTC (permalink / raw)
To: linux-kernel, netdev
Combine the "command" and "completion" locks into a single lock,
on each egress queue, to improve efficiency.
Support the use of 4KB pages in the "egress queue".
Delete the unused "duplicate ACK compression" code.
Filter "bad" (i.e. truncated) packets.
Avoid corrupting "dev->napi_list", by sequentializing modifications.
Deregister for incoming packets during stop, to reduce unexpected
interrupts. Also, encourage active NAPI loops to complete before
we disable NAPI, which would otherwise crash.
Free any pending completions after shutting down LEPP.
Use a single, permanently registered, IRQ handler, to avoid situations
in which the IRQ handler was firing after being freed, and ignore any
"unexpected" interrupts.
Drop egress packets, instead of spinning, if the hardware cannot keep
up, or is disconnected.
Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>
---
drivers/net/tile/tilepro.c | 867 +++++++++++++++++++++++---------------------
1 files changed, 457 insertions(+), 410 deletions(-)
diff --git a/drivers/net/tile/tilepro.c b/drivers/net/tile/tilepro.c
index f901299..eb1b825 100644
--- a/drivers/net/tile/tilepro.c
+++ b/drivers/net/tile/tilepro.c
@@ -44,10 +44,6 @@
#include <linux/tcp.h>
-/* There is no singlethread_cpu, so schedule work on the current cpu. */
-#define singlethread_cpu -1
-
-
/*
* First, "tile_net_init_module()" initializes all four "devices" which
* can be used by linux.
@@ -73,15 +69,16 @@
* return, knowing we will be called again later. Otherwise, we
* reenable the ingress interrupt, and call "napi_complete()".
*
+ * HACK: Since disabling the ingress interrupt is not reliable, we
+ * ignore the interrupt if the global "active" flag is false.
+ *
*
* NOTE: The use of "native_driver" ensures that EPP exists, and that
- * "epp_sendv" is legal, and that "LIPP" is being used.
+ * we are using "LIPP" and "LEPP".
*
* NOTE: Failing to free completions for an arbitrarily long time
* (which is defined to be illegal) does in fact cause bizarre
* problems. The "egress_timer" helps prevent this from happening.
- *
- * NOTE: The egress code can be interrupted by the interrupt handler.
*/
@@ -142,6 +139,7 @@
MODULE_AUTHOR("Tilera");
MODULE_LICENSE("GPL");
+
/*
* Queue of incoming packets for a specific cpu and device.
*
@@ -177,7 +175,7 @@ struct tile_net_cpu {
struct tile_netio_queue queue;
/* Statistics. */
struct tile_net_stats_t stats;
- /* ISSUE: Is this needed? */
+ /* True iff NAPI is enabled. */
bool napi_enabled;
/* True if this tile has succcessfully registered with the IPP. */
bool registered;
@@ -200,20 +198,20 @@ struct tile_net_cpu {
struct tile_net_priv {
/* Our network device. */
struct net_device *dev;
- /* The actual egress queue. */
- lepp_queue_t *epp_queue;
- /* Protects "epp_queue->cmd_tail" and "epp_queue->comp_tail" */
- spinlock_t cmd_lock;
- /* Protects "epp_queue->comp_head". */
- spinlock_t comp_lock;
+ /* Pages making up the egress queue. */
+ struct page *eq_pages;
+ /* Address of the actual egress queue. */
+ lepp_queue_t *eq;
+ /* Protects "eq". */
+ spinlock_t eq_lock;
/* The hypervisor handle for this interface. */
int hv_devhdl;
/* The intr bit mask that IDs this device. */
u32 intr_id;
/* True iff "tile_net_open_aux()" has succeeded. */
- int partly_opened;
- /* True iff "tile_net_open_inner()" has succeeded. */
- int fully_opened;
+ bool partly_opened;
+ /* True iff the device is "active". */
+ bool active;
/* Effective network cpus. */
struct cpumask network_cpus_map;
/* Number of network cpus. */
@@ -228,6 +226,10 @@ struct tile_net_priv {
struct tile_net_cpu *cpu[NR_CPUS];
};
+/* Log2 of the number of small pages needed for the egress queue. */
+#define EQ_ORDER get_order(sizeof(lepp_queue_t))
+/* Size of the egress queue's pages. */
+#define EQ_SIZE (1 << (PAGE_SHIFT + EQ_ORDER))
/*
* The actual devices (xgbe0, xgbe1, gbe0, gbe1).
@@ -284,7 +286,11 @@ static void net_printk(char *fmt, ...)
*/
static void dump_packet(unsigned char *data, unsigned long length, char *s)
{
+ int my_cpu = smp_processor_id();
+
unsigned long i;
+ char buf[128];
+
static unsigned int count;
pr_info("dump_packet(data %p, length 0x%lx s %s count 0x%x)\n",
@@ -294,10 +300,12 @@ static void dump_packet(unsigned char *data, unsigned long length, char *s)
for (i = 0; i < length; i++) {
if ((i & 0xf) == 0)
- sprintf(buf, "%8.8lx:", i);
+ sprintf(buf, "[%02d] %8.8lx:", my_cpu, i);
sprintf(buf + strlen(buf), " %2.2x", data[i]);
- if ((i & 0xf) == 0xf || i == length - 1)
- pr_info("%s\n", buf);
+ if ((i & 0xf) == 0xf || i == length - 1) {
+ strcat(buf, "\n");
+ pr_info("%s", buf);
+ }
}
}
#endif
@@ -356,9 +364,11 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
bool small)
{
/* ISSUE: What should we use here? */
+ /* Without "jumbo", 2 + 1536 should be sufficient. */
unsigned int large_size = NET_IP_ALIGN + TILE_NET_MTU + 100;
- /* Round up to ensure to avoid "false sharing" with last cache line. */
+ /* Avoid "false sharing" with last cache line. */
+ /* FIXME: This is already done by "dev_alloc_skb()". */
unsigned int buffer_size =
(((small ? LIPP_SMALL_PACKET_SIZE : large_size) +
CHIP_L2_LINE_SIZE() - 1) & -CHIP_L2_LINE_SIZE());
@@ -367,6 +377,12 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
* ISSUE: Since CPAs are 38 bits, and we can only encode the
* high 31 bits in a "linux_buffer_t", the low 7 bits must be
* zero, and thus, we must align the actual "va" mod 128.
+ *
+ * FIXME: We could change the underlying code to assume that
+ * the low SIX bits are zero, and the 7th bit is one, and then
+ * we would need to align mod 128 plus 64, which would be what
+ * linux gives us anyway, in theory, and then we could make
+ * LIPP_SMALL_PACKET_SIZE be 64, or maybe 196.
*/
const unsigned long align = 128;
@@ -375,11 +391,15 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
struct sk_buff **skb_ptr;
- /* Note that "dev_alloc_skb()" adds NET_SKB_PAD more bytes, */
+ /* Note that "dev_alloc_skb()" adds NET_SKB_PAD (32) more bytes, */
/* and also "reserves" that many bytes. */
- /* ISSUE: Can we "share" the NET_SKB_PAD bytes with "skb_ptr"? */
+ /* ISSUE: We could "share" the NET_SKB_PAD bytes with "skb_ptr", */
+ /* but currently there is no real need, as NET_SKB_PAD is only 32. */
int len = sizeof(*skb_ptr) + align + buffer_size;
+ /* NOTE: Another 192 (256?) bytes may be used by "skb_shared_info" */
+ /* stored after the bytes. This explains many things. */
+
while (1) {
/* Allocate (or fail). */
@@ -399,13 +419,24 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
if (small)
break;
- /* ISSUE: This has never been observed! */
/* Large buffers must not span a huge page. */
if (((((long)va & ~HPAGE_MASK) + 1535) & HPAGE_MASK) == 0)
break;
+ /* ISSUE: This appears to be "impossible". */
pr_err("Leaking unaligned linux buffer at %p.\n", va);
}
+#ifdef TILE_NET_PARANOIA
+#if CHIP_HAS_CBOX_HOME_MAP()
+ if (hash_default) {
+ HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)va);
+ if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
+ panic("Non-HFH ingress buffer! VA=%p Mode=%d PTE=%llx",
+ va, hv_pte_get_mode(pte), hv_pte_val(pte));
+ }
+#endif
+#endif
+
/* Skip two bytes to satisfy LIPP assumptions. */
/* Note that this aligns IP on a 16 byte boundary. */
/* ISSUE: Do this when the packet arrives? */
@@ -422,16 +453,6 @@ static bool tile_net_provide_needed_buffer(struct tile_net_cpu *info,
/* Make sure "skb_ptr" has been flushed. */
__insn_mf();
-#ifdef TILE_NET_PARANOIA
-#if CHIP_HAS_CBOX_HOME_MAP()
- if (hash_default) {
- HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)va);
- if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
- panic("Non-coherent ingress buffer!");
- }
-#endif
-#endif
-
/* Provide the new buffer. */
tile_net_provide_linux_buffer(info, va, small);
@@ -469,48 +490,64 @@ oops:
* Grab some LEPP completions, and store them in "comps", of size
* "comps_size", and return the number of completions which were
* stored, so the caller can free them.
- *
- * If "pending" is not NULL, it will be set to true if there might
- * still be some pending completions caused by this tile, else false.
*/
-static unsigned int tile_net_lepp_grab_comps(struct net_device *dev,
+static unsigned int tile_net_lepp_grab_comps(lepp_queue_t *eq,
struct sk_buff *comps[],
unsigned int comps_size,
- bool *pending)
+ unsigned int min_size)
{
- struct tile_net_priv *priv = netdev_priv(dev);
-
- lepp_queue_t *eq = priv->epp_queue;
-
unsigned int n = 0;
- unsigned int comp_head;
- unsigned int comp_busy;
- unsigned int comp_tail;
-
- spin_lock(&priv->comp_lock);
-
- comp_head = eq->comp_head;
- comp_busy = eq->comp_busy;
- comp_tail = eq->comp_tail;
+ unsigned int comp_head = eq->comp_head;
+ unsigned int comp_busy = eq->comp_busy;
while (comp_head != comp_busy && n < comps_size) {
comps[n++] = eq->comps[comp_head];
LEPP_QINC(comp_head);
}
- if (pending != NULL)
- *pending = (comp_head != comp_tail);
+ if (n < min_size)
+ return 0;
eq->comp_head = comp_head;
- spin_unlock(&priv->comp_lock);
-
return n;
}
/*
+ * Free some comps, and return true iff there are still some pending.
+ */
+static bool tile_net_lepp_free_comps(struct net_device *dev, bool all)
+{
+ struct tile_net_priv *priv = netdev_priv(dev);
+
+ lepp_queue_t *eq = priv->eq;
+
+ struct sk_buff *olds[64];
+ unsigned int wanted = 64;
+ unsigned int i, n;
+ bool pending;
+
+ spin_lock(&priv->eq_lock);
+
+ if (all)
+ eq->comp_busy = eq->comp_tail;
+
+ n = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+
+ pending = (eq->comp_head != eq->comp_tail);
+
+ spin_unlock(&priv->eq_lock);
+
+ for (i = 0; i < n; i++)
+ kfree_skb(olds[i]);
+
+ return pending;
+}
+
+
+/*
* Make sure the egress timer is scheduled.
*
* Note that we use "schedule if not scheduled" logic instead of the more
@@ -544,21 +581,11 @@ static void tile_net_handle_egress_timer(unsigned long arg)
struct tile_net_cpu *info = (struct tile_net_cpu *)arg;
struct net_device *dev = info->napi.dev;
- struct sk_buff *olds[32];
- unsigned int wanted = 32;
- unsigned int i, nolds = 0;
- bool pending;
-
/* The timer is no longer scheduled. */
info->egress_timer_scheduled = false;
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, &pending);
-
- for (i = 0; i < nolds; i++)
- kfree_skb(olds[i]);
-
- /* Reschedule timer if needed. */
- if (pending)
+ /* Free comps, and reschedule timer if more are pending. */
+ if (tile_net_lepp_free_comps(dev, false))
tile_net_schedule_egress_timer(info);
}
@@ -636,8 +663,39 @@ static bool is_dup_ack(char *s1, char *s2, unsigned int len)
+static void tile_net_discard_aux(struct tile_net_cpu *info, int index)
+{
+ struct tile_netio_queue *queue = &info->queue;
+ netio_queue_impl_t *qsp = queue->__system_part;
+ netio_queue_user_impl_t *qup = &queue->__user_part;
+
+ int index2_aux = index + sizeof(netio_pkt_t);
+ int index2 =
+ ((index2_aux ==
+ qsp->__packet_receive_queue.__last_packet_plus_one) ?
+ 0 : index2_aux);
+
+ netio_pkt_t *pkt = (netio_pkt_t *)((unsigned long) &qsp[1] + index);
+
+ /* Extract the "linux_buffer_t". */
+ unsigned int buffer = pkt->__packet.word;
+
+ /* Convert "linux_buffer_t" to "va". */
+ void *va = __va((phys_addr_t)(buffer >> 1) << 7);
+
+ /* Acquire the associated "skb". */
+ struct sk_buff **skb_ptr = va - sizeof(*skb_ptr);
+ struct sk_buff *skb = *skb_ptr;
+
+ kfree_skb(skb);
+
+ /* Consume this packet. */
+ qup->__packet_receive_read = index2;
+}
+
+
/*
- * Like "tile_net_handle_packets()", but just discard packets.
+ * Like "tile_net_poll()", but just discard packets.
*/
static void tile_net_discard_packets(struct net_device *dev)
{
@@ -650,32 +708,8 @@ static void tile_net_discard_packets(struct net_device *dev)
while (qup->__packet_receive_read !=
qsp->__packet_receive_queue.__packet_write) {
-
int index = qup->__packet_receive_read;
-
- int index2_aux = index + sizeof(netio_pkt_t);
- int index2 =
- ((index2_aux ==
- qsp->__packet_receive_queue.__last_packet_plus_one) ?
- 0 : index2_aux);
-
- netio_pkt_t *pkt = (netio_pkt_t *)
- ((unsigned long) &qsp[1] + index);
-
- /* Extract the "linux_buffer_t". */
- unsigned int buffer = pkt->__packet.word;
-
- /* Convert "linux_buffer_t" to "va". */
- void *va = __va((phys_addr_t)(buffer >> 1) << 7);
-
- /* Acquire the associated "skb". */
- struct sk_buff **skb_ptr = va - sizeof(*skb_ptr);
- struct sk_buff *skb = *skb_ptr;
-
- kfree_skb(skb);
-
- /* Consume this packet. */
- qup->__packet_receive_read = index2;
+ tile_net_discard_aux(info, index);
}
}
@@ -704,7 +738,8 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
netio_pkt_metadata_t *metadata = NETIO_PKT_METADATA(pkt);
- /* Extract the packet size. */
+ /* Extract the packet size. FIXME: Shouldn't the second line */
+ /* get subtracted? Mostly moot, since it should be "zero". */
unsigned long len =
(NETIO_PKT_CUSTOM_LENGTH(pkt) +
NET_IP_ALIGN - NETIO_PACKET_PADDING);
@@ -722,15 +757,6 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/* Compare to "NETIO_PKT_CUSTOM_DATA(pkt)". */
unsigned char *buf = va + NET_IP_ALIGN;
-#ifdef IGNORE_DUP_ACKS
-
- static int other;
- static int final;
- static int keep;
- static int skip;
-
-#endif
-
/* Invalidate the packet buffer. */
if (!hash_default)
__inv_buffer(buf, len);
@@ -745,16 +771,8 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
#ifdef TILE_NET_VERIFY_INGRESS
if (!NETIO_PKT_L4_CSUM_CORRECT_M(metadata, pkt) &&
NETIO_PKT_L4_CSUM_CALCULATED_M(metadata, pkt)) {
- /*
- * FIXME: This complains about UDP packets
- * with a "zero" checksum (bug 6624).
- */
-#ifdef TILE_NET_PANIC_ON_BAD
- dump_packet(buf, len, "rx");
- panic("Bad L4 checksum.");
-#else
+ /* Bug 6624: Includes UDP packets with a "zero" checksum. */
pr_warning("Bad L4 checksum on %d byte packet.\n", len);
-#endif
}
if (!NETIO_PKT_L3_CSUM_CORRECT_M(metadata, pkt) &&
NETIO_PKT_L3_CSUM_CALCULATED_M(metadata, pkt)) {
@@ -769,24 +787,22 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
}
break;
case NETIO_PKT_STATUS_BAD:
-#ifdef TILE_NET_PANIC_ON_BAD
- dump_packet(buf, len, "rx");
- panic("Unexpected BAD packet.");
-#else
- pr_warning("Unexpected BAD %d byte packet.\n", len);
-#endif
+ pr_warning("Unexpected BAD %ld byte packet.\n", len);
}
#endif
filter = 0;
+ /* ISSUE: Filter TCP packets with "bad" checksums? */
+
if (!(dev->flags & IFF_UP)) {
/* Filter packets received before we're up. */
filter = 1;
+ } else if (NETIO_PKT_STATUS_M(metadata, pkt) == NETIO_PKT_STATUS_BAD) {
+ /* Filter "truncated" packets. */
+ filter = 1;
} else if (!(dev->flags & IFF_PROMISC)) {
- /*
- * FIXME: Implement HW multicast filter.
- */
+ /* FIXME: Implement HW multicast filter. */
if (is_unicast_ether_addr(buf)) {
/* Filter packets not for our address. */
const u8 *mine = dev->dev_addr;
@@ -794,65 +810,6 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
}
}
-#ifdef IGNORE_DUP_ACKS
-
- if (len != 66) {
- /* FIXME: Must check "is_tcp_ack(buf, len)" somehow. */
-
- other++;
-
- } else if (index2 ==
- qsp->__packet_receive_queue.__packet_write) {
-
- final++;
-
- } else {
-
- netio_pkt_t *pkt2 = (netio_pkt_t *)
- ((unsigned long) &qsp[1] + index2);
-
- netio_pkt_metadata_t *metadata2 =
- NETIO_PKT_METADATA(pkt2);
-
- /* Extract the packet size. */
- unsigned long len2 =
- (NETIO_PKT_CUSTOM_LENGTH(pkt2) +
- NET_IP_ALIGN - NETIO_PACKET_PADDING);
-
- if (len2 == 66 &&
- NETIO_PKT_FLOW_HASH_M(metadata, pkt) ==
- NETIO_PKT_FLOW_HASH_M(metadata2, pkt2)) {
-
- /* Extract the "linux_buffer_t". */
- unsigned int buffer2 = pkt2->__packet.word;
-
- /* Convert "linux_buffer_t" to "va". */
- void *va2 =
- __va((phys_addr_t)(buffer2 >> 1) << 7);
-
- /* Extract the packet data pointer. */
- /* Compare to "NETIO_PKT_CUSTOM_DATA(pkt)". */
- unsigned char *buf2 = va2 + NET_IP_ALIGN;
-
- /* Invalidate the packet buffer. */
- if (!hash_default)
- __inv_buffer(buf2, len2);
-
- if (is_dup_ack(buf, buf2, len)) {
- skip++;
- filter = 1;
- } else {
- keep++;
- }
- }
- }
-
- if (net_ratelimit())
- pr_info("Other %d Final %d Keep %d Skip %d.\n",
- other, final, keep, skip);
-
-#endif
-
if (filter) {
/* ISSUE: Update "drop" statistics? */
@@ -877,10 +834,7 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/* NOTE: This call also sets "skb->dev = dev". */
skb->protocol = eth_type_trans(skb, dev);
- /* ISSUE: Discard corrupt packets? */
- /* ISSUE: Discard packets with bad checksums? */
-
- /* Avoid recomputing TCP/UDP checksums. */
+ /* Avoid recomputing "good" TCP/UDP checksums. */
if (NETIO_PKT_L4_CSUM_CORRECT_M(metadata, pkt))
skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -912,9 +866,14 @@ static bool tile_net_poll_aux(struct tile_net_cpu *info, int index)
/*
* Handle some packets for the given device on the current CPU.
*
- * ISSUE: The "rotting packet" race condition occurs if a packet
- * arrives after the queue appears to be empty, and before the
- * hypervisor interrupt is re-enabled.
+ * If "tile_net_stop()" is called on some other tile while this
+ * function is running, we will return, hopefully before that
+ * other tile asks us to call "napi_disable()".
+ *
+ * The "rotting packet" race condition occurs if a packet arrives
+ * during the extremely narrow window between the queue appearing to
+ * be empty, and the ingress interrupt being re-enabled. This happens
+ * a LOT under heavy network load.
*/
static int tile_net_poll(struct napi_struct *napi, int budget)
{
@@ -928,7 +887,7 @@ static int tile_net_poll(struct napi_struct *napi, int budget)
unsigned int work = 0;
- while (1) {
+ while (priv->active) {
int index = qup->__packet_receive_read;
if (index == qsp->__packet_receive_queue.__packet_write)
break;
@@ -941,19 +900,24 @@ static int tile_net_poll(struct napi_struct *napi, int budget)
napi_complete(&info->napi);
- /* Re-enable hypervisor interrupts. */
+ if (!priv->active)
+ goto done;
+
+ /* Re-enable the ingress interrupt. */
enable_percpu_irq(priv->intr_id);
- /* HACK: Avoid the "rotting packet" problem. */
+ /* HACK: Avoid the "rotting packet" problem (see above). */
if (qup->__packet_receive_read !=
- qsp->__packet_receive_queue.__packet_write)
- napi_schedule(&info->napi);
-
- /* ISSUE: Handle completions? */
+ qsp->__packet_receive_queue.__packet_write) {
+ /* ISSUE: Sometimes this returns zero, presumably */
+ /* because an interrupt was handled for this tile. */
+ (void)napi_reschedule(&info->napi);
+ }
done:
- tile_net_provide_needed_buffers(info);
+ if (priv->active)
+ tile_net_provide_needed_buffers(info);
return work;
}
@@ -961,6 +925,12 @@ done:
/*
* Handle an ingress interrupt for the given device on the current cpu.
+ *
+ * ISSUE: Sometimes this gets called after "disable_percpu_irq()" has
+ * been called! This is probably due to "pending hypervisor downcalls".
+ *
+ * ISSUE: Is there any race condition between the "napi_schedule()" here
+ * and the "napi_complete()" call above?
*/
static irqreturn_t tile_net_handle_ingress_interrupt(int irq, void *dev_ptr)
{
@@ -969,9 +939,15 @@ static irqreturn_t tile_net_handle_ingress_interrupt(int irq, void *dev_ptr)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Disable hypervisor interrupt. */
+ /* Disable the ingress interrupt. */
disable_percpu_irq(priv->intr_id);
+ /* Ignore unwanted interrupts. */
+ if (!priv->active)
+ return IRQ_HANDLED;
+
+ /* ISSUE: Sometimes "info->napi_enabled" is false here. */
+
napi_schedule(&info->napi);
return IRQ_HANDLED;
@@ -1005,8 +981,7 @@ static int tile_net_open_aux(struct net_device *dev)
*/
{
int epp_home = hv_lotar_to_cpu(epp_lotar);
- struct page *page = virt_to_page(priv->epp_queue);
- homecache_change_page_home(page, 0, epp_home);
+ homecache_change_page_home(priv->eq_pages, EQ_ORDER, epp_home);
}
/*
@@ -1015,9 +990,9 @@ static int tile_net_open_aux(struct net_device *dev)
{
netio_ipp_address_t ea = {
.va = 0,
- .pa = __pa(priv->epp_queue),
+ .pa = __pa(priv->eq),
.pte = hv_pte(0),
- .size = PAGE_SIZE,
+ .size = EQ_SIZE,
};
ea.pte = hv_pte_set_lotar(ea.pte, epp_lotar);
ea.pte = hv_pte_set_mode(ea.pte, HV_PTE_MODE_CACHE_TILE_L3);
@@ -1043,7 +1018,7 @@ static int tile_net_open_aux(struct net_device *dev)
/*
- * Register with hypervisor on each CPU.
+ * Register with hypervisor on the current CPU.
*
* Strangely, this function does important things even if it "fails",
* which is especially common if the link is not up yet. Hopefully
@@ -1092,7 +1067,8 @@ static void tile_net_register(void *dev_ptr)
priv->cpu[my_cpu] = info;
/*
- * Register ourselves with the IPP.
+ * Register ourselves with LIPP. This does a lot of stuff,
+ * including invoking the LIPP registration code.
*/
ret = hv_dev_pwrite(priv->hv_devhdl, 0,
(HV_VirtAddr)&config,
@@ -1101,8 +1077,11 @@ static void tile_net_register(void *dev_ptr)
PDEBUG("hv_dev_pwrite(NETIO_IPP_INPUT_REGISTER_OFF) returned %d\n",
ret);
if (ret < 0) {
- printk(KERN_DEBUG "hv_dev_pwrite NETIO_IPP_INPUT_REGISTER_OFF"
- " failure %d\n", ret);
+ if (ret != NETIO_LINK_DOWN) {
+ printk(KERN_DEBUG "hv_dev_pwrite "
+ "NETIO_IPP_INPUT_REGISTER_OFF failure %d\n",
+ ret);
+ }
info->link_down = (ret == NETIO_LINK_DOWN);
return;
}
@@ -1145,15 +1124,47 @@ static void tile_net_register(void *dev_ptr)
NETIO_IPP_GET_FASTIO_OFF);
PDEBUG("hv_dev_pread(NETIO_IPP_GET_FASTIO_OFF) returned %d\n", ret);
- netif_napi_add(dev, &info->napi, tile_net_poll, 64);
-
/* Now we are registered. */
info->registered = true;
}
/*
- * Unregister with hypervisor on each CPU.
+ * Deregister with hypervisor on the current CPU.
+ *
+ * This simply discards all our credits, so no more packets will be
+ * delivered to this tile. There may still be packets in our queue.
+ *
+ * Also, disable the ingress interrupt.
+ */
+static void tile_net_deregister(void *dev_ptr)
+{
+ struct net_device *dev = (struct net_device *)dev_ptr;
+ struct tile_net_priv *priv = netdev_priv(dev);
+ int my_cpu = smp_processor_id();
+ struct tile_net_cpu *info = priv->cpu[my_cpu];
+
+ /* Disable the ingress interrupt. */
+ disable_percpu_irq(priv->intr_id);
+
+ /* Do nothing else if not registered. */
+ if (info == NULL || !info->registered)
+ return;
+
+ {
+ struct tile_netio_queue *queue = &info->queue;
+ netio_queue_user_impl_t *qup = &queue->__user_part;
+
+ /* Discard all our credits. */
+ __netio_fastio_return_credits(qup->__fastio_index, -1);
+ }
+}
+
+
+/*
+ * Unregister with hypervisor on the current CPU.
+ *
+ * Also, disable the ingress interrupt.
*/
static void tile_net_unregister(void *dev_ptr)
{
@@ -1162,35 +1173,23 @@ static void tile_net_unregister(void *dev_ptr)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- int ret = 0;
+ int ret;
int dummy = 0;
- /* Do nothing if never registered. */
- if (info == NULL)
- return;
+ /* Disable the ingress interrupt. */
+ disable_percpu_irq(priv->intr_id);
- /* Do nothing if already unregistered. */
- if (!info->registered)
+ /* Do nothing else if not registered. */
+ if (info == NULL || !info->registered)
return;
- /*
- * Unregister ourselves with LIPP.
- */
+ /* Unregister ourselves with LIPP/LEPP. */
ret = hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
sizeof(dummy), NETIO_IPP_INPUT_UNREGISTER_OFF);
- PDEBUG("hv_dev_pwrite(NETIO_IPP_INPUT_UNREGISTER_OFF) returned %d\n",
- ret);
- if (ret < 0) {
- /* FIXME: Just panic? */
- pr_err("hv_dev_pwrite NETIO_IPP_INPUT_UNREGISTER_OFF"
- " failure %d\n", ret);
- }
+ if (ret < 0)
+ panic("Failed to unregister with LIPP/LEPP!\n");
- /*
- * Discard all packets still in our NetIO queue. Hopefully,
- * once the unregister call is complete, there will be no
- * packets still in flight on the IDN.
- */
+ /* Discard all packets still in our NetIO queue. */
tile_net_discard_packets(dev);
/* Reset state. */
@@ -1200,11 +1199,6 @@ static void tile_net_unregister(void *dev_ptr)
/* Cancel egress timer. */
del_timer(&info->egress_timer);
info->egress_timer_scheduled = false;
-
- netif_napi_del(&info->napi);
-
- /* Now we are unregistered. */
- info->registered = false;
}
@@ -1212,18 +1206,28 @@ static void tile_net_unregister(void *dev_ptr)
* Helper function for "tile_net_stop()".
*
* Also used to handle registration failure in "tile_net_open_inner()",
- * when "fully_opened" is known to be false, and the various extra
- * steps in "tile_net_stop()" are not necessary. ISSUE: It might be
- * simpler if we could just call "tile_net_stop()" anyway.
+ * when the various extra steps in "tile_net_stop()" are not necessary.
*/
static void tile_net_stop_aux(struct net_device *dev)
{
struct tile_net_priv *priv = netdev_priv(dev);
+ int i;
int dummy = 0;
- /* Unregister all tiles, so LIPP will stop delivering packets. */
+ /*
+ * Unregister all tiles, so LIPP will stop delivering packets.
+ * Also, delete all the "napi" objects (sequentially, to protect
+ * "dev->napi_list").
+ */
on_each_cpu(tile_net_unregister, (void *)dev, 1);
+ for_each_online_cpu(i) {
+ struct tile_net_cpu *info = priv->cpu[i];
+ if (info != NULL && info->registered) {
+ netif_napi_del(&info->napi);
+ info->registered = false;
+ }
+ }
/* Stop LIPP/LEPP. */
if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
@@ -1235,18 +1239,15 @@ static void tile_net_stop_aux(struct net_device *dev)
/*
- * Disable ingress interrupts for the given device on the current cpu.
+ * Disable NAPI for the given device on the current cpu.
*/
-static void tile_net_disable_intr(void *dev_ptr)
+static void tile_net_stop_disable(void *dev_ptr)
{
struct net_device *dev = (struct net_device *)dev_ptr;
struct tile_net_priv *priv = netdev_priv(dev);
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Disable hypervisor interrupt. */
- disable_percpu_irq(priv->intr_id);
-
/* Disable NAPI if needed. */
if (info != NULL && info->napi_enabled) {
napi_disable(&info->napi);
@@ -1256,21 +1257,24 @@ static void tile_net_disable_intr(void *dev_ptr)
/*
- * Enable ingress interrupts for the given device on the current cpu.
+ * Enable NAPI and the ingress interrupt for the given device
+ * on the current cpu.
+ *
+ * ISSUE: Only do this for "network cpus"?
*/
-static void tile_net_enable_intr(void *dev_ptr)
+static void tile_net_open_enable(void *dev_ptr)
{
struct net_device *dev = (struct net_device *)dev_ptr;
struct tile_net_priv *priv = netdev_priv(dev);
int my_cpu = smp_processor_id();
struct tile_net_cpu *info = priv->cpu[my_cpu];
- /* Enable hypervisor interrupt. */
- enable_percpu_irq(priv->intr_id);
-
/* Enable NAPI. */
napi_enable(&info->napi);
info->napi_enabled = true;
+
+ /* Enable the ingress interrupt. */
+ enable_percpu_irq(priv->intr_id);
}
@@ -1288,8 +1292,9 @@ static int tile_net_open_inner(struct net_device *dev)
int my_cpu = smp_processor_id();
struct tile_net_cpu *info;
struct tile_netio_queue *queue;
- unsigned int irq;
+ int result = 0;
int i;
+ int dummy = 0;
/*
* First try to register just on the local CPU, and handle any
@@ -1307,42 +1312,52 @@ static int tile_net_open_inner(struct net_device *dev)
/*
* Now register everywhere else. If any registration fails,
* even for "link down" (which might not be possible), we
- * clean up using "tile_net_stop_aux()".
+ * clean up using "tile_net_stop_aux()". Also, add all the
+ * "napi" objects (sequentially, to protect "dev->napi_list").
+ * ISSUE: Only use "netif_napi_add()" for "network cpus"?
*/
smp_call_function(tile_net_register, (void *)dev, 1);
for_each_online_cpu(i) {
- if (!priv->cpu[i]->registered) {
- tile_net_stop_aux(dev);
- return -EAGAIN;
- }
+ struct tile_net_cpu *info = priv->cpu[i];
+ if (info->registered)
+ netif_napi_add(dev, &info->napi, tile_net_poll, 64);
+ else
+ result = -EAGAIN;
+ }
+ if (result != 0) {
+ tile_net_stop_aux(dev);
+ return result;
}
queue = &info->queue;
- /*
- * Set the device intr bit mask.
- * The tile_net_register above sets per tile __intr_id.
- */
- priv->intr_id = queue->__system_part->__intr_id;
- BUG_ON(!priv->intr_id);
-
- /*
- * Register the device interrupt handler.
- * The __ffs() function returns the index into the interrupt handler
- * table from the interrupt bit mask which should have one bit
- * and one bit only set.
- */
- irq = __ffs(priv->intr_id);
- tile_irq_activate(irq, TILE_IRQ_PERCPU);
- BUG_ON(request_irq(irq, tile_net_handle_ingress_interrupt,
- 0, dev->name, (void *)dev) != 0);
-
- /* ISSUE: How could "priv->fully_opened" ever be "true" here? */
+ if (priv->intr_id == 0) {
+ unsigned int irq;
- if (!priv->fully_opened) {
+ /*
+ * Acquire the irq allocated by the hypervisor. Every
+ * queue gets the same irq. The "__intr_id" field is
+ * "1 << irq", so we use "__ffs()" to extract "irq".
+ */
+ priv->intr_id = queue->__system_part->__intr_id;
+ BUG_ON(priv->intr_id == 0);
+ irq = __ffs(priv->intr_id);
- int dummy = 0;
+ /*
+ * Register the ingress interrupt handler for this
+ * device, permanently.
+ *
+ * We used to call "free_irq()" in "tile_net_stop()",
+ * and then re-register the handler here every time,
+ * but that caused DNP errors in "handle_IRQ_event()"
+ * because "desc->action" was NULL. See bug 9143.
+ */
+ tile_irq_activate(irq, TILE_IRQ_PERCPU);
+ BUG_ON(request_irq(irq, tile_net_handle_ingress_interrupt,
+ 0, dev->name, (void *)dev) != 0);
+ }
+ {
/* Allocate initial buffers. */
int max_buffers =
@@ -1359,18 +1374,21 @@ static int tile_net_open_inner(struct net_device *dev)
if (info->num_needed_small_buffers != 0 ||
info->num_needed_large_buffers != 0)
panic("Insufficient memory for buffer stack!");
+ }
- /* Start LIPP/LEPP and activate "ingress" at the shim. */
- if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
- sizeof(dummy), NETIO_IPP_INPUT_INIT_OFF) < 0)
- panic("Failed to activate the LIPP Shim!\n");
+ /* We are about to be active. */
+ priv->active = true;
- priv->fully_opened = 1;
- }
+ /* Make sure "active" is visible to all tiles. */
+ mb();
- /* On each tile, enable the hypervisor to trigger interrupts. */
- /* ISSUE: Do this before starting LIPP/LEPP? */
- on_each_cpu(tile_net_enable_intr, (void *)dev, 1);
+ /* On each tile, enable NAPI and the ingress interrupt. */
+ on_each_cpu(tile_net_open_enable, (void *)dev, 1);
+
+ /* Start LIPP/LEPP and activate "ingress" at the shim. */
+ if (hv_dev_pwrite(priv->hv_devhdl, 0, (HV_VirtAddr)&dummy,
+ sizeof(dummy), NETIO_IPP_INPUT_INIT_OFF) < 0)
+ panic("Failed to activate the LIPP Shim!\n");
/* Start our transmit queue. */
netif_start_queue(dev);
@@ -1396,9 +1414,9 @@ static void tile_net_open_retry(struct work_struct *w)
* ourselves to try again later; otherwise, tell Linux we now have
* a working link. ISSUE: What if the return value is negative?
*/
- if (tile_net_open_inner(priv->dev))
- schedule_delayed_work_on(singlethread_cpu, &priv->retry_work,
- TILE_NET_RETRY_INTERVAL);
+ if (tile_net_open_inner(priv->dev) != 0)
+ schedule_delayed_work(&priv->retry_work,
+ TILE_NET_RETRY_INTERVAL);
else
netif_carrier_on(priv->dev);
}
@@ -1412,8 +1430,8 @@ static void tile_net_open_retry(struct work_struct *w)
* The open entry point is called when a network interface is made
* active by the system (IFF_UP). At this point all resources needed
* for transmit and receive operations are allocated, the interrupt
- * handler is registered with the OS, the watchdog timer is started,
- * and the stack is notified that the interface is ready.
+ * handler is registered with the OS (if needed), the watchdog timer
+ * is started, and the stack is notified that the interface is ready.
*
* If the actual link is not available yet, then we tell Linux that
* we have no carrier, and we keep checking until the link comes up.
@@ -1468,6 +1486,10 @@ static int tile_net_open(struct net_device *dev)
#endif
priv->partly_opened = 1;
+
+ } else {
+ /* FIXME: Is this possible? */
+ /* printk("Already partly opened.\n"); */
}
/*
@@ -1487,57 +1509,17 @@ static int tile_net_open(struct net_device *dev)
* and then remember to try again later.
*/
netif_carrier_off(dev);
- schedule_delayed_work_on(singlethread_cpu, &priv->retry_work,
- TILE_NET_RETRY_INTERVAL);
+ schedule_delayed_work(&priv->retry_work, TILE_NET_RETRY_INTERVAL);
return 0;
}
-/*
- * Disables a network interface.
- *
- * Returns 0, this is not allowed to fail.
- *
- * The close entry point is called when an interface is de-activated
- * by the OS. The hardware is still under the drivers control, but
- * needs to be disabled. A global MAC reset is issued to stop the
- * hardware, and all transmit and receive resources are freed.
- *
- * ISSUE: Can this can be called while "tile_net_poll()" is running?
- */
-static int tile_net_stop(struct net_device *dev)
+static int tile_net_drain_lipp_buffers(struct tile_net_priv *priv)
{
- struct tile_net_priv *priv = netdev_priv(dev);
-
- bool pending = true;
-
- PDEBUG("tile_net_stop()\n");
-
- /* ISSUE: Only needed if not yet fully open. */
- cancel_delayed_work_sync(&priv->retry_work);
-
- /* Can't transmit any more. */
- netif_stop_queue(dev);
-
- /*
- * Disable hypervisor interrupts on each tile.
- */
- on_each_cpu(tile_net_disable_intr, (void *)dev, 1);
-
- /*
- * Unregister the interrupt handler.
- * The __ffs() function returns the index into the interrupt handler
- * table from the interrupt bit mask which should have one bit
- * and one bit only set.
- */
- if (priv->intr_id)
- free_irq(__ffs(priv->intr_id), dev);
-
- /*
- * Drain all the LIPP buffers.
- */
+ int n = 0;
+ /* Drain all the LIPP buffers. */
while (true) {
int buffer;
@@ -1560,43 +1542,105 @@ static int tile_net_stop(struct net_device *dev)
kfree_skb(skb);
}
+
+ n++;
}
- /* Stop LIPP/LEPP. */
- tile_net_stop_aux(dev);
+ return n;
+}
+
+
+/*
+ * Disables a network interface.
+ *
+ * Returns 0, this is not allowed to fail.
+ *
+ * The close entry point is called when an interface is de-activated
+ * by the OS. The hardware is still under the drivers control, but
+ * needs to be disabled. A global MAC reset is issued to stop the
+ * hardware, and all transmit and receive resources are freed.
+ *
+ * ISSUE: How closely does "netif_running(dev)" mirror "priv->active"?
+ *
+ * Before we are called by "__dev_close()", "netif_running()" will
+ * have been cleared, so no NEW calls to "tile_net_poll()" will be
+ * made by "netpoll_poll_dev()".
+ *
+ * Often, this can cause some tiles to still have packets in their
+ * queues, so we must call "tile_net_discard_packets()" later.
+ *
+ * Note that some other tile may still be INSIDE "tile_net_poll()",
+ * and in fact, many will be, if there is heavy network load.
+ *
+ * Calling "on_each_cpu(tile_net_stop_disable, (void *)dev, 1)" when
+ * any tile is still "napi_schedule()"'d will induce a horrible crash
+ * when "msleep()" is called. This includes tiles which are inside
+ * "tile_net_poll()" which have not yet called "napi_complete()".
+ *
+ * So, we must first try to wait long enough for other tiles to finish
+ * with any current "tile_net_poll()" call, and, hopefully, to clear
+ * the "scheduled" flag. ISSUE: It is unclear what happens to tiles
+ * which have called "napi_schedule()" but which had not yet tried to
+ * call "tile_net_poll()", or which exhausted their budget inside
+ * "tile_net_poll()" just before this function was called.
+ */
+static int tile_net_stop(struct net_device *dev)
+{
+ struct tile_net_priv *priv = netdev_priv(dev);
+ PDEBUG("tile_net_stop()\n");
- priv->fully_opened = 0;
+ /* Start discarding packets. */
+ priv->active = false;
+ /* Make sure "active" is visible to all tiles. */
+ mb();
/*
- * XXX: ISSUE: It appears that, in practice anyway, by the
- * time we get here, there are no pending completions.
+ * On each tile, make sure no NEW packets get delivered, and
+ * disable the ingress interrupt.
+ *
+ * Note that the ingress interrupt can fire AFTER this,
+ * presumably due to packets which were recently delivered,
+ * but it will have no effect.
*/
- while (pending) {
+ on_each_cpu(tile_net_deregister, (void *)dev, 1);
- struct sk_buff *olds[32];
- unsigned int wanted = 32;
- unsigned int i, nolds = 0;
+ /* Optimistically drain LIPP buffers. */
+ (void)tile_net_drain_lipp_buffers(priv);
- nolds = tile_net_lepp_grab_comps(dev, olds,
- wanted, &pending);
+ /* ISSUE: Only needed if not yet fully open. */
+ cancel_delayed_work_sync(&priv->retry_work);
- /* ISSUE: We have never actually seen this debug spew. */
- if (nolds != 0)
- pr_info("During tile_net_stop(), grabbed %d comps.\n",
- nolds);
+ /* Can't transmit any more. */
+ netif_stop_queue(dev);
- for (i = 0; i < nolds; i++)
- kfree_skb(olds[i]);
- }
+ /* Disable NAPI on each tile. */
+ on_each_cpu(tile_net_stop_disable, (void *)dev, 1);
+ /*
+ * Drain any remaining LIPP buffers. NOTE: This "printk()"
+ * has never been observed, but in theory it could happen.
+ */
+ if (tile_net_drain_lipp_buffers(priv) != 0)
+ printk("Had to drain some extra LIPP buffers!\n");
+
+ /* Stop LIPP/LEPP. */
+ tile_net_stop_aux(dev);
+
+ /*
+ * ISSUE: It appears that, in practice anyway, by the time we
+ * get here, there are no pending completions, but just in case,
+ * we free (all of) them anyway.
+ */
+ while (tile_net_lepp_free_comps(dev, true))
+ /* loop */;
/* Wipe the EPP queue. */
- memset(priv->epp_queue, 0, sizeof(lepp_queue_t));
+ memset(priv->eq, 0, sizeof(lepp_queue_t));
/* Evict the EPP queue. */
- finv_buffer(priv->epp_queue, PAGE_SIZE);
+ finv_buffer(priv->eq, EQ_SIZE);
return 0;
}
@@ -1742,17 +1786,15 @@ static int tile_net_tx_tso(struct sk_buff *skb, struct net_device *dev)
unsigned long irqflags;
- lepp_queue_t *eq = priv->epp_queue;
+ lepp_queue_t *eq = priv->eq;
- struct sk_buff *olds[4];
- unsigned int wanted = 4;
+ struct sk_buff *olds[8];
+ unsigned int wanted = 8;
unsigned int i, nolds = 0;
unsigned int cmd_head, cmd_tail, cmd_next;
unsigned int comp_tail;
- unsigned int free_slots;
-
/* Paranoia. */
BUG_ON(skb->protocol != htons(ETH_P_IP));
@@ -1780,34 +1822,32 @@ static int tile_net_tx_tso(struct sk_buff *skb, struct net_device *dev)
/* Enqueue the command. */
- spin_lock_irqsave(&priv->cmd_lock, irqflags);
+ spin_lock_irqsave(&priv->eq_lock, irqflags);
/*
* Handle completions if needed to make room.
* HACK: Spin until there is sufficient room.
*/
- free_slots = lepp_num_free_comp_slots(eq);
- if (free_slots < 1) {
-spin:
- nolds += tile_net_lepp_grab_comps(dev, olds + nolds,
- wanted - nolds, NULL);
- if (lepp_num_free_comp_slots(eq) < 1)
- goto spin;
+ if (lepp_num_free_comp_slots(eq) == 0) {
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+ if (nolds == 0) {
+busy:
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
+ return NETDEV_TX_BUSY;
+ }
}
cmd_head = eq->cmd_head;
cmd_tail = eq->cmd_tail;
- /* NOTE: The "gotos" below are untested. */
-
/* Prepare to advance, detecting full queue. */
cmd_next = cmd_tail + cmd_size;
if (cmd_tail < cmd_head && cmd_next >= cmd_head)
- goto spin;
+ goto busy;
if (cmd_next > LEPP_CMD_LIMIT) {
cmd_next = 0;
if (cmd_next == cmd_head)
- goto spin;
+ goto busy;
}
/* Copy the command. */
@@ -1823,14 +1863,18 @@ spin:
eq->comp_tail = comp_tail;
/* Flush before allowing LEPP to handle the command. */
+ /* ISSUE: Is this the optimal location for the flush? */
__insn_mf();
eq->cmd_tail = cmd_tail;
- spin_unlock_irqrestore(&priv->cmd_lock, irqflags);
-
+ /* NOTE: Using "4" here is more efficient than "0" or "2", */
+ /* and, strangely, more efficient than pre-checking the number */
+ /* of available completions, and comparing it to 4. */
if (nolds == 0)
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, NULL);
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 4);
+
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
/* Handle completions. */
for (i = 0; i < nolds; i++)
@@ -1870,10 +1914,10 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
unsigned int num_frags;
- lepp_queue_t *eq = priv->epp_queue;
+ lepp_queue_t *eq = priv->eq;
- struct sk_buff *olds[4];
- unsigned int wanted = 4;
+ struct sk_buff *olds[8];
+ unsigned int wanted = 8;
unsigned int i, nolds = 0;
unsigned int cmd_size = sizeof(lepp_cmd_t);
@@ -1883,8 +1927,6 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
lepp_cmd_t cmds[LEPP_MAX_FRAGS];
- unsigned int free_slots;
-
/*
* This is paranoia, since we think that if the link doesn't come
@@ -1905,7 +1947,8 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
if (hash_default) {
HV_PTE pte = *virt_to_pte(current->mm, (unsigned long)data);
if (hv_pte_get_mode(pte) != HV_PTE_MODE_CACHE_HASH_L3)
- panic("Non-coherent egress buffer!");
+ panic("Non-HFH egress buffer! VA=%p Mode=%d PTE=%llx",
+ data, hv_pte_get_mode(pte), hv_pte_val(pte));
}
#endif
#endif
@@ -1958,37 +2001,35 @@ static int tile_net_tx(struct sk_buff *skb, struct net_device *dev)
/* Enqueue the commands. */
- spin_lock_irqsave(&priv->cmd_lock, irqflags);
+ spin_lock_irqsave(&priv->eq_lock, irqflags);
/*
* Handle completions if needed to make room.
* HACK: Spin until there is sufficient room.
*/
- free_slots = lepp_num_free_comp_slots(eq);
- if (free_slots < 1) {
-spin:
- nolds += tile_net_lepp_grab_comps(dev, olds + nolds,
- wanted - nolds, NULL);
- if (lepp_num_free_comp_slots(eq) < 1)
- goto spin;
+ if (lepp_num_free_comp_slots(eq) == 0) {
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 0);
+ if (nolds == 0) {
+busy:
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
+ return NETDEV_TX_BUSY;
+ }
}
cmd_head = eq->cmd_head;
cmd_tail = eq->cmd_tail;
- /* NOTE: The "gotos" below are untested. */
-
/* Copy the commands, or fail. */
for (i = 0; i < num_frags; i++) {
/* Prepare to advance, detecting full queue. */
cmd_next = cmd_tail + cmd_size;
if (cmd_tail < cmd_head && cmd_next >= cmd_head)
- goto spin;
+ goto busy;
if (cmd_next > LEPP_CMD_LIMIT) {
cmd_next = 0;
if (cmd_next == cmd_head)
- goto spin;
+ goto busy;
}
/* Copy the command. */
@@ -2005,14 +2046,18 @@ spin:
eq->comp_tail = comp_tail;
/* Flush before allowing LEPP to handle the command. */
+ /* ISSUE: Is this the optimal location for the flush? */
__insn_mf();
eq->cmd_tail = cmd_tail;
- spin_unlock_irqrestore(&priv->cmd_lock, irqflags);
-
+ /* NOTE: Using "4" here is more efficient than "0" or "2", */
+ /* and, strangely, more efficient than pre-checking the number */
+ /* of available completions, and comparing it to 4. */
if (nolds == 0)
- nolds = tile_net_lepp_grab_comps(dev, olds, wanted, NULL);
+ nolds = tile_net_lepp_grab_comps(eq, olds, wanted, 4);
+
+ spin_unlock_irqrestore(&priv->eq_lock, irqflags);
/* Handle completions. */
for (i = 0; i < nolds; i++)
@@ -2261,7 +2306,6 @@ static struct net_device *tile_net_dev_init(const char *name)
int ret;
struct net_device *dev;
struct tile_net_priv *priv;
- struct page *page;
/*
* Allocate the device structure. This allocates "priv", calls
@@ -2285,23 +2329,21 @@ static struct net_device *tile_net_dev_init(const char *name)
INIT_DELAYED_WORK(&priv->retry_work, tile_net_open_retry);
- spin_lock_init(&priv->cmd_lock);
- spin_lock_init(&priv->comp_lock);
+ spin_lock_init(&priv->eq_lock);
- /* Allocate "epp_queue". */
- BUG_ON(get_order(sizeof(lepp_queue_t)) != 0);
- page = alloc_pages(GFP_KERNEL | __GFP_ZERO, 0);
- if (!page) {
+ /* Allocate "eq". */
+ priv->eq_pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, EQ_ORDER);
+ if (!priv->eq_pages) {
free_netdev(dev);
return NULL;
}
- priv->epp_queue = page_address(page);
+ priv->eq = page_address(priv->eq_pages);
/* Register the network device. */
ret = register_netdev(dev);
if (ret) {
pr_err("register_netdev %s failed %d\n", dev->name, ret);
- free_page((unsigned long)priv->epp_queue);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
return NULL;
}
@@ -2310,7 +2352,7 @@ static struct net_device *tile_net_dev_init(const char *name)
ret = tile_net_get_mac(dev);
if (ret < 0) {
unregister_netdev(dev);
- free_page((unsigned long)priv->epp_queue);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
return NULL;
}
@@ -2321,6 +2363,9 @@ static struct net_device *tile_net_dev_init(const char *name)
/*
* Module cleanup.
+ *
+ * FIXME: If compiled as a module, this module cannot be "unloaded",
+ * because the "ingress interrupt handler" is registered permanently.
*/
static void tile_net_cleanup(void)
{
@@ -2331,8 +2376,8 @@ static void tile_net_cleanup(void)
struct net_device *dev = tile_net_devs[i];
struct tile_net_priv *priv = netdev_priv(dev);
unregister_netdev(dev);
- finv_buffer(priv->epp_queue, PAGE_SIZE);
- free_page((unsigned long)priv->epp_queue);
+ finv_buffer(priv->eq, EQ_SIZE);
+ __free_pages(priv->eq_pages, EQ_ORDER);
free_netdev(dev);
}
}
@@ -2355,7 +2400,12 @@ static int tile_net_init_module(void)
}
+module_init(tile_net_init_module);
+module_exit(tile_net_cleanup);
+
+
#ifndef MODULE
+
/*
* The "network_cpus" boot argument specifies the cpus that are dedicated
* to handle ingress packets.
@@ -2391,8 +2441,5 @@ static int __init network_cpus_setup(char *str)
return 0;
}
__setup("network_cpus=", network_cpus_setup);
-#endif
-
-module_init(tile_net_init_module);
-module_exit(tile_net_cleanup);
+#endif
--
1.6.5.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox