Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IP routing config for local network instances #38

Merged
merged 2 commits into from
Dec 14, 2023
Merged

IP routing config for local network instances #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7345c5d
IP routing config for local network instances
milan-zededa Dec 1, 2023
c4ec939
Generate proto files with NI routing config
milan-zededa Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
296 changes: 213 additions & 83 deletions go/config/netinst.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

281 changes: 143 additions & 138 deletions go/info/info.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions proto/config/netinst.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ message NetworkInstanceLispConfig {
bool experimental = 20;
}

message IPRoute {
// Destination network address in the CIDR format: <IP-address>/<prefix-length>
// It is allowed to submit default route with all-zeroes destination network address
// 0.0.0.0/0 or ::/0.
string destination_network = 1;
// Gateway IP address.
// This must be a valid IP address and can not be all-zeroes.
string gateway = 2;
}

message NetworkInstanceConfig {
UUIDandVersion uuidandversion = 1;
string displayname = 2;
Expand Down Expand Up @@ -107,4 +117,33 @@ message NetworkInstanceConfig {

// static DNS entry, if we are running DNS/DHCP service
repeated ZnetStaticDNSEntry dns = 41;
// Enable to use DHCP to automatically propagate routes for uplink subnets
// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always suspect that the question about life and everthing had something to do with IP routing ;-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be do this with 'disable_propagate_connected_routes'? since default case is most people want.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't change the default behavior since we don't know whether that will break existing deployments.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not going to rename, we agreed internally that only connected routes will be propagated, not all routes received from external DHCP server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

// List of IP routes statically added to the network instance routing table.
// Statically routed subnets are also propagated to connected applications
// using DHCP, with gateway set to the network instance bridge IP if it is
// outside of the network instance subnet.
//
// IP route gateway may point to an external endpoint (provided that network
// instance is not air-gapped), or to an IP address of one of the applications
// connected to the network instance.
//
// Static routes are handled independently from connected routes. While connected
// routes are propagated to applications only if enabled by propagate_connected_routes,
// static routes are always propagated. Both connected and statically configured
// routes can be propagated at the same time, there are no restrictions for using both.
//
// Note that the default route (with the bridge IP as the gateway) is automatically
// propagated to connected applications unless explicitly disabled by setting
// NetworkInstanceConfig.ip.gateway to an all-zeroes IP or when the uplink is app-shared
// (not management) and does not have a default route of its own. In the latter case,
// it is possible to enforce default route propagation by configuring a static default
// route for the network instance.
//
// This option is only valid for local network instances. For other types
// of network instances, it will be ignored.
repeated IPRoute static_routes = 43;
eriknordmark marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 1 addition & 0 deletions proto/info/info.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,7 @@ enum APICapability {
// Coming soon:
// API_CAPABILITY_EDGEVIEW = 4; // edgeview and edgeview.token supported
API_CAPABILITY_VOLUME_SNAPSHOTS = 5; // Volume snapshots supported
API_CAPABILITY_NETWORK_INSTANCE_ROUTING = 6; // routing config in NetworkInstanceConfig supported
// Add new values as new EdgeDevConfig API features are implemented
}

Expand Down
24 changes: 13 additions & 11 deletions python/config/netinst_pb2.py
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 24 additions & 24 deletions python/info/info_pb2.py
View file
Open in desktop

Large diffs are not rendered by default.

Loading