canlogserver: Close accsocket and can when tcp client disconnected

[qianfan-Zhao]

Close all can socket and send "FYI" to tcp client when the tcp client disconnected.

Signed-off-by: qianfan Zhao [email protected]

[hartkopp]

What problem do you want to solve? The accept() leads to a fork() and when the child is terminated due to the client termination the child is completely removed (which closes all its sockets, e.g. CAN_RAW). I just don't see the reason for the PR ...

[qianfan-Zhao]

@hartkopp Hi, please try this patch:

do_not_fork.patch.txt

Open canlogserver and 'telnet' it in another computer, disconnect tcp link in the client right now. Wireshark capture those step:

(canlogserver IP: 10.11.12.13, tcp client IP: 10.11.12.1)

1	0.000000000	10.11.12.1	10.11.12.13	TCP	74	52816 → 28700 [SYN] Seq=0 Win=29200 Len=0 MSS=1460 SACK_PERM=1 TSval=4168349364 TSecr=0 WS=128
2	0.000370615	10.11.12.13	10.11.12.1	TCP	74	28700 → 52816 [SYN, ACK] Seq=0 Ack=1 Win=14480 Len=0 MSS=1460 SACK_PERM=1 TSval=397751 TSecr=4168349364 WS=8
3	0.000026419	10.11.12.1	10.11.12.13	TCP	66	52816 → 28700 [ACK] Seq=1 Ack=1 Win=29312 Len=0 TSval=4168349364 TSecr=397751
8	1.268289006	10.11.12.1	10.11.12.13	TCP	66	52816 → 28700 [FIN, ACK] Seq=1 Ack=1 Win=29312 Len=0 TSval=4168355867 TSecr=397751
9	0.005321842	10.11.12.13	10.11.12.1	TCP	66	28700 → 52816 [ACK] Seq=1 Ack=2 Win=14480 Len=0 TSval=398402 TSecr=4168355867

canlogserver doesn't close the tcp link when the client disconnected. (There are no FYI sent from server).

If the can interface got a frame after the tcp client disconnected, canlogserver try write this frame to tcp client. It will got a RST.

13	75.420245719	10.11.12.13	10.11.12.1	TCP	111	28700 → 52816 [PSH, ACK] Seq=1 Ack=1 Win=1810 Len=45 TSval=438966 TSecr=4168355867
14	0.000053205	10.11.12.1	10.11.12.13	TCP	54	52816 → 28700 [RST] Seq=1 Win=0 Len=0

But canlogserver doesn't know the client disconnected, it think this can frame was sent successfully.

# ./canlogserver can0
accepted
Write 45 bytes

The next time when it try sent another can frame, it will got signal PIPE and died due to without signal handler.

# ./canlogserver can0
accepted
Write 45 bytes
shutdown_gra: got signal 13

The tcp logic seems not good.

[yashi]

Hi,

I've been reading canlogserver.c thease days. And indeed it does some odd thing.

The code structure is basicall,

1. wait on accept()
2. when new connection arrived, fork
   • if parent, accept again and blocked
   • if child, break out the loop and keep going
3. open given can interfaces
4. while running, serve a client
5. When client closes the connection, the child get SIGPIPE and die
6. When SIGTERM, app shuts down

When a new client connects, accept() returns, and the parent has a chance to reap SIGPIPED children. It forks and goes back to accept().

For new child, enumerate the list of can interfaces and open them again.

I'd propose

• Close sockets and exit when write() failed, instead of SIGPIPED. That means
  • Ignore SIGPIPE
  • handle error return from write() (or send())
  • get out of the while (running)block by running = false, which doesn't end the server but just the child.

Optionally,

• Open can interfaces before fork() so that all childen keep the same fds and no need to open.
  • assuming that multiple read() (or recvfrom()) from the fds gives you the exact copy of the captured copy, as does the current code do.

The first part goes something like:

diff --git a/canlogserver.c b/canlogserver.c
index 3fe1ad9..6e7b137 100644
--- a/canlogserver.c
+++ b/canlogserver.c
@@ -424,9 +424,10 @@ int main(int argc, char **argv)
                                sprint_canframe(temp+strlen(temp), &frame, 0, maxdlen); 
                                strcat(temp, "\n");
 
-                               if (write(accsocket, temp, strlen(temp)) < 0) {
-                                       perror("writeaccsock");
-                                       return 1;
+                               if (send(accsocket, temp, strlen(temp), MSG_NOSIGNAL) < 0) {
+                                       //perror("writeaccsock");
+                                       running = 0;
+                                       break;
                                }
                    
 #if 0

WDYT?