On Thu, Dec 23, 2021 at 12:00 PM Remi Pommarel [off-list ref] wrote:

On Wed, Dec 22, 2021 at 10:52:20PM +0100, Arnd Bergmann wrote:

quoted

On Wed, Dec 22, 2021 at 8:13 PM Remi Pommarel [off-list ref] wrote:

[...]

quoted

The intention of my broken patch was to make it work for compat mode as I did
in br_dev_siocdevprivate(), as this is now the only bit that remains broken.

This could be done along the lines of the patch below, if you see any value in
it. (not tested, probably not quite right).

Oh ok, because SIOC{S,G}IFBR compat ioctl was painfully done with
old_bridge_ioctl() I didn't think those needed compat. So I adapted and
fixed your patch to get that working.

Ok, thanks!

Here is my test results.

With my initial patch only :
  - 64bit busybox's brctl (working)
    # brctl show
    bridge name     bridge id               STP enabled     interfaces
    br0             8000.000000000000       n

  - CONFIG_COMPAT=y + 32bit busybox's brctl (not working)
    # brctl show
    brctl: SIOCGIFBR: Invalid argument

With both my intial patch and the one below :
  - 64bit busybox's brctl (working)
    # brctl show
    bridge name     bridge id               STP enabled     interfaces
    br0             8000.000000000000       n

  - CONFIG_COMPAT=y + 32bit busybox's brctl (working)
    # brctl show
    bridge name     bridge id               STP enabled     interfaces
    br0             8000.000000000000       n

If you think this has enough value to fix those compatility issues I can
either send the below patch as a V2 replacing my initial one for net
or sending it as a separate patch for net-next. What would you rather
like ?

If 32-bit busybox still uses those ioctls in moderately recent
versions, then it's probably worth doing this, but that would
be up to the bridge maintainers.

Your patch looks good to me, I see you caught a few mistakes
in my prototype. I would however suggest basing it on top of
your original fix, so that can be applied first and backported
to stable kernels, while the new patch would go on top and
not get backported.

If that works with everyone, please submit those two, and add
these tags to the second patch:

Co-developed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>