From: <erik.hugne@ericsson.com>
To: <netdev@vger.kernel.org>, <tipc-discussion@lists.sourceforge.net>,
<jon.maloy@ericsson.com>, <maloy@donjonn.com>
Cc: <ying.xue@windriver.com>, <paul.gortmaker@windriver.com>,
<richard.alpe@ericsson.com>, Erik Hugne <erik.hugne@ericsson.com>
Subject: [PATCH net-next v2 1/6] tipc: allow connection shutdown callback to be invoked in advance
Date: Wed, 5 Mar 2014 08:56:13 +0100 [thread overview]
Message-ID: <1394006178-23966-2-git-send-email-erik.hugne@ericsson.com> (raw)
In-Reply-To: <1394006178-23966-1-git-send-email-erik.hugne@ericsson.com>
From: Ying Xue <ying.xue@windriver.com>
Currently connection shutdown callback function is called when
connection instance is released in tipc_conn_kref_release(), and
receiving packets and sending packets are running in different
threads. Even if connection is closed by the thread of receiving
packets, its shutdown callback may not be called immediately as
the connection reference count is non-zero at that moment. So,
although the connection is shut down by the thread of receiving
packets, the thread of sending packets doesn't know it. Before
its shutdown callback is invoked to tell the sending thread its
connection has been closed, the sending thread may deliver
messages by tipc_conn_sendmsg(), this is why the following error
information appears:
"Sending subscription event failed, no memory"
To eliminate it, allow connection shutdown callback function to
be called before connection id is removed in tipc_close_conn(),
which makes the sending thread know the truth in time that its
socket is closed so that it doesn't send message to it. We also
replace the specific error reporting from topology/config service
with a general one if either service tries to send a message on
a nonexisting/closed connection.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Reviewed-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/config.c | 9 ++-------
net/tipc/server.c | 13 +++++++------
net/tipc/subscr.c | 8 ++------
3 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/net/tipc/config.c b/net/tipc/config.c
index c301a9a..5afe633 100644
--- a/net/tipc/config.c
+++ b/net/tipc/config.c
@@ -376,7 +376,6 @@ static void cfg_conn_msg_event(int conid, struct sockaddr_tipc *addr,
struct tipc_cfg_msg_hdr *req_hdr;
struct tipc_cfg_msg_hdr *rep_hdr;
struct sk_buff *rep_buf;
- int ret;
/* Validate configuration message header (ignore invalid message) */
req_hdr = (struct tipc_cfg_msg_hdr *)buf;
@@ -398,12 +397,8 @@ static void cfg_conn_msg_event(int conid, struct sockaddr_tipc *addr,
memcpy(rep_hdr, req_hdr, sizeof(*rep_hdr));
rep_hdr->tcm_len = htonl(rep_buf->len);
rep_hdr->tcm_flags &= htons(~TCM_F_REQUEST);
-
- ret = tipc_conn_sendmsg(&cfgsrv, conid, addr, rep_buf->data,
- rep_buf->len);
- if (ret < 0)
- pr_err("Sending cfg reply message failed, no memory\n");
-
+ tipc_conn_sendmsg(&cfgsrv, conid, addr, rep_buf->data,
+ rep_buf->len);
kfree_skb(rep_buf);
}
}
diff --git a/net/tipc/server.c b/net/tipc/server.c
index b635ca3..91423ca 100644
--- a/net/tipc/server.c
+++ b/net/tipc/server.c
@@ -87,7 +87,6 @@ static void tipc_clean_outqueues(struct tipc_conn *con);
static void tipc_conn_kref_release(struct kref *kref)
{
struct tipc_conn *con = container_of(kref, struct tipc_conn, kref);
- struct tipc_server *s = con->server;
if (con->sock) {
tipc_sock_release_local(con->sock);
@@ -95,10 +94,6 @@ static void tipc_conn_kref_release(struct kref *kref)
}
tipc_clean_outqueues(con);
-
- if (con->conid)
- s->tipc_conn_shutdown(con->conid, con->usr_data);
-
kfree(con);
}
@@ -181,6 +176,9 @@ static void tipc_close_conn(struct tipc_conn *con)
struct tipc_server *s = con->server;
if (test_and_clear_bit(CF_CONNECTED, &con->flags)) {
+ if (con->conid)
+ s->tipc_conn_shutdown(con->conid, con->usr_data);
+
spin_lock_bh(&s->idr_lock);
idr_remove(&s->conn_idr, con->conid);
s->idr_in_use--;
@@ -413,8 +411,11 @@ int tipc_conn_sendmsg(struct tipc_server *s, int conid,
struct tipc_conn *con;
con = tipc_conn_lookup(s, conid);
- if (!con)
+ if (!con) {
+ pr_err("Connection %d not found on server %s\n", conid,
+ s->name);
return -EINVAL;
+ }
e = tipc_alloc_entry(data, len);
if (!e) {
diff --git a/net/tipc/subscr.c b/net/tipc/subscr.c
index 7cb0bd5..a6ce3bb 100644
--- a/net/tipc/subscr.c
+++ b/net/tipc/subscr.c
@@ -96,20 +96,16 @@ static void subscr_send_event(struct tipc_subscription *sub, u32 found_lower,
{
struct tipc_subscriber *subscriber = sub->subscriber;
struct kvec msg_sect;
- int ret;
msg_sect.iov_base = (void *)&sub->evt;
msg_sect.iov_len = sizeof(struct tipc_event);
-
sub->evt.event = htohl(event, sub->swap);
sub->evt.found_lower = htohl(found_lower, sub->swap);
sub->evt.found_upper = htohl(found_upper, sub->swap);
sub->evt.port.ref = htohl(port_ref, sub->swap);
sub->evt.port.node = htohl(node, sub->swap);
- ret = tipc_conn_sendmsg(&topsrv, subscriber->conid, NULL,
- msg_sect.iov_base, msg_sect.iov_len);
- if (ret < 0)
- pr_err("Sending subscription event failed, no memory\n");
+ tipc_conn_sendmsg(&topsrv, subscriber->conid, NULL, msg_sect.iov_base,
+ msg_sect.iov_len);
}
/**
--
1.8.3.2
next prev parent reply other threads:[~2014-03-05 7:55 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-05 7:56 [PATCH net-next v2 0/6] tipc: refcount and memory leak fixes erik.hugne
2014-03-05 7:56 ` erik.hugne [this message]
2014-03-06 4:56 ` [PATCH net-next v2 1/6] tipc: allow connection shutdown callback to be invoked in advance David Miller
2014-03-06 9:06 ` Erik Hugne
2014-03-09 17:48 ` Ben Hutchings
2014-03-05 7:56 ` [PATCH net-next v2 2/6] tipc: fix connection refcount leak erik.hugne
2014-03-05 7:56 ` [PATCH net-next v2 3/6] tipc: avoid to unnecessary process switch under non-block mode erik.hugne
2014-03-05 7:56 ` [PATCH net-next v2 4/6] tipc: drop subscriber connection id invalidation erik.hugne
2014-03-05 7:56 ` [PATCH net-next v2 5/6] tipc: fix memory leak during module removal erik.hugne
2014-03-05 7:56 ` [PATCH net-next v2 6/6] tipc: don't log disabled tasklet handler errors erik.hugne
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1394006178-23966-2-git-send-email-erik.hugne@ericsson.com \
--to=erik.hugne@ericsson.com \
--cc=jon.maloy@ericsson.com \
--cc=maloy@donjonn.com \
--cc=netdev@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
--cc=richard.alpe@ericsson.com \
--cc=tipc-discussion@lists.sourceforge.net \
--cc=ying.xue@windriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).