On Wed, Jul 31, 2024 at 5:19 AM Ales Musil <[email protected]> wrote: > > On Wed, Jul 31, 2024 at 11:05 AM Dumitru Ceara <[email protected]> wrote: > > > When the QoS stages were merged the documentation wasn't updated > > properly. Also fix up some small style issues in the northd code > > itself. > > > > Fixes: 5dd573757699 ("Merge QoS logical pipelines.") > > Signed-off-by: Dumitru Ceara <[email protected]> > > --- > > northd/northd.c | 11 +++-- > > northd/ovn-northd.8.xml | 92 +++++++++++++---------------------------- > > 2 files changed, 34 insertions(+), 69 deletions(-) > > > > diff --git a/northd/northd.c b/northd/northd.c > > index a8a0b6f94c..fbfd5a7f35 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -7137,7 +7137,6 @@ build_qos(struct ovn_datapath *od, struct > > lflow_table *lflows, > > } > > } > > if (rate) { > > - stage = ingress ? S_SWITCH_IN_QOS : S_SWITCH_OUT_QOS; > > if (burst) { > > ds_put_format(&action, > > "set_meter(%"PRId64", %"PRId64"); ", > > @@ -7164,11 +7163,11 @@ build_qos(struct ovn_datapath *od, struct > > lflow_table *lflows, > > qos->value_action[j]); > > } > > } > > - ds_put_cstr(&action, "next;"); > > - ovn_lflow_add_with_hint(lflows, od, stage, > > - qos->priority, > > - qos->match, ds_cstr(&action), > > - &qos->header_, lflow_ref); > > + ds_put_cstr(&action, "next;"); > > + 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/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > index b06b09ac5f..ba85e4bfd7 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -907,48 +907,21 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 10: <code>from-lport</code> QoS Marking</h3> > > + <h3>Ingress Table 10: <code>from-lport</code> QoS</h3> > > > > <p> > > Logical flows in this table closely reproduce those in the > > - <code>QoS</code> table with the <code>action</code> column set in > > - the <code>OVN_Northbound</code> database for the > > + <code>QoS</code> table with the <code>action</code> or > > + <code>bandwidth</code> column set in the > > + <code>OVN_Northbound</code> database for the > > <code>from-lport</code> direction. > > </p> > > > > <ul> > > <li> > > - For every qos_rules entry in a logical switch with DSCP marking > > - enabled, a flow will be added at the priority mentioned in the > > - QoS table. > > - </li> > > - > > - <li> > > - For every qos_rules entry in a logical switch with packet marking > > - enabled, a flow will be added at the priority mentioned in the > > - QoS table. > > - </li> > > - > > - <li> > > - One priority-0 fallback flow that matches all packets and > > advances to > > - the next table. > > - </li> > > - </ul> > > - > > - <h3>Ingress Table 11: <code>from-lport</code> QoS Meter</h3> > > - > > - <p> > > - Logical flows in this table closely reproduce those in the > > - <code>QoS</code> table with the <code>bandwidth</code> column set > > - in the <code>OVN_Northbound</code> database for the > > - <code>from-lport</code> direction. > > - </p> > > - > > - <ul> > > - <li> > > - For every qos_rules entry in a logical switch with metering > > - enabled, a flow will be added at the priority mentioned in the > > - QoS table. > > + For every qos_rules entry in a logical switch with DSCP marking, > > + packet marking or metering enabled a flow will be added at the > > priority > > + mentioned in the QoS table. > > </li> > > > > <li> > > @@ -957,7 +930,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 12: Load balancing affinity check</h3> > > + <h3>Ingress Table 11: Load balancing affinity check</h3> > > > > <p> > > Load balancing affinity check table contains the following > > @@ -985,7 +958,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 13: LB</h3> > > + <h3>Ingress Table 12: LB</h3> > > > > <ul> > > <li> > > @@ -1065,7 +1038,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 14: Load balancing affinity learn</h3> > > + <h3>Ingress Table 13: Load balancing affinity learn</h3> > > > > <p> > > Load balancing affinity learn table contains the following > > @@ -1096,7 +1069,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 15: Pre-Hairpin</h3> > > + <h3>Ingress Table 14: Pre-Hairpin</h3> > > <ul> > > <li> > > If the logical switch has load balancer(s) configured, then a > > @@ -1114,7 +1087,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 16: Nat-Hairpin</h3> > > + <h3>Ingress Table 15: Nat-Hairpin</h3> > > <ul> > > <li> > > If the logical switch has load balancer(s) configured, then a > > @@ -1149,7 +1122,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 17: Hairpin</h3> > > + <h3>Ingress Table 16: Hairpin</h3> > > <ul> > > <li> > > <p> > > @@ -1187,7 +1160,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress table 18: <code>from-lport</code> ACL evaluation after > > LB</h3> > > + <h3>Ingress table 17: <code>from-lport</code> ACL evaluation after > > LB</h3> > > > > <p> > > Logical flows in this table closely reproduce those in the > > @@ -1272,7 +1245,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 19: <code>from-lport</code> ACL action after LB</h3> > > + <h3>Ingress Table 18: <code>from-lport</code> ACL action after LB</h3> > > > > <p> > > Logical flows in this table decide how to proceed based on the > > values of > > @@ -1312,7 +1285,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 20: Stateful</h3> > > + <h3>Ingress Table 19: Stateful</h3> > > > > <ul> > > <li> > > @@ -1335,7 +1308,7 @@ > > </li> > > </ul> > > > > - <h3>Ingress Table 21: ARP/ND responder</h3> > > + <h3>Ingress Table 20: ARP/ND responder</h3> > > > > <p> > > This table implements ARP/ND responder in a logical switch for known > > @@ -1670,7 +1643,7 @@ output; > > </li> > > </ul> > > > > - <h3>Ingress Table 22: DHCP option processing</h3> > > + <h3>Ingress Table 21: DHCP option processing</h3> > > > > <p> > > This table adds the DHCPv4 options to a DHCPv4 packet from the > > @@ -1731,7 +1704,7 @@ next; > > </li> > > </ul> > > > > - <h3>Ingress Table 23: DHCP responses</h3> > > + <h3>Ingress Table 22: DHCP responses</h3> > > > > <p> > > This table implements DHCP responder for the DHCP replies generated > > by > > @@ -1812,7 +1785,7 @@ output; > > </li> > > </ul> > > > > - <h3>Ingress Table 24 DNS Lookup</h3> > > + <h3>Ingress Table 23 DNS Lookup</h3> > > > > <p> > > This table looks up and resolves the DNS names to the corresponding > > @@ -1841,7 +1814,7 @@ reg0[4] = dns_lookup(); next; > > </li> > > </ul> > > > > - <h3>Ingress Table 25 DNS Responses</h3> > > + <h3>Ingress Table 24 DNS Responses</h3> > > > > <p> > > This table implements DNS responder for the DNS replies generated by > > @@ -1876,7 +1849,7 @@ output; > > </li> > > </ul> > > > > - <h3>Ingress table 26 External ports</h3> > > + <h3>Ingress table 25 External ports</h3> > > > > <p> > > Traffic from the <code>external</code> logical ports enter the > > ingress > > @@ -1919,7 +1892,7 @@ output; > > </li> > > </ul> > > > > - <h3>Ingress Table 27 Destination Lookup</h3> > > + <h3>Ingress Table 26 Destination Lookup</h3> > > > > <p> > > This table implements switching behavior. It contains these logical > > @@ -2117,7 +2090,7 @@ output; > > </li> > > </ul> > > > > - <h3>Ingress Table 28 Destination unknown</h3> > > + <h3>Ingress Table 27 Destination unknown</h3> > > > > <p> > > This table handles the packets whose destination was not found or > > @@ -2330,28 +2303,21 @@ output; > > This is similar to ingress table <code>ACL action</code>. > > </p> > > > > - <h3>Egress Table 6: <code>to-lport</code> QoS Marking</h3> > > - > > - <p> > > - This is similar to ingress table <code>QoS marking</code> except > > - they apply to <code>to-lport</code> QoS rules. > > - </p> > > - > > - <h3>Egress Table 7: <code>to-lport</code> QoS Meter</h3> > > + <h3>Egress Table 6: <code>to-lport</code> QoS</h3> > > > > <p> > > - This is similar to ingress table <code>QoS meter</code> except > > + This is similar to ingress table <code>QoS</code> except > > they apply to <code>to-lport</code> QoS rules. > > </p> > > > > - <h3>Egress Table 8: Stateful</h3> > > + <h3>Egress Table 7: Stateful</h3> > > > > <p> > > This is similar to ingress table <code>Stateful</code> except that > > there are no rules added for load balancing new connections. > > </p> > > > > - <h3>Egress Table 9: Egress Port Security - check</h3> > > + <h3>Egress Table 8: Egress Port Security - check</h3> > > > > <p> > > This is similar to the port security logic in table > > @@ -2380,7 +2346,7 @@ output; > > </li> > > </ul> > > > > - <h3>Egress Table 10: Egress Port Security - Apply</h3> > > + <h3>Egress Table 9: Egress Port Security - Apply</h3> > > > > <p> > > This is similar to the ingress port security logic in ingress table > > -- > > 2.45.2 > > > > > Acked-by: Ales Musil <[email protected]>
Acked-by: Numan Siddique <[email protected]> Not related to this patch: I think ovn-northd.8.xml has been out of sync with the logical flow description for a while. I'm not sure how to fix that. I wonder if users and developers refer to this documentation to understand the OVN pipeline or not. Numan > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] > <https://red.ht/sig> > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
