| Message ID | 20201026181626.1827014-5-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/7] northd: Count mask length and priority correctly for IPv6 addresses. | expand |
On Mon, Oct 26, 2020 at 11:47 PM Ben Pfaff <blp@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 <blp@ovn.org> Acked-by: Numan Siddique <numans@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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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) {
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 <blp@ovn.org> --- northd/ovn-northd.c | 43 +++++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 26 deletions(-)