troglobit / pimd

PIM-SM/SSM multicast routing for UNIX and Linux
http://troglobit.com/projects/pimd/
BSD 3-Clause "New" or "Revised" License
194 stars 86 forks source link

Is changing MAXVIFS from 32 to 64 in the kernel an issue? #172

Closed LouisAtGH closed 3 years ago

LouisAtGH commented 3 years ago

There is an pfSense activity related to "the possible to pass MAXVIFS from 32 to 64 in the kernel and in pimd". It that an issue for the actual pimd v3 build?

troglobit commented 3 years ago

This used to be a HARD kernel limitation, the bits that encode VIFs are in a 32-bit unsigned integer. This is a heritage from the MROUTING stack that both *BSD and Linux have. The pimd user space daemon should be capable of handling more, but that has not been tested for a few years.

  1. The kernel define MAXVIFS must be changed so any user space application can see and utilize this
  2. For pimd there's a configure option called --with-max-vifs=NUM, where NUM is the max
LouisAtGH commented 3 years ago

I surched for "max-vif" in the "git files" and found

"pimd is limited to the number of MAXVIFS interfaces listed in the kernel headers. In Linux see /usr/include/linux/mroute.h. "

Does that imply that under normal conditions pimd automatically reads/gets!! the number from the OS!? So that the possible --max-vifs settings is not necessary at all under normal conditions!? (I specially refer to FreeBSD of course) (If not is there a default value?)

troglobit commented 3 years ago

Yes, that's correct. If the OS header files list 33 as MAXVIFS then pimd will use 33 ... be aware though, this was last tested in 2015 and then only on Linux. I would be surprised if it just worked out of the box.

LouisAtGH commented 3 years ago

If you tell me where it is read I can set a trece and test it

troglobit commented 3 years ago

You asked for it ... it's A LOT! I would definitely recommend against going into this as a beginner in C code, PIM protocol, or kernel knowledge. It is not for the faint of heart.

The MAXVIFS constant is defined in the operating system headers, pimd carries some replacement headers for really old systems with the MROUTING stack but without the headers -- this is not the case for most modern systems, so do not use or rely on the values in the pimd/include directory.

DISCLAIMER: I will not spend any time helping to debug changes related to MAXVIFS. I will only accept patches/pull-requests if the submitter has taken the time to test their patch against at least Linux and *BSD systems, and preferably using the Ixia ANVL protocol suite.

src/kern.c:    char           input[IFNAMSIZ], output[MAXVIFS * (IFNAMSIZ + 2)] = "";
src/rsrr.c: char oifs[MAXVIFS + 1];
src/rsrr.h:    uint8_t  out_vifs[MAXVIFS];      /* outgoing vif bitmask */
src/route.c:    uint8_t old_oifs[MAXVIFS];
src/route.c:    uint8_t new_oifs[MAXVIFS];
src/route.c:    uint8_t new_leaves[MAXVIFS];
src/route.c:    uint8_t new_oifs[MAXVIFS];
src/route.c:    uint8_t old_oifs[MAXVIFS];
src/route.c:    uint8_t new_leaves[MAXVIFS];
src/route.c:    uint8_t oifs[MAXVIFS];
src/route.c:    uint8_t new_joined_oifs[MAXVIFS];  /* The oifs for that particular mrtentry */
src/route.c:    uint8_t old_joined_oifs[MAXVIFS] __attribute__ ((unused));
src/route.c:    uint8_t old_pruned_oifs[MAXVIFS] __attribute__ ((unused));
src/route.c:    uint8_t old_leaves[MAXVIFS] __attribute__ ((unused));
src/route.c:    uint8_t new_leaves[MAXVIFS];
src/route.c:    uint8_t old_asserted_oifs[MAXVIFS] __attribute__ ((unused));
src/route.c:    uint8_t new_real_oifs[MAXVIFS];    /* The result oifs */
src/route.c:    uint8_t old_real_oifs[MAXVIFS];
src/route.c:        uint8_t oifs[MAXVIFS];
src/route.c:    uint8_t new_pruned_oifs[MAXVIFS];
src/vif.c:struct uvif   uvifs[MAXVIFS]; /* array of all virtual interfaces          */
src/vif.c:    for (vifi = 0, v = uvifs; vifi < MAXVIFS; ++vifi, ++v)
src/vif.c:    if ((numvifs + 1) == MAXVIFS) {
src/pim_proto.c:    uint8_t oifs[MAXVIFS];
src/pim_proto.c:    uint8_t pruned_oifs[MAXVIFS];
src/pim_proto.c:    uint8_t entry_oifs[MAXVIFS];
src/debug.c:    char oifs[MAXVIFS+1];
src/debug.c:    char joined_oifs[MAXVIFS+1];
src/debug.c:    char pruned_oifs[MAXVIFS+1];
src/debug.c:    char leaves_oifs[MAXVIFS+1];
src/debug.c:    char asserted_oifs[MAXVIFS+1];
src/debug.c:    char incoming_iif[MAXVIFS+1];
src/igmp_proto.c:    if (vifi >= MAXVIFS)
src/ipc.c:  char asserted_oifs[MAXVIFS+1];
src/ipc.c:  char incoming_iif[MAXVIFS+1];
src/ipc.c:  char joined_oifs[MAXVIFS+1];
src/ipc.c:  char pruned_oifs[MAXVIFS+1];
src/ipc.c:  char leaves_oifs[MAXVIFS+1];
src/ipc.c:  char oifs[MAXVIFS+1];
src/config.c:   if (numvifs == MAXVIFS) {
src/config.c:    for (vifi = 0, v = uvifs; vifi < MAXVIFS; ++vifi, ++v)
src/config.c:    for (vifi = 0, v = uvifs; vifi < MAXVIFS; ++vifi, ++v)
src/mrt.h:    uint8_t         oifs[MAXVIFS];        /* The current result oifs      */
src/mrt.h:    uint8_t         joined_oifs[MAXVIFS];     /* The joined oifs (Join received)  */
src/mrt.h:    uint8_t         pruned_oifs[MAXVIFS];     /* The pruned oifs (Prune received) */
src/mrt.h:    uint8_t         asserted_oifs[MAXVIFS];   /* The asserted oifs (lost Assert)  */
src/mrt.h:    uint8_t         leaves[MAXVIFS];      /* Has directly connected members   */
src/defs.h:extern struct uvif   uvifs[MAXVIFS];
src/vif.h:#define PIMD_VIFM_CLRALL(m)       (memset(m, 0, MAXVIFS))
src/vif.h:#define PIMD_VIFM_COPY(mfrom, mto)    (memcpy(mto, mfrom, MAXVIFS))
src/vif.h:    return memcmp(m1, m2, MAXVIFS) ? 0 : 1;
src/vif.h:    for (n = 0; n < MAXVIFS; n ++) {
src/vif.h:    for (n = 0; n < MAXVIFS; n ++) {
src/vif.h:    for (n = 0; n < MAXVIFS; n ++)
src/vif.h:    for (i = 0; i < MAXVIFS; i++) {
src/vif.h:#define NO_VIF        ((vifi_t)MAXVIFS)  /* An invalid vif index */
LouisAtGH commented 3 years ago

I took a look at the code and did some build-tests. Some findings, I do not guarantee that all my conclusions are correct! Never the less I hope it helps :) Some remarks based on my actual knowledge:

Some findings

MAXVIFS is a constant set during the build process by configure.ac. the result is used to allocate static vif related arrays/structures (: ) (e.g. vif.h is calculating static structs) This implies that if the destination os is compiled with a different maxvif number, there is a problem!

Some test results related to the build process (FreeBSD)

I am not sure where the 32 config.ac generates is coming from. AC_DEFINE_UNQUOTED(CUSTOM_MAX_VIFS, $max_vifs, [Custom MAX VIFs in kernel.])], [max_vifs=32]) ??????

configure.ac

definition files

I could do (and are willing to do) some test with debug statements with a running pimd, but that would only be helpful if maxvifs is dynamic and my actual feeling is that it is not ......

troglobit commented 3 years ago

I understand you're interested in working on this, but please read the disclaimer I posted previously!

Any work on this is bound to run into major problems and cause changes I'm not prepared to take any part in for the foreseeable future. I have other priorities, both in my life personally, and in this project (or other projects I also maintain).

Thank you for understanding.

LouisAtGH commented 3 years ago

Perhaps the temporarily solution is quite simple ...... just raising the default from 32 to 64 !?

troglobit commented 3 years ago

Possibly, but again, that is a kernel/OS define.

LouisAtGH commented 3 years ago

I have to better understand config.ac. I do not yet track down how and where config.ac terminates os-type and the actual maxvifs. Do not worry I will discover that :)

troglobit commented 3 years ago

MAXVIFS is set in a system/kernel/OS header, usually somewhere down in /usr/include/*.

The workaround in configure.ac is just for Linux. Where all open source pieces come from different developers/teams and are glued together in various distributions that often forget to update their system header files.

On the BSDs, when adding support for more than 32 kernel VIFs, adjust the kernel header file, which is part of the system and included automatically by userspace. This is the ideal scenario and does not include changing pimd sources at all.

LouisAtGH commented 3 years ago

Yep on my development system ip-mroute.h is in /usr/include/netinet

configure.ac is de-terminating my computer as freebsd12.2 (host-os). So I added "freebsd12*)" with no action for the moment (I did try CPPFLAGS="-I../usr/include/netinet/", which does not work (conftest.c:11:10: fatal error: 'ac_nonexistent.h' file not found). Setting it to CPPFLAGS="-I../usr/include did raise even more issues (.e.g related to .o files.) To much to investigate for now. And apart from that, my opinion is that somewhere in the future max-vifs should be de-terminated at runtime / at pimd start.

Using a script I am executing among other things following commands: Start with building using git: ./autogen.sh (1), ./configure (with options) (2), make clean all, make dist Then building with port (usiung distribution file): make makesum, make stage (3), make stage-qa, make package

The numbered actions (1,2,3) do generate MAX VIF values.

I used four ^MAX VIFS test^ settings

With those settings I did three tests test-1: mroute = 32, --with-max-vifs=64, AC_DEFINE_UNQUOTED = 99, CPPFLAGS active result: 1) = 64 2) = 64 3) = 99

test-2A: mroute = 32, no override, AC_DEFINE_UNQUOTED = 99, CPPFLAGS active result: 1) = 64 2) = 99 3) = 99

test-2B: mroute = 32, no override, AC_DEFINE_UNQUOTED = 99, CPPFLAGS deactivated result: 1) = 99 2) = 99 3) = 99

Result 1) seems to relay on what happened in the preceding run (not sure)

As noted before the mroute.h MAX VIFS value is dubious, since it is likely, but no one can grantee that the value used for the package target machine is the same.

So as far as I know:

Note here that I think that for FreeBSD MAX VIFS is normally determined by config.ac AC_DEFINE_UNQUOTED standard having a value of 32.

So for the moment based on what I know, I suggest to recompile with a modified AC_DEFINE_UNQUOTED you are running FreeBSD and you need a package supporting more vifs than the default 32. I know a bit dirty ..

Louis

PS doing tests I noticed some errors in config.log onftest.c:11:10: fatal error: 'ac_nonexistent.h' file not found

include

configure: failed program was: | / confdefs.h /

conftest.c:54:6: warning: incompatible redeclaration of library function 'strlcpy' [-Wincompatible-library-redeclaration] char strlcpy ();

conftest.c:55:6: warning: incompatible redeclaration of library function 'strlcat' [-Wincompatible-library-redeclaration] char strlcat (); ^ conftest.c:55:6: note: 'strlcat' is a builtin with type 'unsigned long (char , const char , unsigned long)' 1 warning generated.

troglobit commented 3 years ago

Related to non-Linux system, closing.