IP routing config for local network instances #38
Conversation
milan-zededa
force-pushed
the
ni-routes
branch
from
f10ea01 to
352c04c
Compare
| // into applications connected to them indirectly through local network instances. | ||
| // This option is only valid for local network instances. For other types | ||
| // of network instances, it will be ignored. | ||
| bool propagate_connected_routes = 42; |
There was a problem hiding this comment.
I always suspect that the question about life and everthing had something to do with IP routing ;-)
There was a problem hiding this comment.
Should be do this with 'disable_propagate_connected_routes'? since default case is most people want.
There was a problem hiding this comment.
We can't change the default behavior since we don't know whether that will break existing deployments.
There was a problem hiding this comment.
I'm also considering renaming this to propagate_uplink_routes. After all, uplink port may receive additional routes from an external DHCP server. Shouldn't we propagate all the routes associated with an uplink interface?
There was a problem hiding this comment.
Not going to rename, we agreed internally that only connected routes will be propagated, not all routes received from external DHCP server.
There was a problem hiding this comment.
Makes sense to keep the name and semantics (need to set to true to change from current behavior).
@naiming-zededa Even if most cases (where app instances have multiple adapters) could benefit from having this set it is a bit scary to start doing that as part of an update by changing the default and have a disable field.
|
we can also introduce a new api-capability for this feature, otherwise user configured static-routes in the cloud side and wondering why it does not show up on the device. |
|
Marking this as draft until some design discussions are closed. Then will also make changes requested in review. |
milan-zededa
force-pushed
the
ni-routes
branch
from
352c04c to
c6bf8ef
Compare
Done, added new API capability. |
|
@eriknordmark, @naiming-zededa PR is ready for re-review. |
There was a problem hiding this comment.
LGTM, but as we add this should we also add something about propagate/inject default route or not in the API?
Today we do that by default and can disable it by setting the gateway (for the network if I don't misremember - or is it on the network instance?)
If nothing else, how about documenting that near the comment about static_routes?
These new fields will allow to: - use DHCP to automatically propagate routes for uplink subnets to applications connected to them indirectly through local network instances - configure static IP routes for local network instances and propagate them using DHCP to connected applications Signed-off-by: Milan Lenco <milan@zededa.com>
Signed-off-by: Milan Lenco <milan@zededa.com>
milan-zededa
force-pushed
the
ni-routes
branch
from
c6bf8ef to
c4ec939
Compare
Done, added comments explaining default route propagation. |
These new fields will allow to:
into applications connected to them indirectly through local network
instances
them using DHCP to connected applications