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 1276FA04C1; Wed, 20 Nov 2019 16:06:37 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 079CAA69; Wed, 20 Nov 2019 16:06:36 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 79BA223D for ; Wed, 20 Nov 2019 16:06:34 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Nov 2019 07:06:33 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,222,1571727600"; d="scan'208";a="204755824" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by fmsmga008.fm.intel.com with ESMTP; 20 Nov 2019 07:06:32 -0800 Date: Wed, 20 Nov 2019 23:02:57 +0800 From: Ye Xiaolong To: Shougang Wang , Sun GuinanX Cc: dev@dpdk.org, wenzhuo.lu@intel.com, qiming.yang@intel.com Message-ID: <20191120150257.GE103671@intel.com> References: <20191120080743.26405-1-shougangx.wang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191120080743.26405-1-shougangx.wang@intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix qos sched sample app performance drop 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" Hi, Guinan, could you take a look at this patch as well. On 11/20, Shougang Wang wrote: >Currently MACsec register is set without any conditions when start port. >It should be set only when user needs. To avoid wild value, I add init >function. This patch fixes the issue. > >Fixes: 50556c88104c ("net/ixgbe: fix MACsec setting") > >Signed-off-by: Shougang Wang >--- > drivers/net/ixgbe/ixgbe_ethdev.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c >index 8c1caac18..d54eec1c6 100644 >--- a/drivers/net/ixgbe/ixgbe_ethdev.c >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c >@@ -379,6 +379,8 @@ static int ixgbe_dev_udp_tunnel_port_del(struct rte_eth_dev *dev, > static int ixgbe_filter_restore(struct rte_eth_dev *dev); > static void ixgbe_l2_tunnel_conf(struct rte_eth_dev *dev); > >+static void ixgbe_dev_macsec_init(struct rte_eth_dev *dev); >+ > /* > * Define VF Stats MACRO for Non "cleared on read" register > */ >@@ -1095,6 +1097,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused) > > PMD_INIT_FUNC_TRACE(); > >+ ixgbe_dev_macsec_init(eth_dev); This is not needed as dev_private is allocated by rte_zmalloc_socket. >+ > eth_dev->dev_ops = &ixgbe_eth_dev_ops; > eth_dev->rx_pkt_burst = &ixgbe_recv_pkts; > eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts; >@@ -2545,7 +2549,7 @@ ixgbe_dev_start(struct rte_eth_dev *dev) > uint32_t *link_speeds; > struct ixgbe_tm_conf *tm_conf = > IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private); >- struct ixgbe_macsec_setting *macsec_ctrl = >+ struct ixgbe_macsec_setting *macsec_setting = > IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private); > > PMD_INIT_FUNC_TRACE(); >@@ -2799,9 +2803,11 @@ ixgbe_dev_start(struct rte_eth_dev *dev) > */ > ixgbe_dev_link_update(dev, 0); > >- /* setup the macsec ctrl register */ >- ixgbe_dev_macsec_register_enable(dev, macsec_ctrl); >- >+ /* setup the macsec setting register */ >+ if (macsec_setting->encrypt_en != 0 || >+ macsec_setting->replayprotect_en != 0) { >+ ixgbe_dev_macsec_register_enable(dev, macsec_setting); >+ } Can we safely assume that if encrypt_en and replayprotect_en equals zero, then we don't need to call ixgbe_dev_macsec_register_enable at all? Since this enable routine is more about just encrypt_en/replayprotect_en, is that any usercase when user set macsec offload with both encrypt_en and replayprotect_en are 0? Thanks, Xiaolong > return 0; > > error: >@@ -2827,6 +2833,8 @@ ixgbe_dev_stop(struct rte_eth_dev *dev) > int vf; > struct ixgbe_tm_conf *tm_conf = > IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private); >+ struct ixgbe_macsec_setting *macsec_setting = >+ IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private); > > if (hw->adapter_stopped) > return; >@@ -2834,7 +2842,10 @@ ixgbe_dev_stop(struct rte_eth_dev *dev) > PMD_INIT_FUNC_TRACE(); > > /* disable mecsec register */ >- ixgbe_dev_macsec_register_disable(dev); >+ if (macsec_setting->encrypt_en != 0 || >+ macsec_setting->replayprotect_en != 0) { >+ ixgbe_dev_macsec_register_disable(dev); >+ } > > rte_eal_alarm_cancel(ixgbe_dev_setup_link_alarm_handler, dev); > >@@ -8977,6 +8988,16 @@ ixgbe_dev_macsec_register_disable(struct rte_eth_dev *dev) > ixgbe_enable_sec_tx_path_generic(hw); > } > >+static void >+ixgbe_dev_macsec_init(struct rte_eth_dev *dev) >+{ >+ struct ixgbe_macsec_setting *macsec = >+ IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data->dev_private); >+ >+ macsec->encrypt_en = 0; >+ macsec->replayprotect_en = 0; >+} >+ > RTE_PMD_REGISTER_PCI(net_ixgbe, rte_ixgbe_pmd); > RTE_PMD_REGISTER_PCI_TABLE(net_ixgbe, pci_id_ixgbe_map); > RTE_PMD_REGISTER_KMOD_DEP(net_ixgbe, "* igb_uio | uio_pci_generic | vfio-pci"); >-- >2.17.1 >