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

Mody, Rasesh Rasesh.Mody at cavium.com
Thu May 4 02:14:30 CEST 2017



> From: Yuanhan Liu [mailto:yuanhan.liu at 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 at 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 at 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 at 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. 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!
-Rasesh

> 
> Thanks.
> 
> 	--yliu


More information about the dev mailing list