On Wed, Jan 18, 2023 at 04:01:29PM +0900, Masahiro Yamada wrote:

On Wed, Jan 18, 2023 at 4:23 AM Lucas De Marchi
[off-list ref] wrote:

quoted

On Tue, Jan 17, 2023 at 06:51:44PM +0100, Michal Suchánek wrote:

quoted

Hello,

On Fri, Jan 13, 2023 at 06:18:41PM +0000, Gary Guo wrote:

quoted

On Thu, 12 Jan 2023 14:40:59 -0700
Lucas De Marchi [off-list ref] wrote:

quoted

On Wed, Jan 11, 2023 at 04:11:51PM +0000, Gary Guo wrote:

quoted

struct modversion_info {
- unsigned long crc;
- char name[MODULE_NAME_LEN];
+ /* Offset of the next modversion entry in relation to this one. */
+ u32 next;
+ u32 crc;
+ char name[0];

although not really exported as uapi, this will break userspace as this is
used in the  elf file generated for the modules. I think
this change must be made in a backward compatible way and kmod updated
to deal with the variable name length:

kmod $ git grep "\[64"
libkmod/libkmod-elf.c:  char name[64 - sizeof(uint32_t)];
libkmod/libkmod-elf.c:  char name[64 - sizeof(uint64_t)];

in kmod we have both 32 and 64 because a 64-bit kmod can read both 32
and 64 bit module, and vice versa.

Hi Lucas,

Thanks for the information.

The change can't be "truly" backward compatible, in a sense that
regardless of the new format we choose, kmod would not be able to decode
symbols longer than "64 - sizeof(long)" bytes. So the list it retrieves
is going to be incomplete, isn't it?

What kind of backward compatibility should be expected? It could be:
* short symbols can still be found by old versions of kmod, but not
  long symbols;

That sounds good. Not everyone is using rust, and with this option
people who do will need to upgrade tooling, and people who don't care
don't need to do anything.

that could be it indeed. My main worry here is:

"After the support is added in kmod, kmod needs to be able to output the
correct information regardless if the module is from before/after the
change in the kernel and also without relying on kernel version."
Just changing the struct modversion_info doesn't make that possible.

Maybe adding the long symbols in another section? Or ble
just increase to 512 and add the size to a
"__versions_hdr" section. If we then output a max size per module,
this would offset a little bit the additional size gained for the
modules using rust. And the additional 0's should compress well
so I'm not sure the additional size is that much relevant here.

I also thought of new section(s) for long symbols.



One idea is to have separate sections for CRCs and symbol names.




section __version_crc:
  0x12345678
  0x23456789
  0x34567890


section __version_sym:
 "very_very_very_very_long_symbol"
 "another_very_very_very_very_very_long_symbol"
 "yet_another_very_very_very_very_very_long_symbol"




You can iterate in each section with this:

  crc += sizeof(u32);
  name += strlen(name) + 1;


Benefits:
 - No next pointer
 - No padding
   - *.mod.c is kept human readable.

I like this option. It would be better than the current one.

Lucas De Marchi

BTW, the following is impossible
because the pointer reference to .rodata
is not available at this point?

struct modversion_info {
    u32 crc;
    const char *name:
};



-- 
Best Regards
Masahiro Yamada