diff mbox series

net: ethtool: avoid rebuilds on UTS_RELEASE change

Message ID 20240220194244.2056384-1-jannh@google.com
State New
Headers
Series net: ethtool: avoid rebuilds on UTS_RELEASE change |

Commit Message

Jann Horn Feb. 20, 2024, 7:42 p.m. UTC 
  Currently, when you switch between branches or something like that and
rebuild, net/ethtool/ioctl.c has to be built again because it depends
on UTS_RELEASE.

By instead referencing a string variable stored in another object file,
this can be avoided.

Signed-off-by: Jann Horn <jannh@google.com>
---
(alternatively we could also use the utsname info from the current UTS
namespace, but that'd be a bit of a behavior change, and I wanted to
keep this change a no-op)

 net/ethtool/ioctl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski Feb. 21, 2024, 7:23 p.m. UTC | #1 
On Tue, 20 Feb 2024 20:42:44 +0100 Jann Horn wrote:
> Currently, when you switch between branches or something like that and
> rebuild, net/ethtool/ioctl.c has to be built again because it depends
> on UTS_RELEASE.
> 
> By instead referencing a string variable stored in another object file,
> this can be avoided.
> 
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> (alternatively we could also use the utsname info from the current UTS
> namespace, but that'd be a bit of a behavior change, and I wanted to
> keep this change a no-op)

Is this related to John's work from:
https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/
?
Jann Horn Feb. 21, 2024, 7:25 p.m. UTC | #2 
On Wed, Feb 21, 2024 at 8:23 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 20 Feb 2024 20:42:44 +0100 Jann Horn wrote:
> > Currently, when you switch between branches or something like that and
> > rebuild, net/ethtool/ioctl.c has to be built again because it depends
> > on UTS_RELEASE.
> >
> > By instead referencing a string variable stored in another object file,
> > this can be avoided.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > (alternatively we could also use the utsname info from the current UTS
> > namespace, but that'd be a bit of a behavior change, and I wanted to
> > keep this change a no-op)
>
> Is this related to John's work from:
> https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/
> ?

Ah, I didn't see his patch, but that seems like he had the same idea
(but implemented it less sloppily). You can drop this one then...
Jakub Kicinski Feb. 21, 2024, 7:35 p.m. UTC | #3 
On Wed, 21 Feb 2024 20:25:00 +0100 Jann Horn wrote:
> > Is this related to John's work from:
> > https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/
> > ?  
> 
> Ah, I didn't see his patch, but that seems like he had the same idea
> (but implemented it less sloppily). You can drop this one then...

No preference on my side, just wanted to make sure we don't have 
to decide within netdev which approach is better, not really our 
wheelhouse :)
John Garry Feb. 22, 2024, 8:08 a.m. UTC | #4 
On 21/02/2024 19:35, Jakub Kicinski wrote:

Thanks for including me

> On Wed, 21 Feb 2024 20:25:00 +0100 Jann Horn wrote:
>>> Is this related to John's work from:
>>> https://urldefense.com/v3/__https://lore.kernel.org/all/20240131104851.2311358-1-john.g.garry@oracle.com/__;!!ACWV5N9M2RV99hQ!IghWg9_5HEQ-ng1XiYH9hes6vmgylmcQF5l2RITIrCSB20BzKzOhWWKHrgQZw_DkgKlZRNllglTanuY$
>>> ?
>>
>> Ah, I didn't see his patch, but that seems like he had the same idea
>> (but implemented it less sloppily). You can drop this one then...
> 
> No preference on my side, just wanted to make sure we don't have
> to decide within netdev which approach is better, not really our
> wheelhouse :)

I was not going to pursue the change to introduce uts_release, as we can 
instead use init_uts_ns.name.release or init_utsname()->release instead. 
I think that init_utsname()->release is a bit neater to use, as it 
slightly hides the init_uts_ns structure.

@Jann, please feel free to pursue upstreaming the change in this patch with:

Reviewed-by: John Garry <john.g.garry@oracle.com>

However, note that I do have many other changes pending on this subject, 
see:
https://github.com/johnpgarry/linux/commits/uts-version-string/

I just need to find a bit of time to update and post them. We also need 
to be wary of CONFIG_MODVERSIONS, as discussed in that thread.

Thanks,
John
patchwork-bot+netdevbpf@kernel.org Feb. 24, 2024, 2:10 a.m. UTC | #5 
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 20 Feb 2024 20:42:44 +0100 you wrote:
> Currently, when you switch between branches or something like that and
> rebuild, net/ethtool/ioctl.c has to be built again because it depends
> on UTS_RELEASE.
> 
> By instead referencing a string variable stored in another object file,
> this can be avoided.
> 
> [...]

Here is the summary with links:
  - net: ethtool: avoid rebuilds on UTS_RELEASE change
    https://git.kernel.org/netdev/net-next/c/d2efeb52c344

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 7519b0818b91..575642b3070e 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -26,12 +26,12 @@ 
 #include <linux/sched/signal.h>
 #include <linux/net.h>
 #include <linux/pm_runtime.h>
+#include <linux/utsname.h>
 #include <net/devlink.h>
 #include <net/ipv6.h>
 #include <net/xdp_sock_drv.h>
 #include <net/flow_offload.h>
 #include <linux/ethtool_netlink.h>
-#include <generated/utsrelease.h>
 #include "common.h"
 
 /* State held across locks and calls for commands which have devlink fallback */
@@ -713,7 +713,8 @@  ethtool_get_drvinfo(struct net_device *dev, struct ethtool_devlink_compat *rsp)
 	struct device *parent = dev->dev.parent;
 
 	rsp->info.cmd = ETHTOOL_GDRVINFO;
-	strscpy(rsp->info.version, UTS_RELEASE, sizeof(rsp->info.version));
+	strscpy(rsp->info.version, init_uts_ns.name.release,
+		sizeof(rsp->info.version));
 	if (ops->get_drvinfo) {
 		ops->get_drvinfo(dev, &rsp->info);
 		if (!rsp->info.bus_info[0] && parent)