call fnet_socket_shutdown() in SS_CLOSING cause FNET_ERR_NOTCONN about fnet

it seems the behavior is different from the document from this link

It is the same: [ENOTCONN] The s argument specifies a socket which is not connected.
=> if(sk->state != SS_CONNECTED)
Probably, it is better to look inside a FreeBSD source code to clarify the meaning.

[tranvantruonggit]

I have been traced some of the implementation of the Linux and FreeBSD. My observation is that the implementation of the shutdown generally allow to be successful in case the socket is not closed. With that said, based on the reference implementation is some bsd/linux implementation of the shutdown function, ENOTCONN is not returned in case SS_CLOSING (or SS_DISCONNECTING), SS_CONNECTING, SS_CONNECTED. Reference to the following implementation:

• http://fxr.watson.org/fxr/source/bsd/kern/uipc_socket.c?v=xnu-8792#L4676 (function soshutdown).

• [socket.c - net/tipc/socket.c - Linux source code (v3.7.1) - Bootlin](https://elixir.bootlin.com/linux/v3.7.1/source/net/tipc/socket.c#L1522)

Futhermore, I think the FNET implementation at the moment is that the tcp_connection_state after receiving FIN from the remoted node will be in the state cb->tcpcb_connection_state = CLOSE_WAIT, at the same time, the sk->sk_state is SS_CLOSING.

Honestly, I am not sure my finding is valid based on the other OSes' implementation, because there is no concrete document about this case. However, one of the problem I see is that the fnet' shutdown function always return ENOTCONN if it receives the FIN firstly, it doesn't process to send another FIN to the peer node.

I am eager to know your opinion regarding this.

So we can change the code to:

    if(sk->state == SS_CLOSED) 
    {
        _fnet_socket_set_error(sk, FNET_ERR_NOTCONN); 
        return FNET_ERR;
    }
   else

Are you OK with it?

[tranvantruonggit]

I think that is possible. I will test your suggestion and let you know the outcome within couple of days.On 17 Jul 2023, at 16:19, Andrej Butok ***@***.***> wrote: So we can change the code to: if(sk->state == SS_CLOSED) { _fnet_socket_set_error(sk, FNET_ERR_NOTCONN); return FNET_ERR; } else Are you OK with it? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[tranvantruonggit]

After a few testings, I can confirm the change in the condition if(sk->state == SS_CLOSED) creating the expected behavior.

Do you want me to create the Pull Request or you can update it when you have a chance?

Here is the snippet of code I wrote to do the sanity test the modification. I put here just for the sake of tracking the discussion.

...
    // Check if received data is "exit"
    if (  buffer[0] == 'e' &&
          buffer[1] == 'x'&&
          buffer[2] == 'i'&&
          buffer[3] == 't') {
        // Print shutdown message
        char shutdown_msg[] = "Received 'exit' command. Shutting down writing.\n";
        fnet_socket_send(context->sockfd, shutdown_msg, sizeof(shutdown_msg) - 1,0);
    
    
        fnet_socket_getopt(context->sockfd,SOL_SOCKET,SO_STATE,(void*)&test_socket_state,(fnet_size_t*)&len);
        // Shutdown writing
        test_fnet_return = fnet_socket_shutdown(context->sockfd, SD_WRITE);
        context->state = ETH_CLIENT_CLOSING;
    }
...

The above code check the state of the socket via the variable test_socket_state and read the return of the fnet_socket_shutdown in test_fnet_return

The behaviour before the modification:

• test_socket_state = SS_CLOSING
• test_fnet_return = FNET_ERR

The behaviour after the modification:

• test_socket_state = SS_CLOSING
• test_fnet_return = FNET_OK

Do you want me to create the Pull Request or you can update it when you have a chance?

Yes, if you can. It will be quicker.
I am occupied by different projects now, so it can take much time waiting for me.

[butok]

Merged.

[tranvantruonggit]

After thought, though it is irrelevant, my "fix" for the _fnet_tcp_initial_seq_number_update is not an actual fix (it doesn't change the compiled code), just an typo fix by putting parenthesis in.

I have investigated a bit and it seem Henri de Veer from your the sourceforge thread has addressed it correctly.

I will make another PR to fix that, are you fine with that?

Really sorry for my overlooking in that function.

[butok]

ok