> 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.
