| Message ID | 6800711fb2ebf1fb45da03b23bbc0fb4f0b74fb9.1639698474.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] controller: fix ovn patch port incremental processing | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Thanks, Lorenzo. Acked-by: Mark Michelson <mmichels@redhat.com> On 12/16/21 18:49, Lorenzo Bianconi wrote: > If a patch port that connects a logical router to a logical switch is > added in a different ovn-controller run with respect to the logical > switch peer port, just the router port will be processed and > ovn-controller will miss missing some open-flows for the ls patch port > since the peer pointer was NULL when it has been processed. > Fix the issue always processing both patch ports. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2025623 > Tested-by: Slawek Kaplonski <skaplons@redhat.com> > Fixes: 3ae8470edc64 ("I-P: Handle runtime data changes for pflow_output engine.") > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/physical.c | 49 ++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 22 deletions(-) > > diff --git a/controller/physical.c b/controller/physical.c > index aa1d44bc6..836fc769a 100644 > --- a/controller/physical.c > +++ b/controller/physical.c > @@ -1564,6 +1564,24 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, > sset_destroy(&vtep_chassis); > } > > +static void > +physical_eval_port_binding(struct physical_ctx *p_ctx, > + const struct sbrec_port_binding *pb, > + struct ovn_desired_flow_table *flow_table) > +{ > + struct ofpbuf ofpacts; > + ofpbuf_init(&ofpacts, 0); > + consider_port_binding(p_ctx->sbrec_port_binding_by_name, > + p_ctx->mff_ovn_geneve, p_ctx->ct_zones, > + p_ctx->active_tunnels, > + p_ctx->local_datapaths, > + p_ctx->local_bindings, > + p_ctx->patch_ofports, > + p_ctx->chassis_tunnels, > + pb, p_ctx->chassis, flow_table, &ofpacts); > + ofpbuf_uninit(&ofpacts); > +} > + > bool > physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, > bool removed, struct physical_ctx *p_ctx, > @@ -1585,33 +1603,20 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, > get_local_datapath(p_ctx->local_datapaths, > pb->datapath->tunnel_key); > if (ldp && ldp->localnet_port) { > - struct ofpbuf ofpacts; > ofctrl_remove_flows(flow_table, &ldp->localnet_port->header_.uuid); > - ofpbuf_init(&ofpacts, 0); > - consider_port_binding(p_ctx->sbrec_port_binding_by_name, > - p_ctx->mff_ovn_geneve, p_ctx->ct_zones, > - p_ctx->active_tunnels, > - p_ctx->local_datapaths, > - p_ctx->local_bindings, > - p_ctx->patch_ofports, > - p_ctx->chassis_tunnels, > - ldp->localnet_port, p_ctx->chassis, > - flow_table, &ofpacts); > - ofpbuf_uninit(&ofpacts); > + physical_eval_port_binding(p_ctx, ldp->localnet_port, flow_table); > } > } > > if (!removed) { > - struct ofpbuf ofpacts; > - ofpbuf_init(&ofpacts, 0); > - consider_port_binding(p_ctx->sbrec_port_binding_by_name, > - p_ctx->mff_ovn_geneve, p_ctx->ct_zones, > - p_ctx->active_tunnels, p_ctx->local_datapaths, > - p_ctx->local_bindings, > - p_ctx->patch_ofports, > - p_ctx->chassis_tunnels, pb, > - p_ctx->chassis, flow_table, &ofpacts); > - ofpbuf_uninit(&ofpacts); > + physical_eval_port_binding(p_ctx, pb, flow_table); > + if (!strcmp(pb->type, "patch")) { > + const struct sbrec_port_binding *peer = > + get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb); > + if (peer) { > + physical_eval_port_binding(p_ctx, peer, flow_table); > + } > + } > } > > return true; >
diff --git a/controller/physical.c b/controller/physical.c index aa1d44bc6..836fc769a 100644 --- a/controller/physical.c +++ b/controller/physical.c @@ -1564,6 +1564,24 @@ consider_mc_group(struct ovsdb_idl_index *sbrec_port_binding_by_name, sset_destroy(&vtep_chassis); } +static void +physical_eval_port_binding(struct physical_ctx *p_ctx, + const struct sbrec_port_binding *pb, + struct ovn_desired_flow_table *flow_table) +{ + struct ofpbuf ofpacts; + ofpbuf_init(&ofpacts, 0); + consider_port_binding(p_ctx->sbrec_port_binding_by_name, + p_ctx->mff_ovn_geneve, p_ctx->ct_zones, + p_ctx->active_tunnels, + p_ctx->local_datapaths, + p_ctx->local_bindings, + p_ctx->patch_ofports, + p_ctx->chassis_tunnels, + pb, p_ctx->chassis, flow_table, &ofpacts); + ofpbuf_uninit(&ofpacts); +} + bool physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, bool removed, struct physical_ctx *p_ctx, @@ -1585,33 +1603,20 @@ physical_handle_flows_for_lport(const struct sbrec_port_binding *pb, get_local_datapath(p_ctx->local_datapaths, pb->datapath->tunnel_key); if (ldp && ldp->localnet_port) { - struct ofpbuf ofpacts; ofctrl_remove_flows(flow_table, &ldp->localnet_port->header_.uuid); - ofpbuf_init(&ofpacts, 0); - consider_port_binding(p_ctx->sbrec_port_binding_by_name, - p_ctx->mff_ovn_geneve, p_ctx->ct_zones, - p_ctx->active_tunnels, - p_ctx->local_datapaths, - p_ctx->local_bindings, - p_ctx->patch_ofports, - p_ctx->chassis_tunnels, - ldp->localnet_port, p_ctx->chassis, - flow_table, &ofpacts); - ofpbuf_uninit(&ofpacts); + physical_eval_port_binding(p_ctx, ldp->localnet_port, flow_table); } } if (!removed) { - struct ofpbuf ofpacts; - ofpbuf_init(&ofpacts, 0); - consider_port_binding(p_ctx->sbrec_port_binding_by_name, - p_ctx->mff_ovn_geneve, p_ctx->ct_zones, - p_ctx->active_tunnels, p_ctx->local_datapaths, - p_ctx->local_bindings, - p_ctx->patch_ofports, - p_ctx->chassis_tunnels, pb, - p_ctx->chassis, flow_table, &ofpacts); - ofpbuf_uninit(&ofpacts); + physical_eval_port_binding(p_ctx, pb, flow_table); + if (!strcmp(pb->type, "patch")) { + const struct sbrec_port_binding *peer = + get_binding_peer(p_ctx->sbrec_port_binding_by_name, pb); + if (peer) { + physical_eval_port_binding(p_ctx, peer, flow_table); + } + } } return true;