currently there are 2 QoS pipelines for ingress (ls_in_qos_mark, ls_in_qos_meter) and egress (ls_out_qos_mark, ls_out_qos_meter). This is not necessary as there are no actions across the two pipelines that depend on each other. The two pipelines can be merged.
https://issues.redhat.com/browse/FDP-397 Signed-off-by: Jacob Tanenbaum <jtane...@redhat.com> --- northd/northd.c | 65 +++++++++++++++++---------------------------- northd/northd.h | 46 +++++++++++++++----------------- tests/ovn-northd.at | 24 ++++++++--------- tests/ovn.at | 9 +++---- 4 files changed, 62 insertions(+), 82 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index 1839b7d8b..d5aab756f 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -3484,7 +3484,7 @@ build_lb_vip_actions(const struct ovn_northd_lb *lb, } if (reject) { - int stage = ls_dp ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK) + int stage = ls_dp ? ovn_stage_get_table(S_SWITCH_OUT_QOS) : ovn_stage_get_table(S_ROUTER_OUT_SNAT); ds_clear(action); ds_put_format(action, "reg0 = 0; reject { outport <-> inport; " @@ -6705,7 +6705,7 @@ build_acl_action_lflows(const struct ls_stateful_record *ls_stateful_rec, "/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit. */ " "outport <-> inport; next(pipeline=%s,table=%d); };", ingress ? "egress" : "ingress", - ingress ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK) + ingress ? ovn_stage_get_table(S_SWITCH_OUT_QOS) : ovn_stage_get_table(S_SWITCH_IN_L2_LKUP)); ovn_lflow_metered(lflows, od, stage, 1000, @@ -7081,24 +7081,39 @@ build_qos(struct ovn_datapath *od, struct lflow_table *lflows, struct lflow_ref *lflow_ref) { struct ds action = DS_EMPTY_INITIALIZER; - ovn_lflow_add(lflows, od, S_SWITCH_IN_QOS_MARK, 0, "1", "next;", + ovn_lflow_add(lflows, od, S_SWITCH_IN_QOS, 0, "1", "next;", lflow_ref); - ovn_lflow_add(lflows, od, S_SWITCH_OUT_QOS_MARK, 0, "1", "next;", - lflow_ref); - ovn_lflow_add(lflows, od, S_SWITCH_IN_QOS_METER, 0, "1", "next;", - lflow_ref); - ovn_lflow_add(lflows, od, S_SWITCH_OUT_QOS_METER, 0, "1", "next;", + ovn_lflow_add(lflows, od, S_SWITCH_OUT_QOS, 0, "1", "next;", lflow_ref); for (size_t i = 0; i < od->nbs->n_qos_rules; i++) { struct nbrec_qos *qos = od->nbs->qos_rules[i]; bool ingress = !strcmp(qos->direction, "from-lport") ? true :false; - enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS_MARK : S_SWITCH_OUT_QOS_MARK; + enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS : S_SWITCH_OUT_QOS; static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); int64_t rate = 0; int64_t burst = 0; ds_clear(&action); + for (size_t n = 0; n < qos->n_bandwidth; n++) { + if (!strcmp(qos->key_bandwidth[n], "rate")) { + rate = qos->value_bandwidth[n]; + } else if (!strcmp(qos->key_bandwidth[n], "burst")) { + burst = qos->value_bandwidth[n]; + } + } + if (rate) { + stage = ingress ? S_SWITCH_IN_QOS : S_SWITCH_OUT_QOS; + if (burst) { + ds_put_format(&action, + "set_meter(%"PRId64", %"PRId64"); ", + rate, burst); + } else { + ds_put_format(&action, + "set_meter(%"PRId64"); ", + rate); + } + } for (size_t j = 0; j < qos->n_action; j++) { if (!strcmp(qos->key_action[j], "dscp")) { if (qos->value_action[j] > QOS_MAX_DSCP) { @@ -7115,43 +7130,11 @@ build_qos(struct ovn_datapath *od, struct lflow_table *lflows, qos->value_action[j]); } } - - if (action.length) { ds_put_cstr(&action, "next;"); - ovn_lflow_add_with_hint(lflows, od, stage, qos->priority, - qos->match, ds_cstr(&action), - &qos->header_, lflow_ref); - } - - for (size_t n = 0; n < qos->n_bandwidth; n++) { - if (!strcmp(qos->key_bandwidth[n], "rate")) { - rate = qos->value_bandwidth[n]; - } else if (!strcmp(qos->key_bandwidth[n], "burst")) { - burst = qos->value_bandwidth[n]; - } - } - if (rate) { - stage = ingress ? S_SWITCH_IN_QOS_METER : S_SWITCH_OUT_QOS_METER; - ds_clear(&action); - if (burst) { - ds_put_format(&action, - "set_meter(%"PRId64", %"PRId64"); next;", - rate, burst); - } else { - ds_put_format(&action, - "set_meter(%"PRId64"); next;", - rate); - } - - /* Ingress and Egress QoS Meter Table. - * - * We limit the bandwidth of this flow by adding a meter table. - */ ovn_lflow_add_with_hint(lflows, od, stage, qos->priority, qos->match, ds_cstr(&action), &qos->header_, lflow_ref); - } } ds_destroy(&action); } diff --git a/northd/northd.h b/northd/northd.h index 3f1cd8341..d5161d17e 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -397,27 +397,26 @@ enum ovn_stage { PIPELINE_STAGE(SWITCH, IN, ACL_HINT, 7, "ls_in_acl_hint") \ PIPELINE_STAGE(SWITCH, IN, ACL_EVAL, 8, "ls_in_acl_eval") \ PIPELINE_STAGE(SWITCH, IN, ACL_ACTION, 9, "ls_in_acl_action") \ - PIPELINE_STAGE(SWITCH, IN, QOS_MARK, 10, "ls_in_qos_mark") \ - PIPELINE_STAGE(SWITCH, IN, QOS_METER, 11, "ls_in_qos_meter") \ - PIPELINE_STAGE(SWITCH, IN, LB_AFF_CHECK, 12, "ls_in_lb_aff_check") \ - PIPELINE_STAGE(SWITCH, IN, LB, 13, "ls_in_lb") \ - PIPELINE_STAGE(SWITCH, IN, LB_AFF_LEARN, 14, "ls_in_lb_aff_learn") \ - PIPELINE_STAGE(SWITCH, IN, PRE_HAIRPIN, 15, "ls_in_pre_hairpin") \ - PIPELINE_STAGE(SWITCH, IN, NAT_HAIRPIN, 16, "ls_in_nat_hairpin") \ - PIPELINE_STAGE(SWITCH, IN, HAIRPIN, 17, "ls_in_hairpin") \ - PIPELINE_STAGE(SWITCH, IN, ACL_AFTER_LB_EVAL, 18, \ + PIPELINE_STAGE(SWITCH, IN, QOS, 10, "ls_in_qos") \ + PIPELINE_STAGE(SWITCH, IN, LB_AFF_CHECK, 11, "ls_in_lb_aff_check") \ + PIPELINE_STAGE(SWITCH, IN, LB, 12, "ls_in_lb") \ + PIPELINE_STAGE(SWITCH, IN, LB_AFF_LEARN, 13, "ls_in_lb_aff_learn") \ + PIPELINE_STAGE(SWITCH, IN, PRE_HAIRPIN, 14, "ls_in_pre_hairpin") \ + PIPELINE_STAGE(SWITCH, IN, NAT_HAIRPIN, 15, "ls_in_nat_hairpin") \ + PIPELINE_STAGE(SWITCH, IN, HAIRPIN, 16, "ls_in_hairpin") \ + PIPELINE_STAGE(SWITCH, IN, ACL_AFTER_LB_EVAL, 17, \ "ls_in_acl_after_lb_eval") \ - PIPELINE_STAGE(SWITCH, IN, ACL_AFTER_LB_ACTION, 19, \ + PIPELINE_STAGE(SWITCH, IN, ACL_AFTER_LB_ACTION, 18, \ "ls_in_acl_after_lb_action") \ - PIPELINE_STAGE(SWITCH, IN, STATEFUL, 20, "ls_in_stateful") \ - PIPELINE_STAGE(SWITCH, IN, ARP_ND_RSP, 21, "ls_in_arp_rsp") \ - PIPELINE_STAGE(SWITCH, IN, DHCP_OPTIONS, 22, "ls_in_dhcp_options") \ - PIPELINE_STAGE(SWITCH, IN, DHCP_RESPONSE, 23, "ls_in_dhcp_response") \ - PIPELINE_STAGE(SWITCH, IN, DNS_LOOKUP, 24, "ls_in_dns_lookup") \ - PIPELINE_STAGE(SWITCH, IN, DNS_RESPONSE, 25, "ls_in_dns_response") \ - PIPELINE_STAGE(SWITCH, IN, EXTERNAL_PORT, 26, "ls_in_external_port") \ - PIPELINE_STAGE(SWITCH, IN, L2_LKUP, 27, "ls_in_l2_lkup") \ - PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 28, "ls_in_l2_unknown") \ + PIPELINE_STAGE(SWITCH, IN, STATEFUL, 19, "ls_in_stateful") \ + PIPELINE_STAGE(SWITCH, IN, ARP_ND_RSP, 20, "ls_in_arp_rsp") \ + PIPELINE_STAGE(SWITCH, IN, DHCP_OPTIONS, 21, "ls_in_dhcp_options") \ + PIPELINE_STAGE(SWITCH, IN, DHCP_RESPONSE, 22, "ls_in_dhcp_response") \ + PIPELINE_STAGE(SWITCH, IN, DNS_LOOKUP, 23, "ls_in_dns_lookup") \ + PIPELINE_STAGE(SWITCH, IN, DNS_RESPONSE, 24, "ls_in_dns_response") \ + PIPELINE_STAGE(SWITCH, IN, EXTERNAL_PORT, 25, "ls_in_external_port") \ + PIPELINE_STAGE(SWITCH, IN, L2_LKUP, 26, "ls_in_l2_lkup") \ + PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 27, "ls_in_l2_unknown") \ \ /* Logical switch egress stages. */ \ PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl") \ @@ -426,11 +425,10 @@ enum ovn_stage { PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint") \ PIPELINE_STAGE(SWITCH, OUT, ACL_EVAL, 4, "ls_out_acl_eval") \ PIPELINE_STAGE(SWITCH, OUT, ACL_ACTION, 5, "ls_out_acl_action") \ - PIPELINE_STAGE(SWITCH, OUT, QOS_MARK, 6, "ls_out_qos_mark") \ - PIPELINE_STAGE(SWITCH, OUT, QOS_METER, 7, "ls_out_qos_meter") \ - PIPELINE_STAGE(SWITCH, OUT, STATEFUL, 8, "ls_out_stateful") \ - PIPELINE_STAGE(SWITCH, OUT, CHECK_PORT_SEC, 9, "ls_out_check_port_sec") \ - PIPELINE_STAGE(SWITCH, OUT, APPLY_PORT_SEC, 10, "ls_out_apply_port_sec") \ + PIPELINE_STAGE(SWITCH, OUT, QOS, 6, "ls_out_qos") \ + PIPELINE_STAGE(SWITCH, OUT, STATEFUL, 7, "ls_out_stateful") \ + PIPELINE_STAGE(SWITCH, OUT, CHECK_PORT_SEC, 8, "ls_out_check_port_sec") \ + PIPELINE_STAGE(SWITCH, OUT, APPLY_PORT_SEC, 9, "ls_out_apply_port_sec") \ \ /* Logical router ingress stages. */ \ PIPELINE_STAGE(ROUTER, IN, ADMISSION, 0, "lr_in_admission") \ diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 7893b0540..87c7e49a6 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -10080,6 +10080,7 @@ check ovn-nbctl qos-add ls from-lport 100 ip4 rate=100 burst=1000 check ovn-nbctl qos-add ls from-lport 101 ip4 mark=15 check ovn-nbctl qos-add ls from-lport 102 ip4 dscp=16 check ovn-nbctl qos-add ls from-lport 103 ip4 dscp=17 mark=18 +check ovn-nbctl qos-add ls from-lport 104 ip4 dscp=18 mark=19 rate=100 check ovn-nbctl qos-add ls to-lport 100 ip4 rate=100 burst=1000 check ovn-nbctl qos-add ls to-lport 101 ip4 mark=15 @@ -10088,18 +10089,17 @@ check ovn-nbctl qos-add ls to-lport 103 ip4 dscp=17 mark=18 check ovn-nbctl --wait=sb sync AT_CHECK([ovn-sbctl lflow-list ls | grep qos | ovn_strip_lflows], [0], [dnl - table=??(ls_in_qos_mark ), priority=0 , match=(1), action=(next;) - table=??(ls_in_qos_mark ), priority=101 , match=(ip4), action=(pkt.mark = 15; next;) - table=??(ls_in_qos_mark ), priority=102 , match=(ip4), action=(ip.dscp = 16; next;) - table=??(ls_in_qos_mark ), priority=103 , match=(ip4), action=(ip.dscp = 17; pkt.mark = 18; next;) - table=??(ls_in_qos_meter ), priority=0 , match=(1), action=(next;) - table=??(ls_in_qos_meter ), priority=100 , match=(ip4), action=(set_meter(100, 1000); next;) - table=??(ls_out_qos_mark ), priority=0 , match=(1), action=(next;) - table=??(ls_out_qos_mark ), priority=101 , match=(ip4), action=(pkt.mark = 15; next;) - table=??(ls_out_qos_mark ), priority=102 , match=(ip4), action=(ip.dscp = 16; next;) - table=??(ls_out_qos_mark ), priority=103 , match=(ip4), action=(ip.dscp = 17; pkt.mark = 18; next;) - table=??(ls_out_qos_meter ), priority=0 , match=(1), action=(next;) - table=??(ls_out_qos_meter ), priority=100 , match=(ip4), action=(set_meter(100, 1000); next;) + table=??(ls_in_qos ), priority=0 , match=(1), action=(next;) + table=??(ls_in_qos ), priority=100 , match=(ip4), action=(set_meter(100, 1000); next;) + table=??(ls_in_qos ), priority=101 , match=(ip4), action=(pkt.mark = 15; next;) + table=??(ls_in_qos ), priority=102 , match=(ip4), action=(ip.dscp = 16; next;) + table=??(ls_in_qos ), priority=103 , match=(ip4), action=(ip.dscp = 17; pkt.mark = 18; next;) + table=??(ls_in_qos ), priority=104 , match=(ip4), action=(set_meter(100); ip.dscp = 18; pkt.mark = 19; next;) + table=??(ls_out_qos ), priority=0 , match=(1), action=(next;) + table=??(ls_out_qos ), priority=100 , match=(ip4), action=(set_meter(100, 1000); next;) + table=??(ls_out_qos ), priority=101 , match=(ip4), action=(pkt.mark = 15; next;) + table=??(ls_out_qos ), priority=102 , match=(ip4), action=(ip.dscp = 16; next;) + table=??(ls_out_qos ), priority=103 , match=(ip4), action=(ip.dscp = 17; pkt.mark = 18; next;) ]) AT_CLEANUP diff --git a/tests/ovn.at b/tests/ovn.at index 4d0c7ad53..f04b74210 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -10205,9 +10205,8 @@ check ovn-nbctl --wait=hv set QoS $qos_id bandwidth=rate=100 # check at hv with a qos meter table AT_CHECK([as hv ovs-ofctl dump-meters br-int -O OpenFlow13 | grep rate=100 | wc -l], [0], [1 ]) -AT_CHECK([as hv ovs-ofctl dump-flows br-int -O OpenFlow13 | grep meter | wc -l], [0], [1 +AT_CHECK([as hv ovs-ofctl dump-flows br-int -O OpenFlow13 | grep meter | wc -l], [0], [2 ]) - # Update the DSCP marking check ovn-nbctl --wait=hv set QoS $qos_id action=dscp=63 check_tos 63 @@ -10218,7 +10217,7 @@ check ovn-nbctl --wait=hv set QoS $qos_id bandwidth=rate=4294967295,burst=429496 # check at hv with a qos meter table AT_CHECK([as hv ovs-ofctl dump-meters br-int -O OpenFlow13 | grep burst_size=4294967295 | wc -l], [0], [1 ]) -AT_CHECK([as hv ovs-ofctl dump-flows br-int -O OpenFlow13 | grep meter | wc -l], [0], [1 +AT_CHECK([as hv ovs-ofctl dump-flows br-int -O OpenFlow13 | grep meter | wc -l], [0], [2 ]) check ovn-nbctl --wait=hv set QoS $qos_id match="outport\=\=\"lp2\"" direction="to-lport" @@ -22071,7 +22070,7 @@ check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1 wait_for_ports_up sw0-vir check ovn-nbctl --wait=hv sync AT_CHECK([test 2 = `cat hv1/ovn-controller.log | grep "pinctrl received packet-in" | \ -grep opcode=BIND_VPORT | grep OF_Table_ID=29 | wc -l`]) +grep opcode=BIND_VPORT | grep OF_Table_ID=28 | wc -l`]) wait_row_count Port_Binding 1 logical_port=sw0-vir6 chassis=$hv1_ch_uuid check_row_count Port_Binding 1 logical_port=sw0-vir6 virtual_parent=sw0-p1 @@ -37685,7 +37684,7 @@ tpa=$(ip_to_hex 10 0 0 10) send_garp 1 1 $eth_src $eth_dst $spa $tpa OVS_WAIT_UNTIL([test 1 = `cat hv1/ovn-controller.log | grep "pinctrl received packet-in" | \ -grep opcode=BIND_VPORT | grep OF_Table_ID=29 | wc -l`]) +grep opcode=BIND_VPORT | grep OF_Table_ID=28 | wc -l`]) sleep_controller hv1 -- 2.42.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev