| Message ID | 20190927112703.17745-9-sgarzare@redhat.com |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
| Series | vsock: add multi-transports support | expand |
> From: Stefano Garzarella <sgarzare@redhat.com> > Sent: Friday, September 27, 2019 4:27 AM > > vsock_insert_unbound() was called only when 'sock' parameter of > __vsock_create() was not null. This only happened when > __vsock_create() was called by vsock_create(). > > In order to simplify the multi-transports support, this patch > moves vsock_insert_unbound() at the end of vsock_create(). > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > net/vmw_vsock/af_vsock.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > Reviewed-by: Dexuan Cui <decui@microsoft.com>
On Fri, Sep 27, 2019 at 01:26:58PM +0200, Stefano Garzarella wrote: > vsock_insert_unbound() was called only when 'sock' parameter of > __vsock_create() was not null. This only happened when > __vsock_create() was called by vsock_create(). > > In order to simplify the multi-transports support, this patch > moves vsock_insert_unbound() at the end of vsock_create(). > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > net/vmw_vsock/af_vsock.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) Maybe transports shouldn't call __vsock_create() directly. They always pass NULL as the parent socket, so we could have a more specific function that transports call without a parent sock argument. This would eliminate any concern over moving vsock_insert_unbound() out of this function. In any case, I've checked the code and this patch is correct. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Wed, Oct 09, 2019 at 01:34:23PM +0100, Stefan Hajnoczi wrote: > On Fri, Sep 27, 2019 at 01:26:58PM +0200, Stefano Garzarella wrote: > > vsock_insert_unbound() was called only when 'sock' parameter of > > __vsock_create() was not null. This only happened when > > __vsock_create() was called by vsock_create(). > > > > In order to simplify the multi-transports support, this patch > > moves vsock_insert_unbound() at the end of vsock_create(). > > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > > --- > > net/vmw_vsock/af_vsock.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > Maybe transports shouldn't call __vsock_create() directly. They always > pass NULL as the parent socket, so we could have a more specific > function that transports call without a parent sock argument. This > would eliminate any concern over moving vsock_insert_unbound() out of > this function. In any case, I've checked the code and this patch is > correct. Yes, I agree with you, I can add a new patch to do this cleaning. > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Thanks, Stefano
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index dee69d7ee148..95e6db21e7e1 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -634,9 +634,6 @@ struct sock *__vsock_create(struct net *net, return NULL; } - if (sock) - vsock_insert_unbound(vsk); - return sk; } EXPORT_SYMBOL_GPL(__vsock_create); @@ -1875,6 +1872,8 @@ static const struct proto_ops vsock_stream_ops = { static int vsock_create(struct net *net, struct socket *sock, int protocol, int kern) { + struct sock *sk; + if (!sock) return -EINVAL; @@ -1894,7 +1893,13 @@ static int vsock_create(struct net *net, struct socket *sock, sock->state = SS_UNCONNECTED; - return __vsock_create(net, sock, NULL, GFP_KERNEL, 0, kern) ? 0 : -ENOMEM; + sk = __vsock_create(net, sock, NULL, GFP_KERNEL, 0, kern); + if (!sk) + return -ENOMEM; + + vsock_insert_unbound(vsock_sk(sk)); + + return 0; } static const struct net_proto_family vsock_family_ops = {
vsock_insert_unbound() was called only when 'sock' parameter of __vsock_create() was not null. This only happened when __vsock_create() was called by vsock_create(). In order to simplify the multi-transports support, this patch moves vsock_insert_unbound() at the end of vsock_create(). Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- net/vmw_vsock/af_vsock.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)