On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff <b...@ovn.org> wrote:
>
> Until now, the service monitor MAC has been inlined into logical flow
> matches.  This makes it a little hard to compare flow tables from one
> run or deployment to another, since the service monitor MAC is random
> and will always differ.  This commit changes flow matches to use an
> address set $svc_monitor_mac everywhere that it can be.  This makes
> the flow matches the same in every deployment.
>
> The service monitor MAC is also used in actions to set Ethernet
> addresses.  This can't be replaced by an address set, so these flows
> will still have some differences.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>

Acked-by: Numan Siddique <num...@ovn.org>

Thanks
Numan


> ---
>  northd/ovn-northd.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index f25f5cd82f39..b96e0db516be 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5004,15 +5004,11 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> *lflows)
>      ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;");
>      ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;");
>
> -    char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
> -    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match,
> -                  "next;");
> -    free(svc_check_match);
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> +                  "eth.dst == $svc_monitor_mac", "next;");
>
> -    svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> -    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, svc_check_match,
> -                  "next;");
> -    free(svc_check_match);
> +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
> +                  "eth.src == $svc_monitor_mac", "next;");
>
>      /* If there are any stateful ACL rules in this datapath, we must
>       * send all IP packets through the conntrack action, which handles
> @@ -5170,15 +5166,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> *lflows,
>                    "next;");
>
>      /* Do not send service monitor packets to conntrack. */
> -    char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
>      ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> -                  svc_check_match, "next;");
> -    free(svc_check_match);
> -
> -    svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> +                  "eth.dst == $svc_monitor_mac", "next;");
>      ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
> -                  svc_check_match, "next;");
> -    free(svc_check_match);
> +                  "eth.src == $svc_monitor_mac", "next;");
>
>      /* Allow all packets to go to next tables by default. */
>      ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
> @@ -5831,17 +5822,13 @@ build_acls(struct ovn_datapath *od, struct hmap 
> *lflows,
>
>      /* Add a 34000 priority flow to advance the service monitor reply
>       * packets to skip applying ingress ACLs. */
> -    char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
> -    ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000, svc_check_match,
> -                  "next;");
> -    free(svc_check_match);
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000,
> +                  "eth.dst == $svc_monitor_mac", "next;");
>
>      /* Add a 34000 priority flow to advance the service monitor packets
>       * generated by ovn-controller to skip applying egress ACLs. */
> -    svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac);
> -    ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000, svc_check_match,
> -                  "next;");
> -    free(svc_check_match);
> +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000,
> +                  "eth.src == $svc_monitor_mac", "next;");
>  }
>
>  static void
> @@ -7172,7 +7159,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>          }
>      }
>
> -    char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
>      /* Ingress table 19: Destination lookup, broadcast and multicast handling
>       * (priority 70 - 100). */
>      HMAP_FOR_EACH (od, key_node, datapaths) {
> @@ -7180,7 +7166,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>              continue;
>          }
>
> -        ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, svc_check_match,
> +        ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
> +                      "eth.dst == $svc_monitor_mac",
>                        "handle_svc_check(inport);");
>
>          struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw;
> @@ -7253,7 +7240,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>          ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast",
>                        "outport = \""MC_FLOOD"\"; output;");
>      }
> -    free(svc_check_match);
>
>      /* Ingress table 19: Add IP multicast flows learnt from IGMP/MLD
>       * (priority 90). */
> @@ -11537,6 +11523,11 @@ sync_address_sets(struct northd_context *ctx)
>          shash_add(&sb_address_sets, sb_address_set->name, sb_address_set);
>      }
>
> +    /* Service monitor MAC. */
> +    const char *svc_monitor_macp = svc_monitor_mac;
> +    sync_address_set(ctx, "svc_monitor_mac", &svc_monitor_macp, 1,
> +                     &sb_address_sets);
> +
>      /* sync port group generated address sets first */
>      const struct nbrec_port_group *nb_port_group;
>      NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {
> --
> 2.26.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to