MIME-Version: 1.0
References: <20211222191320.17662-1-repk@triplefau.lt> <CAK8P3a18b63GoPKiTey8KpEusyffbN97gxP+NM3fyZnOYXv5zg@mail.gmail.com>
 <YcRW1ckSr3ZSCDf9@pilgrim>
In-Reply-To: <YcRW1ckSr3ZSCDf9@pilgrim>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Thu, 23 Dec 2021 12:38:14 +0100
Message-ID: <CAK8P3a0PTu2qCHGr63TBMgnjL9fQwn4=7CrURKMHQufffwOg9Q@mail.gmail.com>
Subject: Re: [PATCH net] net: bridge: fix ioctl old_deviceless bridge argument
To:     Remi Pommarel <repk@triplefau.lt>
Cc:     Arnd Bergmann <arnd@arndb.de>, Networking <netdev@vger.kernel.org>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <nikolay@nvidia.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        bridge@lists.linux-foundation.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Dec 23, 2021 at 12:00 PM Remi Pommarel <repk@triplefau.lt> wrote:
>
> On Wed, Dec 22, 2021 at 10:52:20PM +0100, Arnd Bergmann wrote:
> > On Wed, Dec 22, 2021 at 8:13 PM Remi Pommarel <repk@triplefau.lt> wrote:
> [...]
> >
> > 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>