linux-kernel - RE: [PATCH v2 net 2/9] net: enetc: correct the tx

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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <PAXPR04MB8510EA8BC58E151B178F684188C02@PAXPR04MB8510.eurprd04.prod.outlook.com>
Date: Mon, 24 Feb 2025 03:27:10 +0000
From: Wei Fang <wei.fang@....com>
To: Vladimir Oltean <vladimir.oltean@....com>
CC: Claudiu Manoil <claudiu.manoil@....com>, Clark Wang
	<xiaoning.wang@....com>, "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
	"davem@...emloft.net" <davem@...emloft.net>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, Ioana Ciornei
	<ioana.ciornei@....com>, "Y.B. Lu" <yangbo.lu@....com>,
	"michal.swiatkowski@...ux.intel.com" <michal.swiatkowski@...ux.intel.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"imx@...ts.linux.dev" <imx@...ts.linux.dev>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>
Subject: RE: [PATCH v2 net 2/9] net: enetc: correct the tx_swbd statistics

> On Fri, Feb 21, 2025 at 03:42:05AM +0200, Wei Fang wrote:
> > > I'm not sure "correct the statistics" is the best way to describe this
> > > change. Maybe "keep track of correct TXBD count in
> > > enetc_map_tx_tso_buffs()"?
> >
> > Hi Vladimir,
> >
> > Inspired by Michal, I think we don't need to keep the count variable, because
> > we already have index "i", we just need to record the value of the initial i at
> the
> > beginning. So I plan to do this optimization on the net-next tree in the future.
> > So I don't think it is necessary to modify enetc_map_tx_tso_hdr().
> 
> You are saying this in reply to my observation that the title of the
> change does not correctly represent the change. But I don't see how what
> you're saying is connected to that. Currently there exists a "count"
> variable based on which TX BDs are unmapped, and these patches are not
> changing that fact.
> 
> > > stylistic nitpick: I think this implementation choice obscures the fact,
> > > to an unfamiliar reader, that the requirement for an extended TXBD comes
> > > from enetc_map_tx_tso_hdr(). This is because you repeat the condition
> > > for skb_vlan_tag_present(), but it's not obvious it's correlated to the
> > > other one. Something like the change below is more expressive in this
> > > regard, in my opinion:
> 
> It seems you were objecting to this other change suggestion instead.
> Sure, I mean, ignore it if you want, but you're saying "well I'm going
> to change the scheme for net-next, so no point in making the code more
> obviously correct in stable branches", but the stable branches aren't
> going to pick up the net-next patch - they are essentially frozen except
> for bug fixes. I would still recommend writing code that makes the most
> sense for stable (to the extent that this is logically part of fixing a
> bug), and then writing code that makes most sense for net-next, even if
> it implies changing some of it back the way it was.

Okay, agree with you, I will improve the patch.