On 1/13/25 16:01, Stefano Garzarella wrote:
> On Mon, Jan 13, 2025 at 02:51:58PM +0100, Michal Luczaj wrote:
>> On 1/13/25 12:05, Stefano Garzarella wrote:
>>> ...
>>> An alternative approach, which would perhaps allow us to avoid all this,
>>> is to re-insert the socket in the unbound list after calling release()
>>> when we deassign the transport.
>>>
>>> WDYT?
>>
>> If we can't keep the old state (sk_state, transport, etc) on failed
>> re-connect() then reverting back to initial state sounds, uhh, like an
>> option :) I'm not sure how well this aligns with (user's expectations of)
>> good ol' socket API, but maybe that train has already left.
>
> We really want to behave as similar as possible with the other sockets,
> like AF_INET, so I would try to continue toward that train.
I was worried that such connect()/transport error handling may have some
user visible side effects, but I guess I was wrong. I mean you can still
reach a sk_state=TCP_LISTEN with a transport assigned[1], but perhaps
that's a different issue.
I've tried your suggestion on top of this series. Passes the tests.
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index fa9d1b49599b..4718fe86689d 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -492,6 +492,10 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
vsk->transport->release(vsk);
vsock_deassign_transport(vsk);
+ vsock_addr_unbind(&vsk->local_addr);
+ vsock_addr_unbind(&vsk->remote_addr);
+ vsock_insert_unbound(vsk);
+
/* transport's release() and destruct() can touch some socket
* state, since we are reassigning the socket to a new transport
* during vsock_connect(), let's reset these fields to have a
>> Another possibility would be to simply brick the socket on failed (re)connect.
>
> I see, though, this is not the behavior of AF_INET for example, right?
Right.
> Do you have time to investigate/fix this problem?
> If not, I'll try to look into it in the next few days, maybe next week.
I'm happy to help, but it's not like I have any better ideas.
Michal
[1]: E.g. this way:
```
from socket import *
MAX_PORT_RETRIES = 24 # net/vmw_vsock/af_vsock.c
VMADDR_CID_LOCAL = 1
VMADDR_PORT_ANY = -1
hold = []
def take_port(port):
s = socket(AF_VSOCK, SOCK_SEQPACKET)
s.bind((VMADDR_CID_LOCAL, port))
hold.append(s)
return s
s = take_port(VMADDR_PORT_ANY)
_, port = s.getsockname()
for _ in range(MAX_PORT_RETRIES):
port += 1
take_port(port);
s = socket(AF_VSOCK, SOCK_SEQPACKET)
err = s.connect_ex((VMADDR_CID_LOCAL, port))
assert err != 0
print("ok, connect failed; transport set")
s.bind((VMADDR_CID_LOCAL, port+1))
s.listen(16)
```
