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.

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

ncm-network: nmstate default gateway issue #1655

Closed
wdpypere opened this issue Feb 16, 2024 · 7 comments · Fixed by #1656
Closed

ncm-network: nmstate default gateway issue #1655

wdpypere opened this issue Feb 16, 2024 · 7 comments · Fixed by #1656
Milestone

Comments

@wdpypere
Copy link
Contributor

@aka7 I found another discrepancy with nmstate, with gateways. on our site we only set /system/network/default_gateway. something like:

+-/system/network
  $ default_gateway : '10.141.10.250'
  $ domainname : 'domain'
  $ hostname : 'hostname'
  +-interfaces
    +-eth0
      $ broadcast : '10.141.31.255'
      $ driver : 'bnx2'
      $ ip : '10.141.10.62'
      $ netmask : '255.255.224.0'
      +-route
        +-0
...

but in the code I see:

    if ($default_gw) {
        # create only default gw entry if gw entry match interface gateway defined                                                                           
        # otherwise this interface is not the default gw interface.                                                                                          
	if ((defined($iface->{gateway})) and ($iface->{gateway} eq $default_gw)) {
            $default_rt{destination} = '0.0.0.0/0';
            $default_rt{'next-hop-address'} = $default_gw;
            $default_rt{'next-hop-interface'} = $device;
	}
    }

the defined($iface->{gateway}) always fails, so no default gateway is set. My perl is quite poor but it seems the code makes the assumption that a gateway is set on interface level. while network.pm does not. Unless I'm mistaken.

I would expect a route like the following to show up:

Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
0.0.0.0         10.141.10.250   0.0.0.0         UG    0      0        0 eth0
@aka7
Copy link
Contributor

aka7 commented Feb 16, 2024

@wdpypere
probably worth understanding your profile settings. so if I understood this right, you have a default gateway defined only, and non of the interface config has gw defined?
I found this to be tricky with nmstate.
To add a route via nmstate it needs next-hope-interface. So you can't just add a default route without a next-hop-interface, in nmstate config, sadly.
All our profile configs has gw on interface level and a default_gateway defined, which is why I havent yet notice this issue.

So what this part of code currently supports if default_gateway is defined and tries match the same with the interface gw settings so it can pick the next-hope-interface.
I'm not sure how we can fix this, other than you making sure your interface config has gw? This highlights that we may need to add a check in the schema to fail compile. Something to discuss on our calls.

@wdpypere
Copy link
Contributor Author

wdpypere commented Feb 16, 2024

@aka7 yes, we are on the same page. 😄 none of our interfaces have a gw defined. we define gateways on routes and a default gateway.

My preference is of course backwards compatibility on the profiles, but if that is not possible then yes, it would be good if it would fail to compile. a system should have at least a fuctional default gateway.

@stdweird
Copy link
Member

i will have a look at this. default gateway should be something we can set/configure.

@stdweird
Copy link
Member

stdweird commented Feb 16, 2024

@aka7 the schema already has following code

} with {
    if (exists(SELF['default_gateway'])) {
        reachable = false;
        # is there any interface that can reach it?
        foreach (name; data; SELF['interfaces']) {
            if (exists(data['ip']) && exists(data['netmask']) &&
                ip_in_network(SELF['default_gateway'], data['ip'], data['netmask'])) {
                reachable = true;
            };
        };
        if (!reachable) {
            error("No interface with ip/mask found to reach default gateway");
        };
    };
    true;
};

i guess we would need something similar in perl or in our profiles to add a /0 route to the interface.

i looked into nmstate and networkmanager a bit, and it's not that clear how to set it (eg https://askubuntu.com/questions/1073692/network-manager-no-default-route). looks like you need to set the "gateway" value in the networkmanger config file instead of only adding a route. not sure how to do that with nmstate.

@wdpypere i guess the easiest way for now is to add some "run-last" template that based on code above adds a route with prefix 0 to the interface that it can reach.

i do think that this is better done in perl though, but given on the test above, it's not that hard in pan.

@aka7
Copy link
Contributor

aka7 commented Feb 17, 2024

@stdweird I already had chat with Redhat about this, if we can add default route (or any route) without having to add next-hop-interface in nmstate and answer was no. However if we think its valid requirement perhaps I can push them to create a RFE for it.

Thinking about this a bit more, we can perhaps change the perl code to add the next-hop-interface to default DGW entry if DGW falls within the subnet boundary of that interface? instead of what it does currently.

alternatively, we may need to add the support for command option in nmstate too.

aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Feb 18, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Feb 18, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Feb 18, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
@aka7
Copy link
Contributor

aka7 commented Feb 18, 2024

Thinking about this a bit more, we can perhaps change the perl code to add the next-hop-interface to default DGW entry >>if DGW falls within the subnet boundary of that interface? instead of what it does currently.

@wdpypere made a change to pick the right interface without looking at gateway on the interface. I think should fix the issue.
This appears to work for me, well, its a noop for me now, so looks good. Therefore pull this and try it, please?

#1656

Other commits of this is in my other PR which is yet to be merged but are needed here.

@wdpypere
Copy link
Contributor Author

As I commented in the PR, it does fix this issue.

aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Feb 20, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Feb 20, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Mar 19, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Mar 19, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
aka7 pushed a commit to aka7/configuration-modules-core that referenced this issue Mar 19, 2024
nmstate requires next-hop-interface for every route to be added.
Create default route entry only on the interface if default_gateway falls within subnet boundary.

fixes quattor#1655
@jrha jrha added this to the 24.3 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants