Impact of Early Exit Check in Configuration Processing

[miteshkanjariya]

The addition of the early exit check in vtysh_config_from_file changes the configuration processing logic. This code was added by commit 315e903.

@@ -880,6 +880,13 @@ int vtysh_config_from_file(struct vty *vty, FILE *fp)
                if (strmatch(vty_buf_trimmed, "end"))
                        continue;
 
+               if (strmatch(vty_buf_trimmed, "exit") &&
+                   vty->node == CONFIG_NODE) {
+                       fprintf(stderr, "line %d: Warning[%d]...: %s\n", lineno,
+                               vty->node, "early exit from config file");
+                       break;
+               }
+

Before the Change:

If a command failed (e.g., router bgp ASN), the system would:

1. Set retcode to the error status
2. Continue processing the next line in the configuration file
3. Return the error status only after attempting to process all commands

After the Change:
If a command fails (e.g., router bgp 66) and the next line contains "exit":

1. The system sets retcode to the error status from the failed command
2. The early exit check detects the "exit" command
3. The system breaks out of the configuration processing loop
4. The function returns immediately with the error status from the failed command

Specific Failure Scenario:

consider a config file with the following snippet

router bgp 66
exit
--- other commands ---

1. The system attempts to execute router bgp 66
2. This command might fails due to validation checks in BGP backbend handling of the command (e.g., due to invalid ASN or an ASN change etc)
3. The system sets retcode to the error status
4. The system processes the next line and finds "exit"
5. The early exit check detects "exit" at CONFIG_NODE level
6. The system breaks out of the configuration processing loop
7. The function returns immediately with the error status from the failed command

This is a change in behavior of how config is read from frr.conf:
It prevents the system from attempting to execute any commands after the failed command
It changes the behavior of the configuration processing

Summary:

The early exit check was likely added to detect and handle intentional exits from configuration files, but it inadvertently changes the error handling logic. The system now stops processing the configuration file when it encounters an "exit" command after a failed command, which is a significant change and might not be desirable in certain cases.
This change breaks how the config file was read and processed where the system would continue processing the configuration file even if one command fails, allowing for partial configuration success.

[choppsv1]

Can you provide an actual config file that fails the two different ways you are saying? The added code is exiting trying to avoid processing config in a config file as exec level commands.

[miteshkanjariya]

Sample config file which fails:

router bgp 1234 vrf default
bgp router-id 1.1.1.1
neighbor <> remote-as external
address-family ipv4 unicast
redistribute connected  route-map <>
neighbor <> activate
exit-address-family

!**!! Below command returns failure from BGP backend which causes the loop to exit in the vtysh_config_from_file**

router bgp 5678 vrf vrf1 
bgp router-id 2.2.2.2
neighbor A.B.C.D remote-as external
address-family ipv4 unicast
network PREFIX/32
redistribute connected  route-map <>
neighbor A.B.C.D activate
exit-address-family

**!!! Other valid config which would have been processed before the changes in the mentioned commit.**
router bgp 1234 vrf vrf2
bgp router-id 2.2.2.2
neighbor A.B.C.D remote-as external
address-family ipv4 unicast
network PREFIX/32
redistribute connected  route-map <>
neighbor A.B.C.D activate
exit-address-family

[choppsv1]

FWIW when the early exit behavior was modified a topotest was created (tests/topotests/mgmt_config). Previously there were differences in how early exit commands were handled depending on who was processing the config. The change normalized things so it was the same behavior regardless of who was processing the config.

I still need to understand your example case better, but if it's making the behavior more deterministic (which it sounds like it is) then it's likely going to be the solution going forward. Continuing to parse lines when the parser now has no idea what level it should be at seems wrong, if that what happened before.

[choppsv1]

The following config simply parsed correclty for me, can you be more explicit in what is generating the error and why? I added some values that you had given as <> or A.B.C.D but it wasn't clear to me that I should leave them since nothing would parse correctly otherwise.

router bgp 1234 vrf default
bgp router-id 1.1.1.1
neighbor 10.0.1.2 remote-as external
address-family ipv4 unicast
redistribute connected  route-map RM1
neighbor 10.0.1.2 activate
exit-address-family

!**!! Below command returns failure from BGP backend which causes the loop to exit in the vtysh_config_from_file**

router bgp 5678 vrf vrf1
bgp router-id 2.2.2.2
neighbor 10.0.2.3 remote-as external
address-family ipv4 unicast
network 10.0.0.0/8
redistribute connected  route-map RM1
neighbor 10.0.2.3 activate
exit-address-family

!**!!! Other valid config which would have been processed before the changes in the mentioned commit.**

router bgp 1234 vrf vrf2
bgp router-id 2.2.2.2
neighbor 10.0.1.2 remote-as external
address-family ipv4 unicast
network 10.0.0.0/8
redistribute connected  route-map RM1
neighbor 10.0.1.2 activate
exit-address-family