Page MenuHomeFreeBSD

svcj: correctly handle kernels without INET or INET6
Needs ReviewPublic

Authored by ivy on Wed, Apr 23, 10:08 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 26, 5:13 AM
Unknown Object (File)
Fri, Apr 25, 8:01 PM
Unknown Object (File)
Fri, Apr 25, 1:56 AM
Unknown Object (File)
Fri, Apr 25, 1:28 AM
Unknown Object (File)
Fri, Apr 25, 1:23 AM
Unknown Object (File)
Wed, Apr 23, 10:10 PM
Unknown Object (File)
Wed, Apr 23, 8:15 PM
Unknown Object (File)
Wed, Apr 23, 5:39 PM
Subscribers

Details

Reviewers
kevans
des
netchild
Group Reviewers
Jails
rc
Summary

if either INET or INET6 is not enabled in the kernel, then the jail(8)
options ip4=<new|inherit> resp. ip6=<new|inherit> are not available.
detect this case and don't try to provide those options, otherwise
svcjs will not start.

do this automatically (without a warning) so that net_basic, which
includes both netv4 and netv6, continues to work as expected.

if _svcj_ipaddrs is explicitly configured with an address for an IP
version not supported by the kernel, issue a warning but continue to
start the service. this can result in the service being started with
fewer addresses than expected, but never more.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 63665
Build 60549: arc lint + arc unit

Event Timeline

ivy requested review of this revision.Wed, Apr 23, 10:08 AM

Wouldn't it be better to check at point of use rather than at point of initialization?

libexec/rc/rc.subr
1231–1250

What if inet is enabled but inet6 is not, but there are ip6 addrs? If I'm not mistaken, in that case you have set this it to ip6=new above, and this branch is wrongly taken.
To me it looks it's better to simply set a "new" flag, and set both to new after this check and before the "translate service jail options" part.

This revision now requires changes to proceed.Wed, Apr 23, 10:31 AM
libexec/rc/rc.subr
1231–1250

yes, you're right. i'll rethinking this and see if i can address des comment at the same time.

i think this is a more reasonable way to do it, and seems to work in testing.
i'd like to add a proper set of tests for this, but since the test jails run
with path=/, i'm not sure there's a reasonable way to test this kind of thing.

In D49976#1139747, @des wrote:

Wouldn't it be better to check at point of use rather than at point of initialization?

I see your point. On the other hand I think the part which sets "the other one" may be more easy at the current place than at the place of use. I also like the current way of having the case statement "nice" (not convoluted with checks but simply listing what it entails).