Bug 206583 - Unable to load ip_mroute kernel module if VIMAGE is enabled in kernel
Summary: Unable to load ip_mroute kernel module if VIMAGE is enabled in kernel
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Bjoern A. Zeeb
URL:
Keywords: patch, vimage
: 242035 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-01-24 16:53 UTC by Ben Woods
Modified: 2019-11-19 15:39 UTC (History)
4 users (show)

See Also:


Attachments
malloc() array space to unbreak kldloading on VNET kernels (2.29 KB, patch)
2016-01-24 17:24 UTC, Marko Zec
no flags Details | Diff
HEAD: malloc() array space to unbreak kldloading on VNET kernels (2.26 KB, patch)
2016-01-24 17:52 UTC, Ben Woods
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Woods freebsd_committer 2016-01-24 16:53:18 UTC
When running a kernel with VIMAGE support, loading the ip_mroute kernel module will fail.

% sudo kldload -v ip_mroute
kldload: an error occurred while loading the module. Please check dmesg(8) for more details.
% dmesg
linker_load_file: Unsupported file type
% uname -a
FreeBSD sparticus.woods.am 11.0-CURRENT FreeBSD 11.0-CURRENT #1 r294463: Sun Jan 24 17:17:25 CET 2016     root@freenas.woods.am:/usr/obj/usr/src/sys/GENERIC-NODEBUG-VIMAGE  amd64


If I recompile the kernel with the same sources, with the only change being to disable VIMAGE (in this case using the GENERIC-NODEBUG kernel), then after reboot I am able to load the ip_mroute kernel option successfully.

Expected result: ip_mroute kernel module should be able to load regardless of whether VIMAGE support is enabled in the kernel or not.
Comment 1 Ben Woods freebsd_committer 2016-01-24 17:16:48 UTC
Some information provided by Marko Zec on the freebsd-net@freebsd.org mailing list:
https://lists.freebsd.org/pipermail/freebsd-net/2016-January/044447.html

In this particular case the problem is that ip_mroute demands more
space for "virtualized global" variables than what kernel linker has
put aside for each vnet.

Bumping VNET_MODMIN to 24 should circumvent the issue that Ben is
observing.  A more vnet-friendly fix would require refactoring
ip_mroute's arrays so that they get malloc()ed / free()d from SYSINIT
handlers instead of being declared "virtualized global".

Marko

===================================================================
--- vnet.c      (revision 294659)
+++ vnet.c      (working copy)
@@ -170,7 +170,7 @@
  * we want the virtualized global variable space to be page-sized, we
may
  * have more space than that in practice.
  */
-#define        VNET_MODMIN     8192
+#define        VNET_MODMIN     3 * 8192
 #define        VNET_SIZE       roundup2(VNET_BYTES, PAGE_SIZE)
 #define        VNET_MODSIZE    (VNET_SIZE - (VNET_BYTES - VNET_MODMIN))
Comment 2 Ben Woods freebsd_committer 2016-01-24 17:18:57 UTC
It is worth noting that building a kernel with VIMAGE and MROUTING both enabled seems to work fine. This problem only appears when multicast routing is not built into the kernel with the MROUTING option, but VIMAGE support is - in this case attempting to load the ip_mroute kernel module fails.
Comment 3 Marko Zec freebsd_committer 2016-01-24 17:24:17 UTC
Created attachment 166069 [details]
malloc() array space to unbreak kldloading on VNET kernels
Comment 4 Marko Zec freebsd_committer 2016-01-24 17:25:07 UTC
Please try the attached patch with both plain and VIMAGE kernels.

Marko
Comment 5 Ben Woods freebsd_committer 2016-01-24 17:52:18 UTC
Created attachment 166070 [details]
HEAD: malloc() array space to unbreak kldloading on VNET kernels

Thanks for your help Marko. Your patch didn't apply cleanly against head, so I have instead modified it to the attached. Can you please have a quick look and make sure it still looks ok?
Comment 6 Marko Zec freebsd_committer 2016-01-24 19:56:18 UTC
(In reply to Ben Woods from comment #5)

Oops, my patch was based on stable/10 branch.  Your version for head seems fine, pls. go ahead with testing...
Comment 7 Ben Woods freebsd_committer 2016-01-25 08:25:53 UTC
I have tested the attached patch against head, and can confirm it fixes the issue with the ip_mroute module not being able to load on a kernel with VIMAGE enabled.

On a kernel with VIMAGE enabled and this patch applied, I am successfully able to load and then unload the ip_mroute module.

On a GENERIC-NODEBUG kernel (without VIMAGE enabled) and this patch applied, I am successfully able to load and unload the ip_mroute module (no change from without the patch applied).

Note that I have not done exhaustive testing of the multicast routing functionality yet, but can confirm I have not noticed any other strange behaviour with the patch applied.
Comment 8 Ben Woods freebsd_committer 2016-01-27 22:51:51 UTC
I have just attempted to test multicast routing with this patch, but have experienced 2 strange issues:

1. Sometimes I found igmpproxy would fail to start, but if I then unloaded the ip_mroute kernel module, and reloaded it, then igmpproxy would successfully start.

2. Once I began using the multicast routing, my normal unicast network traffic would no longer work. When I unloaded the ip_mroute kernel module my unicast network traffic would begin working again.

I am in the process of building a generic kernel with no patchs and no VIMAGE, to determine if these 2 items are regressions introduced by this patch, or if they also exist in the unpatched generic kernel.
Comment 9 Ben Woods freebsd_committer 2016-01-28 06:22:40 UTC
(In reply to Ben Woods from comment #8)
I can confirm Issue #2 described above is NOT caused by this patch, and is an existing issue I am experiencing with multicast routing and IPFW.

Still investigating whether Issue #1 is related to this patch or not.
Comment 10 Ben Woods freebsd_committer 2016-03-19 16:34:55 UTC
Hi Marko,
What are your thoughts about this patch? Anything more required before committing it?
Thanks,
Ben
Comment 11 Ben Woods freebsd_committer 2016-05-06 22:25:08 UTC
Hi Marko / Bjoern,
Any chance we could consider committing this soon, to ensure it is fixed in time for the FreeBSD 11 code freeze in 1 month?
Regards,
Ben
Comment 12 Bjoern A. Zeeb freebsd_committer 2018-11-02 14:47:23 UTC
Hi Ben,

sorry this never made it.  I was looking for this change in HEAD the other day to get a reference for https://svnweb.freebsd.org/base?view=revision&revision=339930 and couldn't find it.  Only going through a lot of open PRs I now see that it was never committed.  I'll go and have a look even though it's probably no longer needed given the aforementioned change.

You didn't by any chance keep the patch up-to-date?
Comment 13 Bjoern A. Zeeb freebsd_committer 2019-11-19 13:39:21 UTC
*** Bug 242035 has been marked as a duplicate of this bug. ***
Comment 14 Bjoern A. Zeeb freebsd_committer 2019-11-19 13:55:04 UTC
I've updated the patch and put it into review at:

https://reviews.freebsd.org/D22443


You can download it from there to test (I know it compiles) by clicking "Download Raw Diff" on the right side menu.
Comment 15 commit-hook freebsd_committer 2019-11-19 15:39:37 UTC
A commit references this bug:

Author: bz
Date: Tue Nov 19 15:38:55 UTC 2019
New revision: 354857
URL: https://svnweb.freebsd.org/changeset/base/354857

Log:
  Reduce the vnet_set module size of ip_mroute to allow loading as a module.

  With VIMAGE kernels modules get special treatment as they need
  to also keep the original values and make copies for each instance.
  For that a few pages of vnet modspace are provided and the
  kernel-linker and the VNET framework know how to deal with things.
  When the modspace is (almost) full, other modules which would
  overflow the modspace cannot be loaded and kldload will fail.

  ip_mroute uses a lot of variable space, mostly be four big arrays:
  set_vnet 0000000000000510 vnet_entry_multicast_register_if
  set_vnet 0000000000000700 vnet_entry_viftable
  set_vnet 0000000000002000 vnet_entry_bw_meter_timers
  set_vnet 0000000000002800 vnet_entry_bw_upcalls

  Dynamically malloc the three big ones for each instance we need
  and free them again on vnet teardown (the 4th is an ifnet).
  That way they only need module space for a single pointer and
  allow a lot more modules using virtualized variables to be loaded
  on a VNET kernel.

  PR:		206583
  Reviewed by:	hselasky, kp
  MFC after:	3 weeks
  Differential Revision:	https://reviews.freebsd.org/D22443

Changes:
  head/sys/netinet/ip_mroute.c