From:   "Wang, Wei W" <wei.w.wang@intel.com>
To:     Stefano Garzarella <sgarzare@redhat.com>
CC:     "mst@redhat.com" <mst@redhat.com>,
        "stefanha@redhat.com" <stefanha@redhat.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "asias@redhat.com" <asias@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "virtualization@lists.linux-foundation.org" 
        <virtualization@lists.linux-foundation.org>
Subject: RE: [PATCH] virtio/vsock: fix the transport to work with
 VMADDR_CID_ANY
Thread-Topic: [PATCH] virtio/vsock: fix the transport to work with
 VMADDR_CID_ANY
Thread-Index: AQHX4daqV2Omz/OJfUqBSDdTk17B0qwT+K9Q//+PVgCAAXxRUA==
Date:   Fri, 26 Nov 2021 02:15:48 +0000
Message-ID: <97be1440032248a29b5eb75619f8ac4d@intel.com>
References: <20211125071554.16969-1-wei.w.wang@intel.com>
 <7992566c682b46dc9ec2502e44a2fb04@intel.com>
 <20211125104046.wxupkn6uogvohioi@steredhat>
In-Reply-To: <20211125104046.wxupkn6uogvohioi@steredhat>
Accept-Language: en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-reaction: no-action
dlp-version: 11.6.200.16
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk

On Thursday, November 25, 2021 6:41 PM, Stefano Garzarella wrote:
> On Thu, Nov 25, 2021 at 09:27:40AM +0000, Wang, Wei W wrote:
> >On Thursday, November 25, 2021 3:16 PM, Wang, Wei W wrote:
> >> -	/* Update CID in case it has changed after a transport reset event */
> >> -	vsk->local_addr.svm_cid = dst.svm_cid;
> >> -
> >>  	if (space_available)
> >>  		sk->sk_write_space(sk);
> >>
> >
> >Not sure if anybody knows how this affects the transport reset.
> 
> I believe the primary use case is when a guest is migrated.
> 
> After the migration, the transport gets a reset event from the hypervisor and
> all connected sockets are closed. The ones in listen remain open though.
> 
> Also the guest's CID may have changed after migration. So if an application has
> open listening sockets, bound to the old CID, this should ensure that the socket
> continues to be usable.

OK, thanks for sharing the background.

> 
> The patch would then change this behavior.
> 
> So maybe to avoid problems, we could update the CID only if it is different
> from VMADDR_CID_ANY:
> 
> 	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
> 		vsk->local_addr.svm_cid = dst.svm_cid;
> 
> 
> When this code was written, a guest only supported a single transport, so it
> could only have one CID assigned, so that wasn't a problem.
> For that reason I'll add this Fixes tag:
> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")

Sounds good to me.

Thanks,
Wei