From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f195.google.com (mail-wr0-f195.google.com [209.85.128.195]) by dpdk.org (Postfix) with ESMTP id BCE875F72 for ; Thu, 28 Jun 2018 11:13:26 +0200 (CEST) Received: by mail-wr0-f195.google.com with SMTP id b8-v6so4708050wro.6 for ; Thu, 28 Jun 2018 02:13:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=GP5F3w48pNNr2knTm6cgm/mY7fJcFv7KtjHEwe4/BGE=; b=YKTkjQOWUnluApP3siBrvVKd8lRSJE3o37o/2EbSFKL6n356h5NTBW7ZX1AqzAHAUm gXTcIO77GU+HPo3768RJk9Z091b3K4NxFsEqayUsBPtjNlRChlCBfJMpduv5SmTWTycN 83g4fDOLRSpI+HQxA6HXmPu9Ybl7YNpCx8bBwvbjV4txxlIAxbwYUhpMwWZ5nmf9OvYr bDIpY4hez27Hgu8TIEDLkB83+WSr8IovEGINi8F+DvA9BclTynSOdPBHo/EsZAIW9q/e IGKY9r2wa1i8MILCjVUFZTHM/BCFDCSvtcKIt1Mpa/eYbSVo078vlrvl2vzzSziPO+37 UXsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=GP5F3w48pNNr2knTm6cgm/mY7fJcFv7KtjHEwe4/BGE=; b=Gw1VLUsQ3lUqN0+WGrmTp2INm+SXYNSlL3tGd5XD36bR/JHbthwbTLQ3lkzze2Ca7n G8mK1FJzJMhSmt3qJDb5ZVvdFoiPqMdwrSxvbuFt2fy9EPII6UkQVhA9FjzAQNE1mV+l oDe8D88NBB7tL94urj1T1vxV2WV7vzqtoviSDT3ME7JM+FUVEA21l1XoJ2Nj8KrQHGSg 3o9MqGj/FyjwBivtHn0INnqYnNR4gW/GJS+op9W2jkTlRnw+/P81/v75eUUY/CAIy/GJ IuKlvznjdVd8B5/oicMeLkbaiGtsPEZ/VpxFt6nQ2GYhKQrk1ipACaCR/W9UKcXAEiSN pvgg== X-Gm-Message-State: APt69E0Jk+Bgvfq0JEFc2PRB1ZnkNA8+Fg7+v9aZd2lJbf5jAcIeNxaP A0PQaHwRx+rd5WCkZJL2Bc0IVQ== X-Google-Smtp-Source: AAOMgpfPgTxFqjxlQkfnssOUmbxbKeT378Q0KzSzeM9k7qICelExv3iKsYZDkRePrvYqpi7hLZKKtg== X-Received: by 2002:adf:c4f0:: with SMTP id o45-v6mr4614260wrf.173.1530177206541; Thu, 28 Jun 2018 02:13:26 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id s10-v6sm12062997wmb.12.2018.06.28.02.13.25 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 28 Jun 2018 02:13:25 -0700 (PDT) Date: Thu, 28 Jun 2018 11:13:09 +0200 From: Adrien Mazarguil To: Shahaf Shuler Cc: "Xueming(Steven) Li" , "dev@dpdk.org" , Guillaume Gaudonville , Wisam Monther , Raslan Darawsheh , Olga Shern Message-ID: <20180628091309.GB4025@6wind.com> References: <20180525161814.13873-1-adrien.mazarguil@6wind.com> <20180614083047.10812-1-adrien.mazarguil@6wind.com> <20180614083047.10812-7-adrien.mazarguil@6wind.com> <20180627133235.GW4025@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors 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: Thu, 28 Jun 2018 09:13:26 -0000 On Thu, Jun 28, 2018 at 05:57:03AM +0000, Shahaf Shuler wrote: > Wednesday, June 27, 2018 4:33 PM, Adrien Mazarguil: > > Subject: Re: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port representors > > > > On Sun, Jun 24, 2018 at 01:33:31PM +0000, Shahaf Shuler wrote: > > > One more input, > > > > > > Sunday, June 17, 2018 1:15 PM, Shahaf Shuler: > > > > Subject: RE: [dpdk-dev] [PATCH v2 6/7] net/mlx5: probe all port > > > > representors > > > > > > [...] > > > > > > > > > + eth_list = tmp; > > > > > > for (i = 0; i < attr.orig_attr.phys_port_cnt; ++i) { > > > > > > - eth_list[i] = mlx5_dev_spawn_one(dpdk_dev, > > ibv_dev, vf, > > > > > > - &attr, i + 1); > > > > > > - if (eth_list[i]) > > > > > > - continue; > > > > > > - /* Save rte_errno and roll back in case of failure. */ > > > > > > - ret = rte_errno; > > > > > > - while (i--) { > > > > > > - mlx5_dev_close(eth_list[i]); > > > > > > - if (rte_eal_process_type() == > > RTE_PROC_PRIMARY) > > > > > > - rte_free(eth_list[i]->data- > > >dev_private); > > > > > > - > > claim_zero(rte_eth_dev_release_port(eth_list[i])); > > > > > > - } > > > > > > - free(eth_list); > > > > > > - rte_errno = ret; > > > > > > - return NULL; > > > > > > + eth_list[n] = mlx5_dev_spawn_one(dpdk_dev, > > ibv_dev[j], > > > > > vf, > > > > > > + &attr, i + 1, > > > > > > + j ? eth_list[0] : NULL, > > > > > > + j - 1); > > > > > > > > The representor id is according to the sort made by qsort (based on > > > > device names). > > > > A better way may be to set it according to the sysfs information, > > > > like you do in the mlx5_get_ifname function. > > > > What do you think? > > > > > > In fact relaying on linear increasing port numbers is dangerous. In may > > break on special scenarios like BlueField. > > > In BlueField there are representors between the x86 and the ARM cores. > > Those are not VF representors. The phys_port_name of those is -1 and each > > of them belongs to different phys_switch_id. > > > > > > We can argue whether it is correct/not to assign them w/ -1 value, but I > > think the suggested approach above can detect the right "vf_id" for those > > and not break the current behavior on x86. > > > Let me know if you have other suggestions. > > > > I didn't know that. Assuming that with these, there is exactly only one > > representor per device, I think we can manage, the main issue being that "- > > 1" will be difficult to parse as a valid "representor" argument which uses "-" > > for ranges. > > The -1 value is not for the representor id, It is for the id of the entity which exists on the other size of the representor. > The repesentor index is still 0, meaning the command line -w ,representor=0 is correct on this case. > > The problems comes from the assumption you do in your code about the representor id. > What you do currently is to receive the representors and qsort them by device name. then you assign the priv->rep_id based on the qsort output. > Later on when querying the if_name (mlx5_get_ifname) you assume that the phys_port_name of representor (which include the enumeration of what exists on its other side) is the same. > > For x86 it probably works. On BlueField it breaks, as from some reason the phys_port_name is -1. > > My suggestion is to set the priv->rep_id based on the phys_port_name instead of qsort output. Yes, understood. The only drawback using this approach is that mlx5 devices won't be usable at all if no netdevice can be associated with them (e.g. in case it was moved to another netns). Currently all matching IB devs are probed regardless, except they are handled as normal devices when the PMD can't determine whether it's dealing with a representor. > > Anyway, I suggest to deal with Bluefield specifics in a subsequent series. > > This one focuses on and is validated with VF representors only. > > It is related to VF representors only. BlueField is just an example. By "BlueField specifics", I mean the translation of -1 to 0 which so far is specific to BlueField. Another patch is needed for that. For devices where representors are properly numbered starting from 0, we must rely on the uninterpreted phys_port_name value directly, which must be a positive integer instead of a qsort() interpretation in order to properly handle holes in the sequence due to missing devices (netns). I intend to modify this patch as described. -- Adrien Mazarguil 6WIND