From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f174.google.com (mail-ua0-f174.google.com [209.85.217.174]) by dpdk.org (Postfix) with ESMTP id C3C8668A9 for ; Tue, 9 May 2017 23:16:47 +0200 (CEST) Received: by mail-ua0-f174.google.com with SMTP id e55so15026048uaa.2 for ; Tue, 09 May 2017 14:16:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=GUQpcmOZt83fEOTsbbm9IR61U+hxR+oglUt+aqIYpco=; b=EgroVO50Ngc7o4R4mOsLc5uWqiIp11U+iR+PAn80g1Mm3aJnoqUeNlNEPiABrk859F qp+TxL0cF0SHwYxMyY166Y0IqgJCv3Aez9TFLAkE5n6b612aSpi8S+wEMVYUY0pNjwT+ RqSrLHp7Ln+WIflnf9uo7lnKFiYqLom0QoRcJA50+v+DK7N2bEfcAD3EUV2FKf/UXOum A4aEJzzvd/Rmx4oFgflbEskS53Jbj5W6SQ6P+4psjwGquMsbLg7AU5P2/81D3G2iu+wM Qzm/R/a5xRJxy/b+pcsWleXScHgsQ+AsHdSz4MxA4pWpuexxu1BlqmxSirjDUQGdcK5Y ERKw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=GUQpcmOZt83fEOTsbbm9IR61U+hxR+oglUt+aqIYpco=; b=GvSABL9oUoJph/w9BIj4Df/AxUhjfWURJysPVuG+wsBzR/uQVGv/C+1HquZmtn2FIx gGtKfcT1yR8cXzCRpPhZIspRxrVlBCjtAPBf8vtZo5s+yqpYKAbxZPRxwhhVZBw2dBhY AyEV97lTP/1fneaineXyOWOA+5vGFnXqZcoJSAGIFAmtbWMDo134xeD8MtVXH7OeXQyd rIe8E6DBGGNTjd/agntlT7DqWP6KW2ngFCkA0JS5XhfgDBDIA2wpjjBQQtXnNIbK9wI2 MaYEXg6V0Oj0FyVk5jDzLWvZ9gtlQg249HanChk1w4yWw/m/OGLaj/Fywh/VJ2bVrVhd VK4g== X-Gm-Message-State: AODbwcAFiw233V4mS4ssVQ+Y1HPTM6Pm/1YiqbTWzmWH2PRz71lsxgKP xnEqb63hsWf8klHOnUmXAV+/NgTzf4Bp X-Received: by 10.176.3.209 with SMTP id 75mr1184392uau.122.1494364607051; Tue, 09 May 2017 14:16:47 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.14.69 with HTTP; Tue, 9 May 2017 14:16:46 -0700 (PDT) In-Reply-To: References: <1494265458-35709-1-git-send-email-alejandro.lucero@netronome.com> From: Alejandro Lucero Date: Tue, 9 May 2017 22:16:46 +0100 Message-ID: To: "Burakov, Anatoly" Cc: "dev@dpdk.org" , "jerin.jacob@caviumnetworks.com" , "thomas@monjalon.net" Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] vfio: use right index when tracking devices in a vfio group X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 May 2017 21:16:48 -0000 Hi Anatoly, On Tue, May 9, 2017 at 4:18 PM, Burakov, Anatoly wrote: > Hi Alejandro, > > > From: Alejandro Lucero [mailto:alejandro.lucero@netronome.com] > > Sent: Monday, May 8, 2017 6:44 PM > > To: dev@dpdk.org > > Cc: Burakov, Anatoly ; > > jerin.jacob@caviumnetworks.com; thomas@monjalon.net > > Subject: [PATCH] vfio: use right index when tracking devices in a vfio > group > > > > Previous fix for properly handling devices from the same VFIO group > > introduced another bug where the file descriptor of a kernel vfio group > is > > used as the index for tracking number of devices of a vfio group struct > > handled by dpdk vfio code. Instead of the file descriptor itself, the > vfio group > > object that file descriptor is registered with has to be used. > > > > This patch introduces specific functions for incrementing or decrementing > > the device counter for a specific vfio group using the vfio file > descriptor as a > > parameter. Note the code is not optimized as the vfio group is found > > sequentially going through the vfio group array but this should not be a > > problem as this is not related to packet handling at all. > > > > Fixes: a9c349e3a100 ("vfio: fix device unplug when several devices per > > group") > > > > Signed-off-by: Alejandro Lucero > > --- > > lib/librte_eal/linuxapp/eal/eal_vfio.c | 60 > > +++++++++++++++++++++++++++------- > > 1 file changed, 49 insertions(+), 11 deletions(-) > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > index d3eae20..21d126f 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > @@ -172,6 +172,44 @@ > > return -1; > > } > > > > + > > +static int > > +get_vfio_group_idx(int vfio_group_fd) > > +{ > > + int i; > > + for (i = 0; i < VFIO_MAX_GROUPS; i++) > > + if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) > > + return i; > > + return -1; > > +} > > + > > +static void > > +vfio_group_device_get(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + vfio_cfg.vfio_groups[i].devices++; > > Maybe add a check for I < 0? > > Right. I doubted about adding such a check, since being < 0 or > VFIO_MAX_GROUPS are bad news. But better to avoid using a wrong index which could led to a random write elsewhere. I will send another patch version with that check and a RTE_LOG(ERR ... call. And same for the other functions. Thanks > > +} > > + > > +static void > > +vfio_group_device_put(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + vfio_cfg.vfio_groups[i].devices--; > > Same here. > > > +} > > + > > +static int > > +vfio_group_device_count(int vfio_group_fd) { > > + int i; > > + > > + i = get_vfio_group_idx(vfio_group_fd); > > + return vfio_cfg.vfio_groups[i].devices; } > > And here. > > > + > > int > > clear_group(int vfio_group_fd) > > { > > @@ -180,15 +218,14 @@ > > > > if (internal_config.process_type == RTE_PROC_PRIMARY) { > > > > - for (i = 0; i < VFIO_MAX_GROUPS; i++) > > - if (vfio_cfg.vfio_groups[i].fd == vfio_group_fd) { > > - vfio_cfg.vfio_groups[i].group_no = -1; > > - vfio_cfg.vfio_groups[i].fd = -1; > > - vfio_cfg.vfio_groups[i].devices = 0; > > - vfio_cfg.vfio_active_groups--; > > - return 0; > > - } > > - return -1; > > + i = get_vfio_group_idx(vfio_group_fd); > > + if ( i < 0) > > + return -1; > > + vfio_cfg.vfio_groups[i].group_no = -1; > > + vfio_cfg.vfio_groups[i].fd = -1; > > + vfio_cfg.vfio_groups[i].devices = 0; > > + vfio_cfg.vfio_active_groups--; > > + return 0; > > } > > > > /* This is just for SECONDARY processes */ @@ -358,7 +395,7 @@ > > clear_group(vfio_group_fd); > > return -1; > > } > > - vfio_cfg.vfio_groups[vfio_group_fd].devices++; > > + vfio_group_device_get(vfio_group_fd); > > > > return 0; > > } > > @@ -406,7 +443,8 @@ > > /* An VFIO group can have several devices attached. Just when there > > is > > * no devices remaining should the group be closed. > > */ > > - if (--vfio_cfg.vfio_groups[vfio_group_fd].devices == 0) { > > + vfio_group_device_put(vfio_group_fd); > > + if (!vfio_group_device_count(vfio_group_fd)) { > > > > if (close(vfio_group_fd) < 0) { > > RTE_LOG(INFO, EAL, "Error when closing > > vfio_group_fd for %s\n", > > -- > > 1.9.1 > >