> From: Yuanhan Liu [mailto:yuanhan....@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....@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.m...@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: sta...@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.m...@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
