diff mbox series

[ovs-dev,v1,07/12] ovn-br-controller: Add en_pflow_output engine.

Message ID 20250811101006.917943-1-numans@ovn.org
State Changes Requested
Headers show
Series OVN Bridge Controller service. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Numan Siddique Aug. 11, 2025, 10:10 a.m. UTC
From: Numan Siddique <numans@ovn.org>

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 br-controller/automake.mk         |   2 +
 br-controller/en-lflow.h          |   6 +
 br-controller/en-pflow.c          | 192 ++++++++++++++++++++++++++++++
 br-controller/en-pflow.h          |  16 +++
 br-controller/ovn-br-controller.c |   6 +
 5 files changed, 222 insertions(+)
 create mode 100644 br-controller/en-pflow.c
 create mode 100644 br-controller/en-pflow.h

Comments

Mark Michelson Aug. 21, 2025, 9:24 p.m. UTC | #1
On 8/11/25 6:10 AM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>   br-controller/automake.mk         |   2 +
>   br-controller/en-lflow.h          |   6 +
>   br-controller/en-pflow.c          | 192 ++++++++++++++++++++++++++++++
>   br-controller/en-pflow.h          |  16 +++
>   br-controller/ovn-br-controller.c |   6 +
>   5 files changed, 222 insertions(+)
>   create mode 100644 br-controller/en-pflow.c
>   create mode 100644 br-controller/en-pflow.h
> 
> diff --git a/br-controller/automake.mk b/br-controller/automake.mk
> index ea515a85e4..4baea4f6fe 100644
> --- a/br-controller/automake.mk
> +++ b/br-controller/automake.mk
> @@ -6,6 +6,8 @@ br_controller_ovn_br_controller_SOURCES = \
>   	br-controller/en-bridge-data.h \
>   	br-controller/en-lflow.c \
>   	br-controller/en-lflow.h \
> +	br-controller/en-pflow.c \
> +	br-controller/en-pflow.h \
>   	br-controller/ovn-br-controller.c
>   
>   br_controller_ovn_br_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
> diff --git a/br-controller/en-lflow.h b/br-controller/en-lflow.h
> index 39daa30174..093c2849dd 100644
> --- a/br-controller/en-lflow.h
> +++ b/br-controller/en-lflow.h
> @@ -12,6 +12,12 @@
>   #define BR_OFTABLE_LOG_INGRESS_PIPELINE      8
>   #define BR_OFTABLE_SAVE_INPORT               64
>   
> +/* OpenFlow table numbers. */
> +#define BR_OFTABLE_PHY_TO_LOG             0
> +#define BR_OFTABLE_LOG_INGRESS_PIPELINE   8
> +#define BR_OFTABLE_SAVE_INPORT           64

It appears you are redefining BR_OFTABLE_LOG_INGRESS_PIPELINE and 
BR_OFTABLE_SAVE_INPORT.

> +#define BR_OFTABLE_LOG_TO_PHY            65
> +
>   enum engine_node_state en_lflow_output_run(struct engine_node *, void *data);
>   void *en_lflow_output_init(struct engine_node *node, struct engine_arg *arg);
>   void en_lflow_output_cleanup(void *data);
> diff --git a/br-controller/en-pflow.c b/br-controller/en-pflow.c
> new file mode 100644
> index 0000000000..5f4c5b914f
> --- /dev/null
> +++ b/br-controller/en-pflow.c
> @@ -0,0 +1,192 @@
> +/*
> + * 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 <config.h>
> +
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +/* OVS includes. */
> +#include "lib/coverage.h"
> +#include "lib/flow.h"
> +#include "lib/simap.h"
> +#include "lib/vswitch-idl.h"
> +#include "openvswitch/match.h"
> +#include "openvswitch/ofp-actions.h"
> +#include "openvswitch/ofpbuf.h"
> +#include "openvswitch/ofp-parse.h"
> +
> +#include "openvswitch/vlog.h"
> +#include "openvswitch/uuid.h"
> +
> +/* OVN includes. */
> +#include "br-flow-mgr.h"
> +#include "en-lflow.h"
> +#include "en-pflow.h"
> +#include "en-bridge-data.h"
> +#include "include/ovn/logical-fields.h"
> +#include "lib/ovn-br-idl.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_pflow);
> +
> +
> +static void pflow_run(const struct shash *ovn_bridges);
> +static void physical_flows_add(const struct ovn_bridge *);
> +
> +static void put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> +                     struct ofpbuf *);
> +static void put_resubmit(uint8_t table_id, struct ofpbuf *);
> +
> +static void add_interface_flows(const char *bridge,
> +                                const struct ovsrec_interface *iface_rec,
> +                                int64_t, struct ofpbuf *);
> +
> +/* Public functions. */
> +void *en_pflow_output_init(struct engine_node *node OVS_UNUSED,
> +                           struct engine_arg *arg OVS_UNUSED)
> +{
> +    return NULL;
> +}
> +
> +void en_pflow_output_cleanup(void *data_ OVS_UNUSED)
> +{
> +}
> +
> +enum engine_node_state
> +en_pflow_output_run(struct engine_node *node, void *data_  OVS_UNUSED)
> +{
> +    struct ed_type_bridge_data *br_data =
> +        engine_get_input_data("bridge_data", node);
> +
> +    pflow_run(&br_data->bridges);
> +
> +    return EN_UPDATED;
> +}
> +
> +/* Static functions. */
> +static void
> +pflow_run(const struct shash *ovn_bridges)
> +{
> +    br_flow_switch_physical_oflow_tables();
> +    struct shash_node *shash_node;
> +    SHASH_FOR_EACH (shash_node, ovn_bridges) {
> +        physical_flows_add(shash_node->data);
> +    }
> +}
> +
> +static void
> +physical_flows_add(const struct ovn_bridge *br)
> +{
> +    if (!br->ovs_br) {
> +        return;
> +    }
> +
> +    struct ofpbuf ofpacts;
> +    ofpbuf_init(&ofpacts, 0);
> +
> +    struct match match = MATCH_CATCHALL_INITIALIZER;
> +
> +    /* Table 0 and 65, priority 0, actions=NORMAL
> +     * ===================================
> +     *
> +     */
> +    ofpact_put_OUTPUT(&ofpacts)->port = OFPP_NORMAL;
> +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_PHY_TO_LOG, 0,
> +                               br->ovs_br->header_.uuid.parts[0],
> +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_LOG_TO_PHY, 0,
> +                               br->ovs_br->header_.uuid.parts[0],
> +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> +
> +    ofpbuf_clear(&ofpacts);
> +    put_resubmit(BR_OFTABLE_LOG_TO_PHY, &ofpacts);
> +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_SAVE_INPORT, 0,
> +                               br->ovs_br->header_.uuid.parts[0],
> +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> +
> +    for (size_t i = 0; i < br->ovs_br->n_ports; i++) {
> +        const struct ovsrec_port *port_rec = br->ovs_br->ports[i];
> +
> +        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
> +            const struct ovsrec_interface *iface_rec;
> +
> +            iface_rec = port_rec->interfaces[j];
> +            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> +            if (ofport > 0 && ofport != ofp_to_u16(OFPP_LOCAL) &&
> +                smap_get_bool(&iface_rec->external_ids,
> +                              "ovnbr-managed", true)) {

I skimmed but couldn't find any reference documentation for 
ovnbr-managed. It appears to be that if the option is not present or if 
it is set to "true" then the port can be managed by ovn-br-controller. 
Otherwise, the port is unknown to ovn-br-controller.

Should this instead default to "false" and require opting-in to be 
managed by ovn-br-controller?

> +                add_interface_flows(br->db_br->name, iface_rec,
> +                                    ofport, &ofpacts);
> +            }
> +        }
> +    }
> +
> +    ofpbuf_uninit(&ofpacts);
> +}
> +
> +static void
> +put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> +         struct ofpbuf *ofpacts)
> +{
> +    struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
> +                                                       mf_from_id(dst), NULL,
> +                                                       NULL);
> +    ovs_be64 n_value = htonll(value);
> +    bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
> +    bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
> +}
> +
> +static void
> +put_resubmit(uint8_t table_id, struct ofpbuf *ofpacts)
> +{
> +    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(ofpacts);
> +    resubmit->in_port = OFPP_IN_PORT;
> +    resubmit->table_id = table_id;
> +}
> +
> +static void
> +add_interface_flows(const char *bridge,
> +                    const struct ovsrec_interface *iface_rec,
> +                    int64_t ofport, struct ofpbuf *ofpacts_p)
> +{
> +    struct match match = MATCH_CATCHALL_INITIALIZER;
> +    match_set_in_port(&match, u16_to_ofp(ofport));
> +
> +    ofpbuf_clear(ofpacts_p);
> +    put_load(ofport, MFF_LOG_INPORT, 0, 32, ofpacts_p);
> +
> +    uint32_t iface_metadata =
> +        smap_get_uint(&iface_rec->external_ids, "ovn-iface-metadata", 0);
> +    if (iface_metadata) {
> +        put_load(iface_metadata, MFF_METADATA, 0, 32, ofpacts_p);
> +    }
> +
> +    put_resubmit(BR_OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_PHY_TO_LOG, 100,
> +                               iface_rec->header_.uuid.parts[0],
> +                               &match, ofpacts_p,
> +                               &iface_rec->header_.uuid);
> +
> +
> +    match_init_catchall(&match);
> +    match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, ofport);
> +
> +    ofpbuf_clear(ofpacts_p);
> +    ofpact_put_OUTPUT(ofpacts_p)->port = u16_to_ofp(ofport);
> +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_LOG_TO_PHY, 100,
> +                               iface_rec->header_.uuid.parts[0],
> +                               &match, ofpacts_p,
> +                               &iface_rec->header_.uuid);
> +}
> diff --git a/br-controller/en-pflow.h b/br-controller/en-pflow.h
> new file mode 100644
> index 0000000000..85edf53c17
> --- /dev/null
> +++ b/br-controller/en-pflow.h
> @@ -0,0 +1,16 @@
> +#ifndef EN_PFLOW_H
> +#define EN_PFLOW_H 1
> +
> +#include <config.h>
> +
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "lib/inc-proc-eng.h"
> +
> +enum engine_node_state en_pflow_output_run(struct engine_node *, void *data);
> +void *en_pflow_output_init(struct engine_node *node, struct engine_arg *arg);
> +void en_pflow_output_cleanup(void *data);
> +
> +#endif /* EN_PFLOW_H */
> diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
> index 7cfe6ac23d..ae0e192429 100644
> --- a/br-controller/ovn-br-controller.c
> +++ b/br-controller/ovn-br-controller.c
> @@ -37,6 +37,7 @@
>   /* OVN includes. */
>   #include "en-bridge-data.h"
>   #include "en-lflow.h"
> +#include "en-pflow.h"
>   #include "lib/ovn-br-idl.h"
>   #include "lib/inc-proc-eng.h"
>   #include "lib/ovn-util.h"
> @@ -171,6 +172,7 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>       /* Define inc-proc-engine nodes. */
>       ENGINE_NODE(bridge_data);
>       ENGINE_NODE(lflow_output);
> +    ENGINE_NODE(pflow_output);
>       ENGINE_NODE(br_controller_output);
>   
>   #define BRCTL_NODE(NAME) ENGINE_NODE_BR(NAME);
> @@ -191,7 +193,11 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
>   
>       engine_add_input(&en_lflow_output, &en_bridge_data, NULL);
>       engine_add_input(&en_lflow_output, &en_ovnbr_logical_flow, NULL);
> +
> +    engine_add_input(&en_pflow_output, &en_bridge_data, NULL);
> +
>       engine_add_input(&en_br_controller_output, &en_lflow_output, NULL);
> +    engine_add_input(&en_br_controller_output, &en_pflow_output, NULL);
>   
>       struct engine_arg engine_arg = {
>           .ovs_idl = ovs_idl_loop.idl,
Numan Siddique Oct. 8, 2025, 3:23 p.m. UTC | #2
On Thu, Aug 21, 2025 at 5:24 PM Mark Michelson <mmichels@redhat.com> wrote:
>
> On 8/11/25 6:10 AM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
> > ---
> >   br-controller/automake.mk         |   2 +
> >   br-controller/en-lflow.h          |   6 +
> >   br-controller/en-pflow.c          | 192 ++++++++++++++++++++++++++++++
> >   br-controller/en-pflow.h          |  16 +++
> >   br-controller/ovn-br-controller.c |   6 +
> >   5 files changed, 222 insertions(+)
> >   create mode 100644 br-controller/en-pflow.c
> >   create mode 100644 br-controller/en-pflow.h
> >
> > diff --git a/br-controller/automake.mk b/br-controller/automake.mk
> > index ea515a85e4..4baea4f6fe 100644
> > --- a/br-controller/automake.mk
> > +++ b/br-controller/automake.mk
> > @@ -6,6 +6,8 @@ br_controller_ovn_br_controller_SOURCES = \
> >       br-controller/en-bridge-data.h \
> >       br-controller/en-lflow.c \
> >       br-controller/en-lflow.h \
> > +     br-controller/en-pflow.c \
> > +     br-controller/en-pflow.h \
> >       br-controller/ovn-br-controller.c
> >
> >   br_controller_ovn_br_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
> > diff --git a/br-controller/en-lflow.h b/br-controller/en-lflow.h
> > index 39daa30174..093c2849dd 100644
> > --- a/br-controller/en-lflow.h
> > +++ b/br-controller/en-lflow.h
> > @@ -12,6 +12,12 @@
> >   #define BR_OFTABLE_LOG_INGRESS_PIPELINE      8
> >   #define BR_OFTABLE_SAVE_INPORT               64
> >
> > +/* OpenFlow table numbers. */
> > +#define BR_OFTABLE_PHY_TO_LOG             0
> > +#define BR_OFTABLE_LOG_INGRESS_PIPELINE   8
> > +#define BR_OFTABLE_SAVE_INPORT           64
>
> It appears you are redefining BR_OFTABLE_LOG_INGRESS_PIPELINE and
> BR_OFTABLE_SAVE_INPORT.
>

Ack.  I'll address in v2.

> > +#define BR_OFTABLE_LOG_TO_PHY            65
> > +
> >   enum engine_node_state en_lflow_output_run(struct engine_node *, void *data);
> >   void *en_lflow_output_init(struct engine_node *node, struct engine_arg *arg);
> >   void en_lflow_output_cleanup(void *data);
> > diff --git a/br-controller/en-pflow.c b/br-controller/en-pflow.c
> > new file mode 100644
> > index 0000000000..5f4c5b914f
> > --- /dev/null
> > +++ b/br-controller/en-pflow.c
> > @@ -0,0 +1,192 @@
> > +/*
> > + * 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 <config.h>
> > +
> > +#include <getopt.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +
> > +/* OVS includes. */
> > +#include "lib/coverage.h"
> > +#include "lib/flow.h"
> > +#include "lib/simap.h"
> > +#include "lib/vswitch-idl.h"
> > +#include "openvswitch/match.h"
> > +#include "openvswitch/ofp-actions.h"
> > +#include "openvswitch/ofpbuf.h"
> > +#include "openvswitch/ofp-parse.h"
> > +
> > +#include "openvswitch/vlog.h"
> > +#include "openvswitch/uuid.h"
> > +
> > +/* OVN includes. */
> > +#include "br-flow-mgr.h"
> > +#include "en-lflow.h"
> > +#include "en-pflow.h"
> > +#include "en-bridge-data.h"
> > +#include "include/ovn/logical-fields.h"
> > +#include "lib/ovn-br-idl.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(en_pflow);
> > +
> > +
> > +static void pflow_run(const struct shash *ovn_bridges);
> > +static void physical_flows_add(const struct ovn_bridge *);
> > +
> > +static void put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> > +                     struct ofpbuf *);
> > +static void put_resubmit(uint8_t table_id, struct ofpbuf *);
> > +
> > +static void add_interface_flows(const char *bridge,
> > +                                const struct ovsrec_interface *iface_rec,
> > +                                int64_t, struct ofpbuf *);
> > +
> > +/* Public functions. */
> > +void *en_pflow_output_init(struct engine_node *node OVS_UNUSED,
> > +                           struct engine_arg *arg OVS_UNUSED)
> > +{
> > +    return NULL;
> > +}
> > +
> > +void en_pflow_output_cleanup(void *data_ OVS_UNUSED)
> > +{
> > +}
> > +
> > +enum engine_node_state
> > +en_pflow_output_run(struct engine_node *node, void *data_  OVS_UNUSED)
> > +{
> > +    struct ed_type_bridge_data *br_data =
> > +        engine_get_input_data("bridge_data", node);
> > +
> > +    pflow_run(&br_data->bridges);
> > +
> > +    return EN_UPDATED;
> > +}
> > +
> > +/* Static functions. */
> > +static void
> > +pflow_run(const struct shash *ovn_bridges)
> > +{
> > +    br_flow_switch_physical_oflow_tables();
> > +    struct shash_node *shash_node;
> > +    SHASH_FOR_EACH (shash_node, ovn_bridges) {
> > +        physical_flows_add(shash_node->data);
> > +    }
> > +}
> > +
> > +static void
> > +physical_flows_add(const struct ovn_bridge *br)
> > +{
> > +    if (!br->ovs_br) {
> > +        return;
> > +    }
> > +
> > +    struct ofpbuf ofpacts;
> > +    ofpbuf_init(&ofpacts, 0);
> > +
> > +    struct match match = MATCH_CATCHALL_INITIALIZER;
> > +
> > +    /* Table 0 and 65, priority 0, actions=NORMAL
> > +     * ===================================
> > +     *
> > +     */
> > +    ofpact_put_OUTPUT(&ofpacts)->port = OFPP_NORMAL;
> > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_PHY_TO_LOG, 0,
> > +                               br->ovs_br->header_.uuid.parts[0],
> > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_LOG_TO_PHY, 0,
> > +                               br->ovs_br->header_.uuid.parts[0],
> > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > +
> > +    ofpbuf_clear(&ofpacts);
> > +    put_resubmit(BR_OFTABLE_LOG_TO_PHY, &ofpacts);
> > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_SAVE_INPORT, 0,
> > +                               br->ovs_br->header_.uuid.parts[0],
> > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > +
> > +    for (size_t i = 0; i < br->ovs_br->n_ports; i++) {
> > +        const struct ovsrec_port *port_rec = br->ovs_br->ports[i];
> > +
> > +        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
> > +            const struct ovsrec_interface *iface_rec;
> > +
> > +            iface_rec = port_rec->interfaces[j];
> > +            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> > +            if (ofport > 0 && ofport != ofp_to_u16(OFPP_LOCAL) &&
> > +                smap_get_bool(&iface_rec->external_ids,
> > +                              "ovnbr-managed", true)) {
>
> I skimmed but couldn't find any reference documentation for
> ovnbr-managed. It appears to be that if the option is not present or if
> it is set to "true" then the port can be managed by ovn-br-controller.
> Otherwise, the port is unknown to ovn-br-controller.
>
> Should this instead default to "false" and require opting-in to be
> managed by ovn-br-controller?

Thanks for pointing out the missing documentation.  I'll add it in v2.

My idea was that if a bridge is managed by ovn-bridge-controller, then
it owns all
the ports in it.  CMS can opt out certain ports if it wishes.

Shall I change the name to "ovnbr-exclude" (or a better name if
you've) and if set to true, the port will be excluded.

Let me know wdyt.

Thanks
Numan


>
> > +                add_interface_flows(br->db_br->name, iface_rec,
> > +                                    ofport, &ofpacts);
> > +            }
> > +        }
> > +    }
> > +
> > +    ofpbuf_uninit(&ofpacts);
> > +}
> > +
> > +static void
> > +put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> > +         struct ofpbuf *ofpacts)
> > +{
> > +    struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
> > +                                                       mf_from_id(dst), NULL,
> > +                                                       NULL);
> > +    ovs_be64 n_value = htonll(value);
> > +    bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
> > +    bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
> > +}
> > +
> > +static void
> > +put_resubmit(uint8_t table_id, struct ofpbuf *ofpacts)
> > +{
> > +    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(ofpacts);
> > +    resubmit->in_port = OFPP_IN_PORT;
> > +    resubmit->table_id = table_id;
> > +}
> > +
> > +static void
> > +add_interface_flows(const char *bridge,
> > +                    const struct ovsrec_interface *iface_rec,
> > +                    int64_t ofport, struct ofpbuf *ofpacts_p)
> > +{
> > +    struct match match = MATCH_CATCHALL_INITIALIZER;
> > +    match_set_in_port(&match, u16_to_ofp(ofport));
> > +
> > +    ofpbuf_clear(ofpacts_p);
> > +    put_load(ofport, MFF_LOG_INPORT, 0, 32, ofpacts_p);
> > +
> > +    uint32_t iface_metadata =
> > +        smap_get_uint(&iface_rec->external_ids, "ovn-iface-metadata", 0);
> > +    if (iface_metadata) {
> > +        put_load(iface_metadata, MFF_METADATA, 0, 32, ofpacts_p);
> > +    }
> > +
> > +    put_resubmit(BR_OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> > +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_PHY_TO_LOG, 100,
> > +                               iface_rec->header_.uuid.parts[0],
> > +                               &match, ofpacts_p,
> > +                               &iface_rec->header_.uuid);
> > +
> > +
> > +    match_init_catchall(&match);
> > +    match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, ofport);
> > +
> > +    ofpbuf_clear(ofpacts_p);
> > +    ofpact_put_OUTPUT(ofpacts_p)->port = u16_to_ofp(ofport);
> > +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_LOG_TO_PHY, 100,
> > +                               iface_rec->header_.uuid.parts[0],
> > +                               &match, ofpacts_p,
> > +                               &iface_rec->header_.uuid);
> > +}
> > diff --git a/br-controller/en-pflow.h b/br-controller/en-pflow.h
> > new file mode 100644
> > index 0000000000..85edf53c17
> > --- /dev/null
> > +++ b/br-controller/en-pflow.h
> > @@ -0,0 +1,16 @@
> > +#ifndef EN_PFLOW_H
> > +#define EN_PFLOW_H 1
> > +
> > +#include <config.h>
> > +
> > +#include <getopt.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +
> > +#include "lib/inc-proc-eng.h"
> > +
> > +enum engine_node_state en_pflow_output_run(struct engine_node *, void *data);
> > +void *en_pflow_output_init(struct engine_node *node, struct engine_arg *arg);
> > +void en_pflow_output_cleanup(void *data);
> > +
> > +#endif /* EN_PFLOW_H */
> > diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
> > index 7cfe6ac23d..ae0e192429 100644
> > --- a/br-controller/ovn-br-controller.c
> > +++ b/br-controller/ovn-br-controller.c
> > @@ -37,6 +37,7 @@
> >   /* OVN includes. */
> >   #include "en-bridge-data.h"
> >   #include "en-lflow.h"
> > +#include "en-pflow.h"
> >   #include "lib/ovn-br-idl.h"
> >   #include "lib/inc-proc-eng.h"
> >   #include "lib/ovn-util.h"
> > @@ -171,6 +172,7 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >       /* Define inc-proc-engine nodes. */
> >       ENGINE_NODE(bridge_data);
> >       ENGINE_NODE(lflow_output);
> > +    ENGINE_NODE(pflow_output);
> >       ENGINE_NODE(br_controller_output);
> >
> >   #define BRCTL_NODE(NAME) ENGINE_NODE_BR(NAME);
> > @@ -191,7 +193,11 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> >
> >       engine_add_input(&en_lflow_output, &en_bridge_data, NULL);
> >       engine_add_input(&en_lflow_output, &en_ovnbr_logical_flow, NULL);
> > +
> > +    engine_add_input(&en_pflow_output, &en_bridge_data, NULL);
> > +
> >       engine_add_input(&en_br_controller_output, &en_lflow_output, NULL);
> > +    engine_add_input(&en_br_controller_output, &en_pflow_output, NULL);
> >
> >       struct engine_arg engine_arg = {
> >           .ovs_idl = ovs_idl_loop.idl,
>
Mark Michelson Oct. 8, 2025, 6:04 p.m. UTC | #3
On Wed, Oct 8, 2025 at 11:23 AM Numan Siddique <numans@ovn.org> wrote:
>
> On Thu, Aug 21, 2025 at 5:24 PM Mark Michelson <mmichels@redhat.com> wrote:
> >
> > On 8/11/25 6:10 AM, numans@ovn.org wrote:
> > > From: Numan Siddique <numans@ovn.org>
> > >
> > > Signed-off-by: Numan Siddique <numans@ovn.org>
> > > ---
> > >   br-controller/automake.mk         |   2 +
> > >   br-controller/en-lflow.h          |   6 +
> > >   br-controller/en-pflow.c          | 192 ++++++++++++++++++++++++++++++
> > >   br-controller/en-pflow.h          |  16 +++
> > >   br-controller/ovn-br-controller.c |   6 +
> > >   5 files changed, 222 insertions(+)
> > >   create mode 100644 br-controller/en-pflow.c
> > >   create mode 100644 br-controller/en-pflow.h
> > >
> > > diff --git a/br-controller/automake.mk b/br-controller/automake.mk
> > > index ea515a85e4..4baea4f6fe 100644
> > > --- a/br-controller/automake.mk
> > > +++ b/br-controller/automake.mk
> > > @@ -6,6 +6,8 @@ br_controller_ovn_br_controller_SOURCES = \
> > >       br-controller/en-bridge-data.h \
> > >       br-controller/en-lflow.c \
> > >       br-controller/en-lflow.h \
> > > +     br-controller/en-pflow.c \
> > > +     br-controller/en-pflow.h \
> > >       br-controller/ovn-br-controller.c
> > >
> > >   br_controller_ovn_br_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
> > > diff --git a/br-controller/en-lflow.h b/br-controller/en-lflow.h
> > > index 39daa30174..093c2849dd 100644
> > > --- a/br-controller/en-lflow.h
> > > +++ b/br-controller/en-lflow.h
> > > @@ -12,6 +12,12 @@
> > >   #define BR_OFTABLE_LOG_INGRESS_PIPELINE      8
> > >   #define BR_OFTABLE_SAVE_INPORT               64
> > >
> > > +/* OpenFlow table numbers. */
> > > +#define BR_OFTABLE_PHY_TO_LOG             0
> > > +#define BR_OFTABLE_LOG_INGRESS_PIPELINE   8
> > > +#define BR_OFTABLE_SAVE_INPORT           64
> >
> > It appears you are redefining BR_OFTABLE_LOG_INGRESS_PIPELINE and
> > BR_OFTABLE_SAVE_INPORT.
> >
>
> Ack.  I'll address in v2.
>
> > > +#define BR_OFTABLE_LOG_TO_PHY            65
> > > +
> > >   enum engine_node_state en_lflow_output_run(struct engine_node *, void *data);
> > >   void *en_lflow_output_init(struct engine_node *node, struct engine_arg *arg);
> > >   void en_lflow_output_cleanup(void *data);
> > > diff --git a/br-controller/en-pflow.c b/br-controller/en-pflow.c
> > > new file mode 100644
> > > index 0000000000..5f4c5b914f
> > > --- /dev/null
> > > +++ b/br-controller/en-pflow.c
> > > @@ -0,0 +1,192 @@
> > > +/*
> > > + * 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 <config.h>
> > > +
> > > +#include <getopt.h>
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +
> > > +/* OVS includes. */
> > > +#include "lib/coverage.h"
> > > +#include "lib/flow.h"
> > > +#include "lib/simap.h"
> > > +#include "lib/vswitch-idl.h"
> > > +#include "openvswitch/match.h"
> > > +#include "openvswitch/ofp-actions.h"
> > > +#include "openvswitch/ofpbuf.h"
> > > +#include "openvswitch/ofp-parse.h"
> > > +
> > > +#include "openvswitch/vlog.h"
> > > +#include "openvswitch/uuid.h"
> > > +
> > > +/* OVN includes. */
> > > +#include "br-flow-mgr.h"
> > > +#include "en-lflow.h"
> > > +#include "en-pflow.h"
> > > +#include "en-bridge-data.h"
> > > +#include "include/ovn/logical-fields.h"
> > > +#include "lib/ovn-br-idl.h"
> > > +
> > > +VLOG_DEFINE_THIS_MODULE(en_pflow);
> > > +
> > > +
> > > +static void pflow_run(const struct shash *ovn_bridges);
> > > +static void physical_flows_add(const struct ovn_bridge *);
> > > +
> > > +static void put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> > > +                     struct ofpbuf *);
> > > +static void put_resubmit(uint8_t table_id, struct ofpbuf *);
> > > +
> > > +static void add_interface_flows(const char *bridge,
> > > +                                const struct ovsrec_interface *iface_rec,
> > > +                                int64_t, struct ofpbuf *);
> > > +
> > > +/* Public functions. */
> > > +void *en_pflow_output_init(struct engine_node *node OVS_UNUSED,
> > > +                           struct engine_arg *arg OVS_UNUSED)
> > > +{
> > > +    return NULL;
> > > +}
> > > +
> > > +void en_pflow_output_cleanup(void *data_ OVS_UNUSED)
> > > +{
> > > +}
> > > +
> > > +enum engine_node_state
> > > +en_pflow_output_run(struct engine_node *node, void *data_  OVS_UNUSED)
> > > +{
> > > +    struct ed_type_bridge_data *br_data =
> > > +        engine_get_input_data("bridge_data", node);
> > > +
> > > +    pflow_run(&br_data->bridges);
> > > +
> > > +    return EN_UPDATED;
> > > +}
> > > +
> > > +/* Static functions. */
> > > +static void
> > > +pflow_run(const struct shash *ovn_bridges)
> > > +{
> > > +    br_flow_switch_physical_oflow_tables();
> > > +    struct shash_node *shash_node;
> > > +    SHASH_FOR_EACH (shash_node, ovn_bridges) {
> > > +        physical_flows_add(shash_node->data);
> > > +    }
> > > +}
> > > +
> > > +static void
> > > +physical_flows_add(const struct ovn_bridge *br)
> > > +{
> > > +    if (!br->ovs_br) {
> > > +        return;
> > > +    }
> > > +
> > > +    struct ofpbuf ofpacts;
> > > +    ofpbuf_init(&ofpacts, 0);
> > > +
> > > +    struct match match = MATCH_CATCHALL_INITIALIZER;
> > > +
> > > +    /* Table 0 and 65, priority 0, actions=NORMAL
> > > +     * ===================================
> > > +     *
> > > +     */
> > > +    ofpact_put_OUTPUT(&ofpacts)->port = OFPP_NORMAL;
> > > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_PHY_TO_LOG, 0,
> > > +                               br->ovs_br->header_.uuid.parts[0],
> > > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_LOG_TO_PHY, 0,
> > > +                               br->ovs_br->header_.uuid.parts[0],
> > > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > > +
> > > +    ofpbuf_clear(&ofpacts);
> > > +    put_resubmit(BR_OFTABLE_LOG_TO_PHY, &ofpacts);
> > > +    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_SAVE_INPORT, 0,
> > > +                               br->ovs_br->header_.uuid.parts[0],
> > > +                               &match, &ofpacts, &br->ovs_br->header_.uuid);
> > > +
> > > +    for (size_t i = 0; i < br->ovs_br->n_ports; i++) {
> > > +        const struct ovsrec_port *port_rec = br->ovs_br->ports[i];
> > > +
> > > +        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
> > > +            const struct ovsrec_interface *iface_rec;
> > > +
> > > +            iface_rec = port_rec->interfaces[j];
> > > +            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
> > > +            if (ofport > 0 && ofport != ofp_to_u16(OFPP_LOCAL) &&
> > > +                smap_get_bool(&iface_rec->external_ids,
> > > +                              "ovnbr-managed", true)) {
> >
> > I skimmed but couldn't find any reference documentation for
> > ovnbr-managed. It appears to be that if the option is not present or if
> > it is set to "true" then the port can be managed by ovn-br-controller.
> > Otherwise, the port is unknown to ovn-br-controller.
> >
> > Should this instead default to "false" and require opting-in to be
> > managed by ovn-br-controller?
>
> Thanks for pointing out the missing documentation.  I'll add it in v2.
>
> My idea was that if a bridge is managed by ovn-bridge-controller, then
> it owns all
> the ports in it.  CMS can opt out certain ports if it wishes.
>
> Shall I change the name to "ovnbr-exclude" (or a better name if
> you've) and if set to true, the port will be excluded.
>
> Let me know wdyt.
>
> Thanks
> Numan

Your reasoning makes sense to me, Numan. I think documentation of the
ovnbr-managed option would help to make it more clear what the
rationale is.

>
>
> >
> > > +                add_interface_flows(br->db_br->name, iface_rec,
> > > +                                    ofport, &ofpacts);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    ofpbuf_uninit(&ofpacts);
> > > +}
> > > +
> > > +static void
> > > +put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
> > > +         struct ofpbuf *ofpacts)
> > > +{
> > > +    struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
> > > +                                                       mf_from_id(dst), NULL,
> > > +                                                       NULL);
> > > +    ovs_be64 n_value = htonll(value);
> > > +    bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
> > > +    bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
> > > +}
> > > +
> > > +static void
> > > +put_resubmit(uint8_t table_id, struct ofpbuf *ofpacts)
> > > +{
> > > +    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(ofpacts);
> > > +    resubmit->in_port = OFPP_IN_PORT;
> > > +    resubmit->table_id = table_id;
> > > +}
> > > +
> > > +static void
> > > +add_interface_flows(const char *bridge,
> > > +                    const struct ovsrec_interface *iface_rec,
> > > +                    int64_t ofport, struct ofpbuf *ofpacts_p)
> > > +{
> > > +    struct match match = MATCH_CATCHALL_INITIALIZER;
> > > +    match_set_in_port(&match, u16_to_ofp(ofport));
> > > +
> > > +    ofpbuf_clear(ofpacts_p);
> > > +    put_load(ofport, MFF_LOG_INPORT, 0, 32, ofpacts_p);
> > > +
> > > +    uint32_t iface_metadata =
> > > +        smap_get_uint(&iface_rec->external_ids, "ovn-iface-metadata", 0);
> > > +    if (iface_metadata) {
> > > +        put_load(iface_metadata, MFF_METADATA, 0, 32, ofpacts_p);
> > > +    }
> > > +
> > > +    put_resubmit(BR_OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
> > > +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_PHY_TO_LOG, 100,
> > > +                               iface_rec->header_.uuid.parts[0],
> > > +                               &match, ofpacts_p,
> > > +                               &iface_rec->header_.uuid);
> > > +
> > > +
> > > +    match_init_catchall(&match);
> > > +    match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, ofport);
> > > +
> > > +    ofpbuf_clear(ofpacts_p);
> > > +    ofpact_put_OUTPUT(ofpacts_p)->port = u16_to_ofp(ofport);
> > > +    br_flow_add_physical_oflow(bridge, BR_OFTABLE_LOG_TO_PHY, 100,
> > > +                               iface_rec->header_.uuid.parts[0],
> > > +                               &match, ofpacts_p,
> > > +                               &iface_rec->header_.uuid);
> > > +}
> > > diff --git a/br-controller/en-pflow.h b/br-controller/en-pflow.h
> > > new file mode 100644
> > > index 0000000000..85edf53c17
> > > --- /dev/null
> > > +++ b/br-controller/en-pflow.h
> > > @@ -0,0 +1,16 @@
> > > +#ifndef EN_PFLOW_H
> > > +#define EN_PFLOW_H 1
> > > +
> > > +#include <config.h>
> > > +
> > > +#include <getopt.h>
> > > +#include <stdlib.h>
> > > +#include <stdio.h>
> > > +
> > > +#include "lib/inc-proc-eng.h"
> > > +
> > > +enum engine_node_state en_pflow_output_run(struct engine_node *, void *data);
> > > +void *en_pflow_output_init(struct engine_node *node, struct engine_arg *arg);
> > > +void en_pflow_output_cleanup(void *data);
> > > +
> > > +#endif /* EN_PFLOW_H */
> > > diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
> > > index 7cfe6ac23d..ae0e192429 100644
> > > --- a/br-controller/ovn-br-controller.c
> > > +++ b/br-controller/ovn-br-controller.c
> > > @@ -37,6 +37,7 @@
> > >   /* OVN includes. */
> > >   #include "en-bridge-data.h"
> > >   #include "en-lflow.h"
> > > +#include "en-pflow.h"
> > >   #include "lib/ovn-br-idl.h"
> > >   #include "lib/inc-proc-eng.h"
> > >   #include "lib/ovn-util.h"
> > > @@ -171,6 +172,7 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> > >       /* Define inc-proc-engine nodes. */
> > >       ENGINE_NODE(bridge_data);
> > >       ENGINE_NODE(lflow_output);
> > > +    ENGINE_NODE(pflow_output);
> > >       ENGINE_NODE(br_controller_output);
> > >
> > >   #define BRCTL_NODE(NAME) ENGINE_NODE_BR(NAME);
> > > @@ -191,7 +193,11 @@ main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
> > >
> > >       engine_add_input(&en_lflow_output, &en_bridge_data, NULL);
> > >       engine_add_input(&en_lflow_output, &en_ovnbr_logical_flow, NULL);
> > > +
> > > +    engine_add_input(&en_pflow_output, &en_bridge_data, NULL);
> > > +
> > >       engine_add_input(&en_br_controller_output, &en_lflow_output, NULL);
> > > +    engine_add_input(&en_br_controller_output, &en_pflow_output, NULL);
> > >
> > >       struct engine_arg engine_arg = {
> > >           .ovs_idl = ovs_idl_loop.idl,
> >
>
diff mbox series

Patch

diff --git a/br-controller/automake.mk b/br-controller/automake.mk
index ea515a85e4..4baea4f6fe 100644
--- a/br-controller/automake.mk
+++ b/br-controller/automake.mk
@@ -6,6 +6,8 @@  br_controller_ovn_br_controller_SOURCES = \
 	br-controller/en-bridge-data.h \
 	br-controller/en-lflow.c \
 	br-controller/en-lflow.h \
+	br-controller/en-pflow.c \
+	br-controller/en-pflow.h \
 	br-controller/ovn-br-controller.c
 
 br_controller_ovn_br_controller_LDADD = lib/libovn.la $(OVS_LIBDIR)/libopenvswitch.la
diff --git a/br-controller/en-lflow.h b/br-controller/en-lflow.h
index 39daa30174..093c2849dd 100644
--- a/br-controller/en-lflow.h
+++ b/br-controller/en-lflow.h
@@ -12,6 +12,12 @@ 
 #define BR_OFTABLE_LOG_INGRESS_PIPELINE      8
 #define BR_OFTABLE_SAVE_INPORT               64
 
+/* OpenFlow table numbers. */
+#define BR_OFTABLE_PHY_TO_LOG             0
+#define BR_OFTABLE_LOG_INGRESS_PIPELINE   8
+#define BR_OFTABLE_SAVE_INPORT           64
+#define BR_OFTABLE_LOG_TO_PHY            65
+
 enum engine_node_state en_lflow_output_run(struct engine_node *, void *data);
 void *en_lflow_output_init(struct engine_node *node, struct engine_arg *arg);
 void en_lflow_output_cleanup(void *data);
diff --git a/br-controller/en-pflow.c b/br-controller/en-pflow.c
new file mode 100644
index 0000000000..5f4c5b914f
--- /dev/null
+++ b/br-controller/en-pflow.c
@@ -0,0 +1,192 @@ 
+/*
+ * 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 <config.h>
+
+#include <getopt.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+/* OVS includes. */
+#include "lib/coverage.h"
+#include "lib/flow.h"
+#include "lib/simap.h"
+#include "lib/vswitch-idl.h"
+#include "openvswitch/match.h"
+#include "openvswitch/ofp-actions.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/ofp-parse.h"
+
+#include "openvswitch/vlog.h"
+#include "openvswitch/uuid.h"
+
+/* OVN includes. */
+#include "br-flow-mgr.h"
+#include "en-lflow.h"
+#include "en-pflow.h"
+#include "en-bridge-data.h"
+#include "include/ovn/logical-fields.h"
+#include "lib/ovn-br-idl.h"
+
+VLOG_DEFINE_THIS_MODULE(en_pflow);
+
+
+static void pflow_run(const struct shash *ovn_bridges);
+static void physical_flows_add(const struct ovn_bridge *);
+
+static void put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
+                     struct ofpbuf *);
+static void put_resubmit(uint8_t table_id, struct ofpbuf *);
+
+static void add_interface_flows(const char *bridge,
+                                const struct ovsrec_interface *iface_rec,
+                                int64_t, struct ofpbuf *);
+
+/* Public functions. */
+void *en_pflow_output_init(struct engine_node *node OVS_UNUSED,
+                           struct engine_arg *arg OVS_UNUSED)
+{
+    return NULL;
+}
+
+void en_pflow_output_cleanup(void *data_ OVS_UNUSED)
+{
+}
+
+enum engine_node_state
+en_pflow_output_run(struct engine_node *node, void *data_  OVS_UNUSED)
+{
+    struct ed_type_bridge_data *br_data =
+        engine_get_input_data("bridge_data", node);
+
+    pflow_run(&br_data->bridges);
+
+    return EN_UPDATED;
+}
+
+/* Static functions. */
+static void
+pflow_run(const struct shash *ovn_bridges)
+{
+    br_flow_switch_physical_oflow_tables();
+    struct shash_node *shash_node;
+    SHASH_FOR_EACH (shash_node, ovn_bridges) {
+        physical_flows_add(shash_node->data);
+    }
+}
+
+static void
+physical_flows_add(const struct ovn_bridge *br)
+{
+    if (!br->ovs_br) {
+        return;
+    }
+
+    struct ofpbuf ofpacts;
+    ofpbuf_init(&ofpacts, 0);
+
+    struct match match = MATCH_CATCHALL_INITIALIZER;
+
+    /* Table 0 and 65, priority 0, actions=NORMAL
+     * ===================================
+     *
+     */
+    ofpact_put_OUTPUT(&ofpacts)->port = OFPP_NORMAL;
+    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_PHY_TO_LOG, 0,
+                               br->ovs_br->header_.uuid.parts[0],
+                               &match, &ofpacts, &br->ovs_br->header_.uuid);
+    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_LOG_TO_PHY, 0,
+                               br->ovs_br->header_.uuid.parts[0],
+                               &match, &ofpacts, &br->ovs_br->header_.uuid);
+
+    ofpbuf_clear(&ofpacts);
+    put_resubmit(BR_OFTABLE_LOG_TO_PHY, &ofpacts);
+    br_flow_add_physical_oflow(br->db_br->name, BR_OFTABLE_SAVE_INPORT, 0,
+                               br->ovs_br->header_.uuid.parts[0],
+                               &match, &ofpacts, &br->ovs_br->header_.uuid);
+
+    for (size_t i = 0; i < br->ovs_br->n_ports; i++) {
+        const struct ovsrec_port *port_rec = br->ovs_br->ports[i];
+
+        for (size_t j = 0; j < port_rec->n_interfaces; j++) {
+            const struct ovsrec_interface *iface_rec;
+
+            iface_rec = port_rec->interfaces[j];
+            int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
+            if (ofport > 0 && ofport != ofp_to_u16(OFPP_LOCAL) &&
+                smap_get_bool(&iface_rec->external_ids,
+                              "ovnbr-managed", true)) {
+                add_interface_flows(br->db_br->name, iface_rec,
+                                    ofport, &ofpacts);
+            }
+        }
+    }
+
+    ofpbuf_uninit(&ofpacts);
+}
+
+static void
+put_load(uint64_t value, enum mf_field_id dst, int ofs, int n_bits,
+         struct ofpbuf *ofpacts)
+{
+    struct ofpact_set_field *sf = ofpact_put_set_field(ofpacts,
+                                                       mf_from_id(dst), NULL,
+                                                       NULL);
+    ovs_be64 n_value = htonll(value);
+    bitwise_copy(&n_value, 8, 0, sf->value, sf->field->n_bytes, ofs, n_bits);
+    bitwise_one(ofpact_set_field_mask(sf), sf->field->n_bytes, ofs, n_bits);
+}
+
+static void
+put_resubmit(uint8_t table_id, struct ofpbuf *ofpacts)
+{
+    struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(ofpacts);
+    resubmit->in_port = OFPP_IN_PORT;
+    resubmit->table_id = table_id;
+}
+
+static void
+add_interface_flows(const char *bridge,
+                    const struct ovsrec_interface *iface_rec,
+                    int64_t ofport, struct ofpbuf *ofpacts_p)
+{
+    struct match match = MATCH_CATCHALL_INITIALIZER;
+    match_set_in_port(&match, u16_to_ofp(ofport));
+
+    ofpbuf_clear(ofpacts_p);
+    put_load(ofport, MFF_LOG_INPORT, 0, 32, ofpacts_p);
+
+    uint32_t iface_metadata =
+        smap_get_uint(&iface_rec->external_ids, "ovn-iface-metadata", 0);
+    if (iface_metadata) {
+        put_load(iface_metadata, MFF_METADATA, 0, 32, ofpacts_p);
+    }
+
+    put_resubmit(BR_OFTABLE_LOG_INGRESS_PIPELINE, ofpacts_p);
+    br_flow_add_physical_oflow(bridge, BR_OFTABLE_PHY_TO_LOG, 100,
+                               iface_rec->header_.uuid.parts[0],
+                               &match, ofpacts_p,
+                               &iface_rec->header_.uuid);
+
+
+    match_init_catchall(&match);
+    match_set_reg(&match, MFF_LOG_OUTPORT - MFF_REG0, ofport);
+
+    ofpbuf_clear(ofpacts_p);
+    ofpact_put_OUTPUT(ofpacts_p)->port = u16_to_ofp(ofport);
+    br_flow_add_physical_oflow(bridge, BR_OFTABLE_LOG_TO_PHY, 100,
+                               iface_rec->header_.uuid.parts[0],
+                               &match, ofpacts_p,
+                               &iface_rec->header_.uuid);
+}
diff --git a/br-controller/en-pflow.h b/br-controller/en-pflow.h
new file mode 100644
index 0000000000..85edf53c17
--- /dev/null
+++ b/br-controller/en-pflow.h
@@ -0,0 +1,16 @@ 
+#ifndef EN_PFLOW_H
+#define EN_PFLOW_H 1
+
+#include <config.h>
+
+#include <getopt.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "lib/inc-proc-eng.h"
+
+enum engine_node_state en_pflow_output_run(struct engine_node *, void *data);
+void *en_pflow_output_init(struct engine_node *node, struct engine_arg *arg);
+void en_pflow_output_cleanup(void *data);
+
+#endif /* EN_PFLOW_H */
diff --git a/br-controller/ovn-br-controller.c b/br-controller/ovn-br-controller.c
index 7cfe6ac23d..ae0e192429 100644
--- a/br-controller/ovn-br-controller.c
+++ b/br-controller/ovn-br-controller.c
@@ -37,6 +37,7 @@ 
 /* OVN includes. */
 #include "en-bridge-data.h"
 #include "en-lflow.h"
+#include "en-pflow.h"
 #include "lib/ovn-br-idl.h"
 #include "lib/inc-proc-eng.h"
 #include "lib/ovn-util.h"
@@ -171,6 +172,7 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
     /* Define inc-proc-engine nodes. */
     ENGINE_NODE(bridge_data);
     ENGINE_NODE(lflow_output);
+    ENGINE_NODE(pflow_output);
     ENGINE_NODE(br_controller_output);
 
 #define BRCTL_NODE(NAME) ENGINE_NODE_BR(NAME);
@@ -191,7 +193,11 @@  main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
 
     engine_add_input(&en_lflow_output, &en_bridge_data, NULL);
     engine_add_input(&en_lflow_output, &en_ovnbr_logical_flow, NULL);
+
+    engine_add_input(&en_pflow_output, &en_bridge_data, NULL);
+
     engine_add_input(&en_br_controller_output, &en_lflow_output, NULL);
+    engine_add_input(&en_br_controller_output, &en_pflow_output, NULL);
 
     struct engine_arg engine_arg = {
         .ovs_idl = ovs_idl_loop.idl,