@miconda commented on this pull request.

@@ -204,6 +204,10 @@ static inline int msg_send_buffer(

goto error; }

+ if(!dst->id) { + dst->id = con->id; + } +

Any more details about why this is needed? I cannot easy relate it to the commit message.

@@ -828,7 +829,22 @@ static inline int lumps_len(

send_proto_id = send_sock->proto; } /* init recv_address_str, recv_port_str & recv_port_no */ - if(msg->rcv.bind_address) { + if (ksr_tcp_accept_haproxy && msg->rcv.proto_reserved2) { + if (msg->rcv.bind_address->proto == PROTO_TLS && msg->rcv.bind_address != NULL && msg->rcv.bind_address->useinfo.address_str.len > 0) { + recv_address_str = &msg->rcv.bind_address->useinfo.address_str;

It should be formatted with clang-format in order to match the coding style.

@@ -2845,6 +2876,47 @@ int branch_builder(unsigned int hash_index,

return size; }

+static int is_haproxy(struct dest_info *send_info, struct receive_info *haproxy_rcv) {

Can you add some comment to describe the purpose of the function? The name is rather generic, from C code point of view it looks like it tries to figure out if the target via via haproxy...

@@ -396,6 +396,7 @@ typedef struct sip_msg

char *unparsed; /*!< here we stopped parsing*/

struct receive_info rcv; /*!< source & dest ip, ports, proto a.s.o*/ + struct receive_info haproxy_rcv; /*!< source & dest ip, ports, proto a.s.o for the message from haproxy*/

Traffic via haproxy is not very common, does it make sense to add a full value-structure for every message instead of a pointer that is allocated only when it is the case. Practically the size of message increases for common traffic over direct UDP/TCP/TLS/SCTP/WebSocket. On the other hand, cloning in shm has to be done if allocated. This is just to ponder on what would be better...

@@ -1843,13 +1843,17 @@ inline static int handle_io(struct fd_map *fm, short events, int idx)

* handle_io might decide to del. the new connection => * must be in the list */ tcpconn_listadd(tcp_conn_lst, con, c_next, c_prev); - t = get_ticks_raw(); - con->timeout = t + S_TO_TICKS(TCP_CHILD_TIMEOUT); - /* re-activate the timer */ - con->timer.f = tcpconn_read_timeout; - local_timer_reinit(&con->timer); - local_timer_add(&tcp_reader_ltimer, &con->timer, - S_TO_TICKS(TCP_CHILD_TIMEOUT), t); + if(con->rcv.proto_reserved2 && con->type == PROTO_WSS) { + //not setting up timers for haproxy WSS connections

The patch is quite consistent to important parts of the core. It has to be reviewed properly, so far I did a quick walk through, other developers are welcome to jump in.

@ivanuschak commented on this pull request.

@@ -2845,6 +2876,47 @@ int branch_builder(unsigned int hash_index,

return size; }

+static int is_haproxy(struct dest_info *send_info, struct receive_info *haproxy_rcv) {

The function goal is as follows: 1. it retrieves the tcp_connetion_id from the `dest_info` parameter 2. if this TCP connection is a haproxy connection, then the `haproxy_rcv` param is filled with the corresponding haproxy data and the function returns non-zero value 3. otherwise if the TCP connection is not a connection to haproxy then the function returns zero value.

We will add the comments for this function in the source code.

@ivanuschak commented on this pull request.

@@ -1843,13 +1843,17 @@ inline static int handle_io(struct fd_map *fm, short events, int idx)

* handle_io might decide to del. the new connection => * must be in the list */ tcpconn_listadd(tcp_conn_lst, con, c_next, c_prev); - t = get_ticks_raw(); - con->timeout = t + S_TO_TICKS(TCP_CHILD_TIMEOUT); - /* re-activate the timer */ - con->timer.f = tcpconn_read_timeout; - local_timer_reinit(&con->timer); - local_timer_add(&tcp_reader_ltimer, &con->timer, - S_TO_TICKS(TCP_CHILD_TIMEOUT), t); + if(con->rcv.proto_reserved2 && con->type == PROTO_WSS) { + //not setting up timers for haproxy WSS connections

Please pay attention at this particular change - from line 1846 to line 1856.

The purpose of this change is to keep the WSS connection connected. When testing this haproxy change without this particular piece of change (from line 1846 to line 1856) we observed that kamailio was dropping WSS connection by timer, and this drop looked strange, because it happened in about 5-7 seconds after the connection was established. One of the requirement for haproxy was to keep TLS/WSS connection established during all the dialog and not to drop it without any important reason. I tried to find how to make it not to be dropped by timer, but found the only way is to not to set the timer up for WSS. Also the drop cause is still not cleat for me. I don't think this is a good solution, but I was not able to find a better one. I would appreciate if you check this change attentively. If needed I can provide with the logs where kamailio drops WSS connection.