From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AB0BFA052A; Tue, 26 Jan 2021 23:58:36 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2E67B140D0D; Tue, 26 Jan 2021 23:58:36 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 8ED1C140D03 for ; Tue, 26 Jan 2021 23:58:31 +0100 (CET) IronPort-SDR: TUcMsHtJeyiINwvI4GeD7W3cH9dIMQozU5wtfnCf9zjyWJAdBVVhQQMSeflEc+XyrzCCrnWAC7 ESFFmMRhyVag== X-IronPort-AV: E=McAfee;i="6000,8403,9876"; a="176476080" X-IronPort-AV: E=Sophos;i="5.79,377,1602572400"; d="scan'208";a="176476080" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Jan 2021 14:58:30 -0800 IronPort-SDR: upwbvSdD8b3IjSdqSLMJYjM9isAwYH2bJXXAjJLuFCnROfLIy6QIu9jdk/9nkHnA6Coh6FjFV/ UxHM7RdGoGxA== X-IronPort-AV: E=Sophos;i="5.79,377,1602572400"; d="scan'208";a="362164423" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.227.53]) ([10.213.227.53]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Jan 2021 14:58:29 -0800 To: lironh@marvell.com, jerinj@marvell.com Cc: dev@dpdk.org, Yuri Chipchev References: <20201202101212.4717-1-lironh@marvell.com> <20210122191925.24308-1-lironh@marvell.com> <20210122191925.24308-12-lironh@marvell.com> From: Ferruh Yigit Message-ID: Date: Tue, 26 Jan 2021 22:58:25 +0000 MIME-Version: 1.0 In-Reply-To: <20210122191925.24308-12-lironh@marvell.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 11/37] net/mvpp2: save initial configuration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 1/22/2021 7:18 PM, lironh@marvell.com wrote: > From: Yuri Chipchev > > Save configuration that was done prior 'start' as > only then the ppio is being configured. > Can you please give more details on what is saved and why? > Signed-off-by: Yuri Chipchev > Reviewed-by: Liron Himi > --- > drivers/net/mvpp2/mrvl_ethdev.c | 107 +++++++++++++++++++++++++++----- > 1 file changed, 92 insertions(+), 15 deletions(-) > > diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c > index 47b3aa28f..3891313cf 100644 > --- a/drivers/net/mvpp2/mrvl_ethdev.c > +++ b/drivers/net/mvpp2/mrvl_ethdev.c > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: BSD-3-Clause > - * Copyright(c) 2017 Marvell International Ltd. > - * Copyright(c) 2017 Semihalf. > + * Copyright(c) 2017-2021 Marvell International Ltd. > + * Copyright(c) 2017-2021 Semihalf. > * All rights reserved. > */ > > @@ -146,6 +146,15 @@ static int rte_pmd_mrvl_remove(struct rte_vdev_device *vdev); > static void mrvl_deinit_pp2(void); > static void mrvl_deinit_hifs(void); > > +static int > +mrvl_mac_addr_add(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr, > + uint32_t index, uint32_t vmdq __rte_unused); > +static int > +mrvl_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr); > +static int > +mrvl_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on); > +static int mrvl_promiscuous_enable(struct rte_eth_dev *dev); > +static int mrvl_allmulticast_enable(struct rte_eth_dev *dev); > > #define MRVL_XSTATS_TBL_ENTRY(name) { \ > #name, offsetof(struct pp2_ppio_statistics, name), \ > @@ -404,8 +413,12 @@ mrvl_dev_configure(struct rte_eth_dev *dev) > return 0; > } > > - return mrvl_configure_rss(priv, > - &dev->data->dev_conf.rx_adv_conf.rss_conf); > + ret = mrvl_configure_rss(priv, > + &dev->data->dev_conf.rx_adv_conf.rss_conf); > + if (ret < 0) > + return ret; > + > + return 0; What is the difference? As far I can see 'mrvl_configure_rss()' return negative on error and 0 on success. Is this refactoring part of saving config before start? > } > > /** > @@ -492,8 +505,10 @@ mrvl_dev_set_link_up(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > int ret; > > - if (!priv->ppio) > - return -EPERM; > + if (!priv->ppio) { > + dev->data->dev_link.link_status = ETH_LINK_UP; > + return 0; > + } > > ret = pp2_ppio_enable(priv->ppio); > if (ret) > @@ -507,10 +522,13 @@ mrvl_dev_set_link_up(struct rte_eth_dev *dev) > * Set mtu to default DPDK value here. > */ > ret = mrvl_mtu_set(dev, dev->data->mtu); > - if (ret) > + if (ret) { > pp2_ppio_disable(priv->ppio); > + return ret; > + } > > - return ret; > + dev->data->dev_link.link_status = ETH_LINK_UP; > + return 0; > } > > /** > @@ -526,11 +544,18 @@ static int > mrvl_dev_set_link_down(struct rte_eth_dev *dev) > { > struct mrvl_priv *priv = dev->data->dev_private; > + int ret; > > - if (!priv->ppio) > - return -EPERM; > + if (!priv->ppio) { > + dev->data->dev_link.link_status = ETH_LINK_DOWN; > + return 0; > + } > + ret = pp2_ppio_disable(priv->ppio); > + if (ret) > + return ret; > > - return pp2_ppio_disable(priv->ppio); > + dev->data->dev_link.link_status = ETH_LINK_DOWN; > + return 0; > } Similarly, are these changes on the link up/down, related to what described in the commit log? > > /** > @@ -612,6 +637,9 @@ mrvl_dev_start(struct rte_eth_dev *dev) > struct mrvl_priv *priv = dev->data->dev_private; > char match[MRVL_MATCH_LEN]; > int ret = 0, i, def_init_size; > + uint32_t j; > + struct rte_vlan_filter_conf *vfc; > + struct rte_ether_addr *mac_addr; > > if (priv->ppio) > return mrvl_dev_set_link_up(dev); > @@ -677,6 +705,47 @@ mrvl_dev_start(struct rte_eth_dev *dev) > if (ret) > MRVL_LOG(ERR, "Failed to set MTU to %d", dev->data->mtu); > > + if (!rte_is_zero_ether_addr(&dev->data->mac_addrs[0])) > + mrvl_mac_addr_set(dev, &dev->data->mac_addrs[0]); > + > + for (i = 1; i < MRVL_MAC_ADDRS_MAX; i++) { > + mac_addr = &dev->data->mac_addrs[i]; > + > + /* skip zero address */ > + if (rte_is_zero_ether_addr(mac_addr)) > + continue; > + > + mrvl_mac_addr_add(dev, mac_addr, i, 0); > + } > + > + if (dev->data->all_multicast == 1) > + mrvl_allmulticast_enable(dev); > + > + vfc = &dev->data->vlan_filter_conf; > + for (j = 0; j < RTE_DIM(vfc->ids); j++) { > + uint64_t vlan; > + uint64_t vbit; > + uint64_t ids = vfc->ids[j]; > + > + if (ids == 0) > + continue; > + > + while (ids) { > + vlan = 64 * j; > + /* count trailing zeroes */ > + vbit = ~ids & (ids - 1); > + /* clear least significant bit set */ > + ids ^= (ids ^ (ids - 1)) ^ vbit; > + for (; vbit; vlan++) > + vbit >>= 1; > + ret = mrvl_vlan_filter_set(dev, vlan, 1); > + if (ret) { > + MRVL_LOG(ERR, "Failed to setup VLAN filter\n"); > + goto out; > + } > + } > + } > + > /* For default QoS config, don't start classifier. */ > if (mrvl_qos_cfg && > mrvl_qos_cfg->port[dev->data->port_id].use_global_defaults == 0) { > @@ -687,10 +756,16 @@ mrvl_dev_start(struct rte_eth_dev *dev) > } > } > > - ret = mrvl_dev_set_link_up(dev); > - if (ret) { > - MRVL_LOG(ERR, "Failed to set link up"); > - goto out; > + if (dev->data->promiscuous == 1) > + mrvl_promiscuous_enable(dev); > + > + if (dev->data->dev_link.link_status == ETH_LINK_UP) { > + ret = mrvl_dev_set_link_up(dev); > + if (ret) { > + MRVL_LOG(ERR, "Failed to set link up"); > + dev->data->dev_link.link_status = ETH_LINK_DOWN; > + goto out; > + } > } > > /* start tx queues */ > @@ -2936,6 +3011,8 @@ mrvl_eth_dev_create(struct rte_vdev_device *vdev, const char *name) > eth_dev->dev_ops = &mrvl_ops; > eth_dev->data->dev_flags |= RTE_ETH_DEV_AUTOFILL_QUEUE_XSTATS; > > + eth_dev->data->dev_link.link_status = ETH_LINK_UP; > + > rte_eth_dev_probing_finish(eth_dev); > return 0; > out_free: >