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

Remove suppress-fib-pending CLI and make route_check.py check suppress-fib in BGP configuration #3331

Merged
merged 6 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
12 changes: 0 additions & 12 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -2214,18 +2214,6 @@ def synchronous_mode(sync_mode):
config reload -y \n
Option 2. systemctl restart swss""" % sync_mode)

#
# 'suppress-fib-pending' command ('config suppress-fib-pending ...')
#
@config.command('suppress-fib-pending')
@click.argument('state', metavar='<enabled|disabled>', required=True, type=click.Choice(['enabled', 'disabled']))
@clicommon.pass_db
def suppress_pending_fib(db, state):
''' Enable or disable pending FIB suppression. Once enabled, BGP will not advertise routes that are not yet installed in the hardware '''

config_db = db.cfgdb
config_db.mod_entry('DEVICE_METADATA' , 'localhost', {"suppress-fib-pending" : state})

#
# 'yang_config_validation' command ('config yang_config_validation ...')
#
Expand Down
38 changes: 0 additions & 38 deletions doc/Command-Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2610,26 +2610,6 @@ This command displays the routing policy that takes precedence over the other ro
Exit routemap
```

**show suppress-fib-pending**

This command is used to show the status of suppress pending FIB feature.
When enabled, BGP will not advertise routes which aren't yet offloaded.

- Usage:
```
show suppress-fib-pending
```

- Examples:
```
admin@sonic:~$ show suppress-fib-pending
Enabled
```
```
admin@sonic:~$ show suppress-fib-pending
Disabled
```

Go Back To [Beginning of the document](#) or [Beginning of this section](#bgp)

### BGP config commands
Expand Down Expand Up @@ -2722,24 +2702,6 @@ This command is used to remove particular IPv4 or IPv6 BGP neighbor configuratio
admin@sonic:~$ sudo config bgp remove neighbor SONIC02SPINE
```

**config suppress-fib-pending**

This command is used to enable or disable announcements of routes not yet installed in the HW.
Once enabled, BGP will not advertise routes which aren't yet offloaded.

- Usage:
```
config suppress-fib-pending <enabled|disabled>
```

- Examples:
```
admin@sonic:~$ sudo config suppress-fib-pending enabled
```
```
admin@sonic:~$ sudo config suppress-fib-pending disabled
```

Go Back To [Beginning of the document](#) or [Beginning of this section](#bgp)

## Console
Expand Down
32 changes: 22 additions & 10 deletions scripts/route_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,16 @@ def get_asicdb_routes(namespace):
return (selector, subs, sorted(rt))


def is_bgp_suppress_fib_pending_enabled(namespace):
"""
Retruns True if FIB suppression is enabled in BGP config, False otherwise
"""
show_run_cmd = ['show', 'runningconfiguration', 'bgp', '-n', namespace]

output = subprocess.check_output(show_run_cmd, text=True)
return 'bgp suppress-fib-pending' in output


def is_suppress_fib_pending_enabled(namespace):
"""
Returns True if FIB suppression is enabled, False otherwise
Expand Down Expand Up @@ -781,18 +791,20 @@ def check_routes(namespace):
results[namespace] = {}
results[namespace]["Unaccounted_ROUTE_ENTRY_TABLE_entries"] = rt_asic_miss

rt_frr_miss = check_frr_pending_routes(namespace)
if is_bgp_suppress_fib_pending_enabled(namespace):
rt_frr_miss = check_frr_pending_routes(namespace)

if rt_frr_miss:
if namespace not in results:
results[namespace] = {}
results[namespace]["missed_FRR_routes"] = rt_frr_miss
if rt_frr_miss:
if namespace not in results:
results[namespace] = {}
results[namespace]["missed_FRR_routes"] = rt_frr_miss

if results:
if rt_frr_miss and not rt_appl_miss and not rt_asic_miss:
print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} but all routes in APPL_DB and ASIC_DB are in sync".format(namespace))
if is_suppress_fib_pending_enabled(namespace):
mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl)
if results:
if rt_frr_miss and not rt_appl_miss and not rt_asic_miss:
print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} but all "
"routes in APPL_DB and ASIC_DB are in sync".format(namespace))
if is_suppress_fib_pending_enabled(namespace):
mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl)

if results:
print_message(syslog.LOG_WARNING, "Failure results: {", json.dumps(results, indent=4), "}")
Expand Down
19 changes: 4 additions & 15 deletions show/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def get_config_json_by_namespace(namespace):
iface_alias_converter = lazy_object_proxy.Proxy(lambda: clicommon.InterfaceAliasConverter())

#
# Display all storm-control data
# Display all storm-control data
#
def display_storm_all():
""" Show storm-control """
Expand Down Expand Up @@ -462,7 +462,7 @@ def is_mgmt_vrf_enabled(ctx):
return False

#
# 'storm-control' group
# 'storm-control' group
# "show storm-control [interface <interface>]"
#
@cli.group('storm-control', invoke_without_command=True)
Expand Down Expand Up @@ -2108,7 +2108,7 @@ def summary(db):
key_values = key.split('|')
values = db.db.get_all(db.db.STATE_DB, key)
if "local_discriminator" not in values.keys():
values["local_discriminator"] = "NA"
values["local_discriminator"] = "NA"
bfd_body.append([key_values[3], key_values[2], key_values[1], values["state"], values["type"], values["local_addr"],
values["tx_interval"], values["rx_interval"], values["multiplier"], values["multihop"], values["local_discriminator"]])

Expand Down Expand Up @@ -2139,24 +2139,13 @@ def peer(db, peer_ip):
key_values = key.split(delimiter)
values = db.db.get_all(db.db.STATE_DB, key)
if "local_discriminator" not in values.keys():
values["local_discriminator"] = "NA"
values["local_discriminator"] = "NA"
bfd_body.append([key_values[3], key_values[2], key_values[1], values.get("state"), values.get("type"), values.get("local_addr"),
values.get("tx_interval"), values.get("rx_interval"), values.get("multiplier"), values.get("multihop"), values.get("local_discriminator")])

click.echo(tabulate(bfd_body, bfd_headers))


# 'suppress-fib-pending' subcommand ("show suppress-fib-pending")
@cli.command('suppress-fib-pending')
@clicommon.pass_db
def suppress_pending_fib(db):
""" Show the status of suppress pending FIB feature """

field_values = db.cfgdb.get_entry('DEVICE_METADATA', 'localhost')
state = field_values.get('suppress-fib-pending', 'disabled').title()
click.echo(state)


# asic-sdk-health-event subcommand ("show asic-sdk-health-event")
@cli.group(cls=clicommon.AliasedGroup)
def asic_sdk_health_event():
Expand Down
7 changes: 5 additions & 2 deletions tests/route_check_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,11 @@ def run_test(self, ct_data):

def mock_check_output(self, ct_data, *args, **kwargs):
ns = self.extract_namespace_from_args(args[0])
routes = ct_data.get(FRR_ROUTES, {}).get(ns, {})
return json.dumps(routes)
if 'show runningconfiguration bgp' in ' '.join(args[0]):
return 'bgp suppress-fib-pending'
else:
routes = ct_data.get(FRR_ROUTES, {}).get(ns, {})
return json.dumps(routes)

def assert_results(self, ct_data, ret, res):
expect_ret = ct_data.get(RET, 0)
Expand Down
34 changes: 0 additions & 34 deletions tests/suppress_pending_fib_test.py

This file was deleted.

Loading