netdev - RE: [PATCH net] bnx2x: really disable TPA if 'disable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 27 Apr 2015 18:15:44 +0000
From:	Yuval Mintz <Yuval.Mintz@...gic.com>
To:	David Miller <davem@...emloft.net>,
	"mschmidt@...hat.com" <mschmidt@...hat.com>
CC:	netdev <netdev@...r.kernel.org>,
	Dmitry Kravkov <Dmitry.Kravkov@...gic.com>,
	Ariel Elior <Ariel.Elior@...gic.com>
Subject: RE: [PATCH net] bnx2x: really disable TPA if 'disable_tpa' option
 is set

>> bnx2x's 'disable_tpa=1' module option is not respected properly and TPA
>> (transparent packet aggregation) remains enabled. Even though the
>> module option causes LRO to be disabled, TPA is enabled in GRO mode.
>>
>> Additionally, disabling GRO via ethtool then has no effect. One can
>> still observe tpa_* statistics increase and large packets being received
>> in tcpdump.
>>
>> The bug was an unintended consequence of commit aebf6244cd39 "bnx2x: Be
>> more forgiving toward SW GRO".
>>
>> Fix it by following the bp->disable_tpa flag when initializing fp's.
>>
>> Signed-off-by: Michal Schmidt <mschmidt@...hat.com>
>
>Applied, thanks.

Michal - thanks, but Dave -is it possible you gave us only 2.5 hours to
review this one?

I agree that this fixes an incorrect logic, but its incomplete as the bnx2x
'bp->flags' no longer represent the correct logic. I.e., it might cause
additional issues down the road, as the 'fp->mode' and 'fp->disable_tpa'
are no longer in sync.

We already have a fix for this issue, we just haven't sent it to 'net' yet.
Are you willing to give us an additional day to send an alternative fix?
Or do you prefer to take this, and possibly later we'll remove the logic
added by this patch in our own?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html