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
> Patch 0002: OPTIM: stream_interface: return directly if the connection
> flag CO_FL_ERROR has been set
>
> Actually, for patch 0002, the branch out_error in si_conn_send_cb() can
> be also deleted since only the following codes use it:
>
> static void si_conn_send_cb(struct connection *conn)
> {
> ...
> /* OK there are data waiting to be sent */
> if (si_conn_send_loop(conn) < 0)
> goto out_error;
>
> /* OK all done */
> return;
>
> out_error:
> /* Write error on the connection, report the error and stop I/O */
> conn->flags |= CO_FL_ERROR;
> }
>
> And si_conn_send_loop() will return -1 when the connection flag has
> CO_FL_ERROR set. As a result, we even do not need check the return value
> of si_conn_send_loop() since we just want to set CO_FL_ERROR on the
> connection flag and there is. The fix should be as below:
> @@ -1138,15 +1138,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;
> }
>
> If you agree with me, I will merge this fix into patch0002 and send the
> patch again.
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;
}
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.
Thanks!
Willy
