radvd-project / radvd

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

better check for sysctl() availability #37

Closed abrodkin closed 10 years ago

abrodkin commented 10 years ago

As it was mentioned before some Linux ports are dropping sysctl support. But the problem happens if libc in use still installs "sys/sysctl.h" header even if kernel doesn't support sysctl.

This is exactly the case with uClibc and probably other implementations of libc. But if we check for existing "sysctl" symbol in libc we're golden at least in compile-time because libc provides some stub at least.

Signed-off-by: Alexey Brodkin abrodkin@synopsys.com

abrodkin commented 10 years ago

Ooops I completely forgot to modify "device-linux.c" replacing HAVE_SYS_SYSCTL_H with HAVE_SYSCTL. Will do a re-spin. Please disregard this request.

reubenhwk commented 10 years ago

Very cool! Thanks!

I'll review in more detail when I get time. Seems like I had doubts about the sysctl bit a while back. I don't recall what my doubts were. It seems like sysctl is out of favor/style and is being replaced by files in /proc?

When you do a re-spin, please include a tid-bit about why sysctl is being dropped and what it is being replaced with.

Thanks in advance, Reuben

On Wed, Nov 26, 2014 at 1:16 PM, Alexey Brodkin notifications@github.com wrote:

Ooops I completely forgot to modify "device-linux.c" replacing HAVE_SYS_SYSCTL_H with HAVE_SYSCTL. Will do a re-spin. Please disregard this request.

— Reply to this email directly or view it on GitHub https://github.com/reubenhwk/radvd/pull/37#issuecomment-64711766.

abrodkin commented 10 years ago

Do you mean extended explanation about "sysctl" discontinuity in commit message of comment in code itself?

reubenhwk commented 10 years ago

Not an extended explanation. Any background info will do (and of course I can't require anything). The last I really read up on it I only found a LWN article posted in 2007. I'm mainly just wanting to know anything new that I don't already know so I can stay informed. A link to an article, or anything helpful, would be good.

Thanks in advance, Reuben

On Wed, Nov 26, 2014 at 1:43 PM, Alexey Brodkin notifications@github.com wrote:

Do you mean extended explanation about "sysctl" discontinuity in commit message of comment in code itself?

— Reply to this email directly or view it on GitHub https://github.com/reubenhwk/radvd/pull/37#issuecomment-64714909.

reubenhwk commented 10 years ago

Oh, not a comment in the code. Commit message can be short and simple. I'm mainly looking for more info on the topic...

On Wed, Nov 26, 2014 at 1:55 PM, Reuben Hawkins reubenhwk@gmail.com wrote:

Not an extended explanation. Any background info will do (and of course I can't require anything). The last I really read up on it I only found a LWN article posted in 2007. I'm mainly just wanting to know anything new that I don't already know so I can stay informed. A link to an article, or anything helpful, would be good.

Thanks in advance, Reuben

On Wed, Nov 26, 2014 at 1:43 PM, Alexey Brodkin notifications@github.com wrote:

Do you mean extended explanation about "sysctl" discontinuity in commit message of comment in code itself?

— Reply to this email directly or view it on GitHub https://github.com/reubenhwk/radvd/pull/37#issuecomment-64714909.

abrodkin commented 10 years ago

Well, frankly I haven't been tracking this sysctl topic tightly.

The only thing I know and for me this one is the most important that in UAPI headers which are a way to go for all architectures in Linux kernel and a must for new architectures "sysctl" syscall was put under __ARCH_WANT_SYSCALL_DEPRECATED ifdef: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/include/uapi/asm-generic/unistd.h#n813

So technically it's still possible to enable it but the question is when this deprecated section will go away completely?

abrodkin commented 10 years ago

Ok, so I added a change in "device-linux.c" replacing HAVE_SYS_SYSCTL_H with HAVE_SYSCTL and now if the patch looks good to you please consider applying.

Thanks

reubenhwk commented 9 years ago

I merged the request, but it didn't build on linux, so I reverted the change.

On Wed, Nov 26, 2014 at 10:57 PM, Alexey Brodkin notifications@github.com wrote:

Ok, so I added a change in "device-linux.c" replacing HAVE_SYS_SYSCTL_H with HAVE_SYSCTL and now if the patch looks good to you please consider applying.

Thanks

— Reply to this email directly or view it on GitHub https://github.com/reubenhwk/radvd/pull/37#issuecomment-64752716.