From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7F087A0471 for ; Fri, 19 Jul 2019 20:03:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9CC3058C6; Fri, 19 Jul 2019 20:03:26 +0200 (CEST) Received: from mail-pf1-f195.google.com (mail-pf1-f195.google.com [209.85.210.195]) by dpdk.org (Postfix) with ESMTP id EE96B374E for ; Fri, 19 Jul 2019 20:03:25 +0200 (CEST) Received: by mail-pf1-f195.google.com with SMTP id u14so14522502pfn.2 for ; Fri, 19 Jul 2019 11:03:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=VMST2M5kNmvMA0qbDqpbrBPG/wtkJEz0vyM1ozuVDKI=; b=Luw5pwcgtYkH9nhscXOeTRh+bxZdDfsY0p68e8XKfL6/OjvYwJt0LXnaQdfTokUSzJ d1epKhpPWvNtHo0GbSeqLvv0ECUtX/Nth2/Ih+nG9InRDWfFnV9wo/DJ8bHm9J5wsXZt YXLV/NaGBtnxQ9ResJ08IJEKTSmnR8X464nsDVxfTySsUGzSwkNDe/sXv807uRVoiMer zhPpkWhYY36HT1gw7sj6JP9DwdOFjW4PTuvYr3Orw4MFSQ9XQhn36F5sKfMJ4Z3ehw1O RdKXkKj0RB41BDmPbsEhiNcP+LchS3o6G1guiU4mRihhpOpjUUUDrDP+YgrK70e835c/ Lchw== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=VMST2M5kNmvMA0qbDqpbrBPG/wtkJEz0vyM1ozuVDKI=; b=LyJt2UB4HIluxiSlO5dcz05qH7erw1R9Skjl2slLp1gCaSTg0N951FcErxeMiy2NyV eTzR7fzie5jMnujKeMXBm7WIgkg/6hxHtgN9kNZKSuY2CgsvYDYwolPvhakP/paWSDcu yyuISP/ak5RxfW0Zr8DbYtYErrPzS4rUCfLEDAK1O2SOF1v4QyOVOt3ro08iNsFry++5 yCBRqxeobmQISeZ787g+D9MaNeell1QScJeH9nPqDIktuizF17RTppEiLHF5CjgJokbQ 7mYanL9AU7gldPW5fjxeZyEnrLDNZbm1X05mPo7b11k42/D22KhY85OApm2tHLW8vus5 OcTw== X-Gm-Message-State: APjAAAU+SjfacLOEYlAformblb5V9kIHmY0BsOJiUiEdBsjCd8MsjrLC UD9ste4C7BjKqnXk6kqNugQ= X-Google-Smtp-Source: APXvYqx8B20uTHNYRFz0blsD2mbb37KXTr8Dx/4o4w5aI1byRQWcFWZo6LHHqv5ci3FiGVGEvwIekA== X-Received: by 2002:a63:f817:: with SMTP id n23mr55958597pgh.35.1563559404985; Fri, 19 Jul 2019 11:03:24 -0700 (PDT) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id e11sm38060983pfm.35.2019.07.19.11.03.24 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Fri, 19 Jul 2019 11:03:24 -0700 (PDT) Date: Fri, 19 Jul 2019 11:03:17 -0700 From: Stephen Hemminger To: Slava Ovsiienko Cc: "dev@dpdk.org" , Yongseok Koh , Shahaf Shuler Message-ID: <20190719110317.27222925@hermes.lan> In-Reply-To: References: <20190712205425.17781-3-stephen@networkplumber.org> <1563514305-27405-1-git-send-email-viacheslavo@mellanox.com> <1563514305-27405-2-git-send-email-viacheslavo@mellanox.com> <20190719091544.40294c43@hermes.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/2] net/mlx5: cache the associated network device ifindex 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, 19 Jul 2019 16:41:38 +0000 Slava Ovsiienko wrote: > > -----Original Message----- > > From: Stephen Hemminger > > Sent: Friday, July 19, 2019 19:16 > > To: Slava Ovsiienko > > Cc: dev@dpdk.org; Yongseok Koh ; Shahaf Shuler > > > > Subject: Re: [PATCH 1/2] net/mlx5: cache the associated network device > > ifindex > > > > On Fri, 19 Jul 2019 05:31:44 +0000 > > Viacheslav Ovsiienko wrote: > > > > > + /* > > > + * Store associated network device interface index. This index > > > + * is permanent throughout the lifetime of device. We do not spawn > > > + * rte_eth_dev ports without associated network device, and if > > > + * network device is being unbound we get the remove notification > > > + * message and rte_eth_dev port is also detached. So, we may store > > > + * the ifindex here and use the cached value further. The network > > > + * device name can be changed dynamically and should not be > > cached. > > > + */ > > > + assert(spawn->ifindex); > > > + priv->if_index = spawn->ifindex; > > > > This correct, but overkill. > > > > 1. The comment is way too wordy. Please stick to only a couple of lines. > > If you feel more explanation is necessary put that in the commit log. > > I'd prefer to see the issue description in the source, not by searching the git log > for the appropriate commit. But OK, it does not matter. > > > 2. It is perfectly okay to return 0 as a value in dev_info. > > Therefore the assert is unnecessary. > > Valid network interface index cannot be zero. For example, if_nametoindex() > returns zero in case of error. Also, in mlx5 we do not spawn ports without attached > network interfaces. Assert is not related to dev_info, it checks whether > the mlx5_dev_spawn() is called with valid ifindex for valid port (ifindex checked > against zero to validate infiniband port is active). We need this assert here. > > > 3. Where is "Reported-by:" > It is in cover letter: > "Proposed-by: Stephen Hemminger " > Sorry, I forgot to add this one in commit message, will fix. > > > 4. What was wrong with my simpler patch? > Please, see the cover letter. Your patch fixes only the part of problem - > the mlx5_dev_infos_get(). But it is just the case of unsafe mlx5_ifindex() usage. > mlx5_ifindex() itself must be fixed instead. > > WBR, Slava Will your patch be backported to stable? It is critical that primary/secondary work on older releases.