Re: [dpdk-stable] [PATCH 11/11] net/qede: fix to limit CFLAGS to base files

["Mody, Rasesh" <Rasesh.Mody@cavium.com>]

> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, May 03, 2017 7:11 PM
> 
> On Thu, May 04, 2017 at 12:14:30AM +0000, Mody, Rasesh wrote:
> >
> >
> > > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > > Sent: Monday, May 01, 2017 11:15 PM
> > >
> > > On Tue, Apr 25, 2017 at 12:28:46AM -0700, Rasesh Mody wrote:
> > > > From: Rasesh Mody <rasesh.mody@qlogic.com>
> > > >
> > > > 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 <rasesh.mody@qlogic.com>
> > > > ---
> > > >  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.
> >
> > The patch is a Makefile change to restrict the CFLAG only to the base files.
> Once Makefile was changed it exposed few issues with PMD.
> 
> In such case, you could make patches to fix those issues first, one patch for
> one issue, and then put the CFLAG change to the last.
> 
> > Hence, we thought of putting all the changes in single patch since they
> were relevant changes.
> >
> > As you stated most of them are minor fixes. We'll evaluate the patch if
> anything specifically need to go into the stable release and get back.
> 
> Thanks. The answer seems to be "NO" to me though.

We can  skip this patch for stable release backport.

Thanks!
-Rasesh

> 
> 	--yliu