On Mon, 15 Jan 2024 at 14:25, Russell King (Oracle) <linux@xxxxxxxxxxxxxxx> wrote: > > On Mon, Jan 15, 2024 at 01:09:25PM +0000, Dimitri John Ledkov wrote: > > On Mon, 15 Jan 2024 at 12:45, Russell King (Oracle) > > <linux@xxxxxxxxxxxxxxx> wrote: > > > > > > Ping? > > > > > > > The intent is good. > > The implementation is incomplete. > > > > Please respond or address review feedback emailed previously. See > > https://lore.kernel.org/all/CADWks+Z5iZ=P_OAanA-PiePFbMpwtRe3_dF8wRTak8YAi87zvQ@xxxxxxxxxxxxxx/#t > > > Did you test that things are successful wtih kmod 29, 30, 31? > > No I didn't. See my comment below the "---" line: > > "I don't know what the minimum requirement is for SHA3 to work, so I > have chosen a minimum of version 29 for the purposes of this patch." > > > The code to correctly support sha3 in kmod was committed after 31 was > > tagged, and there is no newer tag yet hence the revision that has the > > correct code is v31-6-g510c8b7f74. > > Thanks for the information. > > > If such check is desired, kmod 32 should be tagged and check should > > check for 32. > > "If such a check is desired" ? You mean you prefer systems to segfault > during the installation step when the build system doesn't have a new > enough kmod? > > > If possible please use min-tool-version.sh to set the lower bound of > > kmod that is supported by the build. Assuming module signing is > > generally desired to be supported, the minimum required kmod should be > > set to 26. Otherwise at least modinfo doesn't work. > > That's a separate issue though, and has build-breaking ramifications. > Enforcing a minimum kmod 26 will mean that the kernel will fail if > kmod isn't new enough, whereas someone may be building with module > signing disabled and thus be fine with older kmod. > > These are two separate issues, and I think _this_ _fix_ needs to be > first because the issue is already there and affecting people (me), I don't believe the fix you desire is as critical as you think it is. Majority of people do not compile and install bleeding edge kernels, on an EOL release with a 5 year old kmod, when both are released by the same upstream project (more or less). Also your fix will prevent people to use EOL release for kernel compilation (or introduce requirement to have kmod installed on the build host), when they do not execute install on the same system (but transfer the files into a packaging format / execute depmod using up to date kmod not in path). Thus you may actually break even more people with this change, as kmod versions have never been enforced before. I am still waiting for a response from you: 1) why you attempted this given non-default configuration build - given your obsolete kmod in the installation target system (intentionally attempting to choose config options to suitable for your target) 2) why you want people to prevent compiling such a build, which they can install later on with a compatible kmod (when their build-time and install-time systems are different) Which you have ignored responding to, in the previous emails. I am trying to politely help you, and yet the tone of your emails are very aggressive and very dismissive to me. So far, this patch is a NACK from me. > and then maybe add the minimum version thing _afterwards_ in case it > needs to be reverted. > > Doing it the other way around would make reverting the min-version > thing much harder. > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last! -- Dimitri Sent from Ubuntu Pro https://ubuntu.com/pro
