search

radvd-project / radvd

radvd | Official repository: https://github.com/radvd-project/radvd
https://radvd.litech.org/
Other
203 stars 107 forks source link

[ENHACEMENT] Unneeded process splitting when root #218

Closed audreylace closed 3 months ago

audreylace commented 11 months ago

Observation When running as root it is seems wasteful to fork the process into 2 as both processes can still write into and modify the proc filesystem.

Start of code that seems unneeded if the process is going to run as root anyways and not switch to another user:

https://github.com/radvd-project/radvd/blob/cf213516101c6871dd697612916ed5f4a282b7c1/radvd.c#L385

Impact

Suggested Change If radvd is not going to drop root privileges then the program should just set the file directly using the same API as the privileged child process.

stappersg commented 11 months ago

On Mon, Dec 04, 2023 at 12:36:54PM -0800, Audrey Lace wrote:

When running as root it is seems wasteful to fork the process into 2 as both processes can still write into and modify the proc filesystem.

What is the purpose of this issue?

Please express any goal

Groeten Geert Stappers

P.S. Do also update #219 & #220 -- Silence is hard to parse

audreylace commented 11 months ago

@stappersg for this issue pointing out a potential performance optimization/extra system overhead. Minimal impact at most.

Let me know if there is a format to follow :)

stappersg commented 11 months ago

On Mon, Dec 04, 2023 at 01:09:19PM -0800, Audrey Lace wrote:

@stappersg for this issue pointing out a potential performance optimization/extra system overhead. Minimal impact at most.

In my words "The issue reports that radvd is not perfect" (and I know "Perfect is the enemy of good")

Let me know if there is a format to follow :)

No need to worry about any format, just understand that open issues do consume human energy.

Back to "What is the purpose of this issue?"

If the answer is still

pointing out a potential performance optimization/extra system overhead

then please close this issue.

Geert Stappers Did not ignore this issue and would like to see further information why this issue should remain open. -- Silence is hard to parse

stappersg commented 11 months ago

Back to "What is the purpose of this issue?" If the answer is still

pointing out a potential performance optimization/extra system overhead

then please close this issue

a much better response would be "Now that I know that my input gets feedback, will I continue on preparing a merge request"

stappersg commented 3 months ago

On Sat, Aug 10, 2024 at 11:09:54PM -0700, Audrey Lace wrote:

Closed #218 as not planned.

Thanks