From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f43.google.com (mail-oi0-f43.google.com [209.85.218.43]) by dpdk.org (Postfix) with ESMTP id D20AF2E41 for ; Sat, 8 Aug 2015 00:06:15 +0200 (CEST) Received: by oihn130 with SMTP id n130so63207083oih.2 for ; Fri, 07 Aug 2015 15:06:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=1K05uhvKCckBP/k3/hHBYuXXH/Q2bjO7EgBGZFarAB0=; b=oGZccskU5SXFZg/xoPzzMtGmB0yeTQUhqZfpF8xGF/ZkGY3BbuzlVpSdISRQs4/hr2 ah6XxljMMv/g62LZtLodyp+MaYU5gN3egKnbZRwzd+Jbo/CjL4dIGa/tSVG8BK5BgknH mOLYvyGGq5FhV7ly9V4gmzPYmzzB4zNnYviasnGk0p2kZDykvGagV3gJ7EhNn+zIxNEp MFqSlhq8Ritkp9bxJgk/4++bCh/Bfj7j0ObE3q9bs70+UnH0/9FT1nWHpKXcQypRe3a2 IGFtcPkyfFAz/6HR8kZSRPeN7teR8G6Krltv9eTYveicCAlmvFlZL46sApFOaOCDeOQH pJuw== MIME-Version: 1.0 X-Received: by 10.202.62.11 with SMTP id l11mr5619729oia.103.1438985175075; Fri, 07 Aug 2015 15:06:15 -0700 (PDT) Received: by 10.202.179.195 with HTTP; Fri, 7 Aug 2015 15:06:15 -0700 (PDT) In-Reply-To: <55C41735.1030405@igel.co.jp> References: <1438884241-15599-1-git-send-email-rkerur@gmail.com> <55C41735.1030405@igel.co.jp> Date: Fri, 7 Aug 2015 15:06:15 -0700 Message-ID: From: Ravi Kerur To: Tetsuya Mukawa Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v1] 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, 07 Aug 2015 22:06:16 -0000 Hi Tetsuya, On Thu, Aug 6, 2015 at 7:25 PM, Tetsuya Mukawa wrote: > On 2015/08/07 3:04, Ravi Kerur wrote: > > diff --git a/drivers/net/enic/enic_ethdev.c > b/drivers/net/enic/enic_ethdev.c > > index 8280cea..472ef5a 100644 > > --- a/drivers/net/enic/enic_ethdev.c > > +++ b/drivers/net/enic/enic_ethdev.c > > @@ -36,8 +36,8 @@ > > #include > > #include > > > > -#include > > #include > > +#include > > #include > > #include > > Hi Ravi, > > Do we need this fixing? > > > > > diff --git a/drivers/net/mpipe/mpipe_tilegx.c > b/drivers/net/mpipe/mpipe_tilegx.c > > index 743feef..6e3e304 100644 > > --- a/drivers/net/mpipe/mpipe_tilegx.c > > +++ b/drivers/net/mpipe/mpipe_tilegx.c > > @@ -1582,6 +1582,7 @@ rte_pmd_mpipe_devinit(const char *ifname, > > if (!eth_dev) { > > RTE_LOG(ERR, PMD, "%s: Failed to allocate device.\n", > ifname); > > rte_free(priv); > > + return -ENOMEM; > > How about separating this fixing from the patch, and put it as an one of > cleanup patch series? > > rte_pmd_mpipe_devinit is the init func pointer called via rte_eal_vdev_init. Since we were fixing rte_eal_vdev_init thought of taking care of mpipe issue. If you think it's unrelated to this patch I will send a separate one. > } > > > > RTE_LOG(INFO, PMD, "%s: Initialized mpipe device" > > diff --git a/lib/librte_eal/common/eal_common_dev.c > b/lib/librte_eal/common/eal_common_dev.c > > index 4089d66..82d5693 100644 > > --- a/lib/librte_eal/common/eal_common_dev.c > > +++ b/lib/librte_eal/common/eal_common_dev.c > > > > RTE_LOG(ERR, EAL, "no driver found for %s\n", name); > > @@ -94,6 +99,7 @@ rte_eal_dev_init(void) > > { > > struct rte_devargs *devargs; > > struct rte_driver *driver; > > + uint8_t port_id; > > > > /* > > * Note that the dev_driver_list is populated here > > @@ -108,7 +114,7 @@ rte_eal_dev_init(void) > > continue; > > > > if (rte_eal_vdev_init(devargs->virtual.drv_name, > > - devargs->args)) { > > + devargs->args, &port_id)) { > > After this line, 'port_id' is actually not used by anywhere in this > function. > Also, I guess we will not use port_id in this function in the future. > How about fixing rte_eal_vdev_init() to handle NULL value correctly to > remove port_id from this function? > But I agree your current implementation is also one of choice. > > > diff --git a/lib/librte_ether/rte_ethdev.c > b/lib/librte_ether/rte_ethdev.c > > index 5fe1906..355d709 100644 > > --- a/lib/librte_ether/rte_ethdev.c > > +++ b/lib/librte_ether/rte_ethdev.c > > +int > > +rte_eth_dev_get_port_by_addr(const struct rte_pci_addr *addr, uint8_t > *port_id) > > +{ > > + int i; > > + struct rte_pci_device *pci_dev = NULL; > > + > > + if (addr == NULL || port_id == NULL) { > > + PMD_DEBUG_TRACE("Null pointer is specified\n"); > > + return -EINVAL; > > + } > > + > > + *port_id = RTE_MAX_ETHPORTS; > > + > > + for (i = 0; i < RTE_MAX_ETHPORTS; i++) { > > + > > + pci_dev = rte_eth_devices[i].pci_dev; > > + > > + if (pci_dev != NULL && > > + pci_dev->addr.domain == addr->domain && > > + pci_dev->addr.bus == addr->bus && > > + pci_dev->addr.devid == addr->devid && > > + pci_dev->addr.function == addr->function) { > > You can use rte_eal_compare_pci_addr() here. > Will fix this. > > > + > > + *port_id = i; > > + return 0; > > + } > > + } > > + return -ENODEV; > > +} > > diff --git a/lib/librte_ether/rte_ether_version.map > b/lib/librte_ether/rte_ether_version.map > > index 8345a6c..3d5cb23 100644 > > --- a/lib/librte_ether/rte_ether_version.map > > +++ b/lib/librte_ether/rte_ether_version.map > > @@ -125,5 +125,7 @@ DPDK_2.1 { > > rte_eth_timesync_enable; > > rte_eth_timesync_read_rx_timestamp; > > rte_eth_timesync_read_tx_timestamp; > > + rte_eth_dev_get_port_by_name; > > + rte_eth_dev_get_port_by_addr; > > > > } DPDK_2.0; > > Hi Thomas, > > Could you please make sure API consistency? > Is it ok to add above functions to DPDK_2.1 even though we are in RC > phase, or need to add to DPDK_2.2? > > Same question. If it's targeted for 2.2 then I will modify this. Thanks, Ravi > Thanks, > Tetsuya > > >