stripydog / kplex

kplex marine data multiplexer
Other
82 stars 38 forks source link

Preamble not sent after reconnect #51

Closed jotx closed 3 years ago

jotx commented 3 years ago

I use the preamble feature when connecting to an AIS server which works fine. However, the AIS server (for unknown reasons) sometimes resets the connection and when kplex reconnects (I use persist=fromstart) no preamble is sent.

The documentations says:

If specified with the "persist" option, the preamble string will be sent after each reconnection following connection loss.

I had a quick look in the code but could not find anything obvious. Perhaps the preamble is cleaned up when the connection is reset?

Version:

~$ kplex -V
1.4

My configuration (with private info removed):

[tcp]
direction=in
mode=client
address=1.2.3.4
port=1234
persist=fromstart
keepalive=yes
keepidle=30
keepintvl=10
Keepcnt=3
retry=600
preamble=\x01username\x00password\x00

[tcp]
direction=out
mode=server
address=0.0.0.0
port=5002

Log output:

$ kplex -d 3 -f kplex.conf
kplex DEBUG: kplex starting, config file kplex.conf
kplex DEBUG: _tcp-id1: initialised
kplex DEBUG: _tcp-id2: initialised
kplex DEBUG: _tcp-id2: New connection id 20006 successfully received from 127.0.0.1
kplex DEBUG: _tcp-id2 id 20006: write failed: Broken pipe
kplex DEBUG: Cleaning up data for exiting output interface _tcp-id2 id 20006
kplex DEBUG: _tcp-id1: Read Failed
kplex DEBUG: _tcp-id1: Reconnecting (read) interface
kplex DEBUG: _tcp-id1: Reconnected (read) interface
kplex DEBUG: _tcp-id1: Read Failed
kplex DEBUG: _tcp-id1: Reconnecting (read) interface
kplex DEBUG: _tcp-id1: Reconnected (read) interface
kplex DEBUG: _tcp-id2: New connection id 20007 successfully received from 127.0.0.1
jotx commented 3 years ago

This is perhaps a regression of issue #11.

jotx commented 3 years ago

I'm too lazy to debug this myself but I had a closer look at the code.

Perhaps direction=both is required when using preamble? It would be make sense since we need to send the preamble 😄. If direction=in, ifa->pair will be NULL and reread() will not send the preamble?

        if (ifa->pair) {
            if (!(iftp = (struct if_tcp *) ifa->pair->info)) {
                logerr(errno,"No pair information found for bi-directional tcp connection!");
                nread=-1;
            } else {
                if (ift->shared->nodelay && (setsockopt(ift->fd,IPPROTO_TCP,
                        TCP_NODELAY,&on,sizeof(on)) < 0))
                    logerr(errno,
                            "Could not disable Nagle on new tcp connection");

                iftp->fd = ift->fd;
                if (iftp->shared->preamble)
                    do_preamble(iftp,NULL);
            }
        }

Maybe do_preamble() should be called also when ifa->direction == IN like before commit e654d68?

I'm testing with direction=both now, maybe it works then.

stripydog commented 3 years ago

Thanks for the report. I'll look at this today

stripydog commented 3 years ago

Firstly thanks for submitting this and with such a great level of analysis. It looks like you're right about this being a bug in 1.4 but it seems to have been fixed when the reconnection logic was reworked for re-looking up addresses on each reconnection. The newer implementation looks like it would logically work and I've experimented to confirm that preamble is indeed sent on reconnection with direction=in. Could you possibly confirm that by checking out the develop branch and trying that?

jotx commented 3 years ago

Yep, direction=in + preamble seems to be fixed in the development branch! 👌

(For the record my workaround using direction=both with 1.4 also worked)

stripydog commented 3 years ago

Great. Thanks again for an excellent issue report. I'll add this to known issues today.