Hi Willy,
On 2013/12/4 15:02, Willy Tarreau wrote:
Hi Godbach,
On Wed, Dec 04, 2013 at 11:50:36AM +0800, Godbach wrote:
Hi Willy,
Please check the attachments for the two patches.
Patch 0001: DOC: stick-table can be declared in such sections as
frontend, listen and backend
OK in principle but we could make it even simpler :
stick-table type {ip | integer | string [len <length>] | binary [len
<length>]}
size <size> [expire <expire>] [nopurge] [peers <peersect>]
[store <data_type>]*
- Configure the stickiness table for the current backend
+ Configure the stickiness table for the current section
May be used in sections : defaults | frontend | listen | backend
no | yes | yes | yes
Yes, just section should be OK. :-)
Please check the attachment for patch 0001.
OK in principle, but you'll see that there is another call in
stream_int_chk_snd_conn() :
if (si_conn_send(si->conn) < 0) {
/* Write error on the file descriptor */
fd_stop_both(si->conn->t.sock.fd);
__conn_data_stop_both(si->conn);
si->flags |= SI_FL_ERR;
si->conn->flags |= CO_FL_ERROR;
goto out_wakeup;
}
So it should also be simplified this way :
si_conn_send(si->conn);
if (si->conn->flags & CO_FL_ERROR) {
/* Write error on the file descriptor */
fd_stop_both(si->conn->t.sock.fd);
__conn_data_stop_both(si->conn);
si->flags |= SI_FL_ERR;
goto out_wakeup;
}
Thank you for pointing this. Yes, both stream_int_chk_snd_conn() and
si_conn_send_cb() will call si_conn_send(), and the code
si->conn->flags |= CO_FL_ERROR;
in stream_int_chk_snd_conn() as you listed above can be removed too.
I accept the change provided that you change the comments on top
of si_conn_send() to clearly state that the caller should check
conn->flags for errors.
It seems that the return value of si_conn_send() will not be used
anymore. Furthermore, si_conn_send_cb() does not need to check
conn->flags for errors since it will return directly after calling
si_conn_send().
Please check the attachment for patch 0002.
--
Best Regards,
Godbach
>From f36cee9f55c248bbbc4a47472904de32166ef5af Mon Sep 17 00:00:00 2001
From: Godbach <nylzhao...@gmail.com>
Date: Wed, 4 Dec 2013 16:08:22 +0800
Subject: [PATCH 1/2] DOC: stick-table: modify the description
The stickiness table can be declared in such sections as frontend, listen
and backend, but the original manual only mentioned backend. Modify the
description simply as below:
"current backend" -> "current section"
Signed-off-by: Godbach <nylzhao...@gmail.com>
---
doc/configuration.txt | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index 803d42e..81b4295 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6133,7 +6133,7 @@ stick store-request <pattern> [table <table>] [{if |
unless} <condition>]
stick-table type {ip | integer | string [len <length>] | binary [len <length>]}
size <size> [expire <expire>] [nopurge] [peers <peersect>]
[store <data_type>]*
- Configure the stickiness table for the current backend
+ Configure the stickiness table for the current section
May be used in sections : defaults | frontend | listen | backend
no | yes | yes | yes
--
1.7.7
>From 7ad2aeb849bc95b4734a941b010b1582fe6e9e35 Mon Sep 17 00:00:00 2001
From: Godbach <nylzhao...@gmail.com>
Date: Wed, 4 Dec 2013 16:24:06 +0800
Subject: [PATCH 2/2] OPTIM: stream_interface: return directly if the
connection flag CO_FL_ERROR has been set
The connection flag CO_FL_ERROR will be tested in the functions both
si_conn_recv_cb() and si_conn_send_cb(). If CO_FL_ERROR has been set, out_error
branch will be executed. But the only job of out_error branch is to set
CO_FL_ERROR on connection flag. So it's better return directly than goto
out_error branch under such conditions. As a result, out_error branch becomes
needless and can be removed.
In addition, the caller of si_conn_send_loop() should check conn->flags for
errors instead of the returned value.
Signed-off-by: Godbach <nylzhao...@gmail.com>
---
src/stream_interface.c | 26 +++++++++-----------------
1 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/stream_interface.c b/src/stream_interface.c
index 9f0c26a..8bec27f 100644
--- a/src/stream_interface.c
+++ b/src/stream_interface.c
@@ -610,7 +610,8 @@ static int si_conn_wake_cb(struct connection *conn)
* This function is called to send buffer data to a stream socket.
* It returns -1 in case of unrecoverable error, otherwise zero.
* It calls the transport layer's snd_buf function. It relies on the
- * caller to commit polling changes.
+ * caller to commit polling changes. The caller should check conn->flags
+ * for errors.
*/
static int si_conn_send(struct connection *conn)
{
@@ -821,12 +822,12 @@ static void stream_int_chk_snd_conn(struct
stream_interface *si)
conn_refresh_polling_flags(si->conn);
- if (si_conn_send(si->conn) < 0) {
+ si_conn_send(si->conn);
+ if (si->conn->flags & CO_FL_ERROR) {
/* Write error on the file descriptor */
fd_stop_both(si->conn->t.sock.fd);
__conn_data_stop_both(si->conn);
si->flags |= SI_FL_ERR;
- si->conn->flags |= CO_FL_ERROR;
goto out_wakeup;
}
}
@@ -915,7 +916,7 @@ static void si_conn_recv_cb(struct connection *conn)
* which rejects it before reading it all.
*/
if (conn->flags & CO_FL_ERROR)
- goto out_error;
+ return;
/* stop here if we reached the end of data */
if (conn_data_read0_pending(conn))
@@ -968,7 +969,7 @@ static void si_conn_recv_cb(struct connection *conn)
goto out_shutdown_r;
if (conn->flags & CO_FL_ERROR)
- goto out_error;
+ return;
if (conn->flags & CO_FL_WAIT_ROOM) {
/* the pipe is full or we have read enough data that it
@@ -1098,7 +1099,7 @@ static void si_conn_recv_cb(struct connection *conn)
} /* while !flags */
if (conn->flags & CO_FL_ERROR)
- goto out_error;
+ return;
if (conn_data_read0_pending(conn))
/* connection closed */
@@ -1114,10 +1115,6 @@ static void si_conn_recv_cb(struct connection *conn)
stream_sock_read0(si);
conn_data_read0(conn);
return;
-
- out_error:
- /* Read error on the connection, report the error and stop I/O */
- conn->flags |= CO_FL_ERROR;
}
/*
@@ -1131,7 +1128,7 @@ static void si_conn_send_cb(struct connection *conn)
struct channel *chn = si->ob;
if (conn->flags & CO_FL_ERROR)
- goto out_error;
+ return;
if (si->conn->flags & CO_FL_HANDSHAKE)
/* a handshake was requested */
@@ -1142,15 +1139,10 @@ static void si_conn_send_cb(struct connection *conn)
return;
/* OK there are data waiting to be sent */
- if (si_conn_send(conn) < 0)
- goto out_error;
+ si_conn_send(conn);
/* OK all done */
return;
-
- out_error:
- /* Write error on the connection, report the error and stop I/O */
- conn->flags |= CO_FL_ERROR;
}
/*
--
1.7.7
