[opensuse-kernel] strange (and broken) patch in our kernels
Hello, I accidentally noticed a really strange patch in our master branch and then found it also in other branches, e.g. SLE12 (but not in e.g. SLE11-SP4): patches.fixes/bridge-module-get-put.patch The purpose of this patch is to disallow unloading bridge module if there is at least one bridge device. This causes various problems: 1. It deviates from upstream kernel behaviour which allows unloading the module (removing all existing bridge devices). There should be a good reason for such difference in behaviour and I don't find the reasoning in bsc#267651 convincing. 2. It is inconsistent with to other types of virtual devices: "rmmod bonding" removes all bonding devices, "rmmod 8021q" removes all vlan devices etc. Why should bridge be different? 3. The patch is in fact broken as it only takes/drops the reference if the bridge device is added/deleted using the old ioctl interface (brctl command) but not if it is done using netlink (ip and most likely also wicked). This can result in refcount leak: 12sp0:~ # modprobe bridge 12sp0:~ # brctl addbr br1 12sp0:~ # ip link del br1 12sp0:~ # lsmod | grep '^bridge' bridge 123100 1 or even negative refcount: 12sp0:~ # modprobe bridge 12sp0:~ # ip link add br1 type bridge 12sp0:~ # brctl delbr br1 12sp0:~ # lsmod | grep '^bridge' bridge 123100 -1 This is definitely wrong. I'm afraid we can't get rid of the patch in SLE12 and SLE12-SP1 but I would like to drop it from master/stable so that it doesn't get into future releases (including SLE12-SP2). Does anyone have a strong argument for keeping (and fixing) the patch? Michal Kubeček -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On 24.11.15 at 13:51, <mkubecek@suse.cz> wrote: I accidentally noticed a really strange patch in our master branch and then found it also in other branches, e.g. SLE12 (but not in e.g. SLE11-SP4):
patches.fixes/bridge-module-get-put.patch
The purpose of this patch is to disallow unloading bridge module if there is at least one bridge device.
We've had such a discussion before (a few years back I guess).
This causes various problems:
1. It deviates from upstream kernel behaviour which allows unloading the module (removing all existing bridge devices). There should be a good reason for such difference in behaviour and I don't find the reasoning in bsc#267651 convincing.
Well, both originally and when the second round of discussions happened, others seem to have agreed that the behavior the patch tries to fix is bad. That said, ...
Does anyone have a strong argument for keeping (and fixing) the patch?
... I don't mind the patch being dropped as long as it's not going to be me to have to deal with possible fallout. (After all it shouldn't have been me to try to deal with the problem back when the patch got created.) Jan -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Tue, Nov 24, 2015 at 06:26:36AM -0700, Jan Beulich wrote:
On 24.11.15 at 13:51, <mkubecek@suse.cz> wrote: I accidentally noticed a really strange patch in our master branch and then found it also in other branches, e.g. SLE12 (but not in e.g. SLE11-SP4):
patches.fixes/bridge-module-get-put.patch
The purpose of this patch is to disallow unloading bridge module if there is at least one bridge device.
We've had such a discussion before (a few years back I guess).
This causes various problems:
1. It deviates from upstream kernel behaviour which allows unloading the module (removing all existing bridge devices). There should be a good reason for such difference in behaviour and I don't find the reasoning in bsc#267651 convincing.
Well, both originally and when the second round of discussions happened, others seem to have agreed that the behavior the patch tries to fix is bad. That said, ...
My opinion is the same as https://lists.linuxfoundation.org/pipermail/bridge/2011-April/007634.html i.e. if something is wrong, it's the behaviour of "modprobe -r" (and, of course, people running it without knowing what it does).
Does anyone have a strong argument for keeping (and fixing) the patch?
... I don't mind the patch being dropped as long as it's not going to be me to have to deal with possible fallout.
I can promise to handle bug reports about this (if any). Given that we don't have the patch in SLE11 SP2 through SP4, I believe they shouldn't be too frequent. Michal Kubeček -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Le Tuesday 24 November 2015 à 15:16 +0100, Michal Kubecek a écrit :
On Tue, Nov 24, 2015 at 06:26:36AM -0700, Jan Beulich wrote:
... I don't mind the patch being dropped as long as it's not going to be me to have to deal with possible fallout.
I can promise to handle bug reports about this (if any). Given that we don't have the patch in SLE11 SP2 through SP4, I believe they shouldn't be too frequent.
It was removed from SLES 11 SP2 by: commit c0d37b28a0b0969ddabaac9120ef3dcfdb165f43 Author: Greg Kroah-Hartman <gregkh@suse.de> Date: Fri Sep 23 15:43:59 2011 -0700 remove patches.fixes/bridge-module-get-put.patch as it's not needed anymore. Sadly lacks a reference, but I would say that the problem is that this patch should have been deleted from the master branch at the time too, but wasn't. So not only I'm all from dropping it from the master branch, but I would also suggest dropping it from the stable, openSUSE-13.1, openSUSE 13.2, openSUSE-42.1, SLE12 and SLE12-SP1 branches. The refcount leak you found is a good enough reason to drop the patch IMHO. -- Jean Delvare SUSE L3 Support -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
participants (3)
-
Jan Beulich -
Jean Delvare -
Michal Kubecek