Skip to content

FPM add routing view global option (for FreeBSD for now). #8470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

devnexen
Copy link
Member

@devnexen devnexen commented May 1, 2022

set the route table FIB id to the sockets created within FPM up to
the max set by the system, avoiding having to use setfib command line.

@devnexen devnexen marked this pull request as ready for review May 11, 2022 19:15
@@ -386,6 +406,23 @@ static int fpm_socket_af_unix_listening_socket(struct fpm_worker_pool_s *wp) /*
}
/* }}} */

#ifdef HAVE_SOCKETROUTE
static int fpm_socket_setroute_init(void) /* {{{ */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for not returning bool or zend_result?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no reason in particular indeed :-)

set the route table FIB id to the sockets created within FPM up to
the max set by the system, avoiding having to use setfib command line.
@devnexen devnexen force-pushed the fpm_setfib_option branch from 925bb73 to aac88a3 Compare July 2, 2022 22:40
Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now except few minor comments.

In terms of a test I assume it might be tricky in the pipeline, right? But would it be possible to have something semi manual (e.g. run if there's a different routing table created and skipped if not)?

@devnexen devnexen force-pushed the fpm_setfib_option branch from aac88a3 to b3429d7 Compare July 3, 2022 17:39
@bukka
Copy link
Member

bukka commented Jul 3, 2022

@devnexen Implementation looks good to me. Let me know what you think about the potential test. If you think it doesn't make sense to add, it should be fine if you describe how you test it and if all works fine. But think some basic stub of the test would be awesome!

we change the value for boot time.
Copy link
Member

@bukka bukka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming that you tested it manually with a real routing table...

@devnexen devnexen closed this in 5174ee2 Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants