Re: [ovs-dev] [PATCH ovn v3 1/3] ovn-northd: Remove lflow_add_unique.

2021-06-29 Thread Han Zhou
On Tue, Jun 29, 2021 at 7:04 AM Dumitru Ceara  wrote:
>
> On 6/21/21 8:51 AM, Han Zhou wrote:
> > This patch removes the workaround when adding multicast group related
> > lflows, because the multicast group dependency problem is fixed in
> > ovn-controller in the previous commit.
> >
> > This patch also removes the UniqueFlow/AnnotatedFlow usage in northd
> > DDlog implementation for the same reason.
> >
> > Signed-off-by: Han Zhou 
> > ---
>
> Hi Han,
>
> The changes look good to me.
>
> Acked-by: Dumitru Ceara 
>
> Thanks,
> Dumitru
>

Thanks Dumitru. I applied patch 1/3 - 2/3 of the series to master. I will
update the test case of patch 3/3 with v4.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v3 1/3] ovn-northd: Remove lflow_add_unique.

2021-06-29 Thread Dumitru Ceara
On 6/21/21 8:51 AM, Han Zhou wrote:
> This patch removes the workaround when adding multicast group related
> lflows, because the multicast group dependency problem is fixed in
> ovn-controller in the previous commit.
> 
> This patch also removes the UniqueFlow/AnnotatedFlow usage in northd
> DDlog implementation for the same reason.
> 
> Signed-off-by: Han Zhou 
> ---

Hi Han,

The changes look good to me.

Acked-by: Dumitru Ceara 

Thanks,
Dumitru

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


[ovs-dev] [PATCH ovn v3 1/3] ovn-northd: Remove lflow_add_unique.

2021-06-20 Thread Han Zhou
This patch removes the workaround when adding multicast group related
lflows, because the multicast group dependency problem is fixed in
ovn-controller in the previous commit.

This patch also removes the UniqueFlow/AnnotatedFlow usage in northd
DDlog implementation for the same reason.

Signed-off-by: Han Zhou 
---
 northd/ovn-northd.c  |  93 +++--
 northd/ovn_northd.dl | 232 ---
 tests/ovn-northd.at  |   2 +-
 3 files changed, 143 insertions(+), 184 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 75484c5cd..ae799cda9 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3688,9 +3688,6 @@ build_ports(struct northd_context *ctx,
 sset_destroy(&active_ha_chassis_grps);
 }
 
-/* XXX: The 'ovn_lflow_add_unique*()' functions should be used for logical
- *  flows using a multicast group.
- *  See the comment on 'ovn_lflow_add_unique()' for details. */
 struct multicast_group {
 const char *name;
 uint16_t key;   /* OVN_MIN_MULTICAST...OVN_MAX_MULTICAST. */
@@ -4107,7 +4104,7 @@ static struct hashrow_locks lflow_locks;
  * Version to use when locking is required.
  */
 static void
-do_ovn_lflow_add(struct hmap *lflow_map, bool shared, struct ovn_datapath *od,
+do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od,
  uint32_t hash, enum ovn_stage stage, uint16_t priority,
  const char *match, const char *actions,
  const struct ovsdb_idl_row *stage_hint,
@@ -4117,7 +4114,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, bool shared, 
struct ovn_datapath *od,
 struct ovn_lflow *old_lflow;
 struct ovn_lflow *lflow;
 
-if (shared && use_logical_dp_groups) {
+if (use_logical_dp_groups) {
 old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority, match,
actions, hash);
 if (old_lflow) {
@@ -4141,7 +4138,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, bool shared, 
struct ovn_datapath *od,
 static void
 ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od,
  enum ovn_stage stage, uint16_t priority,
- const char *match, const char *actions, bool shared,
+ const char *match, const char *actions,
  const struct ovsdb_idl_row *stage_hint, const char *where)
 {
 ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od));
@@ -4155,11 +4152,11 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
ovn_datapath *od,
 
 if (use_logical_dp_groups && use_parallel_build) {
 lock_hash_row(&lflow_locks, hash);
-do_ovn_lflow_add(lflow_map, shared, od, hash, stage, priority, match,
+do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
  actions, stage_hint, where);
 unlock_hash_row(&lflow_locks, hash);
 } else {
-do_ovn_lflow_add(lflow_map, shared, od, hash, stage, priority, match,
+do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match,
  actions, stage_hint, where);
 }
 }
@@ -4167,30 +4164,11 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
ovn_datapath *od,
 /* Adds a row with the specified contents to the Logical_Flow table. */
 #define ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
 ACTIONS, STAGE_HINT) \
-ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \
+ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
  STAGE_HINT, OVS_SOURCE_LOCATOR)
 
 #define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
-ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \
- NULL, OVS_SOURCE_LOCATOR)
-
-/* Adds a row with the specified contents to the Logical_Flow table.
- * Combining of this logical flow with already existing ones, e.g., by using
- * Logical Datapath Groups, is forbidden.
- *
- * XXX: ovn-controller assumes that a logical flow using multicast group always
- *  comes after or in the same database update with the corresponding
- *  multicast group.  That will not be the case with datapath groups.
- *  For this reason, the 'ovn_lflow_add_unique*()' functions should be used
- *  for such logical flows.
- */
-#define ovn_lflow_add_unique_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
-   ACTIONS, STAGE_HINT) \
-ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
- STAGE_HINT, OVS_SOURCE_LOCATOR)
-
-#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
-ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
+ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
  NULL, OVS_SOURCE_LOCATOR)
 
 static struct ovn_lflow *
@@ -6461,9 +