* [PATCH net-next 08/16] tipc: Optimize re-initialization of port message header templates
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Removes an unnecessary check in the logic that updates the message
header template for existing ports when a node's network address is
first assigned. There is no longer any need to check to see if the
node's network address has actually changed since the calling routine
has already verified that this is so.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 94d2904..6156c67 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -646,8 +646,6 @@ void tipc_port_reinit(void)
spin_lock_bh(&tipc_port_list_lock);
list_for_each_entry(p_ptr, &ports, port_list) {
msg = &p_ptr->phdr;
- if (msg_orignode(msg) == tipc_own_addr)
- break;
msg_set_prevnode(msg, tipc_own_addr);
msg_set_orignode(msg, tipc_own_addr);
}
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 09/16] tipc: Ensure network address change doesn't impact new port
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Re-orders port creation logic so that the initialization of a new
port's message header template occurs while the port list lock is
held. This ensures that a change to the node's network address that
occurs at the same time as the port is being created does not result
in the template identifying the sender using the former network
address. The new approach guarantees that the new port's template is
using the current network address or that it will be updated when
the address changes.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 6156c67..3b7162c 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -221,18 +221,25 @@ struct tipc_port *tipc_createport_raw(void *usr_handle,
p_ptr->usr_handle = usr_handle;
p_ptr->max_pkt = MAX_PKT_DEFAULT;
p_ptr->ref = ref;
- msg = &p_ptr->phdr;
- tipc_msg_init(msg, importance, TIPC_NAMED_MSG, NAMED_H_SIZE, 0);
- msg_set_origport(msg, ref);
INIT_LIST_HEAD(&p_ptr->wait_list);
INIT_LIST_HEAD(&p_ptr->subscription.nodesub_list);
p_ptr->dispatcher = dispatcher;
p_ptr->wakeup = wakeup;
p_ptr->user_port = NULL;
k_init_timer(&p_ptr->timer, (Handler)port_timeout, ref);
- spin_lock_bh(&tipc_port_list_lock);
INIT_LIST_HEAD(&p_ptr->publications);
INIT_LIST_HEAD(&p_ptr->port_list);
+
+ /*
+ * Must hold port list lock while initializing message header template
+ * to ensure a change to node's own network address doesn't result
+ * in template containing out-dated network address information
+ */
+
+ spin_lock_bh(&tipc_port_list_lock);
+ msg = &p_ptr->phdr;
+ tipc_msg_init(msg, importance, TIPC_NAMED_MSG, NAMED_H_SIZE, 0);
+ msg_set_origport(msg, ref);
list_add_tail(&p_ptr->port_list, &ports);
spin_unlock_bh(&tipc_port_list_lock);
return p_ptr;
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 11/16] tipc: Ensure network address change doesn't impact local connections
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Revises routines that deal with connections between two ports on
the same node to ensure the connection is not impacted if the node's
network address is changed in mid-operation. The routines now treat
the default node address of <0.0.0> as an alias for "this node" in
the following situations:
1) Incoming messages destined to a connected port now handle the alias
properly when validating that the message was sent by the expected
peer port, ensuring that the message will be accepted regardless of
whether it specifies the node's old network address or it's current one.
2) The code which completes connection establishment now handles the
alias properly when determining if the peer port is on the same node
as the connected port.
An added benefit of addressing issue 1) is that some peer port
validation code has been relocated to TIPC's socket subsystem, which
means that validation is no longer done twice when a message is
sent to a non-socket port (such as TIPC's configuration service or
network topology service).
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/node_subscr.c | 2 +-
net/tipc/port.c | 59 ++++++++++++++++++++++++------------------------
net/tipc/port.h | 1 +
net/tipc/socket.c | 3 ++-
4 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/net/tipc/node_subscr.c b/net/tipc/node_subscr.c
index c3c2815..327ffbb 100644
--- a/net/tipc/node_subscr.c
+++ b/net/tipc/node_subscr.c
@@ -45,7 +45,7 @@
void tipc_nodesub_subscribe(struct tipc_node_subscr *node_sub, u32 addr,
void *usr_handle, net_ev_handler handle_down)
{
- if (addr == tipc_own_addr) {
+ if (in_own_node(addr)) {
node_sub->node = NULL;
return;
}
diff --git a/net/tipc/port.c b/net/tipc/port.c
index f1f6b33..616c72f 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -69,6 +69,28 @@ static inline u32 port_peerport(struct tipc_port *p_ptr)
return msg_destport(&p_ptr->phdr);
}
+/*
+ * tipc_port_peer_msg - verify message was sent by connected port's peer
+ *
+ * Handles cases where the node's network address has changed from
+ * the default of <0.0.0> to its configured setting.
+ */
+
+int tipc_port_peer_msg(struct tipc_port *p_ptr, struct tipc_msg *msg)
+{
+ u32 peernode;
+ u32 orignode;
+
+ if (msg_origport(msg) != port_peerport(p_ptr))
+ return 0;
+
+ orignode = msg_orignode(msg);
+ peernode = port_peernode(p_ptr);
+ return (orignode == peernode) ||
+ (!orignode && (peernode == tipc_own_addr)) ||
+ (!peernode && (orignode == tipc_own_addr));
+}
+
/**
* tipc_multicast - send a multicast message to local and remote destinations
*/
@@ -526,25 +548,21 @@ void tipc_port_recv_proto_msg(struct sk_buff *buf)
struct tipc_msg *msg = buf_msg(buf);
struct tipc_port *p_ptr;
struct sk_buff *r_buf = NULL;
- u32 orignode = msg_orignode(msg);
- u32 origport = msg_origport(msg);
u32 destport = msg_destport(msg);
int wakeable;
/* Validate connection */
p_ptr = tipc_port_lock(destport);
- if (!p_ptr || !p_ptr->connected ||
- (port_peernode(p_ptr) != orignode) ||
- (port_peerport(p_ptr) != origport)) {
+ if (!p_ptr || !p_ptr->connected || !tipc_port_peer_msg(p_ptr, msg)) {
r_buf = tipc_buf_acquire(BASIC_H_SIZE);
if (r_buf) {
msg = buf_msg(r_buf);
tipc_msg_init(msg, TIPC_HIGH_IMPORTANCE, TIPC_CONN_MSG,
- BASIC_H_SIZE, orignode);
+ BASIC_H_SIZE, msg_orignode(msg));
msg_set_errcode(msg, TIPC_ERR_NO_PORT);
msg_set_origport(msg, destport);
- msg_set_destport(msg, origport);
+ msg_set_destport(msg, msg_origport(msg));
}
if (p_ptr)
tipc_port_unlock(p_ptr);
@@ -681,6 +699,7 @@ static void port_dispatcher_sigh(void *dummy)
struct tipc_name_seq dseq;
void *usr_handle;
int connected;
+ int peer_invalid;
int published;
u32 message_type;
@@ -701,6 +720,7 @@ static void port_dispatcher_sigh(void *dummy)
up_ptr = p_ptr->user_port;
usr_handle = up_ptr->usr_handle;
connected = p_ptr->connected;
+ peer_invalid = connected && !tipc_port_peer_msg(p_ptr, msg);
published = p_ptr->published;
if (unlikely(msg_errcode(msg)))
@@ -710,8 +730,6 @@ static void port_dispatcher_sigh(void *dummy)
case TIPC_CONN_MSG:{
tipc_conn_msg_event cb = up_ptr->conn_msg_cb;
- u32 peer_port = port_peerport(p_ptr);
- u32 peer_node = port_peernode(p_ptr);
u32 dsz;
tipc_port_unlock(p_ptr);
@@ -720,8 +738,7 @@ static void port_dispatcher_sigh(void *dummy)
if (unlikely(!connected)) {
if (tipc_connect2port(dref, &orig))
goto reject;
- } else if ((msg_origport(msg) != peer_port) ||
- (msg_orignode(msg) != peer_node))
+ } else if (peer_invalid)
goto reject;
dsz = msg_data_sz(msg);
if (unlikely(dsz &&
@@ -773,14 +790,9 @@ err:
case TIPC_CONN_MSG:{
tipc_conn_shutdown_event cb =
up_ptr->conn_err_cb;
- u32 peer_port = port_peerport(p_ptr);
- u32 peer_node = port_peernode(p_ptr);
tipc_port_unlock(p_ptr);
- if (!cb || !connected)
- break;
- if ((msg_origport(msg) != peer_port) ||
- (msg_orignode(msg) != peer_node))
+ if (!cb || !connected || peer_invalid)
break;
tipc_disconnect(dref);
skb_pull(buf, msg_hdr_sz(msg));
@@ -1157,17 +1169,6 @@ int tipc_port_recv_msg(struct sk_buff *buf)
/* validate destination & pass to port, otherwise reject message */
p_ptr = tipc_port_lock(destport);
if (likely(p_ptr)) {
- if (likely(p_ptr->connected)) {
- if ((unlikely(msg_origport(msg) !=
- port_peerport(p_ptr))) ||
- (unlikely(msg_orignode(msg) !=
- port_peernode(p_ptr))) ||
- (unlikely(!msg_connected(msg)))) {
- err = TIPC_ERR_NO_PORT;
- tipc_port_unlock(p_ptr);
- goto reject;
- }
- }
err = p_ptr->dispatcher(p_ptr, buf);
tipc_port_unlock(p_ptr);
if (likely(!err))
@@ -1175,7 +1176,7 @@ int tipc_port_recv_msg(struct sk_buff *buf)
} else {
err = TIPC_ERR_NO_PORT;
}
-reject:
+
return tipc_reject_msg(buf, err);
}
diff --git a/net/tipc/port.h b/net/tipc/port.h
index 0a632a6..301e1bd 100644
--- a/net/tipc/port.h
+++ b/net/tipc/port.h
@@ -201,6 +201,7 @@ int tipc_shutdown(u32 ref);
* The following routines require that the port be locked on entry
*/
int tipc_disconnect_port(struct tipc_port *tp_ptr);
+int tipc_port_peer_msg(struct tipc_port *p_ptr, struct tipc_msg *msg);
/*
* TIPC messaging routines
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index bcb3314..c19fc4a 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -1236,7 +1236,8 @@ static u32 filter_rcv(struct sock *sk, struct sk_buff *buf)
if (msg_mcast(msg))
return TIPC_ERR_NO_PORT;
if (sock->state == SS_CONNECTED) {
- if (!msg_connected(msg))
+ if (!msg_connected(msg) ||
+ !tipc_port_peer_msg(tipc_sk_port(sk), msg))
return TIPC_ERR_NO_PORT;
} else if (sock->state == SS_CONNECTING) {
if (!msg_connected(msg) && (msg_errcode(msg) == 0))
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 10/16] tipc: delete duplicate peerport/peernode helper functions
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Prior to commit 23dd4cce387124ec3ea06ca30d17854ae4d9b772
"tipc: Combine port structure with tipc_port structure"
there was a need for the two sets of helper functions. But
now they are just duplicates. Remove the globally visible
ones, and mark the remaining ones as inline.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 8 ++++----
net/tipc/port.h | 10 ----------
2 files changed, 4 insertions(+), 14 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 3b7162c..f1f6b33 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -59,12 +59,12 @@ static struct sk_buff *port_build_peer_abort_msg(struct tipc_port *, u32 err);
static void port_timeout(unsigned long ref);
-static u32 port_peernode(struct tipc_port *p_ptr)
+static inline u32 port_peernode(struct tipc_port *p_ptr)
{
return msg_destnode(&p_ptr->phdr);
}
-static u32 port_peerport(struct tipc_port *p_ptr)
+static inline u32 port_peerport(struct tipc_port *p_ptr)
{
return msg_destport(&p_ptr->phdr);
}
@@ -1159,9 +1159,9 @@ int tipc_port_recv_msg(struct sk_buff *buf)
if (likely(p_ptr)) {
if (likely(p_ptr->connected)) {
if ((unlikely(msg_origport(msg) !=
- tipc_peer_port(p_ptr))) ||
+ port_peerport(p_ptr))) ||
(unlikely(msg_orignode(msg) !=
- tipc_peer_node(p_ptr))) ||
+ port_peernode(p_ptr))) ||
(unlikely(!msg_connected(msg)))) {
err = TIPC_ERR_NO_PORT;
tipc_port_unlock(p_ptr);
diff --git a/net/tipc/port.h b/net/tipc/port.h
index 9b88531..0a632a6 100644
--- a/net/tipc/port.h
+++ b/net/tipc/port.h
@@ -257,16 +257,6 @@ static inline struct tipc_port *tipc_port_deref(u32 ref)
return (struct tipc_port *)tipc_ref_deref(ref);
}
-static inline u32 tipc_peer_port(struct tipc_port *p_ptr)
-{
- return msg_destport(&p_ptr->phdr);
-}
-
-static inline u32 tipc_peer_node(struct tipc_port *p_ptr)
-{
- return msg_destnode(&p_ptr->phdr);
-}
-
static inline int tipc_port_congested(struct tipc_port *p_ptr)
{
return (p_ptr->sent - p_ptr->acked) >= (TIPC_FLOW_CONTROL_WIN * 2);
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 12/16] tipc: take lock while updating node network address
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
The routine that changes the node's network address now takes TIPC's
network lock in write mode while the main address variable and associated
data structures are being changed; this is needed to ensure that the
link subsystem won't attempt to send a message off-node until the sending
port's message header template has been updated with the node's new
network address.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/net.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/net.c b/net/tipc/net.c
index d4531b0..5fab4ff 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -178,11 +178,12 @@ int tipc_net_start(u32 addr)
tipc_subscr_stop();
tipc_cfg_stop();
+ write_lock_bh(&tipc_net_lock);
tipc_own_addr = addr;
tipc_named_reinit();
tipc_port_reinit();
-
tipc_bclink_init();
+ write_unlock_bh(&tipc_net_lock);
tipc_k_signal((Handler)tipc_subscr_start, 0);
tipc_k_signal((Handler)tipc_cfg_init, 0);
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 13/16] tipc: properly handle off-node send requests with invalid addr
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
There are two send routines that might conceivably be asked by an
application to send a message off-node when the node is still using
the default network address. These now have an added check that
detects this and rejects the message gracefully.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index 616c72f..dc7f916 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -1270,10 +1270,14 @@ int tipc_send2name(u32 ref, struct tipc_name const *name, unsigned int domain,
if (likely(destnode == tipc_own_addr))
res = tipc_port_recv_sections(p_ptr, num_sect,
msg_sect, total_len);
- else
+ else if (tipc_own_addr)
res = tipc_link_send_sections_fast(p_ptr, msg_sect,
num_sect, total_len,
destnode);
+ else
+ res = tipc_port_reject_sections(p_ptr, msg, msg_sect,
+ num_sect, total_len,
+ TIPC_ERR_NO_NODE);
if (likely(res != -ELINKCONG)) {
if (res > 0)
p_ptr->sent++;
@@ -1314,9 +1318,12 @@ int tipc_send2port(u32 ref, struct tipc_portid const *dest,
if (dest->node == tipc_own_addr)
res = tipc_port_recv_sections(p_ptr, num_sect, msg_sect,
total_len);
- else
+ else if (tipc_own_addr)
res = tipc_link_send_sections_fast(p_ptr, msg_sect, num_sect,
total_len, dest->node);
+ else
+ res = tipc_port_reject_sections(p_ptr, msg, msg_sect, num_sect,
+ total_len, TIPC_ERR_NO_NODE);
if (likely(res != -ELINKCONG)) {
if (res > 0)
p_ptr->sent++;
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 14/16] tipc: handle <0.0.0> as an alias for this node on outgoing msgs
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Revises handling of send routines for payload messages to ensure that
they are processed properly even if the node's network address is
changed in mid-operation. The routines now treat the default node
address of <0.0.0> as an alias for "this node" when determining where
to send an outgoing message.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index dc7f916..c50819b 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -1217,7 +1217,7 @@ int tipc_send(u32 ref, unsigned int num_sect, struct iovec const *msg_sect,
p_ptr->congested = 1;
if (!tipc_port_congested(p_ptr)) {
destnode = port_peernode(p_ptr);
- if (likely(destnode != tipc_own_addr))
+ if (likely(!in_own_node(destnode)))
res = tipc_link_send_sections_fast(p_ptr, msg_sect, num_sect,
total_len, destnode);
else
@@ -1267,7 +1267,7 @@ int tipc_send2name(u32 ref, struct tipc_name const *name, unsigned int domain,
msg_set_destport(msg, destport);
if (likely(destport || destnode)) {
- if (likely(destnode == tipc_own_addr))
+ if (likely(in_own_node(destnode)))
res = tipc_port_recv_sections(p_ptr, num_sect,
msg_sect, total_len);
else if (tipc_own_addr)
@@ -1315,7 +1315,7 @@ int tipc_send2port(u32 ref, struct tipc_portid const *dest,
msg_set_destport(msg, dest->ref);
msg_set_hdr_sz(msg, BASIC_H_SIZE);
- if (dest->node == tipc_own_addr)
+ if (in_own_node(dest->node))
res = tipc_port_recv_sections(p_ptr, num_sect, msg_sect,
total_len);
else if (tipc_own_addr)
@@ -1362,7 +1362,7 @@ int tipc_send_buf2port(u32 ref, struct tipc_portid const *dest,
skb_push(buf, BASIC_H_SIZE);
skb_copy_to_linear_data(buf, msg, BASIC_H_SIZE);
- if (dest->node == tipc_own_addr)
+ if (in_own_node(dest->node))
res = tipc_port_recv_msg(buf);
else
res = tipc_send_buf_fast(buf, dest->node);
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 15/16] tipc: Ensure network address change doesn't impact rejected message
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Revises handling of a rejected message to ensure that a locally
originated message is returned properly even if the node's network
address is changed in mid-operation. The routine now treats the
default node address of <0.0.0> as an alias for "this node" when
determining where to send a returned message.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/port.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/port.c b/net/tipc/port.c
index c50819b..0f40b10 100644
--- a/net/tipc/port.c
+++ b/net/tipc/port.c
@@ -444,7 +444,7 @@ int tipc_reject_msg(struct sk_buff *buf, u32 err)
/* send returned message & dispose of rejected message */
src_node = msg_prevnode(msg);
- if (src_node == tipc_own_addr)
+ if (in_own_node(src_node))
tipc_port_recv_msg(rbuf);
else
tipc_link_send(rbuf, src_node, msg_link_selector(rmsg));
--
1.7.9.3
^ permalink raw reply related
* [PATCH net-next 16/16] tipc: Ensure network address change doesn't impact configuration service
From: Paul Gortmaker @ 2012-04-20 21:05 UTC (permalink / raw)
To: davem; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Allan Stephens <allan.stephens@windriver.com>
Enhances command validation done by TIPC's configuration service so
that it works properly even if the node's network address is changed in
mid-operation. The default node address of <0.0.0> is now recognized as an
alias for "this node" even after a new network address has been assigned.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tipc/config.c b/net/tipc/config.c
index f76d3b1..f5458ed 100644
--- a/net/tipc/config.c
+++ b/net/tipc/config.c
@@ -290,7 +290,7 @@ struct sk_buff *tipc_cfg_do_cmd(u32 orig_node, u16 cmd, const void *request_area
/* Check command authorization */
- if (likely(orig_node == tipc_own_addr)) {
+ if (likely(in_own_node(orig_node))) {
/* command is permitted */
} else if (cmd >= 0x8000) {
rep_tlv_buf = tipc_cfg_reply_error_string(TIPC_CFG_NOT_SUPPORTED
--
1.7.9.3
^ permalink raw reply related
* Re: [net-next 04/14] net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state
From: Tom Herbert @ 2012-04-20 21:19 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Alexander Duyck, netdev, gospo, sassmann
In-Reply-To: <1331611432-30109-5-git-send-email-jeffrey.t.kirsher@intel.com>
Hi Jeff,
> @@ -3244,7 +3246,6 @@ static void igb_clean_tx_ring(struct igb_ring *tx_ring)
> buffer_info = &tx_ring->tx_buffer_info[i];
> igb_unmap_and_free_tx_resource(tx_ring, buffer_info);
> }
> - netdev_tx_reset_queue(txring_txq(tx_ring));
>
Why is it necessary to remove this? If rings are being freed with
going through completion path then we would need this reset. Is this
being done elsewhere maybe?
Tom
^ permalink raw reply
* Re: e1000e tx queue timeout in 3.3.0 (bisected to BQL support for e1000e)
From: Tom Herbert @ 2012-04-20 21:21 UTC (permalink / raw)
To: John Fastabend; +Cc: Ben Greear, netdev, e1000-devel list, Eric Dumazet
In-Reply-To: <4F91BC82.2000804@intel.com>
Thanks John for pointers to those. Ben, are you running a kernel with
these patches?
Tom
>
> Tom, did you see these two patches? Maybe this is resolved by
> the second patch.
>
> We needed these to fixup ixgbe and igb (i didn't test e1000e)
> looks like we might want to push these at stable. I don't
> believe they are in 3.3.
>
> commit b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
> Author: Alexander Duyck <alexander.h.duyck@intel.com>
> Date: Tue Feb 7 02:29:06 2012 +0000
>
> net: Add memory barriers to prevent possible race in byte queue limits
>
> This change adds a memory barrier to the byte queue limit code to address a
> possible race as has been seen in the past with the
> netif_stop_queue/netif_wake_queue logic.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Stephen Ko <stephen.s.ko@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
>
>
> commit 5c4903549c05bbb373479e0ce2992573c120654a
> Author: Alexander Duyck <alexander.h.duyck@intel.com>
> Date: Tue Feb 7 02:29:01 2012 +0000
>
> net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state
>
> We are seeing dev_watchdog hangs on several drivers. I suspect this is due
> to the __QUEUE_STATE_STACK_XOFF bit being set prior to a reset for link
> change, and then not being cleared by netdev_tx_reset_queue. This change
> corrects that.
>
> In addition we were seeing dev_watchdog hangs on igb after running the
> ethtool tests. We found this to be due to the fact that the ethtool test
> runs the same logic as ndo_start_xmit, but we were never clearing the XOFF
> flag since the loopback test in ethtool does not do byte queue accounting.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Tested-by: Stephen Ko <stephen.s.ko@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c4903549c05bbb373479e0ce2992573c120654a
>
>
^ permalink raw reply
* Re: e1000e tx queue timeout in 3.3.0 (bisected to BQL support for e1000e)
From: Ben Greear @ 2012-04-20 21:24 UTC (permalink / raw)
To: Tom Herbert; +Cc: John Fastabend, netdev, e1000-devel list
In-Reply-To: <CA+mtBx-K6-O3ea34kmn4LXtx73FqCHF16w7pBSq1fHNN+-DxCA@mail.gmail.com>
On 04/20/2012 02:21 PM, Tom Herbert wrote:
> Thanks John for pointers to those. Ben, are you running a kernel with
> these patches?
I don't think so. I'll add them and re-test.
Thanks,
Ben
>
> Tom
>
>>
>> Tom, did you see these two patches? Maybe this is resolved by
>> the second patch.
>>
>> We needed these to fixup ixgbe and igb (i didn't test e1000e)
>> looks like we might want to push these at stable. I don't
>> believe they are in 3.3.
>>
>> commit b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
>> Author: Alexander Duyck<alexander.h.duyck@intel.com>
>> Date: Tue Feb 7 02:29:06 2012 +0000
>>
>> net: Add memory barriers to prevent possible race in byte queue limits
>>
>> This change adds a memory barrier to the byte queue limit code to address a
>> possible race as has been seen in the past with the
>> netif_stop_queue/netif_wake_queue logic.
>>
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@intel.com>
>> Tested-by: Stephen Ko<stephen.s.ko@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>>
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
>>
>>
>> commit 5c4903549c05bbb373479e0ce2992573c120654a
>> Author: Alexander Duyck<alexander.h.duyck@intel.com>
>> Date: Tue Feb 7 02:29:01 2012 +0000
>>
>> net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state
>>
>> We are seeing dev_watchdog hangs on several drivers. I suspect this is due
>> to the __QUEUE_STATE_STACK_XOFF bit being set prior to a reset for link
>> change, and then not being cleared by netdev_tx_reset_queue. This change
>> corrects that.
>>
>> In addition we were seeing dev_watchdog hangs on igb after running the
>> ethtool tests. We found this to be due to the fact that the ethtool test
>> runs the same logic as ndo_start_xmit, but we were never clearing the XOFF
>> flag since the loopback test in ethtool does not do byte queue accounting.
>>
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@intel.com>
>> Tested-by: Stephen Ko<stephen.s.ko@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c4903549c05bbb373479e0ce2992573c120654a
>>
>>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [net-next 04/14] net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state
From: John Fastabend @ 2012-04-20 21:31 UTC (permalink / raw)
To: Tom Herbert, Alexander Duyck; +Cc: Jeff Kirsher, davem, netdev, gospo, sassmann
In-Reply-To: <CA+mtBx-BWy=KANgSca9f2vxuu1Km_kPMJbQFa+Nq9q_YYvatBQ@mail.gmail.com>
On 4/20/2012 2:19 PM, Tom Herbert wrote:
> Hi Jeff,
>
>> @@ -3244,7 +3246,6 @@ static void igb_clean_tx_ring(struct igb_ring *tx_ring)
>> buffer_info = &tx_ring->tx_buffer_info[i];
>> igb_unmap_and_free_tx_resource(tx_ring, buffer_info);
>> }
>> - netdev_tx_reset_queue(txring_txq(tx_ring));
>>
> Why is it necessary to remove this? If rings are being freed with
> going through completion path then we would need this reset. Is this
> being done elsewhere maybe?
>
Alex moved this into the igb_configure_tx_ring() iirc to catch an ethtool
test case. He assured me when I reviewed the patch that igb_configure_tx_ring()
would always be called before netif_carrier_on() so I think(?) that should
be OK.
The above removed case was called after netif_carrier_off() anyways.
> Tom
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Hello Dear
From: Mis Tuma Ahoussou @ 2012-04-20 21:45 UTC (permalink / raw)
Hello Dear
How are you doing i hope
you are in a good health?
i hope to hear from you soon.
My Regards
Tuma.
^ permalink raw reply
* Re: e1000e tx queue timeout in 3.3.0 (bisected to BQL support for e1000e)
From: Ben Greear @ 2012-04-20 21:56 UTC (permalink / raw)
To: Tom Herbert; +Cc: John Fastabend, netdev, e1000-devel list
In-Reply-To: <CA+mtBx-K6-O3ea34kmn4LXtx73FqCHF16w7pBSq1fHNN+-DxCA@mail.gmail.com>
On 04/20/2012 02:21 PM, Tom Herbert wrote:
> Thanks John for pointers to those. Ben, are you running a kernel with
> these patches?
I just tested this on my e1000e and igb machine. With these patches,
I can no longer reproduce the problem.
So, please make sure those are queued up for 3.3 stable!
Thanks,
Ben
>
> Tom
>
>>
>> Tom, did you see these two patches? Maybe this is resolved by
>> the second patch.
>>
>> We needed these to fixup ixgbe and igb (i didn't test e1000e)
>> looks like we might want to push these at stable. I don't
>> believe they are in 3.3.
>>
>> commit b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
>> Author: Alexander Duyck<alexander.h.duyck@intel.com>
>> Date: Tue Feb 7 02:29:06 2012 +0000
>>
>> net: Add memory barriers to prevent possible race in byte queue limits
>>
>> This change adds a memory barrier to the byte queue limit code to address a
>> possible race as has been seen in the past with the
>> netif_stop_queue/netif_wake_queue logic.
>>
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@intel.com>
>> Tested-by: Stephen Ko<stephen.s.ko@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>>
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b37c0fbe3f6dfba1f8ad2aed47fb40578a254635
>>
>>
>> commit 5c4903549c05bbb373479e0ce2992573c120654a
>> Author: Alexander Duyck<alexander.h.duyck@intel.com>
>> Date: Tue Feb 7 02:29:01 2012 +0000
>>
>> net: Fix issue with netdev_tx_reset_queue not resetting queue from XOFF state
>>
>> We are seeing dev_watchdog hangs on several drivers. I suspect this is due
>> to the __QUEUE_STATE_STACK_XOFF bit being set prior to a reset for link
>> change, and then not being cleared by netdev_tx_reset_queue. This change
>> corrects that.
>>
>> In addition we were seeing dev_watchdog hangs on igb after running the
>> ethtool tests. We found this to be due to the fact that the ethtool test
>> runs the same logic as ndo_start_xmit, but we were never clearing the XOFF
>> flag since the loopback test in ethtool does not do byte queue accounting.
>>
>> Signed-off-by: Alexander Duyck<alexander.h.duyck@intel.com>
>> Tested-by: Stephen Ko<stephen.s.ko@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>>
>> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=5c4903549c05bbb373479e0ce2992573c120654a
>>
>>
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] macvlan/macvtap: Fix vlan tagging on user read
From: Basil Gor @ 2012-04-20 23:11 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev, David S. Miller
In-Reply-To: <20120418193312.GA24516@nanobar>
I did some additional code review, and it's easier to show on stack traces and
by comparing macvtap with tun/tap driver.
tun/tap device does not need to care about vlan tag stuff, as it gets skb with
vlan id in the header and vlan_tci is not used.
[97493.070321] tun_net_xmit devname vnet0 vlan_tci 0 vlan 0 proto 8100 len 64
[97493.070327] Pid: 0, comm: swapper/2 Tainted: G O 3.3.1-3.fc16.x86_64 #1
[97493.070331] Call Trace:
[97493.070334] <IRQ> [<ffffffffa02c6827>] tun_net_xmit+0x47/0x260 [tun]
[97493.070347] [<ffffffff814e8072>] dev_hard_start_xmit+0x332/0x6d0 <------ __vlan_put_tag is called
[97493.070355] [<ffffffff81503f5a>] sch_direct_xmit+0xfa/0x1d0
[97493.070364] [<ffffffff814e85b5>] dev_queue_xmit+0x1a5/0x640
[97493.070377] [<ffffffffa057c180>] ? br_flood+0xc0/0xc0 [bridge]
[97493.070395] [<ffffffffa057c380>] ? __br_deliver+0x100/0x100 [bridge]
[97493.070409] [<ffffffffa057c380>] ? __br_deliver+0x100/0x100 [bridge]
[97493.070423] [<ffffffffa057c1ec>] br_dev_queue_push_xmit+0x6c/0xa0 [bridge]
[97493.070438] [<ffffffffa057c380>] ? __br_deliver+0x100/0x100 [bridge]
[97493.070457] [<ffffffffa057c242>] br_forward_finish+0x22/0x60 [bridge]
[97493.070471] [<ffffffffa057c380>] ? __br_deliver+0x100/0x100 [bridge]
[97493.070485] [<ffffffffa057c3dd>] __br_forward+0x5d/0xb0 [bridge]
[97493.070495] [<ffffffff814dafe4>] ? skb_clone+0x54/0xb0
[97493.070508] [<ffffffffa057bf1e>] deliver_clone+0x3e/0x60 [bridge]
[97493.070523] [<ffffffffa057c143>] br_flood+0x83/0xc0 [bridge]
[97493.070534] [<ffffffffa057c525>] br_flood_forward+0x15/0x20 [bridge]
[97493.070544] [<ffffffffa057d256>] br_handle_frame_finish+0x246/0x2a0 [bridge]
[97493.070555] [<ffffffffa057d444>] br_handle_frame+0x194/0x260 [bridge]
[97493.070567] [<ffffffffa057d2b0>] ? br_handle_frame_finish+0x2a0/0x2a0 [bridge]
[97493.070581] [<ffffffff814e56de>] __netif_receive_skb+0x1be/0x5c0 <------ vlan_untag is called
[97493.070594] [<ffffffff81088ba2>] ? default_wake_function+0x12/0x20
[97493.070604] [<ffffffff814e5ef1>] process_backlog+0xb1/0x170
[97493.070613] [<ffffffff814e718b>] net_rx_action+0x12b/0x270
[97493.070623] [<ffffffff8108daed>] ? sched_clock_cpu+0xbd/0x110
[97493.070633] [<ffffffff8105efb8>] __do_softirq+0xb8/0x230
[97493.070644] [<ffffffff810e3c30>] ? handle_irq_event+0x50/0x70
[97493.070654] [<ffffffff815fd49c>] call_softirq+0x1c/0x30
[97493.070662] [<ffffffff81016455>] do_softirq+0x65/0xa0
[97493.070668] [<ffffffff8105f3ce>] irq_exit+0x9e/0xc0
[97493.070675] [<ffffffff815fdd03>] do_IRQ+0x63/0xe0
[97493.070682] [<ffffffff815f42ae>] common_interrupt+0x6e/0x6e
[97493.070686] <EOI> [<ffffffff8131b236>] ? intel_idle+0xe6/0x150
[97493.070697] [<ffffffff8131b218>] ? intel_idle+0xc8/0x150
[97493.070705] [<ffffffff814a4071>] cpuidle_idle_call+0xc1/0x280
[97493.070713] [<ffffffff8101322f>] cpu_idle+0xcf/0x120
[97493.070720] [<ffffffff815e3b1f>] start_secondary+0x282/0x284
but macvtap device gets skb with vlan tag extacted in vlan_tci, and as original driver
code was mostly based on tun/tap driver vlan thing was missed.
[98143.863560] macvtap_receive devname macvtap0 vlan_tci 100a vlan 10 proto 0800 len 90
[98143.863570] macvtap_forward devname macvtap0 vlan_tci 100a vlan 10 proto 0800 len 90
[98143.863578] Pid: 0, comm: swapper/2 Tainted: G O 3.3.1-3.fc16.x86_64 #1
[98143.863583] Call Trace:
[98143.863587] <IRQ> [<ffffffffa026819c>] macvtap_forward+0x8c/0x1b0 [macvtap]
[98143.863606] [<ffffffffa0268314>] macvtap_receive+0x54/0x60 [macvtap]
[98143.863623] [<ffffffffa02c05db>] macvlan_handle_frame+0xbb/0x2c0 [macvlan]
[98143.863635] [<ffffffffa02c0520>] ? macvlan_broadcast+0x160/0x160 [macvlan]
[98143.863646] [<ffffffff814e56de>] __netif_receive_skb+0x1be/0x5c0 <------ vlan_untag is called
[98143.863653] [<ffffffff814e67e3>] netif_receive_skb+0x23/0x90
[98143.863660] [<ffffffff814e6c09>] ? dev_gro_receive+0x1b9/0x2b0
[98143.863667] [<ffffffff814e6950>] napi_skb_finish+0x50/0x70
[98143.863673] [<ffffffff814e6f45>] napi_gro_receive+0xf5/0x140
[98143.863697] [<ffffffffa0241fab>] e1000_receive_skb+0x5b/0x70 [e1000e]
[98143.863718] [<ffffffffa0244b21>] e1000_clean_rx_irq+0x2f1/0x400 [e1000e]
[98143.863737] [<ffffffffa02432e8>] e1000_clean+0x78/0x2c0 [e1000e]
[98143.863745] [<ffffffff814e718b>] net_rx_action+0x12b/0x270
[98143.863752] [<ffffffff8108daed>] ? sched_clock_cpu+0xbd/0x110
[98143.863759] [<ffffffff8105efb8>] __do_softirq+0xb8/0x230
[98143.863767] [<ffffffff810e3c30>] ? handle_irq_event+0x50/0x70
[98143.863775] [<ffffffff815fd49c>] call_softirq+0x1c/0x30
[98143.863782] [<ffffffff81016455>] do_softirq+0x65/0xa0
[98143.863788] [<ffffffff8105f3ce>] irq_exit+0x9e/0xc0
[98143.863796] [<ffffffff815fdd03>] do_IRQ+0x63/0xe0
[98143.863803] [<ffffffff815f42ae>] common_interrupt+0x6e/0x6e
[98143.863807] <EOI> [<ffffffff8131b236>] ? intel_idle+0xe6/0x150
[98143.863818] [<ffffffff8131b218>] ? intel_idle+0xc8/0x150
[98143.863826] [<ffffffff814a4071>] cpuidle_idle_call+0xc1/0x280
[98143.863834] [<ffffffff8101322f>] cpu_idle+0xcf/0x120
[98143.863841] [<ffffffff815e3b1f>] start_secondary+0x282/0x284
and as Eric Biederman noted, why not add vlan header back at the last moment? in
macvtap_put_user. And it would work for user space applications which read
/dev/tapX, but in kvm case actual reading is done by vhost_net driver. And this
driver actually does skb_peek on macvtap queue to get next packet size before
reading (in handle_rx).
[98143.863878] vhost peek_head_len devname macvtap0 vlan_tci 100a vlan 10 proto 0800 len 90
so, it gets skb len without vlan tag and then performs read with buffer smaller then needed
[98143.863885] macvtap_do_read buflen 102 <--- 90 + (vnet_hdr_sz 12 bytes)
[98143.863889] devname macvtap0 vlan_tci 100a vlan 10 proto 0800 len 90
__vlan_put_tag is called here
[98143.863894] macvtap_do_read reallen 106 <--- 90 + 4 + (vnet_hdr_sz 12)
[98143.863898] devname macvtap0 vlan_tci 0 vlan 0 proto 8100 len 94
[98143.863904] Pid: 7289, comm: vhost-7236 Tainted: G O 3.3.1-3.fc16.x86_64 #1
[98143.863935] Call Trace:
[98143.863944] [<ffffffffa0268593>] macvtap_do_read+0x243/0x420 [macvtap]
[98143.863954] [<ffffffff81088b90>] ? try_to_wake_up+0x2b0/0x2b0
[98143.863962] [<ffffffffa02687ba>] macvtap_recvmsg+0x4a/0x70 [macvtap]
[98143.863971] [<ffffffffa02e149e>] handle_rx+0x39e/0x6e0 [vhost_net]
[98143.863983] [<ffffffffa02e17f5>] handle_rx_net+0x15/0x20 [vhost_net]
[98143.863996] [<ffffffffa02de84c>] vhost_worker+0xcc/0x150 [vhost_net]
[98143.864008] [<ffffffffa02de780>] ? __vhost_add_used_n+0x110/0x110 [vhost_net]
[98143.864020] [<ffffffff81079af3>] kthread+0x93/0xa0
[98143.864032] [<ffffffff815fd3a4>] kernel_thread_helper+0x4/0x10
[98143.864044] [<ffffffff81079a60>] ? kthread_freezable_should_stop+0x70/0x70
[98143.864056] [<ffffffff815fd3a0>] ? gs_change+0x13/0x13
things get more interesting when we take another case in account. When one kvm guest sends
packet on the same macvlan to another guest macvtap gets skb with vlan id in the header
and vlan_tci is not used.
[99564.523943] macvtap_forward devname (null) vlan_tci 0 vlan 0 proto 8100 len 94
[99564.523946] Pid: 8849, comm: vhost-8797 Tainted: G O 3.3.1-3.fc16.x86_64 #1
[99564.523947] Call Trace:
[99564.523952] [<ffffffffa02de19c>] macvtap_forward+0x8c/0x1b0 [macvtap]
[99564.523963] [<ffffffffa02c0502>] macvlan_broadcast+0x142/0x160 [macvlan]
[99564.523967] [<ffffffffa02c146d>] macvlan_start_xmit+0x14d/0x178 [macvlan]
[99564.523969] [<ffffffffa02df378>] macvtap_get_user+0x388/0x420 [macvtap]
[99564.523971] [<ffffffffa02df43b>] macvtap_sendmsg+0x2b/0x30 [macvtap]
[99564.523973] [<ffffffffa026bb3d>] handle_tx+0x2dd/0x620 [vhost_net]
[99564.523976] [<ffffffffa026beb5>] handle_tx_kick+0x15/0x20 [vhost_net]
[99564.523978] [<ffffffffa026884c>] vhost_worker+0xcc/0x150 [vhost_net]
[99564.523980] [<ffffffffa0268780>] ? __vhost_add_used_n+0x110/0x110 [vhost_net]
[99564.523984] [<ffffffff81079af3>] kthread+0x93/0xa0
[99564.523987] [<ffffffff815fd3a4>] kernel_thread_helper+0x4/0x10
[99564.523989] [<ffffffff81079a60>] ? kthread_freezable_should_stop+0x70/0x70
[99564.523991] [<ffffffff815fd3a0>] ? gs_change+0x13/0x13
[99564.523999] vhost peek_head_len devname (null) vlan_tci 0 vlan 0 proto 8100 len 94
[99564.524003] macvtap_do_read buflen 106
[99564.524004] macvtap_do_read reallen 106
And we definitely want to have common rules for all cases. So we either
1. restore vlan headers from vlan_tci for any packets coming outside of macvlan in
macvtap_receive and we don't need to fix vhost_net and we preserve same vlan id
policy that tun/tap driver have. (my original patch)
or
2. we extract vlan ids for packets coming from the same macvlan, fixing vhost_net to
take vlan_tci into account and restoring vlan headers on macvtap_put_user
or please propose another solution.
Basil Gor
On Wed, Apr 18, 2012 at 11:33:12PM +0400, Basil Gor wrote:
> On Wed, Apr 18, 2012 at 11:54:52AM -0700, Eric W. Biederman wrote:
> > Basil Gor <basilgor@gmail.com> writes:
> >
> > > Vlan tag is restored during buffer transmit to a network device (bridge
> > > port) in bridging code in case of tun/tap driver. In case of macvtap it
> > > has to be done explicitly. Otherwise vlan_tci is ignored and user always
> > > gets untagged packets.
> > >
> > > Scenario tested:
> > > kvm guests (that use vlans) migration from bridged network to macvtap
> > > revealed that packets delivered to guests are always untagged. Dumping
> > > and comparing sk_buff in case of tap and macvtap driver showed that
> > > macvtap does not restore vlan_tci.
> > >
> > > With current patch applied I was able to get working network, kvm guests
> > > get correctly tagged packets and can reach each other when macvtap in
> > > bridge mode (both with no vlans and through vlan interfaces).
> >
> > My first impression is that this is the wrong place to add a vlan
> > header back.
> >
> > You need to keep the vlan information in vlan_tci until just
> > before the packet is delivered to userspace. Which would suggest
> > the best place for these games is macvtap_put_user.
> >
> > Elsewhere vlan headers should not be explicitly stored in the packet.
> >
> > At least that was the rule last I looked.
> >
> > Eric
> >
> >
> This sounds right, and macvtap_put_user was the first place where I
> put vlan header adding. But qemu-kvm does smth like get pending data
> size and then read, and when I put code in macvtap_put_user qemu
> supplied buffer 4 bytes smaller then needed and packets were
> truncated. On the other hand tun/tap driver never keeps vlan info in
> vlan_tci because you can't do any vlan operations on it I think. So I
> decided to restore vlan header just before adding it to macvtap queue.
>
> But I'll try to look deeper in it.
> Thanks
> > > Signed-off-by: Basil Gor <basilgor@gmail.com>
> > > ---
> > > drivers/net/macvtap.c | 9 +++++++++
> > > 1 files changed, 9 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> > > index 0427c65..a6802b9 100644
> > > --- a/drivers/net/macvtap.c
> > > +++ b/drivers/net/macvtap.c
> > > @@ -1,6 +1,7 @@
> > > #include <linux/etherdevice.h>
> > > #include <linux/if_macvlan.h>
> > > #include <linux/interrupt.h>
> > > +#include <linux/if_vlan.h>
> > > #include <linux/nsproxy.h>
> > > #include <linux/compat.h>
> > > #include <linux/if_tun.h>
> > > @@ -254,6 +255,14 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
> > > if (skb_queue_len(&q->sk.sk_receive_queue) >= dev->tx_queue_len)
> > > goto drop;
> > >
> > > + if (vlan_tx_tag_present(skb)) {
> > > + skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
> > > + if (unlikely(!skb))
> > > + return NET_RX_DROP;
> > > +
> > > + skb->vlan_tci = 0;
> > > + }
> > > +
> > > skb_queue_tail(&q->sk.sk_receive_queue, skb);
> > > wake_up_interruptible_poll(sk_sleep(&q->sk), POLLIN | POLLRDNORM | POLLRDBAND);
> > > return NET_RX_SUCCESS;
^ permalink raw reply
* Re: [PATCH 1/2] workqueue: Catch more locking problems with flush_work()
From: Stephen Boyd @ 2012-04-20 23:15 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, netdev, Ben Dooks
In-Reply-To: <20120420173529.GD32324@google.com>
On 4/20/2012 10:35 AM, Tejun Heo wrote:
>
> All that's necessary is acquiring and releasing work->lockdep_map.
> There's no need to nest start_flush_work() inside it. Without
> nesting, there's nothing to worry about ABBA lockdeps.
Yes I see. This patch should work then?
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5abf42f..038cf64 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2506,6 +2506,9 @@ bool flush_work(struct work_struct *work)
{
struct wq_barrier barr;
+ lock_map_acquire(&work->lockdep_map);
+ lock_map_release(&work->lockdep_map);
+
if (start_flush_work(work, &barr, true)) {
wait_for_completion(&barr.done);
destroy_work_on_stack(&barr.work);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* [PATCH v2] macvlan/macvtap: Fix vlan tagging on user read
From: Basil Gor @ 2012-04-20 23:20 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Eric W. Biederman, Basil Gor
Vlan tag is restored during buffer transmit to a network device (bridge
port) in bridging code in case of tun/tap driver. In case of macvtap it
has to be done explicitly. Otherwise vlan_tci is ignored and user always
gets untagged packets.
Scenario tested:
kvm guests (that use vlans) migration from bridged network to macvtap
revealed that packets delivered to guests are always untagged. Dumping
and comparing sk_buff in case of tap and macvtap driver showed that
macvtap does not restore vlan_tci.
With current patch applied I was able to get working network, kvm guests
get correctly tagged packets and can reach each other when macvtap in
bridge mode (both with no vlans and through vlan interfaces).
Changes from original version:
vlan header restoring code is moved from macvtap_forward to
macvtap_receive
Signed-off-by: Basil Gor <basilgor@gmail.com>
---
drivers/net/macvtap.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 0427c65..5e025d9 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1,6 +1,7 @@
#include <linux/etherdevice.h>
#include <linux/if_macvlan.h>
#include <linux/interrupt.h>
+#include <linux/if_vlan.h>
#include <linux/nsproxy.h>
#include <linux/compat.h>
#include <linux/if_tun.h>
@@ -271,6 +272,15 @@ drop:
static int macvtap_receive(struct sk_buff *skb)
{
skb_push(skb, ETH_HLEN);
+
+ if (vlan_tx_tag_present(skb)) {
+ skb = __vlan_put_tag(skb, vlan_tx_tag_get(skb));
+ if (unlikely(!skb))
+ return NET_RX_DROP;
+
+ skb->vlan_tci = 0;
+ }
+
return macvtap_forward(skb->dev, skb);
}
--
1.7.6.5
^ permalink raw reply related
* restoring IP multicast addresses when restarting the interface.
From: Flavio Leitner @ 2012-04-21 0:25 UTC (permalink / raw)
To: netdev; +Cc: Herbert Xu
Hello folks,
I was told that ntp in multicast mode was not working if you
restart the interface (ifdown <iface> ; ifup <iface) on an
older kernel.
It seemed obvious to me, but when I tried to reproduce this
upstream (last kernel of today), I noticed that the kernel
does restore the previous added multicast addresses. Therefore,
there is no issues.
I found that this is due to mainly this commit below which
doesn't remove idev when the addresses are deleted.
commit 6363097cc4d182f93788131b5d8f72aa91d950a0
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu Jun 7 18:35:38 2007 -0700
[IPV4]: Do not remove idev when addresses are cleared
Now that we create idev before addresses are added, it no longer makes
sense to remove them when addresses are all deleted.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
Although the new behavior seems nice and save some user space
work, I think it was unintentional and likely to be a bug.
What you guys think?
thanks,
fbl
^ permalink raw reply
* [PATCHv2] workqueue: Catch more locking problems with flush_work()
From: Stephen Boyd @ 2012-04-21 0:28 UTC (permalink / raw)
To: Tejun Heo; +Cc: linux-kernel, netdev, Ben Dooks, Yong Zhang
In-Reply-To: <20120420173529.GD32324@google.com>
If a workqueue is flushed with flush_work() lockdep checking can
be circumvented. For example:
static DEFINE_MUTEX(mutex);
static void my_work(struct work_struct *w)
{
mutex_lock(&mutex);
mutex_unlock(&mutex);
}
static DECLARE_WORK(work, my_work);
static int __init start_test_module(void)
{
schedule_work(&work);
return 0;
}
module_init(start_test_module);
static void __exit stop_test_module(void)
{
mutex_lock(&mutex);
flush_work(&work);
mutex_unlock(&mutex);
}
module_exit(stop_test_module);
would not always print a warning when flush_work() was called.
In this trivial example nothing could go wrong since we are
guaranteed module_init() and module_exit() don't run concurrently,
but if the work item is schedule asynchronously we could have a
scenario where the work item is running just at the time flush_work()
is called resulting in a classic ABBA locking problem.
Add a lockdep hint by acquiring and releasing the work item
lockdep_map in flush_work() so that we always catch this
potential deadlock scenario.
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
kernel/workqueue.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5abf42f..038cf64 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2506,6 +2506,9 @@ bool flush_work(struct work_struct *work)
{
struct wq_barrier barr;
+ lock_map_acquire(&work->lockdep_map);
+ lock_map_release(&work->lockdep_map);
+
if (start_flush_work(work, &barr, true)) {
wait_for_completion(&barr.done);
destroy_work_on_stack(&barr.work);
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related
* Re: [PATCH 1/2] workqueue: Catch more locking problems with flush_work()
From: Yong Zhang @ 2012-04-21 0:32 UTC (permalink / raw)
To: Stephen Boyd; +Cc: linux-kernel, Tejun Heo, netdev, Ben Dooks
In-Reply-To: <20120420083202.GB17846@zhy>
On Fri, Apr 20, 2012 at 4:32 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> On Fri, Apr 20, 2012 at 01:18:19AM -0700, Stephen Boyd wrote:
>> On 4/20/2012 12:18 AM, Yong Zhang wrote:
>> > On Thu, Apr 19, 2012 at 11:26:47PM -0700, Stephen Boyd wrote:
>> >> complain in the case where the work is not queued. That case is not a
>> >> false positive. We will get a lockdep warning if the work is running
>> > IIRC, flush_work() is just a nop when a work is not queued nor running.
>>
>> Agreed, but it's better to always print a lockdep warning instead of
>> only when the deadlock is going to occur.
>
> It will IMHO.
After reconsidering this issue, I recognize I'm wrong from the beginning.
My suggestion will only warn on the real deadlock. It doesn't follow
what lockdep want to achieve. Sorry for that.
BTW, your latest patch should work :)
Thanks,
Yong
>
>>
>> >
>> >> (when start_flush_work() returns true) solely with the
>> >> lock_map_acquire() on cwq->wq->lockdep_map.
>> > Yeah, that is the point we use lockdep to detect deadlock for workqueue.
>> >
>> > But when looking at start_flush_work(), for some case
>> > !(cwq->wq->saved_max_active == 1 || cwq->wq->flags & WQ_RESCUER),
>> > lock_map_acquire_read() is called, but recursive read is not added to
>> > the chain list. So when lock_map_acquire_read(&cwq->wq->lockdep_map)
>> > is called, deadlock will not be detected. I hope you don't hit that
>> > special case.
>>
>> Hmm. Originally I had what you suggested in my patch but I left it out
>> because I wasn't sure if it would cause false positives.
>> Do you see any
>> possibility for false positives?
>
> No, I don't. My test indeed show nothing (just build and boot).
>
>>I'll add it back in if not.
>
> It's great if you can try it :)
>
> Thanks,
> Yong
--
Only stand for myself
^ permalink raw reply
* Re: [PATCHv2] workqueue: Catch more locking problems with flush_work()
From: Yong Zhang @ 2012-04-21 0:34 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Tejun Heo, linux-kernel, netdev, Ben Dooks
In-Reply-To: <1334968130-20724-1-git-send-email-sboyd@codeaurora.org>
On Sat, Apr 21, 2012 at 8:28 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> If a workqueue is flushed with flush_work() lockdep checking can
> be circumvented. For example:
>
> static DEFINE_MUTEX(mutex);
>
> static void my_work(struct work_struct *w)
> {
> mutex_lock(&mutex);
> mutex_unlock(&mutex);
> }
>
> static DECLARE_WORK(work, my_work);
>
> static int __init start_test_module(void)
> {
> schedule_work(&work);
> return 0;
> }
> module_init(start_test_module);
>
> static void __exit stop_test_module(void)
> {
> mutex_lock(&mutex);
> flush_work(&work);
> mutex_unlock(&mutex);
> }
> module_exit(stop_test_module);
>
> would not always print a warning when flush_work() was called.
> In this trivial example nothing could go wrong since we are
> guaranteed module_init() and module_exit() don't run concurrently,
> but if the work item is schedule asynchronously we could have a
> scenario where the work item is running just at the time flush_work()
> is called resulting in a classic ABBA locking problem.
>
> Add a lockdep hint by acquiring and releasing the work item
> lockdep_map in flush_work() so that we always catch this
> potential deadlock scenario.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
> ---
> kernel/workqueue.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 5abf42f..038cf64 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -2506,6 +2506,9 @@ bool flush_work(struct work_struct *work)
> {
> struct wq_barrier barr;
>
> + lock_map_acquire(&work->lockdep_map);
> + lock_map_release(&work->lockdep_map);
> +
> if (start_flush_work(work, &barr, true)) {
> wait_for_completion(&barr.done);
> destroy_work_on_stack(&barr.work);
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
--
Only stand for myself
^ permalink raw reply
* Re: [PATCH net-next 00/16] tipc: publication lists and zero node handling
From: David Miller @ 2012-04-21 0:45 UTC (permalink / raw)
To: paul.gortmaker; +Cc: netdev, allan.stephens, ying.xue
In-Reply-To: <1334955924-907-1-git-send-email-paul.gortmaker@windriver.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Fri, 20 Apr 2012 17:05:08 -0400
> This series of commits largely focuses on two things. The 1st is
> to categorize the TIPC publication lists, so better control over
> the publications can be achieved. The 2nd is largely to improve
> corner cases around the tipc address changes and how the <0.0.0>
> tipc address is interpreted.
>
> What remains after this, is less than 10 commits originating from
> what was salvaged from the tipc-1.7 sourceforge fork, so I'm happy to
> see the end of this task so close. I'll be looking at those shortly.
>
> The tipc tests (tipcTS <--> tipcTC) have been used in both directions
> between a 32bit and a 64bit machine to do runtime testing, and the
> entire series has been confirmed to be compile time bisectable too.
>
> Credit to Al for all the heavy lifting. I've just refactored things
> a bit and made minor changes here and there.
Pulled, thanks Paul.
Please remove the inline tags from static functions, let the compiler
decide what to do. The only spot appropriate to use inline tags these
days is helper functions defined in header files.
^ permalink raw reply
* Re: [PATCH v3.4-rc 0/9] pch_gbe: ptp bug fixes
From: David Miller @ 2012-04-21 0:50 UTC (permalink / raw)
To: richardcochran; +Cc: netdev, tshimizu818
In-Reply-To: <cover.1334949388.git.richardcochran@gmail.com>
From: Richard Cochran <richardcochran@gmail.com>
Date: Fri, 20 Apr 2012 22:09:19 +0200
> The content in this patch series originally was posted to netdev by
> Takahiro on March 23, 2012, as a loose group of five patches. These
> patches addressed problems pointed out in review, both on and off
> list.
>
> I have taken his work and rebased, squashed, and split it into the
> present form, and I also wrote commit messages that more fully explain
> the changes. Also, I added the last two patches myself.
3.4-rcX is not the appropriate tree to base this work on, whereas
net-next is.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 00/19] net: Sysctl simplifications and enhancements
From: David Miller @ 2012-04-21 1:24 UTC (permalink / raw)
To: xemul; +Cc: ebiederm, netdev, serge, gaofeng, pablo, shemminger
In-Reply-To: <4F912240.8010802@parallels.com>
From: Pavel Emelyanov <xemul@parallels.com>
Date: Fri, 20 Apr 2012 12:45:52 +0400
> On 04/20/2012 03:17 AM, Eric W. Biederman wrote:
>>
>> Summary:
>> - Kill approximately 400 lines of code
>> - Allow all networking sysctls with just CAP_NET_ADMIN
>> - Hide all networking sysctls that don't apply to your current network namespace.
>> - Uniformly register flat sysctl tables not sysctl tables with .child entries
>> - Readable string paths for registering sysctls
>>
>> Eric W. Biederman (19):
>> net: Implement register_net_sysctl.
>> net sysctl: Register an empty /proc/sys/net
>> net sysctl: Initialize the network sysctls sooner to avoid problems.
>> net: Kill register_sysctl_rotable
>> net: Move all of the network sysctls without a namespace into init_net.
>> net core: Remove unneded creation of an empty net/core sysctl directory
>> net ipv6: Remove unneded registration of an empty net/ipv6/neigh
>> net ipv4: Remove the unneeded registration of an empty net/ipv4/neigh
>> net ax25: Simplify and cleanup the ax25 sysctl handling.
>> net llc: Don't use sysctl tables with .child entries.
>> net ipv6: Don't use sysctl tables with .child entries.
>> net neighbour: Convert to use register_net_sysctl
>> net decnet: Convert to use register_net_sysctl
>> net ipv6: Convert addrconf to use register_net_sysctl
>> net ipv4: Convert devinet to use register_net_sysctl
>> net: Convert nf_conntrack_proto to use register_net_sysctl
>> net: Convert all sysctl registrations to register_net_sysctl
>> net: Delete all remaining instances of ctl_path
>> net: Remove register_net_sysctl_table
>
> After resolving issues with Eric
>
> Acked-by: Pavel Emelyanov <xemul@parallels.com>
Series applied, thanks everyone.
^ 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