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();
}
+#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();
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();
> }
>
> +#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();
> 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
