From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id E963B4CB5; Mon, 5 Nov 2018 17:34:32 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Nov 2018 08:34:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,468,1534834800"; d="scan'208";a="93814839" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by FMSMGA003.fm.intel.com with ESMTP; 05 Nov 2018 08:34:22 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 5 Nov 2018 08:34:21 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 5 Nov 2018 08:34:21 -0800 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.161]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.117]) with mapi id 14.03.0415.000; Tue, 6 Nov 2018 00:34:19 +0800 From: "Zhang, Qi Z" To: "Zhao1, Wei" , "dev@dpdk.org" CC: "stable@dpdk.org" , "yamashita.hideyuki@po.ntt-tx.co.jp" , "stephen@networkplumber.org" Thread-Topic: [PATCH v2] net/ixgbe: check jumbo frame enable parameter Thread-Index: AQHUcb3W7r0DXzUc9kS66FUidtmAFqU6E3SAgAdM1uA= Date: Mon, 5 Nov 2018 16:34:18 +0000 Message-ID: <039ED4275CED7440929022BC67E70611532DF8C2@SHSMSX103.ccr.corp.intel.com> References: <1541058926-3576-1-git-send-email-wei.zhao1@intel.com> <1541060916-39754-1-git-send-email-wei.zhao1@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTJhOGI5YmItYzY4ZC00NjNjLWFkZmMtOTUyNzk1MDZhNDcwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiZ3p5VStnc2h3ZXFRMm9EQ3UrYW8xVmVjaVwvenhyVUJHXC96Vm5OMFdWWmVaallsUUo1c0xjcEJaR2hpeDFVblBtIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: check jumbo frame enable parameter 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: Mon, 05 Nov 2018 16:34:33 -0000 Hi Wei: > -----Original Message----- > From: Zhao1, Wei > Sent: Thursday, November 1, 2018 1:40 AM > To: dev@dpdk.org > Cc: stable@dpdk.org; Zhang, Qi Z ; > yamashita.hideyuki@po.ntt-tx.co.jp; stephen@networkplumber.org > Subject: RE: [PATCH v2] net/ixgbe: check jumbo frame enable parameter >=20 > Add the background for this patch. > https://mails.dpdk.org/archives/dev/2018-November/117771.html > https://mails.dpdk.org/archives/dev/2018-November/117772.html >=20 >=20 >=20 > > -----Original Message----- > > From: Zhao1, Wei > > Sent: Thursday, November 1, 2018 4:29 PM > > To: Zhao1, Wei ; dev@dpdk.org > > Cc: stable@dpdk.org; Zhang, Qi Z ; > > yamashita.hideyuki@po.ntt-tx.co.jp; stephen@networkplumber.org > > Subject: [PATCH v2] net/ixgbe: check jumbo frame enable parameter > > > > There is necessary to do some check of max packet size boundary for > > code safe in order to avoid error in register setting when enable jumbo= . > > > > Signed-off-by: Wei Zhao > > > > --- > > > > v2: > > -add more log infomation and fix build issue > > --- > > drivers/net/ixgbe/ixgbe_ethdev.c | 4 +--- > > drivers/net/ixgbe/ixgbe_ethdev.h | 2 ++ > > drivers/net/ixgbe/ixgbe_rxtx.c | 31 > > +++++++++++++++++++++++++++++++ > > 3 files changed, 34 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > > b/drivers/net/ixgbe/ixgbe_ethdev.c > > index 5a2c351..d65a911 100644 > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > @@ -173,8 +173,6 @@ static int > > ixgbe_dev_queue_stats_mapping_set(struct rte_eth_dev *eth_dev, > > uint8_t is_rx); > > static int ixgbe_fw_version_get(struct rte_eth_dev *dev, char > *fw_version, > > size_t fw_size); > > -static void ixgbe_dev_info_get(struct rte_eth_dev *dev, > > - struct rte_eth_dev_info *dev_info); > > static const uint32_t *ixgbe_dev_supported_ptypes_get(struct > > rte_eth_dev *dev); static void ixgbevf_dev_info_get(struct > > rte_eth_dev *dev, > > struct rte_eth_dev_info *dev_info); @@ - > > 3692,7 +3690,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused > > struct rte_eth_dev *dev, > > return 0; > > } > > > > -static void > > +void > > ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info > > *dev_info) { > > struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); diff -- > > git a/drivers/net/ixgbe/ixgbe_ethdev.h > > b/drivers/net/ixgbe/ixgbe_ethdev.h > > index d0b9396..a474be4 100644 > > --- a/drivers/net/ixgbe/ixgbe_ethdev.h > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h > > @@ -727,6 +727,8 @@ int ixgbe_action_rss_same(const struct > > rte_flow_action_rss *comp, > > const struct rte_flow_action_rss *with); int > > ixgbe_config_rss_filter(struct rte_eth_dev *dev, > > struct ixgbe_rte_flow_rss_conf *conf, bool add); > > +void ixgbe_dev_info_get(struct rte_eth_dev *dev, > > + struct rte_eth_dev_info *dev_info); > > > > static inline int > > ixgbe_ethertype_filter_lookup(struct ixgbe_filter_info *filter_info, > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 2f0262a..94051ca 100644 > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c > > @@ -4814,6 +4814,34 @@ void __attribute__((cold)) > > return 0; > > } > > > > +static int __attribute__((cold)) > > +ixgbe_dev_jumboenable_check(struct rte_eth_dev *dev, > > + uint16_t max_rx_pkt_len) it's better to return Boolean for this function > > +{ > > + struct rte_eth_dev_info dev_info; > > + > > + ixgbe_dev_info_get(dev, &dev_info); > > + > > + /* check that max packet size is within the allowed range */ > > + if (max_rx_pkt_len < ETHER_MIN_MTU) { > > + PMD_INIT_LOG(ERR, "max packet size is too small."); > > + return -EINVAL; > > + } Why this is only needed for DEV_RX_OFFLOAD_JUMBO_FRAME?=20 And we will compare to ETHER_MAX_LEN which is a bigger value, this looks re= dundant.=20 > > + > > + if (max_rx_pkt_len > dev_info.max_rx_pktlen) { > > + PMD_INIT_LOG(ERR, "max packet size is too big."); > > + return -EINVAL; > > + } dev_info.max_rx_pktlen =3D 15872, it should be replaced by a macro, then we= don't need to call ixgbe_dev_info_get here. and why this is only needed for DEV_RX_OFFLOAD_JUMBO_FRAME? > > + > > + /* check jumbo mode if needed */ > > + if (max_rx_pkt_len < ETHER_MAX_LEN) { > > + PMD_INIT_LOG(ERR, "No need to enable jumbo."); > > + return -EINVAL; And I agree with Stephen's comment, Looks all of these is common for all drivers, it should be moved to ether l= ayer. > > + } > > + > > + return 0; > > +} > > + > > /* > > * Initializes Receive Unit. > > */ > > @@ -4865,6 +4893,9 @@ int __attribute__((cold)) > > * Configure jumbo frame support, if any. > > */ > > if (rx_conf->offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) { > > + if (ixgbe_dev_jumboenable_check(dev, rx_conf- > > >max_rx_pkt_len)) > > + return -EINVAL; > > + > > hlreg0 |=3D IXGBE_HLREG0_JUMBOEN; > > maxfrs =3D IXGBE_READ_REG(hw, IXGBE_MAXFRS); > > maxfrs &=3D 0x0000FFFF; > > -- > > 1.8.3.1