Usage of NEXT_STATEMENT policy result in RT-7.11-import_export_multi_test

[r4huluk]

Describe the bug

When policy result is RoutingPolicy_PolicyResultType_NEXT_STATEMENT in last statement of a policy, policy evaluation should continue to the next policy. If no policy exists, evaluation should continue to the default policy. If such a policy is used as a condition inside another policy (using call-policy condition), there is no next statement or next policy to continue evaluation and the route being evaluated is not explicitly accepted.

RT-7.11-import_export_multi_test has a policy [POLICY-2] that uses a nested policy [POLICY-1] having a last statement with RoutingPolicy_PolicyResultType_NEXT_STATEMENT and expects the policy evaluation of the nested policy to return True. Is this the right expectation?

[POLICY-1]

policy-definition match_community_regex {
        config {
            name match_community_regex;
        }
        statements {
            statement match_community_regex {
                config {
                    name match_community_regex;
                }
                conditions {
                    openconfig-bgp-policy:bgp-conditions {
                        match-community-set {
                            config {
                                community-set regex-community;
                                match-set-options ANY;
                            }
                        }
                    }
                }                      
                actions {
                    config {
                        policy-result NEXT_STATEMENT;
                    }
                }
            }
        }
    }

[POLICY-2]

policy-definition multiPolicy {
        config {
            name multiPolicy;
        }
        statements {
            statement reject_route_community {
                config {
                    name reject_route_community;
                }
                conditions {
                    openconfig-bgp-policy:bgp-conditions {
                        match-community-set {
                           config {
                                community-set reject_communities;
                                match-set-options ANY;
                            }
                        }
                    }
                }
                actions {
                    config {
                        policy-result REJECT_ROUTE;
                    }
                }
            }
            statement if_30_and_not_20_nested_reject {
                config {
                    name if_30_and_not_20_nested_reject;
                }
                conditions {           
                    config {
                        call-policy match_community_regex; <<<<< POLICY-1 is referenced
                    }
                    openconfig-bgp-policy:bgp-conditions {
                        match-community-set {
                            config {
                                community-set accept_communities;
                                match-set-options INVERT;
                            }
                        }
                    }
                }
                actions {
                    config {
                        policy-result REJECT_ROUTE;
                    }
                }
            }
            statement add_communities_if_missing {
                config {
                    name add_communities_if_missing;
                }
                actions {
                    config {
                        policy-result NEXT_STATEMENT;
                    }
                    openconfig-bgp-policy:bgp-actions {
                        set-community {
                            config {
                                method REFERENCE;
                                options ADD;
                            }
                            reference {
                                config {
                                    community-set-refs add-communities;
                                }
                            }
                        }              
                    }
                }
            }
            statement match_comm_and_prefix_add_2_community_sets {
                config {
                    name match_comm_and_prefix_add_2_community_sets;
                }
                conditions {
                    match-prefix-set {
                        config {
                            prefix-set prefix-set-5;
                            match-set-options ANY;
                        }
                    }
                    openconfig-bgp-policy:bgp-conditions {
                        match-community-set {
                            config {
                                community-set my_community;
                                match-set-options ANY;
                            }
                        }
                    }
                }
                actions {
                    config {
                        policy-result NEXT_STATEMENT;
                    }
                    openconfig-bgp-policy:bgp-actions {
                        config {
                            set-local-pref 5;
                        }
                        set-community {
                            config {
                                method REFERENCE;
                                options ADD;
                            }
                            reference {
                                config {
                                    community-set-refs [ add_comm_60 add_comm_70 ];
                                }
                            }
                        }
                    }
                }
            }
            statement match_comm_and_prefix_add_2_community_sets_V6 {
                config {
                    name match_comm_and_prefix_add_2_community_sets_V6;
                }
                conditions {
                    match-prefix-set {
                        config {
                            prefix-set prefix-set-5_V6;
                            match-set-options ANY;
                        }
                    }
                    openconfig-bgp-policy:bgp-conditions {
                        match-community-set {
                            config {
                                community-set my_community;
                                match-set-options ANY;
                            }
                        }
                    }
                }
                actions {
                    config {
                        policy-result NEXT_STATEMENT;
                    }
                    openconfig-bgp-policy:bgp-actions {
                        config {
                            set-local-pref 5;
                        }
                        set-community {
                            config {
                                method REFERENCE;
                                options ADD;
                            }
                            reference {
                                config {
                                    community-set-refs [ add_comm_60 add_comm_70 ];
                                }
                            }
                        }
                    }
                }
            }
            statement match_aspath_set_med {
                config {
                    name match_aspath_set_med;
                }
                actions {
                    config {
                        policy-result ACCEPT_ROUTE;
                    }
                    openconfig-bgp-policy:bgp-actions {
                        config {
                            set-med 100;
                        }
                    }
                }
            }
        }
    }

[dplore]

Hi @r4huluk , If the route being evaluated in POLICY-1 has a community matching the first (and only) statement, POLICY-1 evaluation POLICY-1 should return TRUE.

If the route being evaluated in POLICY-1 does NOT have a community matching POLICY-1, then POLICY-1 should return FALSE.

[r4huluk]

Hi @dplore, Policy evaluation returns TRUE when the route is Accepted by an ACCEPT policy action in one of the statements. Otherwise, the result is FALSE. IMO, it is the ACCEPT action that makes the policy evaluation engine return result as TRUE. A route may match the condition specified in the statement, but it only enables the policy evaluation engine to execute the action specified for that statement.

In the above example, it wouldn't be appropriate for POLICY-1 to return TRUE just because the conditions in the first (and only) statement matched as the intent of the policy was to not ACCEPT the route but to move on to the next statement (or policy if there was a chain).

IMO, a change is needed in test script to not use a policy with a last statement having RoutingPolicy_PolicyResultType_NEXT_STATEMENT as nested policy. Nested policy can be a different policy with same statements as POLICY-1 and an ACCEPT policy action in the first (and only) statement instead of NEXT_STATEMENT

[dplore]

Hi, you are correct. We should revise the README and code to end in ACCEPT_ROUTE.

Ref: https://github.com/openconfig/public/blob/2c81874b3199f35a0e34b90bef91a74308e0b29b/release/models/policy/openconfig-routing-policy.yang#L790-L810