Move common firewall rules to respective packages #2553
Conversation
github-actions
bot
added
3. topic: babel
mkg20001
changed the title
mkg20001
changed the title
mkg20001
force-pushed
the
mmfd
branch
2 times, most recently
from
363908b to
5191d0e
Compare
There was a problem hiding this comment.
I tested this and checked the babel startup. There is a regression though, babel won't start anymore, because 300-gluon-mesh-babel-mkconfig fails to create /etc/gluon-babeld.conf because the IPv6 address is now not yet set on the loopback interface in UCI. As now 300-layer3-ip6 will be run after 300-gluon-mesh-babel-mkconfig. Before the change 300-gluon-mesh-babel-ip6 was called before 300-gluon-mesh-babel-mkconfig. Renaming 300-layer3-ip6 to 250-layer3-ip6 fixes this issue and makes babeld start fine again.
Also I think you can also move package/{gluon-mesh-babel => gluon-layer3-common}/luasrc/lib/gluon/radvd/argument in this pull-request, too.
Other than that looks good to me.
Edit: A check_site.lua should also be added to gluon-layer3-common for any site/domain settings it uses/requires. prefix6, next_node.ip6 and node_prefix6 if I'm not missing anything. And check if something in the check_site.lua of the other packages might need to be updated after the code moving, too.
|
babel explicitly requires node_client_prefix6. This is true for l3 also or should this actually be optional? |
My understanding of check_site.lua is, that checks for anything should be added to a specific package where it would crash or misbehave during runtime otherwise. I don't see the gluon-layer3-common package using node_client_prefix6 anywhere, so should not need to be added in there. At the moment it is optional for gluon-l3roamd and mandatory for gluon-mesh-babel. I've made a pull-request at #2569 with an idea on how to generally make node_client_prefix6 optional (and deprecated) in the site config though. Feel free to test and comment. |
|
By the way, do we need mmfd for anything? If it isn't needed for l3roamd, I'd like to get rid of it, as it adds a lot of complexity. |
|
Both babel and OLSRD need it for respondd. |
mkg20001
force-pushed
the
mmfd
branch
2 times, most recently
from
3f0386e to
1873049
Compare
| @@ -1,4 +1,3 @@ | |||
| need_string_match(in_domain({'node_prefix6'}), '^[%x:]+/64$') | |||
There was a problem hiding this comment.
In my opinion the node_prefix6 check should stay in here, even if gluon-layer3-common checks it, too:
$ git grep "site\." package/gluon-mesh-babel/
package/gluon-mesh-babel/luasrc/lib/gluon/upgrade/300-gluon-mesh-babel-mkconfig:file:write("out ip " .. site.next_node.ip6() .. "/128 deny\n")
package/gluon-mesh-babel/luasrc/lib/gluon/upgrade/300-gluon-mesh-babel-mkconfig:file:write("redistribute ip " .. site.next_node.ip6() .. "/128 deny\n")
package/gluon-mesh-babel/luasrc/lib/gluon/upgrade/300-gluon-mesh-babel-mkconfig:file:write("redistribute ip " .. site.prefix6() .. " eq 128 allow\n")
package/gluon-mesh-babel/luasrc/lib/gluon/upgrade/300-gluon-mesh-babel-mkconfig:file:write("redistribute ip " .. site.node_client_prefix6() .. " eq 128 allow\n")
package/gluon-mesh-babel/luasrc/lib/gluon/upgrade/300-gluon-mesh-babel-mkconfig:file:write("redistribute ip " .. site.node_prefix6() .. " eq 128 allow\n")
package/gluon-mesh-babel/src/respondd.c: fprintf(stderr, "get_addresses: could not obtain mesh-prefix from site.conf. Not adding addresses to json data\n");
package/gluon-mesh-babel/src/respondd.c: fprintf(stderr, "get_addresses: could not convert mesh-prefix from site.conf to string\n");
There was a problem hiding this comment.
In my opinion we should not duplicate checks that are already provided by dependencies. It may even make sense to have the l3-common package check values it doesn't use itself if all actual L3 protocol package need them, so we consolidate common checks.
There was a problem hiding this comment.
I've moved node_prefix6. Is that enough? Anything else left?
No description provided.