* Re: [PATCH V3 8/8] net: backlog functions rename
From: Eric Dumazet @ 2010-03-05 6:32 UTC (permalink / raw)
To: Zhu Yi; +Cc: davem, netdev
In-Reply-To: <1267761707-15605-8-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> sk_add_backlog -> __sk_add_backlog
> sk_add_backlog_limited -> sk_add_backlog
>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
> include/net/sock.h | 6 +++---
> net/core/sock.c | 2 +-
> net/dccp/minisocks.c | 2 +-
> net/ipv4/tcp_ipv4.c | 2 +-
> net/ipv4/tcp_minisocks.c | 2 +-
> net/ipv4/udp.c | 2 +-
> net/ipv6/tcp_ipv6.c | 2 +-
> net/ipv6/udp.c | 4 ++--
> net/llc/llc_c_ac.c | 2 +-
> net/llc/llc_conn.c | 2 +-
> net/sctp/input.c | 4 ++--
> net/tipc/socket.c | 2 +-
> net/x25/x25_dev.c | 2 +-
> 13 files changed, 17 insertions(+), 17 deletions(-)
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH V3 7/8] x25: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:30 UTC (permalink / raw)
To: Zhu Yi; +Cc: davem, netdev, Andrew Hendry
In-Reply-To: <1267761707-15605-7-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make x25 adapt to the limited socket backlog change.
>
> Cc: Andrew Hendry <andrew.hendry@gmail.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH V3 6/8] tipc: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:30 UTC (permalink / raw)
To: Zhu Yi; +Cc: davem, netdev, Jon Maloy, Allan Stephens
In-Reply-To: <1267761707-15605-6-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make tipc adapt to the limited socket backlog change.
>
> Cc: Jon Maloy <jon.maloy@ericsson.com>
> Cc: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
> net/tipc/socket.c | 6 ++++--
> 1 files changed, 4 insertions(+), 2 deletions(-)
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH V3 5/8] sctp: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:28 UTC (permalink / raw)
To: Zhu Yi; +Cc: davem, netdev, Vlad Yasevich, Sridhar Samudrala
In-Reply-To: <1267761707-15605-5-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make sctp adapt to the limited socket backlog change.
>
> Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
> Cc: Sridhar Samudrala <sri@us.ibm.com>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
This patch looks wrong.
> -static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> +static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
> {
> struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
> struct sctp_ep_common *rcvr = chunk->rcvr;
> + int ret;
>
> - /* Hold the assoc/ep while hanging on the backlog queue.
> - * This way, we know structures we need will not disappear from us
> - */
> - if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
> - sctp_association_hold(sctp_assoc(rcvr));
> - else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
> - sctp_endpoint_hold(sctp_ep(rcvr));
> - else
> - BUG();
> + ret = sk_add_backlog_limited(sk, skb);
> + if (!ret) {
> + /* Hold the assoc/ep while hanging on the backlog queue.
> + * This way, we know structures we need will not disappear
> + * from us
> + */
> + if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
> + sctp_association_hold(sctp_assoc(rcvr));
> + else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
> + sctp_endpoint_hold(sctp_ep(rcvr));
> + else
> + BUG();
> + }
> + return ret;
>
> - sk_add_backlog(sk, skb);
> }
>
As advertized by comment, we should hold the association *before*
accessing backlog queue.
If order is not important, comment should be relaxed somehow ?
^ permalink raw reply
* Re: [PATCH V3 4/8] llc: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:22 UTC (permalink / raw)
To: Zhu Yi; +Cc: davem, netdev, Arnaldo Carvalho de Melo
In-Reply-To: <1267761707-15605-4-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make llc adapt to the limited socket backlog change.
>
> Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH V3 3/8] udp: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:21 UTC (permalink / raw)
To: Zhu Yi
Cc: davem, netdev, Alexey Kuznetsov, Pekka Savola (ipv6),
Patrick McHardy
In-Reply-To: <1267761707-15605-3-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make udp adapt to the limited socket backlog change.
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
> net/ipv4/udp.c | 6 ++++--
> net/ipv6/udp.c | 28 ++++++++++++++++++----------
> 2 files changed, 22 insertions(+), 12 deletions(-)
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [PATCH V3 2/8] tcp: use limited socket backlog
From: Eric Dumazet @ 2010-03-05 6:19 UTC (permalink / raw)
To: Zhu Yi
Cc: davem, netdev, Alexey Kuznetsov, Pekka Savola (ipv6),
Patrick McHardy
In-Reply-To: <1267761707-15605-2-git-send-email-yi.zhu@intel.com>
Le vendredi 05 mars 2010 à 12:01 +0800, Zhu Yi a écrit :
> Make tcp adapt to the limited socket backlog change.
>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
> Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
> Cc: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Zhu Yi <yi.zhu@intel.com>
> ---
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
I'll submit a followup patch to add a MIB counter if your patch gets in.
^ permalink raw reply
* [PATCH V3 8/8] net: backlog functions rename
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Zhu Yi
In-Reply-To: <1267761707-15605-7-git-send-email-yi.zhu@intel.com>
sk_add_backlog -> __sk_add_backlog
sk_add_backlog_limited -> sk_add_backlog
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
include/net/sock.h | 6 +++---
net/core/sock.c | 2 +-
net/dccp/minisocks.c | 2 +-
net/ipv4/tcp_ipv4.c | 2 +-
net/ipv4/tcp_minisocks.c | 2 +-
net/ipv4/udp.c | 2 +-
net/ipv6/tcp_ipv6.c | 2 +-
net/ipv6/udp.c | 4 ++--
net/llc/llc_c_ac.c | 2 +-
net/llc/llc_conn.c | 2 +-
net/sctp/input.c | 4 ++--
net/tipc/socket.c | 2 +-
net/x25/x25_dev.c | 2 +-
13 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 2516d76..170353d 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -592,7 +592,7 @@ static inline int sk_stream_memory_free(struct sock *sk)
}
/* OOB backlog add */
-static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
+static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
{
if (!sk->sk_backlog.tail) {
sk->sk_backlog.head = sk->sk_backlog.tail = skb;
@@ -604,12 +604,12 @@ static inline void sk_add_backlog(struct sock *sk, struct sk_buff *skb)
}
/* The per-socket spinlock must be held here. */
-static inline int sk_add_backlog_limited(struct sock *sk, struct sk_buff *skb)
+static inline int sk_add_backlog(struct sock *sk, struct sk_buff *skb)
{
if (sk->sk_backlog.len >= max(sk->sk_backlog.limit, sk->sk_rcvbuf << 1))
return -ENOBUFS;
- sk_add_backlog(sk, skb);
+ __sk_add_backlog(sk, skb);
sk->sk_backlog.len += skb->truesize;
return 0;
}
diff --git a/net/core/sock.c b/net/core/sock.c
index 6e22dc9..61a65a2 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -340,7 +340,7 @@ int sk_receive_skb(struct sock *sk, struct sk_buff *skb, const int nested)
rc = sk_backlog_rcv(sk, skb);
mutex_release(&sk->sk_lock.dep_map, 1, _RET_IP_);
- } else if (sk_add_backlog_limited(sk, skb)) {
+ } else if (sk_add_backlog(sk, skb)) {
bh_unlock_sock(sk);
atomic_inc(&sk->sk_drops);
goto discard_and_relse;
diff --git a/net/dccp/minisocks.c b/net/dccp/minisocks.c
index af226a0..0d508c3 100644
--- a/net/dccp/minisocks.c
+++ b/net/dccp/minisocks.c
@@ -254,7 +254,7 @@ int dccp_child_process(struct sock *parent, struct sock *child,
* in main socket hash table and lock on listening
* socket does not protect us more.
*/
- sk_add_backlog(child, skb);
+ __sk_add_backlog(child, skb);
}
bh_unlock_sock(child);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 4baf194..1915f7d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1682,7 +1682,7 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v4_do_rcv(sk, skb);
}
- } else if (sk_add_backlog_limited(sk, skb)) {
+ } else if (sk_add_backlog(sk, skb)) {
bh_unlock_sock(sk);
goto discard_and_relse;
}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index f206ee5..4199bc6 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -728,7 +728,7 @@ int tcp_child_process(struct sock *parent, struct sock *child,
* in main socket hash table and lock on listening
* socket does not protect us more.
*/
- sk_add_backlog(child, skb);
+ __sk_add_backlog(child, skb);
}
bh_unlock_sock(child);
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index e7eb47f..7af756d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1371,7 +1371,7 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
rc = __udp_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog_limited(sk, skb)) {
+ else if (sk_add_backlog(sk, skb)) {
bh_unlock_sock(sk);
goto drop;
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index c4ea9d5..2c378b1 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1740,7 +1740,7 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v6_do_rcv(sk, skb);
}
- } else if (sk_add_backlog_limited(sk, skb)) {
+ } else if (sk_add_backlog(sk, skb)) {
bh_unlock_sock(sk);
goto discard_and_relse;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 6480491..3c0c9c7 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -583,7 +583,7 @@ static void flush_stack(struct sock **stack, unsigned int count,
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
udpv6_queue_rcv_skb(sk, skb1);
- else if (sk_add_backlog_limited(sk, skb1)) {
+ else if (sk_add_backlog(sk, skb1)) {
kfree_skb(skb1);
bh_unlock_sock(sk);
goto drop;
@@ -758,7 +758,7 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
udpv6_queue_rcv_skb(sk, skb);
- else if (sk_add_backlog_limited(sk, skb)) {
+ else if (sk_add_backlog(sk, skb)) {
atomic_inc(&sk->sk_drops);
bh_unlock_sock(sk);
sock_put(sk);
diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index 019c780..86d6985 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -1437,7 +1437,7 @@ static void llc_process_tmr_ev(struct sock *sk, struct sk_buff *skb)
llc_conn_state_process(sk, skb);
else {
llc_set_backlog_type(skb, LLC_EVENT);
- sk_add_backlog(sk, skb);
+ __sk_add_backlog(sk, skb);
}
}
}
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index c0539ff..a12144d 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -827,7 +827,7 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
else {
dprintk("%s: adding to backlog...\n", __func__);
llc_set_backlog_type(skb, LLC_PACKET);
- if (sk_add_backlog_limited(sk, skb))
+ if (sk_add_backlog(sk, skb))
goto drop_unlock;
}
out:
diff --git a/net/sctp/input.c b/net/sctp/input.c
index cbc0636..3d74b26 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -341,7 +341,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
sctp_bh_lock_sock(sk);
if (sock_owned_by_user(sk)) {
- if (sk_add_backlog_limited(sk, skb))
+ if (sk_add_backlog(sk, skb))
sctp_chunk_free(chunk);
else
backloged = 1;
@@ -375,7 +375,7 @@ static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
struct sctp_ep_common *rcvr = chunk->rcvr;
int ret;
- ret = sk_add_backlog_limited(sk, skb);
+ ret = sk_add_backlog(sk, skb);
if (!ret) {
/* Hold the assoc/ep while hanging on the backlog queue.
* This way, we know structures we need will not disappear
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 22bfbc3..4b235fc 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1322,7 +1322,7 @@ static u32 dispatch(struct tipc_port *tport, struct sk_buff *buf)
if (!sock_owned_by_user(sk)) {
res = filter_rcv(sk, buf);
} else {
- if (sk_add_backlog_limited(sk, buf))
+ if (sk_add_backlog(sk, buf))
res = TIPC_ERR_OVERLOAD;
else
res = TIPC_OK;
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index a9da0dc..52e3042 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -53,7 +53,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
if (!sock_owned_by_user(sk)) {
queued = x25_process_rx_frame(sk, skb);
} else {
- queued = !sk_add_backlog_limited(sk, skb);
+ queued = !sk_add_backlog(sk, skb);
}
bh_unlock_sock(sk);
sock_put(sk);
--
1.6.3.3
^ permalink raw reply related
* [PATCH V3 7/8] x25: use limited socket backlog
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Zhu Yi, Andrew Hendry
In-Reply-To: <1267761707-15605-6-git-send-email-yi.zhu@intel.com>
Make x25 adapt to the limited socket backlog change.
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
net/x25/x25_dev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index 3e1efe5..a9da0dc 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -53,7 +53,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
if (!sock_owned_by_user(sk)) {
queued = x25_process_rx_frame(sk, skb);
} else {
- sk_add_backlog(sk, skb);
+ queued = !sk_add_backlog_limited(sk, skb);
}
bh_unlock_sock(sk);
sock_put(sk);
--
1.6.3.3
^ permalink raw reply related
* [PATCH V3 2/8] tcp: use limited socket backlog
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem
Cc: netdev, Zhu Yi, Alexey Kuznetsov, Pekka Savola (ipv6),
Patrick McHardy
In-Reply-To: <1267761707-15605-1-git-send-email-yi.zhu@intel.com>
Make tcp adapt to the limited socket backlog change.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
net/ipv4/tcp_ipv4.c | 6 ++++--
net/ipv6/tcp_ipv6.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c3588b4..4baf194 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1682,8 +1682,10 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v4_do_rcv(sk, skb);
}
- } else
- sk_add_backlog(sk, skb);
+ } else if (sk_add_backlog_limited(sk, skb)) {
+ bh_unlock_sock(sk);
+ goto discard_and_relse;
+ }
bh_unlock_sock(sk);
sock_put(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6963a6b..c4ea9d5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1740,8 +1740,10 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v6_do_rcv(sk, skb);
}
- } else
- sk_add_backlog(sk, skb);
+ } else if (sk_add_backlog_limited(sk, skb)) {
+ bh_unlock_sock(sk);
+ goto discard_and_relse;
+ }
bh_unlock_sock(sk);
sock_put(sk);
--
1.6.3.3
^ permalink raw reply related
* [PATCH V3 3/8] udp: use limited socket backlog
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem
Cc: netdev, Zhu Yi, Alexey Kuznetsov, Pekka Savola (ipv6),
Patrick McHardy
In-Reply-To: <1267761707-15605-2-git-send-email-yi.zhu@intel.com>
Make udp adapt to the limited socket backlog change.
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
net/ipv4/udp.c | 6 ++++--
net/ipv6/udp.c | 28 ++++++++++++++++++----------
2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 608a544..e7eb47f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1371,8 +1371,10 @@ int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
rc = __udp_queue_rcv_skb(sk, skb);
- else
- sk_add_backlog(sk, skb);
+ else if (sk_add_backlog_limited(sk, skb)) {
+ bh_unlock_sock(sk);
+ goto drop;
+ }
bh_unlock_sock(sk);
return rc;
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 52b8347..6480491 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -583,16 +583,20 @@ static void flush_stack(struct sock **stack, unsigned int count,
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
udpv6_queue_rcv_skb(sk, skb1);
- else
- sk_add_backlog(sk, skb1);
+ else if (sk_add_backlog_limited(sk, skb1)) {
+ kfree_skb(skb1);
+ bh_unlock_sock(sk);
+ goto drop;
+ }
bh_unlock_sock(sk);
- } else {
- atomic_inc(&sk->sk_drops);
- UDP6_INC_STATS_BH(sock_net(sk),
- UDP_MIB_RCVBUFERRORS, IS_UDPLITE(sk));
- UDP6_INC_STATS_BH(sock_net(sk),
- UDP_MIB_INERRORS, IS_UDPLITE(sk));
+ continue;
}
+drop:
+ atomic_inc(&sk->sk_drops);
+ UDP6_INC_STATS_BH(sock_net(sk),
+ UDP_MIB_RCVBUFERRORS, IS_UDPLITE(sk));
+ UDP6_INC_STATS_BH(sock_net(sk),
+ UDP_MIB_INERRORS, IS_UDPLITE(sk));
}
}
/*
@@ -754,8 +758,12 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
bh_lock_sock(sk);
if (!sock_owned_by_user(sk))
udpv6_queue_rcv_skb(sk, skb);
- else
- sk_add_backlog(sk, skb);
+ else if (sk_add_backlog_limited(sk, skb)) {
+ atomic_inc(&sk->sk_drops);
+ bh_unlock_sock(sk);
+ sock_put(sk);
+ goto discard;
+ }
bh_unlock_sock(sk);
sock_put(sk);
return 0;
--
1.6.3.3
^ permalink raw reply related
* [PATCH V3 5/8] sctp: use limited socket backlog
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Zhu Yi, Vlad Yasevich, Sridhar Samudrala
In-Reply-To: <1267761707-15605-4-git-send-email-yi.zhu@intel.com>
Make sctp adapt to the limited socket backlog change.
Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
Cc: Sridhar Samudrala <sri@us.ibm.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
net/sctp/input.c | 42 +++++++++++++++++++++++++++---------------
net/sctp/socket.c | 3 +++
2 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/net/sctp/input.c b/net/sctp/input.c
index c0c973e..cbc0636 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -75,7 +75,7 @@ static struct sctp_association *__sctp_lookup_association(
const union sctp_addr *peer,
struct sctp_transport **pt);
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb);
/* Calculate the SCTP checksum of an SCTP packet. */
@@ -265,8 +265,13 @@ int sctp_rcv(struct sk_buff *skb)
}
if (sock_owned_by_user(sk)) {
+ if (sctp_add_backlog(sk, skb)) {
+ sctp_bh_unlock_sock(sk);
+ sctp_chunk_free(chunk);
+ skb = NULL; /* sctp_chunk_free already freed the skb */
+ goto discard_release;
+ }
SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_BACKLOG);
- sctp_add_backlog(sk, skb);
} else {
SCTP_INC_STATS_BH(SCTP_MIB_IN_PKT_SOFTIRQ);
sctp_inq_push(&chunk->rcvr->inqueue, chunk);
@@ -336,8 +341,10 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
sctp_bh_lock_sock(sk);
if (sock_owned_by_user(sk)) {
- sk_add_backlog(sk, skb);
- backloged = 1;
+ if (sk_add_backlog_limited(sk, skb))
+ sctp_chunk_free(chunk);
+ else
+ backloged = 1;
} else
sctp_inq_push(inqueue, chunk);
@@ -362,22 +369,27 @@ done:
return 0;
}
-static void sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
+static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
{
struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
struct sctp_ep_common *rcvr = chunk->rcvr;
+ int ret;
- /* Hold the assoc/ep while hanging on the backlog queue.
- * This way, we know structures we need will not disappear from us
- */
- if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
- sctp_association_hold(sctp_assoc(rcvr));
- else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
- sctp_endpoint_hold(sctp_ep(rcvr));
- else
- BUG();
+ ret = sk_add_backlog_limited(sk, skb);
+ if (!ret) {
+ /* Hold the assoc/ep while hanging on the backlog queue.
+ * This way, we know structures we need will not disappear
+ * from us
+ */
+ if (SCTP_EP_TYPE_ASSOCIATION == rcvr->type)
+ sctp_association_hold(sctp_assoc(rcvr));
+ else if (SCTP_EP_TYPE_SOCKET == rcvr->type)
+ sctp_endpoint_hold(sctp_ep(rcvr));
+ else
+ BUG();
+ }
+ return ret;
- sk_add_backlog(sk, skb);
}
/* Handle icmp frag needed error. */
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index f6d1e59..dfc5c12 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3720,6 +3720,9 @@ SCTP_STATIC int sctp_init_sock(struct sock *sk)
SCTP_DBG_OBJCNT_INC(sock);
percpu_counter_inc(&sctp_sockets_allocated);
+ /* Set socket backlog limit. */
+ sk->sk_backlog.limit = sysctl_sctp_rmem[1];
+
local_bh_disable();
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
local_bh_enable();
--
1.6.3.3
^ permalink raw reply related
* [PATCH V3 4/8] llc: use limited socket backlog
From: Zhu Yi @ 2010-03-05 4:01 UTC (permalink / raw)
To: davem; +Cc: netdev, Zhu Yi, Arnaldo Carvalho de Melo
In-Reply-To: <1267761707-15605-3-git-send-email-yi.zhu@intel.com>
Make llc adapt to the limited socket backlog change.
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
net/llc/llc_conn.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index a8dde9b..c0539ff 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -827,7 +827,8 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
else {
dprintk("%s: adding to backlog...\n", __func__);
llc_set_backlog_type(skb, LLC_PACKET);
- sk_add_backlog(sk, skb);
+ if (sk_add_backlog_limited(sk, skb))
+ goto drop_unlock;
}
out:
bh_unlock_sock(sk);
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions
From: Joe Perches @ 2010-03-05 2:46 UTC (permalink / raw)
To: Andrew Morton, David Miller
Cc: Linus Torvalds, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <20100304170030.f97c6630.akpm@linux-foundation.org>
dOn Thu, 2010-03-04 at 17:00 -0800, Andrew Morton wrote:
> On Thu, 4 Mar 2010 16:56:09 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Wed, 3 Mar 2010 22:27:23 -0800 Joe Perches <joe@perches.com> wrote:
> > > Save ~60k in a defconfig
> > > Use %pV and struct va_format
> > > Format arguments are verified before printk
> > Well that doesn't work very well.
> >
> > drivers/net/pcmcia/pcnet_cs.c:117: error: 'dev_info' redeclared as different kind of symbol
> > include/linux/device.h:645: error: previous declaration of 'dev_info' was here
> >
> > there are lots of other dev_info's which will fail plus perhaps
> > dev_err's, etc.
> >
> btw, this may be kludgeable aroundable by doing
>
> int _dev_info(const struct device *dev, const char *fmt, ...);
>
> #define dev_info(...) _dev_info(...)
>
> which will use the preprocessor's separation of `foo' from `foo()' to
> fix things up.
>
> But it would be better to rename all those dev_info's to device_info or
> whatever, IMO.
Perhaps using the function name _dev_info() and adding
the macro above is an acceptable workaround until the
dev_info struct names or local variable names could be
renamed if necessary.
dev_err and dev_warn exist in source comments only and
don't seem to be a problem.
David Miller? Do you have an opinion on the renaming
of dev_info local variables and structs?
^ permalink raw reply
* Re: [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions
From: Andrew Morton @ 2010-03-05 1:00 UTC (permalink / raw)
To: Joe Perches, Linus Torvalds, Nick Andrew, linux-kernel,
Greg Kroah-Hartman
In-Reply-To: <20100304165609.20849b93.akpm@linux-foundation.org>
On Thu, 4 Mar 2010 16:56:09 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 3 Mar 2010 22:27:23 -0800 Joe Perches <joe@perches.com> wrote:
>
> > Save ~60k in a defconfig
> >
> > Use %pV and struct va_format
> > Format arguments are verified before printk
>
> Well that doesn't work very well.
>
> drivers/net/pcmcia/pcnet_cs.c:117: error: 'dev_info' redeclared as different kind of symbol
> include/linux/device.h:645: error: previous declaration of 'dev_info' was here
>
> there are lots of other dev_info's which will fail plus perhaps
> dev_err's, etc.
>
btw, this may be kludgeable aroundable by doing
int _dev_info(const struct device *dev, const char *fmt, ...);
#define dev_info(...) _dev_info(...)
which will use the preprocessor's separation of `foo' from `foo()' to
fix things up.
But it would be better to rename all those dev_info's to device_info or
whatever, IMO.
^ permalink raw reply
* Re: [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions
From: Andrew Morton @ 2010-03-05 0:56 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <f2b24d484347c083fa87856a75c1d96102af9005.1267682641.git.joe@perches.com>
On Wed, 3 Mar 2010 22:27:23 -0800 Joe Perches <joe@perches.com> wrote:
> Save ~60k in a defconfig
>
> Use %pV and struct va_format
> Format arguments are verified before printk
Well that doesn't work very well.
drivers/net/pcmcia/pcnet_cs.c:117: error: 'dev_info' redeclared as different kind of symbol
include/linux/device.h:645: error: previous declaration of 'dev_info' was here
there are lots of other dev_info's which will fail plus perhaps
dev_err's, etc.
^ permalink raw reply
* Re: [PATCH kernel 2.6.32-rc5] pcnet_cs: add cis of PreMax PE-200 ethernet pcmcia card
From: Ben Hutchings @ 2010-03-05 0:02 UTC (permalink / raw)
To: Ken Kawasaki; +Cc: netdev, David Woodhouse
In-Reply-To: <20091025111000.2d97e595.ken_kawasaki@spring.nifty.jp>
[-- Attachment #1: Type: text/plain, Size: 1710 bytes --]
On Sun, 2009-10-25 at 11:10 +0900, Ken Kawasaki wrote:
> Hi,
>
> >> > Random question: are CIS files copyrightable?
> >>
> >> The CIS contains the IRQ, ioport-range, voltage information etc
> >> like the PCI config space.
> >> So I think it is not copyrightable.
> >> but Sierra Wireless provided this CIS by GPL.
> >[...]
>
> >If it is subject to copyright - which apparently they think it is - then
> >when redistributing under GPL we must be able to provide the 'preferred
> >form for modification'. So that leads to the question, is this blob or
> >hex file the preferred form?
>
>
> (1)The CIS contains data only,
> does not contain the code to be executed.
> (2)Sierra Wireless provided the binary of the CIS only.
> (3)These CIS are stable and does not need to be modified.
> (3)The content of the CIS can be seen by the dump_cis utility.
>
> So if someone want to modify the CIS , I think it is OK
> to use hex editor.
The CIS files under firmware/ are, according to firmware/WHENCE,
'developed by the pcmcia-cs project'. So I had a look at what's left of
the pcmcia-cs project and its last release, and found that it includes:
- the tool 'pack_cis' which compiles a text version of CIS to the
standard binary format
- text versions of most of the CIS files
Since the text versions contain comments which are excluded from the
binary format, I believe they should be considered the 'preferred form
for modification' and included in the linux-2.6 and linux-firmware trees
along with the binaries.
Ben.
--
Ben Hutchings
Q. Which is the greater problem in the world today, ignorance or apathy?
A. I don't know and I couldn't care less.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Linus Torvalds @ 2010-03-04 23:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Joe Perches, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <20100304143837.af39845d.akpm@linux-foundation.org>
On Thu, 4 Mar 2010, Andrew Morton wrote:
>
> What would I need to do to make it recur more than once? Include a %pV
> in a string, like dev_printk("%s", %%pV")?
I would argue that if somebody does that, they're just broken.
The single level of recursion is really nice - it solves a real annoyance.
But if somebody starts trying to do multiple levels, that's just crazy.
Of course, I guess we don't relly have any way of sanely _counting_ the
recursion level, so we'd have to just trust people to not do crazy things.
And that's a big thing to take on trust ;)
Linus
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Jan Engelhardt @ 2010-03-04 22:55 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Serge E. Hallyn, Sukadev Bhattiprolu, Pavel Emelyanov,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <m1pr3j92x8.fsf@fess.ebiederm.org>
On Thursday 2010-03-04 22:45, Eric W. Biederman wrote:
>
>So an unshare of the pid namespace that doesn't really take effect
>until we fork may actually be usable from pam, and in fact is probably
>the preferred implementation. It looks like neither openssh nor login
>from util-linux-ng will cope properly with getting any pid back from
>wait() except the pid of their child.
Correct; I can tell from experience with pam_mount. GDM for example is
very unhappy if you fork/exit processes in PAM modules and don't hide
the fact by bending SIGCHLD from gdm_handler to mypam_handler (which
itself is racy, suppose GDM re-set the SIGCHLD handler midway through).
(In this particular case however, I'd prefer if login programs like GDM
just ignored any PIDs they did not spawn in the first place instead of
moaning around.)
^ permalink raw reply
* Re: [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf
From: Andrew Morton @ 2010-03-04 22:38 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, Nick Andrew, linux-kernel, Greg Kroah-Hartman,
netdev
In-Reply-To: <cover.1267682641.git.joe@perches.com>
On Wed, 3 Mar 2010 22:27:21 -0800
Joe Perches <joe@perches.com> wrote:
> (Typo'ed Linus' email address, added Greg KH and netdev)
What is the networking significance here?
> dev_<level> macros use a lot of repetitive string space.
>
> Eliminate the string prefixes and function arguments from all the macro uses
> and consolidate them in functions.
>
> This patchset saves about 60K.
>
> This implementation also adds the ability to use a struct va_format to
> emit a format string along with va_list arguments.
>
> This %pV implementation should not be used without a wrapper that
> does printf argument verification like the dev_<level> functions.
>
> Inspired a bit by Nick Andrew's patches and Linus' comments in December 2008
> http://lkml.org/lkml/2008/12/6/15
> http://lkml.org/lkml/2008/12/6/101
>
Looks like a reasonable approach, although I didn't check how much
additional stack the recursion will take. Bear in mind that printk()
can be called from super-deep contexts.
What would I need to do to make it recur more than once? Include a %pV
in a string, like dev_printk("%s", %%pV")?
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-04 21:45 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Sukadev Bhattiprolu, Pavel Emelyanov, Linux Netdev List,
containers, Netfilter Development Mailinglist, Ben Greear
In-Reply-To: <m13a0hmblr.fsf@fess.ebiederm.org>
ebiederm@xmission.com (Eric W. Biederman) writes:
> "Serge E. Hallyn" <serue@us.ibm.com> writes:
>
>> Quoting Eric W. Biederman (ebiederm@xmission.com):
>>> Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> writes:
>>>
>>> > Eric W. Biederman [ebiederm@xmission.com] wrote:
>>> > |
>>> > | I think replacing a struct pid for another struct pid allocated in
>>> > | descendant pid_namespace (but has all of the same struct upid values
>>> > | as the first struct pid) is a disastrous idea. It destroys the
>>> >
>>> > True. Sorry, I did not mean we would need a new 'struct pid' for an
>>> > existing process. I think we talked earlier of finding a way of attaching
>>> > additional pid numbers to the same struct pid.
>>>
>>> I just played with this and if you make the semantics of unshare(CLONE_NEWPID)
>>> to be that you become the idle task aka pid 0, and not the init task pid 1 the
>>> implementation is trivial.
>>
>> Heh, and then (browsing through your copy_process() patch hunks) the next
>> forked task becomes the child reaper for the new pidns? <shrug> why not
>> I guess.
>>
>> Now if that child reaper then gets killed, will the idle task get killed too?
>
> No.
>
>> And if not, then idle task can just re-populating the new pidns with new
>> idle tasks...
>
> After zap_pid_namespace interesting...
>
>> If this brought us a step closer to entering an existing pidns that would
>> be one thing, but is there actually any advantage to being able to
>> unshare a new pidns? Oh, I guess there is - PAM can then use it at
>> login, which might be neat.
>
> I have to say that the semantics of my patch are unworkable for
> unshare. Unless I am mistaken for PAM to use it requires that the
> current process fully change and become what it needs to be.
> Requiring an extra fork to fully complete the process is a problem.
>
> Scratch one bright idea.
Maybe not. I just looked and in the vast majority of cases the login
process goes like this.
{
setup stuff include pam
child = fork();
if (!child) {
setuid()
exec /bin/bash
}
waitpid(child);
pam and other cleanup
}
So an unshare of the pid namespace that doesn't really take effect
until we fork may actually be usable from pam, and in fact is probably
the preferred implementation. It looks like neither openssh nor login
from util-linux-ng will cope properly with getting any pid back from
wait() except the pid of their child. It looks like they both with
terminate. Which means if you login in a new pid namespace (where the
unsharing process becomes pid 1) and call nohup everything will get
killed and you will be logged out.
Eric
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Eric W. Biederman @ 2010-03-04 21:14 UTC (permalink / raw)
To: Octavian Purdila
Cc: Cong Wang, David Miller, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <201003042211.23460.opurdila@ixiacom.com>
Octavian Purdila <opurdila@ixiacom.com> writes:
> On Thursday 04 March 2010 21:14:07 you wrote:
>> Cong Wang <amwang@redhat.com> writes:
>> > David Miller wrote:
>> >> Eric B., could you look over the first two patches (which touch the
>> >> sysctl core) and give some review and ACK/NACK?
>> >
>> > ping Eric W. Biederman ... :)
>>
>> I have looked and it is not easy to tell by simple review if
>> correctness has been maintained in the proc cleanup patch.
>> Furthermore the code even after the cleanups still feels like code
>> that is trying too hard to do too much.
>>
>>
>> I think the example set by bitmap_parse_user in it's user interface is
>> a good example to follow when constructing bitmap helper functions.
>> Including having the actual parsing code should live in lib/bitmap.c
>>
>> The users of bitmap_parse_user do something very nice. They allocate
>> a temporary bitmap. Parse the userspace value into the temporary
>> bitmap, and then move that new bitmap into the kernel data structures.
>> For your large bitmap this seems like the idea way to handle it. That
>> guarantees no weird intermediate failure states, and really makes the
>> the bitmap feel like a single value.
>>
>> I would add the restriction that the values in the list of ranges
>> always must be increasing, and in general restrict the set of accepted
>> values as much as possible. If we don't accept it now we don't have
>> to worry about some userspace application relying on some unitended
>> side effect a few years into the future.
>>
>
> Eric, thanks for taking the time to go over it again. I now do share you
> opinion that we need to be more atomic. How about this simple approach:
>
> 1. Allocate new kernel buffer for the text and copy the whole userspace buffer
> into it.
> 2. Allocate temporary buffer for bitmap.
> 3. Parse the kernel text buffer into the temp bitmap.
> 4. Copy the temp bitmap into the final bitmap.
>
> This is simple and clean but it has the disadvantage of potentially allocating
> a large chunk of memory, although even in the case that all ports are going to
> be set the temporary buffer will not go over 390K, which is now not an issue
> anymore for kmalloc, right?
More than page size will always be an issue when memory is
sufficiently fragmented. I expect we can do just about as simply
with a small sliding window. Perhaps 50 characters.
However that might not be worth worrying about. Anything set by
a human being and anything we expect in practice is going to be
much shorter.
>> I think it is a serious bug that you clear the destination bitmap
>> in the middle of parsing it. That will either open or close all
>> ports in the middle of parsing, and I can't see how that would
>> ever be a good thing.
>>
>
> Even when doing the copy from the temp bitmap you still have some inconsistent
> bitmap state while copying.
>
> We could solve by replacing the old buffer with the new one + RCU.
True.
I was thinking a simple pass through that updated it a bit at a time
(if it was different), but rcu or some other form of locking would be
even more consistent. The joy with splitting the parser for the rest
of the sysctl code is that this is a separate decision that can be made
to accommodate the specific user.
Eric
^ permalink raw reply
* [PATCH] s2io: Fixing debug message
From: leitao @ 2010-03-04 20:40 UTC (permalink / raw)
To: sreenivasa.honnur; +Cc: netdev, Breno Leitao
Currently s2io is dumping debug messages using the interface name
before it was allocated, showing a message like the following:
s2io: eth%d: Ring Mem PHY: 0x7ef80000
s2io: s2io_reset: Resetting XFrame card eth%d
This patch just fixes it, printing the pci bus information for
the card instead of the interface name.
Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
---
drivers/net/s2io.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 3c4836d..2a913be 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -923,8 +923,8 @@ static int init_shared_mem(struct s2io_nic *nic)
tmp_v_addr = mac_control->stats_mem;
mac_control->stats_info = (struct stat_block *)tmp_v_addr;
memset(tmp_v_addr, 0, size);
- DBG_PRINT(INIT_DBG, "%s: Ring Mem PHY: 0x%llx\n", dev->name,
- (unsigned long long)tmp_p_addr);
+ DBG_PRINT(INIT_DBG, "%s: Ring Mem PHY: 0x%llx\n",
+ dev_name(&nic->pdev->dev), (unsigned long long)tmp_p_addr);
mac_control->stats_info->sw_stat.mem_allocated += mem_allocated;
return SUCCESS;
}
@@ -3480,7 +3480,7 @@ static void s2io_reset(struct s2io_nic *sp)
struct swStat *swstats;
DBG_PRINT(INIT_DBG, "%s: Resetting XFrame card %s\n",
- __func__, sp->dev->name);
+ __func__, pci_name(sp->pdev));
/* Back up the PCI-X CMD reg, dont want to lose MMRBC, OST settings */
pci_read_config_word(sp->pdev, PCIX_COMMAND_REGISTER, &(pci_cmd));
--
1.6.0.2
^ permalink raw reply related
* Re: [Open-FCoE] Request for Advice from Steve Ma
From: Steve Ma @ 2010-03-04 20:40 UTC (permalink / raw)
To: Joe Eykholt; +Cc: berthiaume_wayne, devel, netdev
In-Reply-To: <4B8EA858.8020308@cisco.com>
On Wed, Mar 3, 2010 at 10:20 AM, Joe Eykholt <jeykholt@cisco.com> wrote:
> berthiaume_wayne@emc.com wrote:
>>
>> Wouldn't it be better to pass this as part of the SCSI block so it would
>> be transport agnostic?
>
> I'm not sure I see what you mean.
>
> This is transport-specific already, where we're putting
> a FCP command together and sending it out via FCoE. When the NIC gets the
> sk_buff
> to send, it only has the FCP command and any meta-data in the sk_buff.
> There's no
> access to the scsi_cmnd, if that's what you mean. Putting a pointer to that
> in
> the sk_buff would solve the problem, but seems like a layering problem.
> What am I overlooking?
>
> See more comments at end.
>
>>
>> Regards,
>> Wayne.
>> -----Original Message-----
>> From: devel-bounces@open-fcoe.org [mailto:devel-bounces@open-fcoe.org] On
>> Behalf Of Steve Ma
>> Sent: Monday, March 01, 2010 6:17 PM
>> To: Joe Eykholt
>> Cc: devel@open-fcoe.org
>> Subject: Re: [Open-FCoE] Request for Advice from Steve Ma
>>
>> On Mon, Mar 1, 2010 at 2:39 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>
>>> Steve Ma wrote:
>>>>
>>>> On Mon, Mar 1, 2010 at 12:08 PM, Joe Eykholt <jeykholt@cisco.com> wrote:
>>>>>
>>>>> Steve Ma wrote:
>>>>>>
>>>>>> On Mon, Mar 1, 2010 at 11:38 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> Steve Ma wrote:
>>>>>>>>
>>>>>>>> On Mon, Mar 1, 2010 at 10:49 AM, Joe Eykholt <jeykholt@cisco.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>> Steve Ma wrote:
>>>>>>>>>>
>>>>>>>>>> On Mon, Mar 1, 2010 at 10:06 AM, Robert Love
>>>>>>>>>> <robert.w.love@intel.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Mon, 2010-03-01 at 10:04 -0800, Robert Love wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Mon, 2010-03-01 at 09:26 -0800, Steve Ma wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Guys,
>>>>>>>>>>>>>
>>>>>>>>>>>> Hi Steve
>>>>>>>>>>>>
>>>>>>>>>>>>> When the SCSI mid-layer passes a SCSI command to the Open-FCoE,
>>>>>>>>>>>>> the
>>>>>>>>>>>>> fc_queuecomamnd receives the pointer of struct scsi_cmnd. This
>>>>>>>>>>>>> data
>>>>>>>>>>>>> structure contains two scatter-gather lists:
>>>>>>>>>>>>>
>>>>>>>>>>>>> struct scsi_data_buffer sdb;
>>>>>>>>>>>>> struct scsi_data_buffer *prot_sdb;
>>>>>>>>>>>>>
>>>>>>>>>>>>> The Open-FCoE stack does transmitting and receiving of sdb
>>>>>>>>>>>>> to/from
>>>>>>>>>>>>> the
>>>>>>>>>>>>> NIC driver. Now I am in need of transmitting and receiving both
>>>>>>>>>>>>> of
>>>>>>>>>>>>> them to the NIC driver and, in addition, I also need to pass
>>>>>>>>>>>>> down
>>>>>>>>>>>>> the
>>>>>>>>>>>>> return value of the mid layer routine
>>>>>>>>>>>>>
>>>>>>>>>>>>> unsigned char scsi_get_prot_op(struct scsi_cmnd *scmd)
>>>>>>>>>>>>>
>>>>>>>>>>>>> What would be the best way to implement?
>>>>>>>>>>>>
>>>>>>>>>>>> At a high level you're going to need to go through netdev so
>>>>>>>>>>>> engaging
>>>>>>>>>>>> their mailing list would be a good start (CC'ing this list would
>>>>>>>>>>>> be
>>>>>>>>>>>> nice). For FCoE offloaded devices that are netdev based we have
>>>>>>>>>>>> these
>>>>>>>>>>>> routines in the netdev structure-
>>>>>>>>>>>
>>>>>>>>>>> This should say, "in struct net_device_ops."
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I have already seen those ndo routines. I may be able to make use
>>>>>>>>>> of
>>>>>>>>>> the DDP rotuines. But for transmit, I am still not sure what the
>>>>>>>>>> best
>>>>>>>>>> way to do.
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> Steve
>>>>>>>>>
>>>>>>>>> Is the extra buffer descriptor for DIF bytes or what?
>>>>>>>>
>>>>>>>> Yes, there are 8-bytes of protection data for DIF/DIX.
>>>>>>>>
>>>>>>>>> How would it be supported and used in a non-offloaded case?
>>>>>>>>
>>>>>>>> Currently we are intended to do this in offloaded case.
>>>>>>>> I would disable the protection if offload (LSO/DDP) is disabled.
>>>>>>>>
>>>>>>>>> I mean, what would the code look like in fc_fcp.c?
>>>>>>>>
>>>>>>>> Not sure yet. I will try to make the code as simple as possible in
>>>>>>>> the
>>>>>>>> fcoe level, and do as much work as possible in the NIC driver and
>>>>>>>> the
>>>>>>>> ASIC.
>>>>>>>>
>>>>>>>>> What should the frames look like? Would there just be additional
>>>>>>>>> DIF bytes sent/received after every so many data bytes? If so,
>>>>>>>>> would it work (from a protocol standpoint, maybe not efficiently)
>>>>>>>>> to just merge the sg_lists?
>>>>>>>>
>>>>>>>> I have the same thought of merging the sglists, and in the NIC
>>>>>>>> driver
>>>>>>>> if I can figure out the correct pointer to the 8-bytes at the end, I
>>>>>>>> can assign an additional TxD for it. I am not sure if I have better
>>>>>>>> choice.
>>>>>>>
>>>>>>> If the NIC driver is just going to merge the SG lists, then why
>>>>>>> not do it at the fc_fcp layer? That way it would work for the
>>>>>>> non-offloaded case as well, and you wouldn't need any additional
>>>>>>> offload interfaces.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Joe
>>>>>>>
>>>>>>>
>>>>>> No, what I am thinking is to merge the two sglists in fc_fcp, thus the
>>>>>> 8-byte protection data will be part of the frame (skb) passed down to
>>>>>> the NIC driver.
>>>>>
>>>>> That's what I was suggesting. I thought you were suggesting passing
>>>>> both SG lists separately to the NIC. Perhaps I misunderstood.
>>>>>
>>>>>> It will be a driver for our future NIC. In the NIC driver, I will have
>>>>>> to assign TxDs for the frame data, and an additional TxD for the
>>>>>> 8-byte. So to my understanding, the NIC driver is to separate the
>>>>>> frame data and the 8-byte tag fields. I would let the ASIC to to the
>>>>>> validation job for the protection data. If there is no-offload, the
>>>>>> validation would be a big performance burden if they are done in the
>>>>>> OS driver.
>>>>>
>>>>> Wouldn't DIF need to be validated again when the data is used?
>>>>
>>>> This depends on the value returned from scsi_get_prot_op(). Either
>>>> OS<-->HBA is protected or HBA<-->Target is protected or both
>>>> (end-to-end). HBA is a checkpoint we are responsible for.
>>>>
>>>>> Otherwise other parts of the data path aren't covered ... the
>>>>> DIF wouldn't be completely end-to-end. But verification at
>>>>> multiple points along the path is good.
>>>>>
>>>>> I was mostly thinking about sending the DIF where it's already
>>>>> been verified. Receiving the DIF is more difficult since you
>>>>> don't know exactly how the data will be divided into frames,
>>>>> but it's the same effect: a merged SG list.
>>>>
>>>> With your support, this is the best approach I can work on.
>>>>
>>>> BTW. I want to pass the return value of scsi_get_prot_op() from fc_fcp
>>>> to the NIC driver during transmitting of a SCSI command, what would be
>>>> the best approach? It is only a byte.
>>>
>>> You could pass it to fcoe_xmit() or whatever LLD in the fcoe_rcv_info.
>>> There's only one byte free there (without stretching skb->cb[]),
>>> but you could encode it in fr_flags, which has 7 bits free.
>>> skb->cb[] can't be used for this outside of libfc/fcoe though,
>>> because it's allowed to be reused by any intermediate layers.
>>> But since skb->cb also contains the fr_fsp, you could put it in there,
>>> and call scsi_get_prot_op() from the fcoe if needed.
>>>
>>> After fcoe_xmit(), prot_op could be passed to the NIC some other way.
>>> Perhaps by appending it after the data (to be stripped off later),
>>> or using some existing field in the sk_buff.
>>>
>>> This is a long-winded way of saying "I don't know." Someone who
>>> deals more with the guts of sk_buffs will probably suggest something.
>>>
>>> Joe
>>>
>>>
>>>
>> Appending seems to be a clean way to do. So I will have 9 bytes in
>> total appended to the data frame.
>>
>> Thanks
>> Steve
>
> I regret suggesting appending. I no longer thing that's such a good idea.
> If it is after the FCoE Encapsulation, then it might be OK, but we would
> have to know that the NIC will take it off. Also, it will look wrong
> for tcpdump.
>
> Look into other ways of attaching meta-data to sk_buffs. There might
> be something already available.
>
> Joe
>
>
I'll keep on looking.
Thanks.
Steve Ma
^ permalink raw reply
* Re: 2.6.33 dies on modprobe
From: M G Berberich @ 2010-03-04 20:38 UTC (permalink / raw)
To: Américo Wang
Cc: linux-kernel, Linux Kernel Network Developers, Greg Kroah-Hartman
In-Reply-To: <20100304153042.GA4614@hack>
Am Donnerstag, den 04. März schrieb Américo Wang:
> On Thu, Mar 04, 2010 at 12:04:38AM +0100, M G Berberich wrote:
> >Am Mittwoch, den 03. März schrieb Américo Wang:
> >> On Wed, Mar 3, 2010 at 5:08 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> > On Wed, Mar 3, 2010 at 4:58 PM, Américo Wang <xiyou.wangcong@gmail.com> wrote:
> >> >> Ok, below is my patch, I am not sure it could fix the BUG for you,
> >> >> but it could fix the WARNING. But perhaps they are related.
> >> >>
> >> >> Please give it a try.
> >> >>
> >> >
> >> > Oops! Ignore the patch, it should not be correct, I will send a
> >> > correct version soon.
> >> > Sorry.
> >> >
> >>
> >> Here we go:
> >
> >Seems to fix the warning, but not the modprobe-Oops.
> >
>
> Thanks much for your testing!
>
> Could you please try to turn on DEBUG_KOBJECT and boot your kernel
> with "initcall_debug"? And then show us your dmesg.
If I'm doing this (enabled CONFIG_DEBUG_KERNEL and
CONFIG_DEBUG_KOBJECT), the kernel dies immediately - no output.
MfG
bmg
--
„Des is völlig wurscht, was heut beschlos- | M G Berberich
sen wird: I bin sowieso dagegn!“ | berberic@fmi.uni-passau.de
(SPD-Stadtrat Kurt Schindler; Regensburg) | www.fmi.uni-passau.de/~berberic
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox