From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 99E02377E; Tue, 2 May 2017 08:19:08 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 May 2017 23:19:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,403,1488873600"; d="scan'208";a="81360571" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by orsmga002.jf.intel.com with ESMTP; 01 May 2017 23:19:05 -0700 Date: Tue, 2 May 2017 14:15:27 +0800 From: Yuanhan Liu To: Rasesh Mody Cc: dev@dpdk.org, Rasesh Mody , Dept-EngDPDKDev@cavium.com, stable@dpdk.org, Ferruh Yigit , Thomas Monjalon Message-ID: <20170502061527.GB3005@yliu-dev.sh.intel.com> References: <1493105326-31984-1-git-send-email-rasesh.mody@cavium.com> <1493105326-31984-11-git-send-email-rasesh.mody@cavium.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1493105326-31984-11-git-send-email-rasesh.mody@cavium.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH 11/11] net/qede: fix to limit CFLAGS to base files 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, 02 May 2017 06:19:09 -0000 On Tue, Apr 25, 2017 at 12:28:46AM -0700, Rasesh Mody wrote: > From: Rasesh Mody > > Changes included in this fix > - limit CFLAGS to base files > - fix to remove/mark unused members > - add checks for debug config option > - make qede_set_mtu() and qede_udp_dst_port_del() static and others > non-static as appropriate > - move local APIs qede_vlan_offload_set() and qede_rx_cqe_to_pkt_type() > - initialize variables as required When there are so many items in one single patch, it basically means it's done wrongly. Generally, we should make one patch for each item. > Fixes: ec94dbc57362 ("qede: add base driver") > Cc: stable@dpdk.org It's also not a good idea to put "Cc: stable" tag in a huge fix patch. It's very likely it won't apply cleanly to a stable/LTS release. For instance, I failed to apply it to 16.11.2 (LTS). > > Signed-off-by: Rasesh Mody > --- > drivers/net/qede/Makefile | 32 ++++----- > drivers/net/qede/base/ecore.h | 4 +- > drivers/net/qede/base/ecore_int_api.h | 4 +- > drivers/net/qede/qede_ethdev.c | 120 ++++++++++++++++++--------------- > drivers/net/qede/qede_ethdev.h | 32 ++++----- > drivers/net/qede/qede_fdir.c | 13 +--- > drivers/net/qede/qede_if.h | 4 ++ > drivers/net/qede/qede_main.c | 8 +-- > drivers/net/qede/qede_rxtx.c | 118 +++++++++++++++++--------------- > 9 files changed, 171 insertions(+), 164 deletions(-) It's also a clear sign of bad patch: too many changes for a single bug fix patch. Most of them look like minor fixes to me. So my question is are there any important items really should be picked for stable and LTS release? More specifically, do they really fix any (fatal) issues? If no, I will drop it. If yes, please send a (or some) patch with the real fixes backported only to stable ML, so that I could pick them. Thanks. --yliu