Message-ID: <20250925081452.7u7hvvgac62xavk5@skbuf>
Date: Thu, 25 Sep 2025 11:14:52 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
	Hauke Mehrtens <hauke@...ke-m.de>, Andrew Lunn <andrew@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net 0/2] lantiq_gswip fixes

On Thu, Sep 25, 2025 at 09:35:54AM +0200, Paolo Abeni wrote:
> On 9/24/25 6:18 AM, Daniel Golle wrote:
> > On Mon, Sep 22, 2025 at 11:34:52AM -0700, Jakub Kicinski wrote:
> >> On Mon, 22 Sep 2025 14:07:17 +0300 Vladimir Oltean wrote:
> >>> - I don't think your local_termination.sh exercises the bug fixed by
> >>>   patch "[1/2] net: dsa: lantiq_gswip: move gswip_add_single_port_br()
> >>>   call to port_setup()". The port has to be initially down before
> >>>   joining a bridge, and be brought up afterwards. This can be tested
> >>>   manually. In local_termination.sh, although bridge_create() runs
> >>>   "ip link set $h2 up" after "ip link set $h2 master br0", $h2 was
> >>>   already up due to "simple_if_init $h2".
> >>
> >> Waiting for more testing..
> > 
> > I've added printk statements to illustrate the function calls to
> > gswip_port_enable() and gswip_port_setup(), and tested both the current
> > 'net' without (before.txt) and with (after.txt) patch
> > "net: dsa: lantiq_gswip: move gswip_add_single_port_br() call to port_setup()"
> > applied. This makes it obvious that gswip_port_enable() calls
> > gswip_add_single_port_br() even though the port is at this point
> > already a member of another bridge.
> 
> Out of sheer ignorance is not clear to me why gswip_port_enable() is
> apparently invoked only once while gswip_port_setup() is apparently
> invoked for each dsa port.

All ports have to be set up during probing, but only the ports in use
(in the "after" log, these are the CPU port and one user port) need to
be enabled. The user port has a netdev, so it is enabled on ndo_open().
The CPU port doesn't have a netdev, it is always enabled.

> > I'm ready to do more testing or spray for printk over it, just let me
> > know.
> > 
> >>
> >>> - If the vast majority of users make use of this driver through OpenWrt,
> >>>   and if backporting to the required trees is done by OpenWrt and the
> >>>   fixes' presence in linux-stable is not useful, I can offer to resend
> >>>   this set plus the remaining patches all together through the net-next
> >>>   tree, and avoid complications such as merge conflicts.
> >>
> >> FWIW I don't even see a real conflict when merging this. git seems to
> >> be figuring things out on its own.
> > 
> > My concern here was the upcoming merge of the 'net' tree with the
> > 'net-next' tree which now already contains the splitting of the driver
> > into .h and .c file, and moved both into a dedicated folder.
> > This may result in needing (trivial) manual intervention.
> 
> AFAICT, when Jakub wrote 'merging this' he referred exactly to the 'net'
> -> 'net-next' merge.
> 
> > It would be great if all of Vladimir's patches can be merged without
> > a long delay, so more patches adding support for newer hardware can
> > be added during the next merge window. Especially the conversion of
> > the open-coded register access functions to be replaced by regmap_*
> > calls should only be committed after Vladimir's fixes.
> 
> This should not be a problem. Even moving these patches to net-next,
> they could be applied before the upcoming net-next PR (if Vladimir
> repost them soon enough). Even in the worst case scenario - targeting
> net-next and missing this PR - there should not be any delay for
> follow-up patches, as such patches will likely have to wait for the
> merge window closure anyway.
> 
> Given the above, that we are very close to 6.17, and the fixed here is
> quite old, I suggest moving this series to net-next - unless someone
> comes with a good reasoning to do otherwise. @Vladimr, could you please
> re-post for net-next?

So since I got my testing results which I'm now satisfied with, they
don't actually need reposting. If you can pick them up for the "net"
pull request and they're merged back into "net-next" later today, it
should be even better than reposting them for "net-next" - which may
require some avoidable rework such as dropping the "Fixes" tags.

I can repost them to "net-next" as well, along with more patches that
are waiting for these to be merged, but this isn't my preferred route
today, as it no longer seems to be the simplest path.

Powered by blists - more mailing lists