Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757225AbdIHVqb (ORCPT ); Fri, 8 Sep 2017 17:46:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:60686 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757162AbdIHVqa (ORCPT ); Fri, 8 Sep 2017 17:46:30 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C68D121B81 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=sstabellini@kernel.org Date: Fri, 8 Sep 2017 14:46:29 -0700 (PDT) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Boris Ostrovsky cc: Stefano Stabellini , xen-devel@lists.xen.org, linux-kernel@vger.kernel.org, jgross@suse.com, Stefano Stabellini Subject: Re: [PATCH v3 06/13] xen/pvcalls: implement bind command In-Reply-To: <3591ec0c-1556-dd20-1cc9-092eb1792c42@oracle.com> Message-ID: References: <1501541855-7354-1-git-send-email-sstabellini@kernel.org> <1501541855-7354-6-git-send-email-sstabellini@kernel.org> <3591ec0c-1556-dd20-1cc9-092eb1792c42@oracle.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4162 Lines: 124 On Sat, 12 Aug 2017, Boris Ostrovsky wrote: > On 07/31/2017 06:57 PM, Stefano Stabellini wrote: > > Send PVCALLS_BIND to the backend. Introduce a new structure, part of > > struct sock_mapping, to store information specific to passive sockets. > > > > Introduce a status field to keep track of the status of the passive > > socket. > > > > Signed-off-by: Stefano Stabellini > > CC: boris.ostrovsky@oracle.com > > CC: jgross@suse.com > > --- > > drivers/xen/pvcalls-front.c | 59 > > +++++++++++++++++++++++++++++++++++++++++++++ > > drivers/xen/pvcalls-front.h | 3 +++ > > 2 files changed, 62 insertions(+) > > > > diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c > > index 379b8fb..5ccef34 100644 > > --- a/drivers/xen/pvcalls-front.c > > +++ b/drivers/xen/pvcalls-front.c > > @@ -59,6 +59,13 @@ struct sock_mapping { > > wait_queue_head_t inflight_conn_req; > > } active; > > + struct { > > + /* Socket status */ > > +#define PVCALLS_STATUS_UNINITALIZED 0 > > +#define PVCALLS_STATUS_BIND 1 > > +#define PVCALLS_STATUS_LISTEN 2 > > + uint8_t status; > > + } passive; > > }; > > }; > > @@ -308,6 +315,58 @@ int pvcalls_front_connect(struct socket *sock, struct > > sockaddr *addr, > > return ret; > > } > > +int pvcalls_front_bind(struct socket *sock, struct sockaddr *addr, int > > addr_len) > > +{ > > + struct pvcalls_bedata *bedata; > > + struct sock_mapping *map = NULL; > > + struct xen_pvcalls_request *req; > > + int notify, req_id, ret; > > + > > + if (!pvcalls_front_dev) > > + return -ENOTCONN; > > + if (addr->sa_family != AF_INET || sock->type != SOCK_STREAM) > > > No protocol check? (in the previous patch too) As there is only one protocol for AF_INET SOCK_STREAM sockets, it is not possible to get a "wrong" protocol here. In fact the field is not even saved in struct socket or struct sockaddr. The check should be done at the time the socket is created (corresponding to the "socket" syscall), by the caller of pvcalls_front_socket. I made sure to have a comment in pvcalls_front_socket to clarify it: * Check socket type here, AF_INET and protocol checks are done * by the caller. > > + return -ENOTSUPP; > > + bedata = dev_get_drvdata(&pvcalls_front_dev->dev); > > + > > + map = (struct sock_mapping *) READ_ONCE(sock->sk->sk_send_head); > > + if (map == NULL) > > + return -EINVAL; > > + > > + spin_lock(&bedata->pvcallss_lock); > > + ret = get_request(bedata, &req_id); > > + if (ret < 0) { > > + spin_unlock(&bedata->pvcallss_lock); > > + return ret; > > + } > > + req = RING_GET_REQUEST(&bedata->ring, req_id); > > + req->req_id = req_id; > > + map->sock = sock; > > + req->cmd = PVCALLS_BIND; > > + req->u.bind.id = (uint64_t) map; > > + memcpy(req->u.bind.addr, addr, sizeof(*addr)); > > + req->u.bind.len = addr_len; > > + > > + init_waitqueue_head(&map->passive.inflight_accept_req); > > + > > + map->active_socket = false; > > + > > + bedata->ring.req_prod_pvt++; > > + RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&bedata->ring, notify); > > + spin_unlock(&bedata->pvcallss_lock); > > + if (notify) > > + notify_remote_via_irq(bedata->irq); > > + > > + wait_event(bedata->inflight_req, > > + READ_ONCE(bedata->rsp[req_id].req_id) == req_id); > > + > > + map->passive.status = PVCALLS_STATUS_BIND; > > + ret = bedata->rsp[req_id].ret; > > + /* read ret, then set this rsp slot to be reused */ > > + smp_mb(); > > + WRITE_ONCE(bedata->rsp[req_id].req_id, PVCALLS_INVALID_ID); > > + return 0; > > +} > > + > > static const struct xenbus_device_id pvcalls_front_ids[] = { > > { "pvcalls" }, > > { "" } > > diff --git a/drivers/xen/pvcalls-front.h b/drivers/xen/pvcalls-front.h > > index 63b0417..8b0a274 100644 > > --- a/drivers/xen/pvcalls-front.h > > +++ b/drivers/xen/pvcalls-front.h > > @@ -6,5 +6,8 @@ > > int pvcalls_front_socket(struct socket *sock); > > int pvcalls_front_connect(struct socket *sock, struct sockaddr *addr, > > int addr_len, int flags); > > +int pvcalls_front_bind(struct socket *sock, > > + struct sockaddr *addr, > > + int addr_len); > > #endif > > >