Extend -net-disconnect-ok to close unsaveable unix sockets on save #10996
Conversation
| @@ -438,15 +438,15 @@ func (e *connectionedEndpoint) Listen(ctx context.Context, backlog int) *syserr. | |||
| } | |||
|
|
|||
| // Accept accepts a new connection. | |||
| func (e *connectionedEndpoint) Accept(ctx context.Context, peerAddr *Address) (Endpoint, *syserr.Error) { | |||
| func (e *connectionedEndpoint) Accept(ctx context.Context, peerAddr *Address, opts UnixSocketOpts) (Endpoint, *syserr.Error) { | |||
There was a problem hiding this comment.
It looks like Accept is the only method that actually utilizes UnixSocketOpts. Can we remove it from the various Connect methods?
There was a problem hiding this comment.
It's also used in pkg/sentry/fsimpl/gofer/socket to implement BoundEndpoint::UnidirectionalConnect and BoundEndpoint::BidirectionalConnect for gofer-backed connectable sockets, both of which are used in the implementation of connect(2).
| @@ -163,6 +169,10 @@ func (c *HostConnectedEndpoint) Send(ctx context.Context, data [][]byte, control | |||
| return 0, false, syserr.ErrInvalidEndpointState | |||
| } | |||
|
|
|||
| if c.IsSendClosed() { | |||
There was a problem hiding this comment.
Could the socket be closed for send but still openable for recv?
There was a problem hiding this comment.
I believe so - a shutdown(2) syscall may only close it one way.
| @@ -187,7 +196,7 @@ type Endpoint interface { | |||
| // | |||
| // peerAddr if not nil will be populated with the address of the connected | |||
| // peer on a successful accept. | |||
| Accept(ctx context.Context, peerAddr *Address) (Endpoint, *syserr.Error) | |||
| Accept(ctx context.Context, peerAddr *Address, opts UnixSocketOpts) (Endpoint, *syserr.Error) | |||
There was a problem hiding this comment.
Same as above: it looks like this is the only method that actually uses UnixSocketOpts, so maybe we should remove it from the other methods.
|
I think I've responded to all comments (or pushed changes in response). Thanks so much for the review, and for your help! |
As discussed in gvisor issue #10897, this change extends the -net-disconnect-ok flag to handle unsaveable Unix domain sockets. When enabled, any Unix socket backed by host FDs that cannot be saved will instead be closed before checkpointing, and will be saved in a closed state. This makes it possible to checkpoint containers even when they have connected to host-mounted Unix sockets.
Most of the changes in the diff involve passing the new setting to where host-backed domain sockets are created -- if there's a less invasive approach, please let me know.