icmp: hide icmp_bandlimit_uninit() under VIMAGE

[glebius]

Summary

The uninitialization may be executed only on a kernel with VIMAGE.

[kp]

When we build without VIMAGE VNET_SYSUNINIT translates to SYSUNINIT, so this patch means we leak V_icmp_rates[i].cr_rate on shutdown.

That's not exactly a critical problem, but this is technically wrong.

[glebius]

In D44476#1014446, @kp wrote:

When we build without VIMAGE VNET_SYSUNINIT translates to SYSUNINIT, so this patch means we leak V_icmp_rates[i].cr_rate on shutdown.
That's not exactly a critical problem, but this is technically wrong.

I don't agree with that. We don't deallocate memory on shutdown in general case. We do not have a matching SYSUNINIT for every SYSINIT that mallocs. Keeping a function to deallocate memory on shutdown is the actual waste of memory - it grows kernel text, which is wired.

[kp]

In D44476#1014459, @glebius wrote:

In D44476#1014446, @kp wrote:

When we build without VIMAGE VNET_SYSUNINIT translates to SYSUNINIT, so this patch means we leak V_icmp_rates[i].cr_rate on shutdown.
That's not exactly a critical problem, but this is technically wrong.

I don't agree with that. We don't deallocate memory on shutdown in general case. We do not have a matching SYSUNINIT for every SYSINIT that mallocs. Keeping a function to deallocate memory on shutdown is the actual waste of memory - it grows kernel text, which is wired.

That's an argument one could make, yes. I'm inclined to disagree, mostly because not doing the cleanups made the whole vimage introduction a lot harder, but it's a reasonable argument.

I was going to suggest changing VNET_SYSUNINIT to be empty in the !VIMAGE case, but there are cases where we want to do SYSUNINIT anyway, because we want to clean things up on module unload.
That doesn't apply here, of course.

I see there are a number of other VNET_SYSUNINIT's that have the '#ifdef VIMAGE' guard around them. Including one in netipsec/key.c, where it might be wrong due to module unload scenario.

[zlei]

Looks good to me.