From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 8B382235 for ; Wed, 5 Jul 2017 15:59:46 +0200 (CEST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Jul 2017 06:59:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,312,1496127600"; d="scan'208";a="104671366" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.91]) ([10.237.220.91]) by orsmga004.jf.intel.com with ESMTP; 05 Jul 2017 06:59:44 -0700 To: Beilei Xing , wenzhuo.lu@intel.com Cc: dev@dpdk.org References: <1499156575-15046-1-git-send-email-beilei.xing@intel.com> <1499222024-64581-1-git-send-email-beilei.xing@intel.com> From: Ferruh Yigit Message-ID: <5a29e420-9692-670e-d3e4-f61c20c2a6b3@intel.com> Date: Wed, 5 Jul 2017 14:59:43 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1499222024-64581-1-git-send-email-beilei.xing@intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: remove MTU setting limitation when port's stopped 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: Wed, 05 Jul 2017 13:59:47 -0000 On 7/5/2017 3:33 AM, Beilei Xing wrote: > Currently, setting MTU will fail if MTU requires the support > of scattered packets before scatter is enabled. > This patch allows setting this special MTU when device is > stopped, cause scatter will be re-configured during starting > port. > > Signed-off-by: Beilei Xing > --- > v2 changes: > - change commit log > > drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index fb5574e..ece5d6c 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -4812,6 +4812,7 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) > struct rte_eth_dev_info dev_info; > uint32_t frame_size = mtu + ETHER_HDR_LEN + ETHER_CRC_LEN; > struct rte_eth_rxmode *rx_conf = &dev->data->dev_conf.rxmode; > + struct rte_eth_dev_data *dev_data = dev->data; > > ixgbe_dev_info_get(dev, &dev_info); > > @@ -4819,13 +4820,15 @@ ixgbe_dev_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) > if ((mtu < ETHER_MIN_MTU) || (frame_size > dev_info.max_rx_pktlen)) > return -EINVAL; > > - /* refuse mtu that requires the support of scattered packets when this > - * feature has not been enabled before. > + /* If device is started, refuse mtu that requires the support of > + * scattered packets when this feature has not been enabled before. > */ > - if (!rx_conf->enable_scatter && > + if (dev_data->dev_started && !rx_conf->enable_scatter && Should dev->data->scattered_rx be used here? rx_conf->enable_scatter is configuration value provided by user, but dev->data->scattered_rx keeps the actual status of the feature. Driver may decide to enable scattered rx even tough configuration didn't asked for it, this behavior is what this patch relies on. Can you confirm that there is no other case that can prevent enabling scattered rx? Otherwise this patch will let setting MTU bigger than mbuf and if somehow scattered rx is not enabled, this may break the Rx path I guess. > (frame_size + 2 * IXGBE_VLAN_TAG_SIZE > > - dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)) > + dev->data->min_rx_buf_size - RTE_PKTMBUF_HEADROOM)) { > + PMD_INIT_LOG(ERR, "Stop port first."); > return -EINVAL; > + } > > hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > hlreg0 = IXGBE_READ_REG(hw, IXGBE_HLREG0); >