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

feat: XE to OC QoS #271

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

feat: XE to OC QoS #271

wants to merge 13 commits into from

Conversation

jrouliez
Copy link
Contributor

New feat: XE to OC QoS

@jrouliez jrouliez changed the title XE to OC QoS feat: XE to OC QoS Feb 12, 2024
@@ -40,6 +40,11 @@ def check_xe_features(oc_self, nso_props) -> None:
# OpenConfig System
xe_system_program_service(oc_self, nso_props)

<<<<<<< HEAD

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# OpenConfig QoS
if nso_props.service.oc_qos__qos:
xe_qos_program_service(oc_self, nso_props)
=======

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

@jrouliez jrouliez Feb 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to remove this?
# OpenConfig QoS
if nso_props.service.oc_qos__qos:
xe_qos_program_service(oc_self, nso_props)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just line 47

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -78,3 +83,4 @@ def clean_xe_cdb(nso_props) -> None:
device.ios__ntp.trusted_key.delete()
device.ios__router.bgp.delete()
device.ios__router.ospf.delete()
>>>>>>> main

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@stmosher
Copy link

Please remove the "env" folder

@@ -0,0 +1,280 @@
# -*- mode: python; python-indent: 4 -*-
from translation.openconfig_xe.common import xe_system_get_interface_ip_address

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xe_system_get_interface_ip_address isn't used in the file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@stmosher
Copy link

Add lines to xr_main.py and nx_main.py to raise errors if OC QoS configurations are sent for XR and NX devices.

"openconfig-qos:conditions": {
"openconfig-qos:ipv4": {
"openconfig-qos:config": {
"openconfig-qos:dscp": modify_dscp(class_map["match"]["ip"]["dscp"]),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function modify_dscp returns a list. "openconfig-qos:dscp" should be a string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

"openconfig-qos:conditions": {
"openconfig-qos:ipv4": {
"openconfig-qos:config": {
"openconfig-qos:dscp": modify_dscp(class_map["match"]["dscp"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function modify_dscp returns a list. "openconfig-qos:dscp" should be a string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

device_cdb.ios__class_map.create(c_map.name)
for t_map in c_map.terms.term:
pmap_cmap[t_map.actions.config.target_group] = c_map.name
# Configure "match ip dscp"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ip dscp" or "dscp" (for IPv4 and IPv6) should be based on the c_map.type. The c_map.type will determine the availability of "t_map.conditions.[ ipv4, ipv6, etc...]"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean:
if type: ETHERNET ---> use 'dscp'
if type: IPv4 ---> use 'ip dscp'
or something else?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this code:
for c_map in nso_props.service.oc_qos__qos.classifiers.classifier:
pmap_cmap = {}
# Configure class-map
if device_cdb.ios__class_map.exists(c_map.name) and c_map.name != 'class-default':
del device_cdb.ios__class_map[c_map.name]
elif c_map.name == 'class-default':
for t_map in c_map.terms.term:
pmap_cmap[t_map.actions.config.target_group] = c_map.name
else:
device_cdb.ios__class_map.create(c_map.name)
for t_map in c_map.terms.term:
pmap_cmap[t_map.actions.config.target_group] = c_map.name
# Configure "match ip dscp"
if t_map.conditions.ipv4.config.protocol == 4:
# Configure multiple dscp statements
if t_map.conditions.ipv4.config.dscp_set:

The code is already looking into IPv4 terms (if t_map.conditions.ipv4.config.protocol == 4:) to determine if the term is using IPv4, but the term could be IPv6. For instance if c_map.type == "IPv6" (or something like that), then the term would be t_map.conditions.ipv6....

     Added QoS checks on NX-OS and IOS-XR.
     Fixed modify_dscp function.
if "cir" in cd_new_police:
cir = cd_new_police["cir"]

return cir

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cir needs to be an integer

if "bc" in cd_new_police:
bc = cd_new_police["bc"]

return bc

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bc needs to be an integer

if "pir" in cd_new_police:
pir = cd_new_police["pir"]

return pir

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pir needs to be an integer

if "pir-be" in cd_new_police and "be" in cd_new_police["pir-be"]:
be = cd_new_police["pir-be"]["be"]

return be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be needs to be an integer

if "cir" in cm_new_police:
cir = cm_new_police["cir"]

return cir

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cir needs to be an integer

if "bc" in cm_new_police:
bc = cm_new_police["bc"]

return bc

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bc needs to be an integer

if "pir" in cm_new_police:
pir = cm_new_police["pir"]

return pir

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pir needs to be an integer

if "pir-be" in cm_new_police and "be" in cm_new_police["pir-be"]:
be = cm_new_police["pir-be"]["be"]

return be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be needs to be an integer

},
"openconfig-qos:two-rate-three-color": {
"openconfig-qos:config": {
"openconfig-qos:cir": set_qos_cir_percent(cm_new_percent),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to cir-pct

"openconfig-qos:config": {
"openconfig-qos:cir": set_qos_cir_percent(cm_new_percent),
"openconfig-qos:bc": set_qos_bc_percent(cm_new_percent),
"openconfig-qos:pir": set_qos_pir_percent(cm_new_percent),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to pir-pct


openconfig_class_map.append({
"openconfig-qos:name": "class-default",
"openconfig-qos:config": {"openconfig-qos:name:": "class-default"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:name"

if len(class_map["match"]["ip"]["dscp"]) == 1:
openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:name"

openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"],
"openconfig-qos:type:": "IPV4"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:type"

openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"],
"openconfig-qos:type:": "IPV4"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:type"

elif len(class_map["match"]["ip"]["dscp"]) > 1:
openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:name"

if len(class_map["match"]["dscp"]) == 1:
openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:name"

elif len(class_map["match"]["dscp"]) > 1:
openconfig_class_map.append({
"openconfig-qos:name": class_map["name"],
"openconfig-qos:config": {"openconfig-qos:name:": class_map["name"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"openconfig-qos:name"

openconfig-qos:ipv4:
openconfig-qos:config:
openconfig-qos:dscp-set:
- '13'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 12 DSCP

openconfig-qos:config:
openconfig-qos:dscp-set:
- '13'
- '15'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 14 DSCP

},
"openconfig-qos:one-rate-two-color": {
"openconfig-qos:config": {
"openconfig-qos:cir": policy_map["class-default"]["class"][0]["priority"]["kilo-bits"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cir needs to be in bps

},
"openconfig-qos:one-rate-two-color": {
"openconfig-qos:config": {
"openconfig-qos:cir": class_name["priority"]["kilo-bits"],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cir needs to be in bps


# Openconfig QoS
if is_qos_configured(nso_props):
raise NotImplementedError('openconfig-qos has not yet been implemented for XR')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exception for NX

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened to xe_acls.py's code?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened to xr_acls.py's code?

return violate_percent, drop


def set_qos_interface(config_leftover, openconfig_interface, interface, interface_list, intf_to_sched_list, openconfig_scheduler):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only interfaces with QoS policies should be added here. This is adding all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants