Re: [ovs-dev] [PATCH ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.

2024-02-02 Thread Dumitru Ceara
On 1/30/24 22:21, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> This new engine now maintains the load balancer and NAT data of a
> logical router which was earlier part of northd engine node data.
> The main inputs to this engine are:
>- northd node
>- lr_nat node
>- lb_data node
> 
> A record for each logical router is maintained in the 'lr_stateful_table'
> hmap table and this record
>- stores the lb related data
>- embeds the 'lr_nat' record.
> 
> This engine node becomes an input to 'lflow' node.
> 
> Signed-off-by: Numan Siddique 
> ---

I have two small comments, style related, below.  With those addressed:

Acked-by: Dumitru Ceara 

[...]

> +static void lr_stateful_table_build(struct lr_stateful_table *,
> +   const struct lr_nat_table *,
> +   const struct ovn_datapaths *lr_datapaths,
> +   const struct hmap *lb_datapaths_map,
> +   const struct hmap *lbgrp_datapaths_map);

Nit: indentation.

[...]

> +
> +static inline bool
> +lr_stateful_has_tracked_data(struct lr_stateful_tracked_data *trk_data) {

Nit: curly brace on next line.

Thanks,
Dumitru

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.

2024-01-30 Thread 0-day Robot
Bleep bloop.  Greetings Numan Siddique, 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:
ERROR: Inappropriate bracing around statement
#100 FILE: northd/en-lr-nat.h:95:
HMAP_FOR_EACH (LR_NAT_REC, key_node, &(TABLE)->entries)

ERROR: Inappropriate bracing around statement
#849 FILE: northd/en-lr-stateful.h:74:
HMAP_FOR_EACH (LR_LB_NAT_REC, key_node, &(TABLE)->entries)

Lines checked: 3014, Warnings: 0, Errors: 2


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 ovn v6 02/13] northd: Add a new engine 'lr_stateful' to manage lr's stateful data.

2024-01-30 Thread numans
From: Numan Siddique 

This new engine now maintains the load balancer and NAT data of a
logical router which was earlier part of northd engine node data.
The main inputs to this engine are:
   - northd node
   - lr_nat node
   - lb_data node

A record for each logical router is maintained in the 'lr_stateful_table'
hmap table and this record
   - stores the lb related data
   - embeds the 'lr_nat' record.

This engine node becomes an input to 'lflow' node.

Signed-off-by: Numan Siddique 
---
 lib/stopwatch-names.h|   1 +
 northd/automake.mk   |   2 +
 northd/en-lflow.c|   4 +
 northd/en-lr-nat.h   |   3 +
 northd/en-lr-stateful.c  | 659 
 northd/en-lr-stateful.h  | 112 ++
 northd/en-sync-sb.c  |  54 +--
 northd/inc-proc-northd.c |  13 +-
 northd/northd.c  | 710 ---
 northd/northd.h  | 146 +++-
 northd/ovn-northd.c  |   1 +
 tests/ovn-northd.at  |  62 
 12 files changed, 1250 insertions(+), 517 deletions(-)
 create mode 100644 northd/en-lr-stateful.c
 create mode 100644 northd/en-lr-stateful.h

diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index 782d64320a..e5e41fbfd8 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -30,5 +30,6 @@
 #define PORT_GROUP_RUN_STOPWATCH_NAME "port_group_run"
 #define SYNC_METERS_RUN_STOPWATCH_NAME "sync_meters_run"
 #define LR_NAT_RUN_STOPWATCH_NAME "lr_nat_run"
+#define LR_STATEFUL_RUN_STOPWATCH_NAME "lr_stateful"
 
 #endif
diff --git a/northd/automake.mk b/northd/automake.mk
index a477105470..b886356c9c 100644
--- a/northd/automake.mk
+++ b/northd/automake.mk
@@ -26,6 +26,8 @@ northd_ovn_northd_SOURCES = \
northd/en-lb-data.h \
northd/en-lr-nat.c \
northd/en-lr-nat.h \
+   northd/en-lr-stateful.c \
+   northd/en-lr-stateful.h \
northd/inc-proc-northd.c \
northd/inc-proc-northd.h \
northd/ipam.c \
diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index f1fa92cdfb..97c7f383cd 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -20,6 +20,7 @@
 
 #include "en-lflow.h"
 #include "en-lr-nat.h"
+#include "en-lr-stateful.h"
 #include "en-northd.h"
 #include "en-meters.h"
 
@@ -43,6 +44,8 @@ lflow_get_input_data(struct engine_node *node,
 engine_get_input_data("sync_meters", node);
 struct ed_type_lr_nat_data *lr_nat_data =
 engine_get_input_data("lr_nat", node);
+struct ed_type_lr_stateful *lr_stateful_data =
+engine_get_input_data("lr_stateful", node);
 
 lflow_input->nbrec_bfd_table =
 EN_OVSDB_GET(engine_get_input("NB_bfd", node));
@@ -66,6 +69,7 @@ lflow_get_input_data(struct engine_node *node,
 lflow_input->lr_ports = &northd_data->lr_ports;
 lflow_input->ls_port_groups = &pg_data->ls_port_groups;
 lflow_input->lr_nats = &lr_nat_data->lr_nats;
+lflow_input->lr_stateful_table = &lr_stateful_data->table;
 lflow_input->meter_groups = &sync_meters_data->meter_groups;
 lflow_input->lb_datapaths_map = &northd_data->lb_datapaths_map;
 lflow_input->svc_monitor_map = &northd_data->svc_monitor_map;
diff --git a/northd/en-lr-nat.h b/northd/en-lr-nat.h
index 79390f8db8..16b166ee05 100644
--- a/northd/en-lr-nat.h
+++ b/northd/en-lr-nat.h
@@ -91,6 +91,9 @@ struct lr_nat_table {
 const struct lr_nat_record * lr_nat_table_find_by_index(
 const struct lr_nat_table *, size_t od_index);
 
+#define LR_NAT_TABLE_FOR_EACH(LR_NAT_REC, TABLE) \
+HMAP_FOR_EACH (LR_NAT_REC, key_node, &(TABLE)->entries)
+
 struct ed_type_lr_nat_data {
 struct lr_nat_table lr_nats;
 
diff --git a/northd/en-lr-stateful.c b/northd/en-lr-stateful.c
new file mode 100644
index 00..4404caaf93
--- /dev/null
+++ b/northd/en-lr-stateful.c
@@ -0,0 +1,659 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+
+/* OVS includes */
+#include "include/openvswitch/hmap.h"
+#include "lib/bitmap.h"
+#include "lib/socket-util.h"
+#include "lib/uuidset.h"
+#include "openvswitch/util.h"
+#include "openvswitch/vlog.h"
+#include "stopwatch.h"
+
+/* OVN includes */
+#include "en-lb-data.h"
+#include "en-lr-nat.h"
+#include "en-lr-stateful.h"
+#include "lib/inc-proc-eng.h"
+#include "lib/lb.h"
+#include "lib/ovn-nb-idl.h"
+#include "lib/ovn-sb-idl.h"
+#include "lib/ovn-util.h"
+#include "lib/stopwatch-names.h"
+#include "northd.h"
+