On Thu, Jun 07, 2018 at 05:42:44PM +0100, Ben Hutchings wrote: > On Thu, 2018-06-07 at 18:16 +0200, Greg Kroah-Hartman wrote: > > On Thu, Jun 07, 2018 at 04:49:57PM +0100, Ben Hutchings wrote: > > > On Thu, 2018-06-07 at 17:22 +0530, Sriram R wrote: > > > > Hi Ben, > > > > > > > > On 2018-06-04 23:22, Ben Hutchings wrote: > > > > > On Mon, 2018-05-14 at 08:48 +0200, Greg Kroah-Hartman wrote: > > > > > > 4.4-stable review patch. If anyone has any objections, please let me > > > > > > know. > > > > > > > > > > > > ------------------ > > > > > > > > > > > > From: Vasanthakumar Thiagarajan <vthiagar@xxxxxxxxxxxxxxxx> > > > > > > > > > > > > commit 2f38c3c01de945234d23dd163e3528ccb413066d upstream. > > > > > > > > > > > > Chipset from QCA99X0 onwards (QCA99X0, QCA9984, QCA4019 & future) > > > > > > rx_hdr_status is not padded to align in 4-byte boundary. Define a > > > > > > new hw_params field to handle different alignment behaviour between > > > > > > different hw. This patch fixes improper retrieval of rfc1042 header > > > > > > with QCA4019. This patch along with "ath10k: Properly remove padding > > > > > > from the start of rx payload" will fix traffic failure in ethernet > > > > > > decap mode for QCA4019. > > > > > > > > > > > > Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@xxxxxxxxxxxxxxxx> > > > > > > Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> > > > > > > Signed-off-by: Sriram R <srirrama@xxxxxxxxxxxxxx> > > > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > > > > > > > > > > [...] > > > > > > > > > > I'm curious as to why this backport doesn't include the change to > > > > > ath10k_htt_rx_h_find_rfc1042(). I understand that the addition of the > > > > > new field is a dependency for the following patch, but shouldn't the > > > > > fix included in the upstream commit also be applied to 4.4? > > > > > > > > > > > > > Our main intention with this patchset [1] was to provide fix for > > > > replay detection security issue seen in ath10k driver which needed to be > > > > in the stable releases. > > > > > > > > And, as per stable tree guidelines we wanted the patchset to have only > > > > one and this important fix . > > > > > > OK, I think the problem here is that the rules say "must" when what's > > > really meant is "should". So the rule "It must fix only one thing." > > > really means that commits that each make a single logical change are > > > strongly preferred. > > > > > > It does not mean that upstream commits should be trimmed down to > > > conform to this. Greg generally considers it more important to avoid > > > changes to the upstream commit, where possible. Right, Greg? > > > > > > And speaking only for myself, I particularly dislike stable backports > > > that are significantly different from the original upstream commit but > > > don't mention this difference in the commit message. > > > > I _STRONGLY_ dislike backports that are different than what is in > > Linus's tree and normally I catch it when someone tries to do that. I > > missed this one here, and that's not ok on my part for missing that, and > > for the authors part in doing that :( > > > > So, what to do here, should I revert this series and take a fixed-up > > one? What exactly is the stable tree now missing because of this > > mistake? > > If you apply the attached patch, that should complete the backporting > of commit 2f38c3c01de9. I'm not able to test the driver but the change > looks reasonable and it builds OK. Looks good, thanks for this, now queued up. greg k-h