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

Jump to bottom

Update CoPP design with new always_enabled field info #962

Merged
merged 1 commit into from
Mar 20, 2022
Merged

Update CoPP design with new always_enabled field info #962

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 39 additions & 26 deletions doc/copp/CoPP Config and Management.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ queue = number; strict queue priority. Higher number means higher priori
trap_action = packet_action; trap action which will be applied to all trap_ids for this group.
trap_priority = trap_priority

;Settings for embedded policer.
;Settings for embedded policer.
meter_type = "packets" | "bytes"
mode = "sr_tcm" | "tr_tcm" | "storm"
color = "aware" | "blind"
Expand All @@ -43,8 +43,8 @@ pir = number ;packets or bytes depending on the meter_type value
green_action = packet_action
yellow_action = packet_action
red_action = packet_action
genetlink_name = genetlink_name ;[Optional] "psample" for sFlow
genetlink_mcgrp_name = multicast group name; ;[Optional] "packets" for sFlow
genetlink_name = genetlink_name ;[Optional] "psample" for sFlow
genetlink_mcgrp_name = multicast group name; ;[Optional] "packets" for sFlow
```
```
key = "COPP_TRAP|name"
Expand All @@ -63,7 +63,18 @@ state = "ok"
```

### coppmgr
Introduce a *new* CoPP manager, that subscribes for the Config DB CoPP Tables and Feature Tables. Based on the feature enablement, ```coppmgr``` handles the logic to resolve whether a CoPP table shall be written to APP DB for orchagent consumption. Inorder to reduce changes to copporch and for backward compatibility during warmboot, ```coppmgr``` shall use the existing APP_DB schema and implement internal logic to convert the proposed ConfigDB entries to APP DB entries. Similar to existing swss managers, an entry with state "ok" shall be added to STATE_DB. `coppmgrd` must be started by [supervisord.conf](https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-orchagent/supervisord.conf) first, before any other process is started in swss. During init, ```coppmgr``` shall read the ```copp_cfg.json``` file and apply the default configuration. The default shall not be part of Config_DB entries. ```coppmgr``` shall also read from the config_db during init and apply the logic to merge if there are same entries within the ```copp_cfg.json``` file.
Introduce a *new* CoPP manager, that subscribes for the Config DB CoPP Tables and Feature Tables. Based on the feature enablement, ```coppmgr``` handles the logic to resolve whether a CoPP table shall be written to APP DB for orchagent consumption. Inorder to reduce changes to copporch and for backward compatibility during warmboot, ```coppmgr``` shall use the existing APP_DB schema and implement internal logic to convert the proposed ConfigDB entries to APP DB entries. Similar to existing swss managers, an entry with state "ok" shall be added to STATE_DB. `coppmgrd` must be started by [supervisord.conf](https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-orchagent/supervisord.conf) first, before any other process is started in swss. During init, ```coppmgr``` shall read the ```copp_cfg.json``` file and apply the default configuration. The default shall not be part of Config_DB entries. ```coppmgr``` shall also read from the config_db during init and apply the logic to merge if there are same entries within the ```copp_cfg.json``` file.

Trap name added to copp_cfg.json file has to match a feature name exist in FEATURE table in Config DB.
In order to handle traps which has no associated feature (such as arp, ip2me), a new field called "always_enabled" will be added to COPP_TRAP table in Config DB.

With the new "always_enabled" field, coppmgr will determine if a trap should be installed.

- If a trap has "always_enabled":"true" field, install it.

- If the associated feature is enabled, install the trap.

Now, traps which have no associated feature, will be installed only if "always_enabled" field value is "true".

### copporch
```copporch``` shall only be a consumer of APP DB CoPP Table. It is not expected to handle feature logic and the current handling of features like sFlow, NAT shall be revisited and removed to be added as part of ```coppmgr```. However `copporch` must be able to handle any new trap_id getting added or removed from an existing CoPP table, and handle attribute value set for a trap group
Expand All @@ -75,7 +86,7 @@ Handling of CoPP config json file shall be removed from ```dockers/docker-orchag
This file shall be modified to be compatible to Config DB schema, currently placed at ```swssconfig/sample/00-copp.config.json```. It shall be renamed to ```copp_cfg.j2``` and moved to ```files/image_config/copp/copp_cfg.j2```

### copp_cfg.json
A new file ```copp_cfg.json``` shall be introduced to include default CoPP tables from copp_cfg.j2 placed under ```/etc/sonic```. This file shall be read by ```coppmgr``` during initialization.
A new file ```copp_cfg.json``` shall be introduced to include default CoPP tables from copp_cfg.j2 placed under ```/etc/sonic```. This file shall be read by ```coppmgr``` during initialization.

### Default CoPP Tables
There are two proposals for loading the default CoPP Tables.
Expand All @@ -99,22 +110,22 @@ However, this approach has the following limitations:
In warmboot scenarios, as init_cfg.json is not read during warmboot and may result in ***new*** CoPP Tables in the "to" image not getting applied.
In coldboot, if user saves the config and the new release happened to have new CoPP values, it will not be applied since previous default values are present in config_db

2. ```coppmgr``` reading directly from ```copp_cfg.json``` and apply the default CoPP Tables.
2. ```coppmgr``` reading directly from ```copp_cfg.json``` and apply the default CoPP Tables.

For this design, considering the limitations, proposal is to go ahead with second option.
For this design, considering the limitations, proposal is to go ahead with second option.

## Warmboot and Backward Compatibility
It is desirable to have warmboot functionality from previous release versions of Sonic. Since the existing schema has COPP Group name/key with protocol names (e.g `"COPP_TABLE:trap.group.bgp.lacp"`, there is a limitation in adding any new protocol or trap to an existing CoPP group and seamlessly migrate. However, with this proposal, the following options are considered:

1. The implementation is to do a migration of APP DB entries to new schema.
2. Let ```db_migrator``` remove all previously saved APP_DB entries and let syncd reconcile and remove the TRAP entries during warmboot. Later, ```coppmgr``` can reapply the traps afresh. Also remove COPP_TABLES from being saved in [backup_database](https://github.com/Azure/sonic-utilities/blob/master/scripts/fast-reboot#L234). This may require additional tests to confirm system behaviour during warmboot.
2. Let ```db_migrator``` remove all previously saved APP_DB entries and let syncd reconcile and remove the TRAP entries during warmboot. Later, ```coppmgr``` can reapply the traps afresh. Also remove COPP_TABLES from being saved in [backup_database](https://github.com/Azure/sonic-utilities/blob/master/scripts/fast-reboot#L234). This may require additional tests to confirm system behaviour during warmboot.
3. Remove '00-copp.config.json' from being included for checksum calculation in ```files/build_scripts/generate_asic_config_checksum.py```

In addition, the implementation must ensure that backward compatibility is maintained. If the system is boot-up with an *old* config file, the default CoPP tables are to be loaded from ```copp_cfg.json``` and expected to work seamlessly.

## Limitations
1. In case of downgrade, the config_db entries shall be present as stale entries as there is no subscribers for the table. Functionality would be same as supported by the downgraded version
2. This proposal expects the table names to be consistent across multiple releases.
2. This proposal expects the table names to be consistent across multiple releases.
3. User is expected to resolve any conflicts, say for a trap id or group, that arises due to default values from the Sonic binary vs same trap or group currently or previously configured by the user.

## CLI
Expand All @@ -124,7 +135,7 @@ CLI support to add/modify CoPP tables is not scoped as part of this design. In f

1. User shall be able to change policer values for a queue
2. User shall be able to change the queue for a protocol/trap
3. User shall be able to delete a group/trap.
3. User shall be able to delete a group/trap.
*In the current proposal, this would mean to keep the key in config_db with empty attributes.

# Flows
Expand All @@ -139,7 +150,7 @@ The following flow captures scenarios for ```boot-up``` sequence and ```config r

## Copp Manager flow

The following flow captures CoPP manager functionality.
The following flow captures CoPP manager functionality.

![](https://github.com/Azure/SONiC/blob/master/images/copp/CoppManager_1.png)

Expand All @@ -162,13 +173,13 @@ The following flow captures CoPP manager functionality.
"trap_action":"trap",
"trap_priority":"4",
},

"COPP_GROUP|queue4_group2": {
"queue": "4",
"trap_action":"trap",
"trap_priority":"4",
},

"COPP_GROUP|queue4_group3": {
"queue": "4",
"trap_action":"copy",
Expand All @@ -189,10 +200,11 @@ The following flow captures CoPP manager functionality.
"trap_ids": "lldp",
"trap_group": "queue4_group2"
},

"COPP_TRAP|arp": {
"trap_ids": "arp_req,arp_resp,neigh_discovery",
"trap_group": "queue4_group3"
"trap_group": "queue4_group3",
"always_enabled": "true"
},

"COPP_GROUP|queue1_group1": {
Expand All @@ -205,17 +217,18 @@ The following flow captures CoPP manager functionality.
"cbs":"6000",
"red_action":"drop"
},

"COPP_TRAP|ip2me": {
"trap_ids": "ip2me",
"trap_group": "queue1_group1"
"trap_group": "queue1_group1",
"always_enabled": "true"
},

"COPP_TRAP|nat": {
"trap_ids": "src_nat_miss,dest_nat_miss",
"trap_group": "queue1_group1"
},

"COPP_GROUP|queue2_group1": {
"queue": "2",
"trap_action":"trap",
Expand All @@ -236,7 +249,7 @@ The following flow captures CoPP manager functionality.
}
```

*queue4_group2 is added for backward compatibility. Refer "existing" APP_DB entries below
*queue4_group2 is added for backward compatibility. Refer "existing" APP_DB entries below

### APP DB

Expand All @@ -249,14 +262,14 @@ The following sample APP DB entries shall be created by `coppmgr` by merging the
"trap_action":"trap",
"trap_priority":"4",
},

"COPP_TABLE:queue4_group2": {
"trap_ids": "lldp",
"queue": "4",
"trap_action":"trap",
"trap_priority":"4",
},

"COPP_TABLE:queue4_group3": {
"trap_ids": "arp_req,arp_resp,neigh_discovery",
"queue": "4",
Expand All @@ -267,11 +280,11 @@ The following sample APP DB entries shall be created by `coppmgr` by merging the
"cir":"600",
"cbs":"600",
"red_action":"drop"
}
}
```

### APP DB (Existing in swss, For reference purpose only)

```
[
{
Expand Down Expand Up @@ -333,4 +346,4 @@ The following sample APP DB entries shall be created by `coppmgr` by merging the
}
]
```