[dpdk-dev] [dpdk-stable] [PATCH 11/11] net/qede: fix to limit CFLAGS to base files
Mody, Rasesh
Rasesh.Mody at cavium.com
Sun May 7 07:15:47 CEST 2017
> From: Yuanhan Liu [mailto:yuanhan.liu at 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 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.
>
> 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
More information about the dev
mailing list