<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Thanks for the comments. <br>
To understand the consequences of ignoring the err, we need to look
at what is going on. <br>
We get a softIRQ when a connection packet (tcp SYN). It is critical
to note that we may not<br>
get a softIRQ<u> for every connection s</u>ince connection packets
can arrive<br>
back-to-back (as happened in this bug). So, one softIRQ could be
delivered for > 1 pending accept.<br>
<u>This is the KEY point. </u><br>
<br>
If we terminate the loop calling o2net_accept_one() upon seeing an
error, what happens<br>
to the rest of the connections in the queue. If no new connection
arrives for hours, no new softIRQ<br>
will be delivered, and the connections will just sit in the queue.<br>
<br>
Thanks,<br>
-Tariq<br>
<br>
<br>
<div class="moz-cite-prefix">On 1/24/2014 1:55 PM, Mark Fasheh
wrote:<br>
</div>
<blockquote cite="mid:20140124215554.GC24361@wotan.suse.de"
type="cite">
<pre wrap="">On Fri, Jan 24, 2014 at 12:47:02PM -0800, <a class="moz-txt-link-abbreviated" href="mailto:akpm@linux-foundation.org">akpm@linux-foundation.org</a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">From: Tariq Saeed <a class="moz-txt-link-rfc2396E" href="mailto:tariq.x.saeed@oracle.com"><tariq.x.saeed@oracle.com></a>
Subject: ocfs2/o2net: incorrect to terminate accepting connections loop upon rejecting an invalid one
When o2net-accept-one() rejects an illegal connection, it terminates the
loop picking up the remaining queued connections. This fix will continue
accepting connections till the queue is emtpy.
Addresses Orabug 17489469.
</pre>
</blockquote>
<pre wrap="">
Thanks for sending this, review comments below.
</pre>
<blockquote type="cite">
<pre wrap="">diff -puN fs/ocfs2/cluster/tcp.c~ocfs2-o2net-incorrect-to-terminate-accepting-connections-loop-upon-rejecting-an-invalid-one fs/ocfs2/cluster/tcp.c
--- a/fs/ocfs2/cluster/tcp.c~ocfs2-o2net-incorrect-to-terminate-accepting-connections-loop-upon-rejecting-an-invalid-one
+++ a/fs/ocfs2/cluster/tcp.c
@@ -1826,7 +1826,7 @@ int o2net_register_hb_callbacks(void)
/* ------------------------------------------------------------ */
-static int o2net_accept_one(struct socket *sock)
+static int o2net_accept_one(struct socket *sock, int *more)
{
int ret, slen;
struct sockaddr_in sin;
@@ -1837,6 +1837,7 @@ static int o2net_accept_one(struct socke
struct o2net_node *nn;
BUG_ON(sock == NULL);
+ *more = 0;
ret = sock_create_lite(sock->sk->sk_family, sock->sk->sk_type,
sock->sk->sk_protocol, &new_sock);
if (ret)
@@ -1848,6 +1849,7 @@ static int o2net_accept_one(struct socke
if (ret < 0)
goto out;
+ *more = 1;
new_sock->sk->sk_allocation = GFP_ATOMIC;
ret = o2net_set_nodelay(new_sock);
@@ -1949,8 +1951,15 @@ out:
static void o2net_accept_many(struct work_struct *work)
{
struct socket *sock = o2net_listen_sock;
- while (o2net_accept_one(sock) == 0)
+ int more;
+ int err;
+
+ for (;;) {
+ err = o2net_accept_one(sock, &more);
+ if (!more)
+ break;
</pre>
</blockquote>
<pre wrap="">
We're throwing out 'err' here and trusting the variable 'more'. However, err
could be set and more would be 0 regardless of whether there actually are
more connections to be had. This makes more sense given when 'more' is set:
if (err)
break;
/* only trust the value of 'more' when err == 0 */
if (more)
break;
Thanks,
--Mark
--
Mark Fasheh
</pre>
</blockquote>
<br>
</body>
</html>