Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.
Thanks a bunch Lorenzo. Acked-by: Mark Michelson I plan to merge this in a few hours, giving a bit of time in case others want to review this. When I merge, I'll fold in Dumitru's suggestion. On 2/2/24 11:03, Dumitru Ceara wrote: On 2/2/24 16:41, Lorenzo Bianconi wrote: Add the capbility to mark (through pkt.mark) incoming/outgoing packets in logical_switch datapath according to user configured QoS rule. Co-developed-by: Dumitru Ceara Reported-at: https://issues.redhat.com/browse/FDP-42 Signed-off-by: Lorenzo Bianconi --- Changes since v2: - set max dscp/mark value in ovn-nb.ovsschema - add unit-test for new dscp/mark boundaries Changes since v1: - move qos packet mark action in QOS_MARK ls {ingress/egress} stage --- NEWS | 2 + northd/northd.c | 33 +--- northd/ovn-northd.8.xml | 6 +++ ovn-nb.ovsschema | 8 ++-- ovn-nb.xml| 12 +- tests/ovn-nbctl.at| 8 +++- tests/ovn-northd.at | 81 ++ tests/ovn.at | 83 +++ utilities/ovn-nbctl.8.xml | 5 ++- utilities/ovn-nbctl.c | 27 ++--- 10 files changed, 245 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 6553bd078..a8beb09fb 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ Post v23.09.0 - Support selecting encapsulation IP based on the source/destination VIF's settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more details. + - Add the capability to mark (through pkt.mark) incoming/outgoing packets +in the logical switch datapath according to user configured QoS rule. OVN v23.09.0 - 15 Sep 2023 -- diff --git a/northd/northd.c b/northd/northd.c index d2091d4bc..a77919af3 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct chassis_features *features, ds_destroy(&actions); } +#define QOS_MAX_DSCP 63 + static void build_qos(struct ovn_datapath *od, struct hmap *lflows) { struct ds action = DS_EMPTY_INITIALIZER; @@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) { 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; +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 j = 0; j < qos->n_action; j++) { +if (strcmp(qos->key_action[j], "dscp") && +strcmp(qos->key_action[j], "mark")) { +continue; +} + This check seems redundant we recheck the qos->key_action[j] just below. Would it be possible to remove it when applying the patch to the main branch (if the patch is accepted)? Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.
On 2/2/24 16:41, Lorenzo Bianconi wrote: > Add the capbility to mark (through pkt.mark) incoming/outgoing packets > in logical_switch datapath according to user configured QoS rule. > > Co-developed-by: Dumitru Ceara > Reported-at: https://issues.redhat.com/browse/FDP-42 > Signed-off-by: Lorenzo Bianconi > --- > Changes since v2: > - set max dscp/mark value in ovn-nb.ovsschema > - add unit-test for new dscp/mark boundaries > Changes since v1: > - move qos packet mark action in QOS_MARK ls {ingress/egress} stage > --- > NEWS | 2 + > northd/northd.c | 33 +--- > northd/ovn-northd.8.xml | 6 +++ > ovn-nb.ovsschema | 8 ++-- > ovn-nb.xml| 12 +- > tests/ovn-nbctl.at| 8 +++- > tests/ovn-northd.at | 81 ++ > tests/ovn.at | 83 +++ > utilities/ovn-nbctl.8.xml | 5 ++- > utilities/ovn-nbctl.c | 27 ++--- > 10 files changed, 245 insertions(+), 20 deletions(-) > > diff --git a/NEWS b/NEWS > index 6553bd078..a8beb09fb 100644 > --- a/NEWS > +++ b/NEWS > @@ -18,6 +18,8 @@ Post v23.09.0 >- Support selecting encapsulation IP based on the source/destination VIF's > settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more > details. > + - Add the capability to mark (through pkt.mark) incoming/outgoing packets > +in the logical switch datapath according to user configured QoS rule. > > OVN v23.09.0 - 15 Sep 2023 > -- > diff --git a/northd/northd.c b/northd/northd.c > index d2091d4bc..a77919af3 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct > chassis_features *features, > ds_destroy(&actions); > } > > +#define QOS_MAX_DSCP 63 > + > static void > build_qos(struct ovn_datapath *od, struct hmap *lflows) { > struct ds action = DS_EMPTY_INITIALIZER; > @@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap > *lflows) { > 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; > +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 j = 0; j < qos->n_action; j++) { > +if (strcmp(qos->key_action[j], "dscp") && > +strcmp(qos->key_action[j], "mark")) { > +continue; > +} > + This check seems redundant we recheck the qos->key_action[j] just below. Would it be possible to remove it when applying the patch to the main branch (if the patch is accepted)? Thanks, Dumitru ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.
Bleep bloop. Greetings Lorenzo Bianconi, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 286 characters long (recommended limit is 79) #402 FILE: utilities/ovn-nbctl.8.xml:466: [--may-exist] qos-add switch direction priority match [mark=mark] [dscp=dscp] [rate=rate [burst=burst]] WARNING: Line is 94 characters long (recommended limit is 79) WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator WARNING: Line lacks whitespace around operator #425 FILE: utilities/ovn-nbctl.c:286: qos-add SWITCH DIRECTION PRIORITY MATCH [rate=RATE [burst=BURST]] [dscp=DSCP] [mark=MARK]\n\ Lines checked: 505, Warnings: 5, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 ovn] northd: Add qos packet marking.
Add the capbility to mark (through pkt.mark) incoming/outgoing packets in logical_switch datapath according to user configured QoS rule. Co-developed-by: Dumitru Ceara Reported-at: https://issues.redhat.com/browse/FDP-42 Signed-off-by: Lorenzo Bianconi --- Changes since v2: - set max dscp/mark value in ovn-nb.ovsschema - add unit-test for new dscp/mark boundaries Changes since v1: - move qos packet mark action in QOS_MARK ls {ingress/egress} stage --- NEWS | 2 + northd/northd.c | 33 +--- northd/ovn-northd.8.xml | 6 +++ ovn-nb.ovsschema | 8 ++-- ovn-nb.xml| 12 +- tests/ovn-nbctl.at| 8 +++- tests/ovn-northd.at | 81 ++ tests/ovn.at | 83 +++ utilities/ovn-nbctl.8.xml | 5 ++- utilities/ovn-nbctl.c | 27 ++--- 10 files changed, 245 insertions(+), 20 deletions(-) diff --git a/NEWS b/NEWS index 6553bd078..a8beb09fb 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,8 @@ Post v23.09.0 - Support selecting encapsulation IP based on the source/destination VIF's settting. See ovn-controller(8) 'external_ids:ovn-encap-ip' for more details. + - Add the capability to mark (through pkt.mark) incoming/outgoing packets +in the logical switch datapath according to user configured QoS rule. OVN v23.09.0 - 15 Sep 2023 -- diff --git a/northd/northd.c b/northd/northd.c index d2091d4bc..a77919af3 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -8363,6 +8363,8 @@ build_acls(struct ovn_datapath *od, const struct chassis_features *features, ds_destroy(&actions); } +#define QOS_MAX_DSCP 63 + static void build_qos(struct ovn_datapath *od, struct hmap *lflows) { struct ds action = DS_EMPTY_INITIALIZER; @@ -8376,21 +8378,40 @@ build_qos(struct ovn_datapath *od, struct hmap *lflows) { 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; +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 j = 0; j < qos->n_action; j++) { +if (strcmp(qos->key_action[j], "dscp") && +strcmp(qos->key_action[j], "mark")) { +continue; +} + if (!strcmp(qos->key_action[j], "dscp")) { -ds_clear(&action); -ds_put_format(&action, "ip.dscp = %"PRId64"; next;", +if (qos->value_action[j] > QOS_MAX_DSCP) { +VLOG_WARN_RL(&rl, "Bad 'dscp' value %"PRId64" in qos " + UUID_FMT, qos->value_action[j], + UUID_ARGS(&qos->header_.uuid)); +continue; +} + +ds_put_format(&action, "ip.dscp = %"PRId64"; ", + qos->value_action[j]); +} else if (!strcmp(qos->key_action[j], "mark")) { +ds_put_format(&action, "pkt.mark = %"PRId64"; ", qos->value_action[j]); -ovn_lflow_add_with_hint(lflows, od, stage, -qos->priority, -qos->match, ds_cstr(&action), -&qos->header_); } } +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_); +} + for (size_t n = 0; n < qos->n_bandwidth; n++) { if (!strcmp(qos->key_bandwidth[n], "rate")) { rate = qos->value_bandwidth[n]; diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 068d47e1a..9583abeff 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -915,6 +915,12 @@ QoS table. + +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. + + One priority-0 fallback flow that matches all packets and advances to the next table. diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index b2e0993e0..12ddce3f6 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", -"version": "7.2.0", -"cksum": "1069338687 34162", +"version": "7.3.0", +"cksum": "4237284067 34178", "tables": { "NB_Global": { "columns": { @@ -293,10 +293,10 @@ "enum": ["set", ["from