[sr-dev] Re: [kamailio/kamailio] extented haproxy protocol support (PR #3731)

@miconda commented on this pull request. 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. 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. 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... 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... * 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. -- Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/pull/3731#pullrequestreview-1849803321 You are receiving this because you are subscribed to this thread. Message ID: &lt;kamailio/kamailio/pull/3731/review/1849803321(a)github.com&gt;