From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 391541D8A; Tue, 12 Sep 2017 15:53:14 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7AB09356E6; Tue, 12 Sep 2017 13:53:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7AB09356E6 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=aconole@redhat.com Received: from dhcp-25-97.bos.redhat.com (unknown [10.18.25.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DE2DA6E21D; Tue, 12 Sep 2017 13:53:12 +0000 (UTC) From: Aaron Conole To: John Daley Cc: ferruh.yigit@intel.com, dev@dpdk.org, stable@dpdk.org References: <20170911185833.11458-1-johndale@cisco.com> Date: Tue, 12 Sep 2017 09:53:11 -0400 In-Reply-To: <20170911185833.11458-1-johndale@cisco.com> (John Daley's message of "Mon, 11 Sep 2017 11:58:33 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 12 Sep 2017 13:53:13 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] net/enic: fix multi-process operation 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, 12 Sep 2017 13:53:14 -0000 John Daley writes: > - Use rte_malloc() instead of malloc() for the per device 'vdev' structure > so that it can be shared across processes. > - Only initialize the device if the process type is RTE_PROC_PRIMARY > - Only allow the primary process to do queue setup, start/stop, promisc > allmulticast, mac add/del, mtu. > > Fixes: fefed3d1e62c ("enic: new driver") > Cc: stable@dpdk.org > > Signed-off-by: John Daley > --- I was first considering that this should be multiple patches, but then I realized that it wouldn't actually make sense (there's no way to do it and keep the driver from being broken w.r.t. primary/secondary). Reviewed-by: Aaron Conole > doc/guides/nics/features/enic.ini | 1 + > drivers/net/enic/base/vnic_dev.c | 10 +++++++-- > drivers/net/enic/enic_ethdev.c | 43 +++++++++++++++++++++++++++++++++++++++ > drivers/net/enic/enic_main.c | 7 +++++++ > 4 files changed, 59 insertions(+), 2 deletions(-) > > diff --git a/doc/guides/nics/features/enic.ini b/doc/guides/nics/features/enic.ini > index 0de3ef53c..498341f07 100644 > --- a/doc/guides/nics/features/enic.ini > +++ b/doc/guides/nics/features/enic.ini > @@ -25,6 +25,7 @@ L3 checksum offload = Y > L4 checksum offload = Y > Packet type parsing = Y > Basic stats = Y > +Multiprocess aware = Y > BSD nic_uio = Y > Linux UIO = Y > Linux VFIO = Y > diff --git a/drivers/net/enic/base/vnic_dev.c b/drivers/net/enic/base/vnic_dev.c > index 49b36555b..162e9c2f2 100644 > --- a/drivers/net/enic/base/vnic_dev.c > +++ b/drivers/net/enic/base/vnic_dev.c > @@ -1063,7 +1063,7 @@ void vnic_dev_unregister(struct vnic_dev *vdev) > vdev->free_consistent(vdev->priv, > sizeof(struct vnic_devcmd_fw_info), > vdev->fw_info, vdev->fw_info_pa); > - kfree(vdev); > + rte_free(vdev); > } > } > > @@ -1072,7 +1072,13 @@ struct vnic_dev *vnic_dev_register(struct vnic_dev *vdev, > unsigned int num_bars) > { > if (!vdev) { > - vdev = kzalloc(sizeof(struct vnic_dev), GFP_ATOMIC); > + char name[NAME_MAX]; > + snprintf((char *)name, sizeof(name), "%s-vnic", > + pdev->device.name); > + vdev = (struct vnic_dev *)rte_zmalloc_socket(name, > + sizeof(struct vnic_dev), > + RTE_CACHE_LINE_SIZE, > + pdev->device.numa_node); > if (!vdev) > return NULL; > } > diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c > index da8fec2d0..33a3f87e2 100644 > --- a/drivers/net/enic/enic_ethdev.c > +++ b/drivers/net/enic/enic_ethdev.c > @@ -142,6 +142,10 @@ enicpmd_dev_filter_ctrl(struct rte_eth_dev *dev, > static void enicpmd_dev_tx_queue_release(void *txq) > { > ENICPMD_FUNC_TRACE(); > + > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > enic_free_wq(txq); > } > > @@ -196,6 +200,9 @@ static int enicpmd_dev_tx_queue_setup(struct rte_eth_dev *eth_dev, > int ret; > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > ENICPMD_FUNC_TRACE(); > if (queue_idx >= ENIC_WQ_MAX) { > dev_err(enic, > @@ -272,6 +279,10 @@ static int enicpmd_dev_rx_queue_stop(struct rte_eth_dev *eth_dev, > static void enicpmd_dev_rx_queue_release(void *rxq) > { > ENICPMD_FUNC_TRACE(); > + > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > enic_free_rq(rxq); > } > > @@ -310,6 +321,10 @@ static int enicpmd_dev_rx_queue_setup(struct rte_eth_dev *eth_dev, > struct enic *enic = pmd_priv(eth_dev); > > ENICPMD_FUNC_TRACE(); > + > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > /* With Rx scatter support, two RQs are now used on VIC per RQ used > * by the application. > */ > @@ -378,6 +393,9 @@ static int enicpmd_dev_configure(struct rte_eth_dev *eth_dev) > int ret; > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > ENICPMD_FUNC_TRACE(); > ret = enic_set_vnic_res(enic); > if (ret) { > @@ -404,6 +422,9 @@ static int enicpmd_dev_start(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > ENICPMD_FUNC_TRACE(); > return enic_enable(enic); > } > @@ -416,6 +437,9 @@ static void enicpmd_dev_stop(struct rte_eth_dev *eth_dev) > struct rte_eth_link link; > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > enic_disable(enic); > memset(&link, 0, sizeof(link)); > @@ -513,7 +537,11 @@ static void enicpmd_dev_promiscuous_enable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > + > enic->promisc = 1; > enic_add_packet_filter(enic); > } > @@ -522,6 +550,9 @@ static void enicpmd_dev_promiscuous_disable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > enic->promisc = 0; > enic_add_packet_filter(enic); > @@ -531,6 +562,9 @@ static void enicpmd_dev_allmulticast_enable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > enic->allmulti = 1; > enic_add_packet_filter(enic); > @@ -540,6 +574,9 @@ static void enicpmd_dev_allmulticast_disable(struct rte_eth_dev *eth_dev) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > enic->allmulti = 0; > enic_add_packet_filter(enic); > @@ -551,6 +588,9 @@ static int enicpmd_add_mac_addr(struct rte_eth_dev *eth_dev, > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > ENICPMD_FUNC_TRACE(); > return enic_set_mac_address(enic, mac_addr->addr_bytes); > } > @@ -559,6 +599,9 @@ static void enicpmd_remove_mac_addr(struct rte_eth_dev *eth_dev, uint32_t index) > { > struct enic *enic = pmd_priv(eth_dev); > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return; > + > ENICPMD_FUNC_TRACE(); > enic_del_mac_address(enic, index); > } > diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c > index 1d956cd94..9b0439b96 100644 > --- a/drivers/net/enic/enic_main.c > +++ b/drivers/net/enic/enic_main.c > @@ -1181,6 +1181,9 @@ int enic_set_mtu(struct enic *enic, uint16_t new_mtu) > old_mtu = eth_dev->data->mtu; > config_mtu = enic->config.mtu; > > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return -E_RTE_SECONDARY; > + > if (new_mtu > enic->max_mtu) { > dev_err(enic, > "MTU not updated: requested (%u) greater than max (%u)\n", > @@ -1332,6 +1335,10 @@ int enic_probe(struct enic *enic) > > dev_debug(enic, " Initializing ENIC PMD\n"); > > + /* if this is a secondary process the hardware is already initialized */ > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return 0; > + > enic->bar0.vaddr = (void *)pdev->mem_resource[0].addr; > enic->bar0.len = pdev->mem_resource[0].len;