Attention is currently required from: jolly.
14 comments:
Patchset:
TODO: figure out if it makes sense to provide a default qos, and with with ip-dscp, etc.
File include/osmocom/sigtran/osmo_ss7.h:
Patch Set #1, Line 155: * QoS Class
Better create a new private ss7_qos_class.h + ss7_qos_class.c inside src/, and avoid prefixing it with osmo_ to avoid making them public until there's a real need for them to be public. This way we can extend without breaking the API.
Patch Set #1, Line 167: osmo_ss7_qos_class_find(struct osmo_ss7_instance *inst, uint8_t qos_class);
This one can go in ss7_instance.h + ss7_instance.c
File src/Makefile.am:
Patch Set #1, Line 92: ss7_qos_class_vty.c \
good, you need to do the same with ss7_qos_class.c and .h.
File src/ss7_asp_vty.c:
Patch Set #1, Line 414: "qos-class " IP_QOS_CLASS_RANGE_STR,
SS7_QOS_CLASS_RANGE_STR
Patch Set #1, Line 423: asp->cfg.ip_dscp = 0;
why do you keep asp->cfg.ip_dscp? IMHO it should be removed now that we have the qos-class stuff.
Patch Set #1, Line 443: DEFUN_ATTR(asp_ip_dscps, asp_ip_dscp_cmd,
This should be deprecated in some way imho.
File src/ss7_instance.c:
Patch Set #1, Line 363: llist_for_each_entry(xua, &inst->xua_servers, list) {
Cisco ITP doesn't have qos-class node for "listen" node, but I think it's actually a good idea to use it here.
Patch Set #1, Line 392: void osmo_ss7_qos_class_update(struct osmo_ss7_qos_class *qos)
This can go into ss7_qos_class.c too. btw I don't recall seeing it declared in the header file?
File src/ss7_qos_class_vty.c:
Patch Set #1, Line 112: if (qos->ip_dscp) {
I think is "qos" exists it should always print the "qos class" node, even if empty.
Patch Set #1, Line 114: if (qos->ip_dscp)
this is always true in this path?
File src/ss7_vty.c:
Patch Set #1, Line 1409: /* then dump ASPs, as ASs reference them */
I think you can simplify and drop all the connectors "first, then, now, finally" ;)
Patch Set #1, Line 1442: return ss7_vty_node_qos_class_go_parent(vty);
Are you sure the func is needed? or simply apply it directly here like others?
remove this empty line.
To view, visit change 43667. To unsubscribe, or for help writing mail filters, visit settings.