netdev - Re: [PATCH net v2] net: mscc: ocelot: Fix crash when adding interface under a lag

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] [day] [month] [year] [list]

Message-ID: <20251220210808.325isrbvmhjp3tlg@skbuf>
Date: Sat, 20 Dec 2025 23:08:08 +0200
From: Vladimir Oltean <vladimir.oltean@....com>
To: Jerry Wu <w.7erry@...mail.com>
Cc: christophe.jaillet@...adoo.fr, UNGLinuxDriver@...rochip.com,
	alexandre.belloni@...tlin.com, andrew+netdev@...n.ch,
	claudiu.manoil@....com, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, pabeni@...hat.com
Subject: Re: [PATCH net v2] net: mscc: ocelot: Fix crash when adding
 interface under a lag

On Sat, Dec 20, 2025 at 08:36:14PM +0000, Jerry Wu wrote:
> Dear Linux Kernel communities,
> 
> On Sat, Dec 20, 2025 at 20:00 UTC Vladimir Oltean Wrote
> > The 4th item in maintainer-netdev.rst is "don't repost your patches
> > within one 24h period". This would have given me more than 4 minutes
> between your v2 and... v2 (?!) to leave extra comments.
> 
> > The area below "---" in the patch is discarded when applying the patch.
> > It is recommended that you use it for patch change information between
> > versions. You copied a bunch of new people in v2 which have no reference
> > to v1. Find your patches on https://lore.kernel.org/netdev/ and
> > https://lore.kernel.org/lkml/ and reference them, and explain the
> > changes you've made.
> 
> Thank you for your kind suggestion. I'll learn to leverage it in my future
> contribution. And I want to explain that the repeated patch was sent due
> to some network issues as I thought in first email failed. The latest
> patch is the correct one.

Ok, but the content of the two v2 patches is not identical. One compiles,
and the other doesn't. Any change to a patch should constitute a new
version.

> 
> The context link is
> https://lore.kernel.org/netdev/20251220180113.724txltmrkxzyaql@skbuf/T/

You should provide the link to your own code submission and not to a reply, i.e.
https://lore.kernel.org/lkml/tencent_9E2B81D645D04DFE191C86F128212F842B05@qq.com/
In this case, this is not just a pedantic comment, because you didn't
post v1 to netdev, so your patch is [not found] when searched from that
lore instance rather than from lkml.

> 
> > Because the "bond" variable is used only once, I had a review comment in
> > v1 to delete it, and leave the code with just this:
> 
> > bond_mask = ocelot_get_bond_mask(ocelot, ocelot_port->bond);
> 
> > You didn't leave any reason for disregarding this element of the feedback.
> 
> Sorry for the missing. I reserved the `bond` variable as near line 2355
> 
> >		for (port = lag; port < ocelot->num_phys_ports; port++) {
> >			struct ocelot_port *ocelot_port = ocelot->ports[port];
> >
> >			if (!ocelot_port)
> >				continue;
> >
> >			if (ocelot_port->bond == bond)
> >				visited |= BIT(port);
> >		}
> 
> I noticed that the bond variable would be used again so reserved it.
> Sorry again for any inconvenience caused. If there is any information
> needed or improper contribution practice from me please let me know as
> I also found some other issues, being preparing to continue reporting.

Ok, so the reason is that "bond" is not used just once as I thought. It
is used one more time here:

		/* Mark all ports in the same LAG as visited to avoid applying
		 * the same config again.
		 */
		for (port = lag; port < ocelot->num_phys_ports; port++) {
			struct ocelot_port *ocelot_port = ocelot->ports[port];

			if (!ocelot_port)
				continue;

			if (ocelot_port->bond == bond)
						 ~~~~
				visited |= BIT(port);
		}

In that case yes, please disregard this comment, we need the variable saved.

After the 24 hour cool-off period, can you please resend the proper variant
of v2 as a new v3 with the change log added? To avoid the situation where
the wrong patch is applied.