From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) by dpdk.org (Postfix) with ESMTP id 264F65A9D for ; Fri, 21 Aug 2015 05:33:25 +0200 (CEST) Received: by pawq9 with SMTP id q9so42309266paw.3 for ; Thu, 20 Aug 2015 20:33:24 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=2QrGtLs3J4JGDHxrtcOFWcRU2sbvABqguYvV8LdDm4s=; b=d+qUTNBKWTCjIwqgLsYb1YdD8FKyx7lv8as7/YOVFRW7Qxy1BUK0owG4S6/7ksUdTx TarzKpqGgmtCpWW+mE2sGC4A7IrOAiV6DmT/KiasYlu2T/TKIjHYR8Xwdw56J5D20SEu cHHvpxGLr7BodlY01aarjh6UE1rEQJcXq2n/lyvrCqaHEzVV/g8D0P4Aei1N/dWPWcP0 xc/YaoaWsrplqEqqmmuwxTRgVXR/khaNgkKxTiWTdbIWLVLZGNipo2xkbrhQJKUY1qPd 7indf1Hc5bQJVc3baD19lQfCKpc4Ms7HCwr47ZKzzJIcy/rhXj68qtOLzoaeN39FC2Dx fN9g== X-Gm-Message-State: ALoCoQn2O6Uz+g4KyPyRywQPc36lHqhggb40vJ8WSkfuN5qZPK2gramt2DSreFB8EZOZj0MxH6xb X-Received: by 10.69.3.228 with SMTP id bz4mr13074464pbd.79.1440128004441; Thu, 20 Aug 2015 20:33:24 -0700 (PDT) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by smtp.googlemail.com with ESMTPSA id ph5sm5939084pac.2.2015.08.20.20.33.22 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Aug 2015 20:33:23 -0700 (PDT) Message-ID: <55D69C00.2020609@igel.co.jp> Date: Fri, 21 Aug 2015 12:33:20 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Ravi Kerur , David Marchand References: <1440013341-29659-1-git-send-email-rkerur@gmail.com> <1440013376-29715-1-git-send-email-rkerur@gmail.com> <55D53655.1040808@igel.co.jp> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2] Change rte_eal_vdev_init to update port_id X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 21 Aug 2015 03:33:25 -0000 On 2015/08/21 4:16, Ravi Kerur wrote: > > > /** > > * Uninitalize a driver specified by name. > > @@ -125,6 +127,38 @@ int rte_eal_vdev_init(const char *name, > const char *args); > > */ > > int rte_eal_vdev_uninit(const char *name); > > > > +/** > > + * Given name, return port_id associated with the device. > > + * > > + * @param name > > + * Name associated with device. > > + * @param port_id > > + * The port identifier of the device. > > + * > > + * @return > > + * - 0: Success. > > + * - -EINVAL: NULL string (name) > > + * - -ENODEV failure > > Please define above in 'rte_ethdev.h.' > > > Hi Tetsuya, > > I would like to take a step back and explain why function declarations > are in rte_dev.h and not in rte_ethdev.h > > Approach 1: > Initially I thought of modifying driver init routine to return/update > port_id as the init routine is the place port_id gets allocated and it > would have been clean approach. However, it required changes to all > PMD_VDEV driver init routine to modify function signature for the > changes which I thought may be an overkill. > > Approach 2: > Instead I chose to define 2 functions in librte_ether/rte_ethdev.c and > make use of it. In this approach new functions are invoked from > librte_eal/common/.c to get port_id. If I had new function > declarations in rte_ethdev.h and included that file in > librte_eal/common/.c files it creates circular dependancy and > compilation fails, hence I took hybrid approach of definitions in > librte_ether and declarations in librte_eal. > > Please let me know if there is a better approach to take care of your > comments. As it stands declarations cannot be moved to rte_ethdev.h > for compilation reasons. > > Thanks, > Ravi > Hi Ravi, (Adding David) I appreciate your description. I understand why you define the functions in rte_dev.h. About Approach2, I don't know a way to implement cleanly. I guess if we define the functions in rte_dev.h, the developers who want to use the functions will be confused because the functions are implemented in ethdev.c, but it is needed to include rte_dev.h. To avoid such a confusion, following implementation might be worked, but I am not sure this cording style is allowed in eal library. ---------------------------- Define the functions in rte_ethdev.h, then fix librte_eal/common/.c files like below ex) lib/librte_eal/common/eal_common_dev.c ---------------------------- +#include #include #include #include #include "eal_private.h" +extern int rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id); +extern int rte_eth_dev_get_port_by_addr(const struct rte_pci_addr *addr, uint8_t *port_id); ---------------------------- In this case, the developer might be able to notice that above usage in eal library is some kind of exception. But I guess the DPDK code won't be clean if we start having a exception. So it might be good to choose Approach1, because apparently it is straight forward. Anyone won't be confused and complained about coding style. Hi David, Could you please let us know what you think? Do you have a good approach for this? Thanks, Tetsuya