[ovs-dev,1/2] mirror: Enhance port-mirroring to support local OVS port as target.

Today the mirror feature in OVN supports only tunnel to a remote
destinations. This patch adds the support for mirroring to a local OVS
port. It is particularly useful for monitoring traffic that is offloaded
thus not possible to be intercepted by regular tools such as tcpdump.
With this feature, traffic to/from a virtual function that is offloaded
to hardware can be mirrored to a dedicated (pre-configured) OVS port
(which can be another virtual function that is dedicated for
interception purposes).

Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 NEWS                      |   1 +
 controller/mirror.c       | 134 ++++++++++++++++++++++++++++++++------
 ovn-nb.ovsschema          |   7 +-
 ovn-nb.xml                |  12 +++-
 ovn-sb.ovsschema          |   9 +--
 ovn-sb.xml                |  17 +++--
 tests/ovn.at              |  97 ++++++++++++++++++++++++++-
 utilities/ovn-nbctl.8.xml |  14 ++--
 utilities/ovn-nbctl.c     |  64 ++++++++++--------
 9 files changed, 287 insertions(+), 68 deletions(-)

Bleep bloop.  Greetings Han Zhou, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#551 FILE: utilities/ovn-nbctl.c:276:
  mirror-add NAME TYPE [INDEX] FILTER {IP | MIRROR-ID} \n\

WARNING: Line lacks whitespace around operator
#562 FILE: utilities/ovn-nbctl.c:285:
                                local interface with external-ids:mirror-id\n\

WARNING: Line lacks whitespace around operator
#563 FILE: utilities/ovn-nbctl.c:286:
                                matching MIRROR-ID\n\

Lines checked: 693, Warnings: 3, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

Hi Han,

For both patches in the series,

Acked-by: Mark Michelson <mmichels@redhat.com>

I want to put a reply to patch 2 directly, but unfortunately I can't 
find it in my email. Sometimes patches get sent to spam, but in this 
case, it's not there either.

On 5/5/23 02:32, Han Zhou wrote:
> Today the mirror feature in OVN supports only tunnel to a remote
> destinations. This patch adds the support for mirroring to a local OVS
> port. It is particularly useful for monitoring traffic that is offloaded
> thus not possible to be intercepted by regular tools such as tcpdump.
> With this feature, traffic to/from a virtual function that is offloaded
> to hardware can be mirrored to a dedicated (pre-configured) OVS port
> (which can be another virtual function that is dedicated for
> interception purposes).
> 
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>   NEWS                      |   1 +
>   controller/mirror.c       | 134 ++++++++++++++++++++++++++++++++------
>   ovn-nb.ovsschema          |   7 +-
>   ovn-nb.xml                |  12 +++-
>   ovn-sb.ovsschema          |   9 +--
>   ovn-sb.xml                |  17 +++--
>   tests/ovn.at              |  97 ++++++++++++++++++++++++++-
>   utilities/ovn-nbctl.8.xml |  14 ++--
>   utilities/ovn-nbctl.c     |  64 ++++++++++--------
>   9 files changed, 287 insertions(+), 68 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 60467581a1ba..f49d4049c332 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -14,6 +14,7 @@ Post v23.03.0
>       existing behaviour of flooding these arp requests to all attached Ports.
>     - Always allow IPv6 Router Discovery, Neighbor Discovery, and Multicast
>       Listener Discovery protocols, regardless of ACLs defined.
> +  - Support using local OVS port as port-mirroring target.
>   
>   OVN v23.03.0 - 03 Mar 2023
>   --------------------------
> diff --git a/controller/mirror.c b/controller/mirror.c
> index 6657369665fe..fd36c7650c41 100644
> --- a/controller/mirror.c
> +++ b/controller/mirror.c
> @@ -54,10 +54,12 @@ static struct ovn_mirror *ovn_mirror_find(struct shash *ovn_mirrors,
>   static void ovn_mirror_delete(struct ovn_mirror *);
>   static void ovn_mirror_add_lport(struct ovn_mirror *, struct local_binding *);
>   static void sync_ovn_mirror(struct ovn_mirror *, struct ovsdb_idl_txn *,
> -                            const struct ovsrec_bridge *);
> +                            const struct ovsrec_bridge *,
> +                            struct shash *ovs_mirror_ports);
>   
>   static void create_ovs_mirror(struct ovn_mirror *, struct ovsdb_idl_txn *,
> -                              const struct ovsrec_bridge *);
> +                              const struct ovsrec_bridge *,
> +                              struct shash *ovs_mirror_ports);
>   static void sync_ovs_mirror_ports(struct ovn_mirror *,
>                                     const struct ovsrec_bridge *);
>   static void delete_ovs_mirror(struct ovn_mirror *,
> @@ -69,6 +71,8 @@ static void set_mirror_iface_options(struct ovsrec_interface *,
>   static const struct ovsrec_port *get_iface_port(
>       const struct ovsrec_interface *, const struct ovsrec_bridge *);
>   
> +static void build_ovs_mirror_ports(const struct ovsrec_bridge *,
> +                                   struct shash *ovs_mirror_ports);
>   
>   void
>   mirror_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> @@ -105,7 +109,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
>       }
>   
>       struct shash ovn_mirrors = SHASH_INITIALIZER(&ovn_mirrors);
> -    struct shash tmp_mirrors = SHASH_INITIALIZER(&tmp_mirrors);
> +    struct shash ovs_local_mirror_ports =
> +        SHASH_INITIALIZER(&ovs_local_mirror_ports);
>   
>       /* Iterate through sb mirrors and build the 'ovn_mirrors'. */
>       const struct sbrec_mirror *sb_mirror;
> @@ -137,6 +142,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
>           return;
>       }
>   
> +    build_ovs_mirror_ports(br_int, &ovs_local_mirror_ports);
> +
>       /* Iterate through the local bindings and if the local binding's 'pb' has
>        * mirrors associated, add it to the ovn_mirror. */
>       struct shash_node *node;
> @@ -161,7 +168,7 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
>        * create/update or delete the ovsrec mirror(s). */
>        SHASH_FOR_EACH (node, &ovn_mirrors) {
>           struct ovn_mirror *m = node->data;
> -        sync_ovn_mirror(m, ovs_idl_txn, br_int);
> +        sync_ovn_mirror(m, ovs_idl_txn, br_int, &ovs_local_mirror_ports);
>        }
>   
>       SHASH_FOR_EACH_SAFE (node, &ovn_mirrors) {
> @@ -170,9 +177,52 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
>       }
>   
>       shash_destroy(&ovn_mirrors);
> +    shash_destroy(&ovs_local_mirror_ports);
>   }
>   
>   /* Static functions. */
> +
> +/* Builds mapping from mirror-id to ovsrec_port.
> + */
> +static void
> +build_ovs_mirror_ports(const struct ovsrec_bridge *br_int,
> +                       struct shash *ovs_mirror_ports)
> +{
> +    int i;
> +    for (i = 0; i < br_int->n_ports; i++) {
> +        const struct ovsrec_port *port_rec = br_int->ports[i];
> +        int j;
> +
> +        if (!strcmp(port_rec->name, br_int->name)) {
> +            continue;
> +        }
> +
> +        for (j = 0; j < port_rec->n_interfaces; j++) {
> +            const struct ovsrec_interface *iface_rec;
> +
> +            iface_rec = port_rec->interfaces[j];
> +            const char *mirror_id = smap_get(&iface_rec->external_ids,
> +                                             "mirror-id");
> +            if (mirror_id) {
> +                const struct ovsrec_port *p = shash_find_data(ovs_mirror_ports,
> +                                                              mirror_id);
> +                if (!p) {
> +                    shash_add(ovs_mirror_ports, mirror_id, port_rec);
> +                } else {
> +                    static struct vlog_rate_limit rl =
> +                        VLOG_RATE_LIMIT_INIT(1, 5);
> +                    VLOG_WARN_RL(
> +                        &rl,
> +                        "Invalid configuration: same mirror-id [%s] is "
> +                        "configured on interfaces of ports: [%s] and [%s]. "
> +                        "Ignoring the configuration on interface [%s]",
> +                        mirror_id, port_rec->name, p->name, iface_rec->name);
> +                }
> +            }
> +        }
> +    }
> +}
> +
>   static struct ovn_mirror *
>   ovn_mirror_create(char *mirror_name)
>   {
> @@ -266,7 +316,8 @@ check_and_update_interface_table(const struct sbrec_mirror *sb_mirror,
>   
>   static void
>   sync_ovn_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn *ovs_idl_txn,
> -                const struct ovsrec_bridge *br_int)
> +                const struct ovsrec_bridge *br_int,
> +                struct shash *ovs_mirror_ports)
>   {
>       if (should_delete_ovs_mirror(m)) {
>           /* Delete the ovsrec mirror. */
> @@ -281,9 +332,31 @@ sync_ovn_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn *ovs_idl_txn,
>       }
>   
>       if (m->sb_mirror && !m->ovs_mirror) {
> -        create_ovs_mirror(m, ovs_idl_txn, br_int);
> -    } else {
> +        create_ovs_mirror(m, ovs_idl_txn, br_int, ovs_mirror_ports);
> +        if (!m->ovs_mirror) {
> +            return;
> +        }
> +    } else if (strcmp(m->sb_mirror->type, "local")) {
>           check_and_update_interface_table(m->sb_mirror, m->ovs_mirror);
> +
> +        /* For upgradability, set the "ovn-owned" in case it was not set when
> +         * the port was created. */
> +        if (!smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> +                                   "ovn-owned", false)) {
> +            ovsrec_port_update_external_ids_setkey(m->ovs_mirror->output_port,
> +                                                   "ovn-owned", "true");
> +        }
> +    } else {
> +        /* type is local, check mirror-id */
> +        const struct ovsrec_port *mirror_port =
> +            shash_find_data(ovs_mirror_ports, m->sb_mirror->sink);
> +        if (!mirror_port) {
> +            delete_ovs_mirror(m, br_int);
> +            return;
> +        }
> +        if (mirror_port != m->ovs_mirror->output_port) {
> +            ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
> +        }
>       }
>   
>       sync_ovs_mirror_ports(m, br_int);
> @@ -321,25 +394,39 @@ get_iface_port(const struct ovsrec_interface *iface,
>   
>   static void
>   create_ovs_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn *ovs_idl_txn,
> -                  const struct ovsrec_bridge *br_int)
> +                  const struct ovsrec_bridge *br_int,
> +                  struct shash *ovs_mirror_ports)
>   {
> -    struct ovsrec_interface *iface = ovsrec_interface_insert(ovs_idl_txn);
> -    char *port_name = xasprintf("ovn-%s", m->name);
> +    const struct ovsrec_port *mirror_port;
> +    if (!strcmp(m->sb_mirror->type, "local")) {
> +        mirror_port = shash_find_data(ovs_mirror_ports, m->sb_mirror->sink);
> +        if (!mirror_port) {
> +            return;
> +        }
> +    } else {
> +        struct ovsrec_interface *iface = ovsrec_interface_insert(ovs_idl_txn);
> +        char *port_name = xasprintf("ovn-%s", m->name);
>   
> -    ovsrec_interface_set_name(iface, port_name);
> -    ovsrec_interface_set_type(iface, m->sb_mirror->type);
> -    set_mirror_iface_options(iface, m->sb_mirror);
> +        ovsrec_interface_set_name(iface, port_name);
> +        ovsrec_interface_set_type(iface, m->sb_mirror->type);
> +        set_mirror_iface_options(iface, m->sb_mirror);
>   
> -    struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> -    ovsrec_port_set_name(port, port_name);
> -    ovsrec_port_set_interfaces(port, &iface, 1);
> -    ovsrec_bridge_update_ports_addvalue(br_int, port);
> +        struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> +        ovsrec_port_set_name(port, port_name);
> +        ovsrec_port_set_interfaces(port, &iface, 1);
> +        ovsrec_bridge_update_ports_addvalue(br_int, port);
>   
> -    free(port_name);
> +        const struct smap port_external_ids =
> +            SMAP_CONST1(&port_external_ids, "ovn-owned", "true");
> +        ovsrec_port_set_external_ids(port, &port_external_ids);
> +
> +        free(port_name);
> +        mirror_port = port;
> +    }
>   
>       m->ovs_mirror = ovsrec_mirror_insert(ovs_idl_txn);
>       ovsrec_mirror_set_name(m->ovs_mirror, m->name);
> -    ovsrec_mirror_set_output_port(m->ovs_mirror, port);
> +    ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
>   
>       const struct smap external_ids =
>           SMAP_CONST1(&external_ids, "ovn-owned", "true");
> @@ -393,8 +480,13 @@ sync_ovs_mirror_ports(struct ovn_mirror *m, const struct ovsrec_bridge *br_int)
>   static void
>   delete_ovs_mirror(struct ovn_mirror *m, const struct ovsrec_bridge *br_int)
>   {
> -    ovsrec_bridge_update_ports_delvalue(br_int, m->ovs_mirror->output_port);
>       ovsrec_bridge_update_mirrors_delvalue(br_int, m->ovs_mirror);
> -    ovsrec_port_delete(m->ovs_mirror->output_port);
> +    bool ovn_owned = smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> +                                   "ovn-owned", false);
> +    if (ovn_owned) {
> +        ovsrec_bridge_update_ports_delvalue(br_int,
> +                                            m->ovs_mirror->output_port);
> +        ovsrec_port_delete(m->ovs_mirror->output_port);
> +    }
>       ovsrec_mirror_delete(m->ovs_mirror);
>   }
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index 4836a219f93d..dd5632562578 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
>   {
>       "name": "OVN_Northbound",
> -    "version": "7.0.0",
> -    "cksum": "94023179 33468",
> +    "version": "7.0.1",
> +    "cksum": "441625132 33536",
>       "tables": {
>           "NB_Global": {
>               "columns": {
> @@ -315,7 +315,8 @@
>                   "sink":{"type": "string"},
>                   "type": {"type": {"key": {"type": "string",
>                                             "enum": ["set", ["gre",
> -                                                           "erspan"]]}}},
> +                                                           "erspan",
> +                                                           "local"]]}}},
>                   "index": {"type": "integer"},
>                   "external_ids": {
>                       "type": {"key": "string", "value": "string",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 0552eff199a0..20b59df48f4e 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2693,14 +2693,19 @@ or
>       <column name="sink">
>         <p>
>           The value of this field represents the destination/sink of the mirror.
> -        The value it takes is an IP address of the sink port.
> +        If the <var>type</var> is <code>gre</code> or <code>erspan</code>,
> +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> +        For a <var>type</var> of <code>local</code>, this field defines a
> +        local interface on the OVS integration bridge to be used as the
> +        mirror destination. The interface must possess external-ids:mirror-id
> +        that matches this string.
>         </p>
>       </column>
>   
>       <column name="type">
>         <p>
> -        The value of this field represents the type of the tunnel used for
> -        sending the mirrored packets.
> +        The value of this field specifies the mirror type - <code>gre</code>,
> +        <code>erspan</code> or <code>local</code>.
>         </p>
>       </column>
>   
> @@ -2710,6 +2715,7 @@ or
>           tunnel type is <code>gre</code>, this field represents the
>           <code>GRE</code> key value and if the configured tunnel type is
>           <code>erspan</code> it represents the <code>erspan_idx</code> value.
> +        It is ignored if the type is <code>local</code>.
>         </p>
>       </column>
>   
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 79ba6841e53a..d5ac393f9210 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
>   {
>       "name": "OVN_Southbound",
> -    "version": "20.27.0",
> -    "cksum": "4078371916 30328",
> +    "version": "20.27.1",
> +    "cksum": "1439763681 30400",
>       "tables": {
>           "SB_Global": {
>               "columns": {
> @@ -151,8 +151,9 @@
>                                                         "to-lport"]]}}},
>                   "sink":{"type": "string"},
>                   "type": {"type": {"key": {"type": "string",
> -                                            "enum": ["set",
> -                                                     ["gre", "erspan"]]}}},
> +                                          "enum": ["set", ["gre",
> +                                                           "erspan",
> +                                                           "local"]]}}},
>                   "index": {"type": "integer"},
>                   "external_ids": {
>                       "type": {"key": "string", "value": "string",
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index a77f8f4efb38..9599e55f44e5 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -2907,20 +2907,29 @@ tcp.flags = RST;
>       <column name="sink">
>         <p>
>           The value of this field represents the destination/sink of the mirror.
> +        If the <var>type</var> is <code>gre</code> or <code>erspan</code>,
> +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> +        For a <var>type</var> of <code>local</code>, this field defines a
> +        local interface on the OVS integration bridge to be used as the
> +        mirror destination. The interface must possess external-ids:mirror-id
> +        that matches this string.
>         </p>
>       </column>
>   
>       <column name="type">
>         <p>
> -        The value of this field represents the type of the tunnel used for
> -        sending the mirrored packets
> +        The value of this field specifies the mirror type - <code>gre</code>,
> +        <code>erspan</code> or <code>local</code>.
>         </p>
>       </column>
>   
>       <column name="index">
>         <p>
> -        The value of this field represents the key/idx depending on the
> -        tunnel type configured
> +        The value of this field represents the tunnel ID. If the configured
> +        tunnel type is <code>gre</code>, this field represents the
> +        <code>GRE</code> key value and if the configured tunnel type is
> +        <code>erspan</code> it represents the <code>erspan_idx</code> value.
> +        It is ignored if the type is <code>local</code>.
>         </p>
>       </column>
>   
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 213ad18fa50b..4c124e0e58d9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -16427,7 +16427,7 @@ AT_CLEANUP
>   ])
>   
>   OVN_FOR_EACH_NORTHD([
> -AT_SETUP([Mirror])
> +AT_SETUP([Mirror - remote])
>   AT_KEYWORDS([Mirror])
>   ovn_start
>   
> @@ -16655,6 +16655,101 @@ OVN_CLEANUP([hv1], [hv2])
>   AT_CLEANUP
>   ])
>   
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([Mirror - local])
> +AT_KEYWORDS([Mirror])
> +ovn_start
> +
> +ovn-nbctl ls-add ls1
> +# Create logical port ls1-lp1 in ls1
> +ovn-nbctl lsp-add ls1 ls1-lp1 \
> +-- lsp-set-addresses ls1-lp1 "00:00:00:01:01:02 10.0.0.2"
> +ovn-nbctl lsp-add ls1 ls1-lp2 \
> +-- lsp-set-addresses ls1-lp2 "00:00:00:01:01:03 10.0.0.3"
> +
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys -- set bridge br-phys other-config:hwaddr=\"00:00:00:01:02:00\"
> +ovn_attach n1 br-phys 192.168.1.11
> +ovn-appctl -t ovn-controller vlog/set file:dbg
> +
> +ovs-vsctl -- add-port br-int vif1 -- \
> +    set interface vif1 external-ids:iface-id=ls1-lp1 \
> +    options:tx_pcap=hv1/vif1-tx.pcap \
> +    options:rxq_pcap=hv1/vif1-rx.pcap \
> +    ofport-request=1
> +ovs-vsctl -- add-port br-int vif2 -- \
> +    set interface vif2 external-ids:iface-id=ls1-lp2 \
> +    options:tx_pcap=hv1/vif2-tx.pcap \
> +    options:rxq_pcap=hv1/vif2-rx.pcap \
> +    ofport-request=2
> +
> +# Add two ports as mirroring target
> +ovs-vsctl -- add-port br-int mirror1 -- \
> +    set interface mirror1 external-ids:mirror-id=sink1 \
> +    options:tx_pcap=hv1/mirror1-tx.pcap \
> +    options:rxq_pcap=hv1/mirror1-rx.pcap \
> +    ofport-request=3
> +ovs-vsctl -- add-port br-int mirror2 -- \
> +    set interface mirror2 external-ids:mirror-id=sink2 \
> +    options:tx_pcap=hv1/mirror2-tx.pcap \
> +    options:rxq_pcap=hv1/mirror2-rx.pcap \
> +    ofport-request=4
> +
> +# Create a NB mirror use DB 'create' command.
> +uuid1=$(ovn-nbctl create mirror name=mirror-from-lp1 type=local sink=sink1 filter=from-lport)
> +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror-from-lp1
> +
> +# Create another NB mirror use 'mirror-add' command.
> +check ovn-nbctl mirror-add mirror-to-lp2 local to-lport sink2
> +check ovn-nbctl lsp-attach-mirror ls1-lp2 mirror-to-lp2
> +
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list mirror | grep mirror | sort], [0], [dnl
> +mirror-from-lp1
> +mirror-to-lp2
> +])
> +
> +# Update to wrong mirror-id, the mirror should be deleted.
> +check ovn-nbctl set mirror $uuid1 sink=xxx
> +check ovn-nbctl --wait=hv sync
> +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list mirror | grep mirror | sort], [0], [dnl
> +mirror-to-lp2
> +])
> +
> +# Change back, and the mirror should be created back
> +check ovn-nbctl set mirror $uuid1 sink=sink1
> +check ovn-nbctl --wait=hv sync
> +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list mirror | grep mirror | sort], [0], [dnl
> +mirror-from-lp1
> +mirror-to-lp2
> +])
> +
> +# Send a packet from lp1 to lp2, should be mirrored to both mirror ports.
> +packet="inport==\"ls1-lp1\" && eth.src==00:00:00:01:01:02 && eth.dst==00:00:00:01:01:03 &&
> +        ip4 && ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 && icmp"
> +AT_CHECK([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +exp_packet="eth.src==00:00:00:01:01:02 && eth.dst==00:00:00:01:01:03 && ip4 &&
> +            ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 && icmp"
> +echo $exp_packet | ovstest test-ovn expr-to-packets | sort > expout
> +
> +OVS_WAIT_UNTIL([
> +    rcv_mirror1=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/mirror1-tx.pcap > mirror1.packets && cat mirror1.packets | wc -l`
> +    rcv_mirror2=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" hv1/mirror2-tx.pcap > mirror2.packets && cat mirror2.packets | wc -l`
> +    echo $rcv_mirror1 $rcv_mirror2
> +    test $rcv_mirror1 -eq 1 -a $rcv_mirror2 -eq 1])
> +
> +AT_CHECK([cat mirror1.packets | sort], [0], [expout])
> +AT_CHECK([cat mirror2.packets | sort], [0], [expout])
> +
> +OVN_CLEANUP([hv1])
> +AT_CLEANUP
> +])
> +
>   OVN_FOR_EACH_NORTHD([
>   AT_SETUP([Mirror test bulk updates])
>   AT_KEYWORDS([Mirror test bulk updates])
> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> index 54dbdb791e52..c450b9a5ba7d 100644
> --- a/utilities/ovn-nbctl.8.xml
> +++ b/utilities/ovn-nbctl.8.xml
> @@ -1546,7 +1546,7 @@
>       <h2> Mirror commands</h2>
>       <dl>
>         <dt><code>mirror-add</code> <var>m</var> <var>type</var>
> -      <var>index</var> <var>filter</var> <var>dest</var></dt>
> +      [<var>index</var>] <var>filter</var> <var>dest</var></dt>
>         <dd>
>           <p>
>             Creates a new mirror in the <code>Mirror</code>
> @@ -1556,12 +1556,13 @@
>   
>           <p>
>             <var>type</var> specifies the mirror type - <code>gre</code>
> -          or <code>erspan</code>.
> +          , <code>erspan</code> or <code>local</code>.
>           </p>
>   
>           <p>
>             <var>index</var> specifies the tunnel index value (which is
> -          an integer).
> +          an integer) if the <var>type</var> is <code>gre</code> or
> +          <code>erspan</code>.
>           </p>
>   
>           <p>
> @@ -1570,7 +1571,12 @@
>           </p>
>   
>           <p>
> -          <var>dest</var> specifies the mirror destination IP (v4 or v6).
> +          <var>dest</var> specifies the mirror destination IP (v4 or v6)
> +          if the <var>type</var> is <code>gre</code> or <code>erspan</code>.
> +          For a <var>type</var> of <code>local</code>, this field defines a
> +          local interface on the OVS integration bridge to be used as the
> +          mirror destination. The interface must possess external-ids:mirror-id
> +          that matches this string.
>           </p>
>         </dd>
>   
> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> index 9399f9462bb1..5f2fbfecfe5b 100644
> --- a/utilities/ovn-nbctl.c
> +++ b/utilities/ovn-nbctl.c
> @@ -273,15 +273,17 @@ QoS commands:\n\
>     qos-list SWITCH           print QoS rules for SWITCH\n\
>   \n\
>   Mirror commands:\n\
> -  mirror-add NAME TYPE INDEX FILTER IP\n\
> +  mirror-add NAME TYPE [INDEX] FILTER {IP | MIRROR-ID} \n\
>                               add a mirror with given name\n\
> -                            specify TYPE 'gre' or 'erspan'\n\
> +                            specify TYPE 'gre', 'erspan', or 'local'\n\
>                               specify the tunnel INDEX value\n\
>                                   (indicates key if GRE\n\
>                                    erpsan_idx if ERSPAN)\n\
>                               specify FILTER for mirroring selection\n\
>                                   'to-lport' / 'from-lport'\n\
> -                            specify Sink / Destination i.e. Remote IP\n\
> +                            specify Sink / Destination i.e. Remote IP, or a\n\
> +                                local interface with external-ids:mirror-id\n\
> +                                matching MIRROR-ID\n\
>     mirror-del [NAME]         remove mirrors\n\
>     mirror-list               print mirrors\n\
>   \n\
> @@ -7406,17 +7408,19 @@ parse_mirror_filter(const char *arg, const char **selection_p)
>   }
>   
>   static char * OVS_WARN_UNUSED_RESULT
> -parse_mirror_tunnel_type(const char *arg, const char **type_p)
> +parse_mirror_type(const char *arg, const char **type_p)
>   {
>       /* Validate type.  Only require the first letter. */
>       if (arg[0] == 'g') {
>           *type_p = "gre";
>       } else if (arg[0] == 'e') {
>           *type_p = "erspan";
> +    } else if (arg[0] == 'l') {
> +        *type_p = "local";
>       } else {
>           *type_p = NULL;
> -        return xasprintf("%s: type must be \"gre\" or "
> -                         "\"erspan\"", arg);
> +        return xasprintf("%s: type must be \"gre\", "
> +                         "\"erspan\", or \"local\"", arg);
>       }
>       return NULL;
>   }
> @@ -7435,16 +7439,16 @@ static void
>   nbctl_mirror_add(struct ctl_context *ctx)
>   {
>       const char *filter = NULL;
> -    const char *sink_ip = NULL;
> +    const char *sink = NULL;
>       const char *type = NULL;
>       const char *name = NULL;
> -    char *new_sink_ip = NULL;
>       int64_t index;
>       char *error = NULL;
>       const struct nbrec_mirror *mirror_check = NULL;
> +    int pos = 1;
>   
>       /* Mirror Name */
> -    name = ctx->argv[1];
> +    name = ctx->argv[pos++];
>       NBREC_MIRROR_FOR_EACH (mirror_check, ctx->idl) {
>           if (!strcmp(mirror_check->name, name)) {
>               ctl_error(ctx, "Mirror with %s name already exists.",
> @@ -7453,40 +7457,44 @@ nbctl_mirror_add(struct ctl_context *ctx)
>           }
>       }
>   
> -    /* Tunnel Type - GRE/ERSPAN */
> -    error = parse_mirror_tunnel_type(ctx->argv[2], &type);
> +    /* Type - gre/erspan/local */
> +    error = parse_mirror_type(ctx->argv[pos++], &type);
>       if (error) {
>           ctx->error = error;
>           return;
>       }
>   
> -    /* tunnel index / GRE key / ERSPAN idx */
> -    if (!str_to_long(ctx->argv[3], 10, (long int *) &index)) {
> -        ctl_error(ctx, "Invalid Index");
> -        return;
> +    if (strcmp(type, "local")) {
> +        /* tunnel index / GRE key / ERSPAN idx */
> +        if (!str_to_long(ctx->argv[pos++], 10, (long int *) &index)) {
> +            ctl_error(ctx, "Invalid Index");
> +            return;
> +        }
>       }
>   
>       /* Filter for mirroring */
> -    error = parse_mirror_filter(ctx->argv[4], &filter);
> +    error = parse_mirror_filter(ctx->argv[pos++], &filter);
>       if (error) {
>           ctx->error = error;
>           return;
>       }
>   
>       /* Destination / Sink details */
> -    sink_ip = ctx->argv[5];
> +    sink = ctx->argv[pos++];
>   
> -    /* check if it is a valid ip */
> -    new_sink_ip = normalize_ipv4_addr_str(sink_ip);
> -    if (!new_sink_ip) {
> -        new_sink_ip = normalize_ipv6_addr_str(sink_ip);
> -    }
> +    /* check if it is a valid ip unless it is type 'local' */
> +    if (strcmp(type, "local")) {
> +        char *new_sink_ip = normalize_ipv4_addr_str(sink);
> +        if (!new_sink_ip) {
> +            new_sink_ip = normalize_ipv6_addr_str(sink);
> +        }
>   
> -    if (!new_sink_ip) {
> -        ctl_error(ctx, "Invalid sink ip: %s", sink_ip);
> -        return;
> +        if (!new_sink_ip) {
> +            ctl_error(ctx, "Invalid sink ip: %s", sink);
> +            return;
> +        }
> +        free(new_sink_ip);
>       }
> -    free(new_sink_ip);
>   
>       /* Create the mirror. */
>       struct nbrec_mirror *mirror = nbrec_mirror_insert(ctx->txn);
> @@ -7494,7 +7502,7 @@ nbctl_mirror_add(struct ctl_context *ctx)
>       nbrec_mirror_set_index(mirror, index);
>       nbrec_mirror_set_filter(mirror, filter);
>       nbrec_mirror_set_type(mirror, type);
> -    nbrec_mirror_set_sink(mirror, sink_ip);
> +    nbrec_mirror_set_sink(mirror, sink);
>   
>   }
>   
> @@ -7672,7 +7680,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
>         NULL, "", RO },
>   
>       /* mirror commands. */
> -    { "mirror-add", 5, 5,
> +    { "mirror-add", 4, 5,
>         "NAME TYPE INDEX FILTER IP",
>         nbctl_pre_mirror_add, nbctl_mirror_add, NULL, "--may-exist", RW },
>       { "mirror-del", 0, 1, "[NAME]",

On Mon, May 8, 2023 at 11:06 AM Mark Michelson <mmichels@redhat.com> wrote:
>
> Hi Han,
>
> For both patches in the series,
>
> Acked-by: Mark Michelson <mmichels@redhat.com>
>
Thanks Mark. I applied to main.

> I want to put a reply to patch 2 directly, but unfortunately I can't
> find it in my email. Sometimes patches get sent to spam, but in this
> case, it's not there either.
That's weird. At least I see both emails in my inbox.

Regards,
Han

>
> On 5/5/23 02:32, Han Zhou wrote:
> > Today the mirror feature in OVN supports only tunnel to a remote
> > destinations. This patch adds the support for mirroring to a local OVS
> > port. It is particularly useful for monitoring traffic that is offloaded
> > thus not possible to be intercepted by regular tools such as tcpdump.
> > With this feature, traffic to/from a virtual function that is offloaded
> > to hardware can be mirrored to a dedicated (pre-configured) OVS port
> > (which can be another virtual function that is dedicated for
> > interception purposes).
> >
> > Signed-off-by: Han Zhou <hzhou@ovn.org>
> > ---
> >   NEWS                      |   1 +
> >   controller/mirror.c       | 134 ++++++++++++++++++++++++++++++++------
> >   ovn-nb.ovsschema          |   7 +-
> >   ovn-nb.xml                |  12 +++-
> >   ovn-sb.ovsschema          |   9 +--
> >   ovn-sb.xml                |  17 +++--
> >   tests/ovn.at              |  97 ++++++++++++++++++++++++++-
> >   utilities/ovn-nbctl.8.xml |  14 ++--
> >   utilities/ovn-nbctl.c     |  64 ++++++++++--------
> >   9 files changed, 287 insertions(+), 68 deletions(-)
> >
> > diff --git a/NEWS b/NEWS
> > index 60467581a1ba..f49d4049c332 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -14,6 +14,7 @@ Post v23.03.0
> >       existing behaviour of flooding these arp requests to all attached
Ports.
> >     - Always allow IPv6 Router Discovery, Neighbor Discovery, and
Multicast
> >       Listener Discovery protocols, regardless of ACLs defined.
> > +  - Support using local OVS port as port-mirroring target.
> >
> >   OVN v23.03.0 - 03 Mar 2023
> >   --------------------------
> > diff --git a/controller/mirror.c b/controller/mirror.c
> > index 6657369665fe..fd36c7650c41 100644
> > --- a/controller/mirror.c
> > +++ b/controller/mirror.c
> > @@ -54,10 +54,12 @@ static struct ovn_mirror *ovn_mirror_find(struct
shash *ovn_mirrors,
> >   static void ovn_mirror_delete(struct ovn_mirror *);
> >   static void ovn_mirror_add_lport(struct ovn_mirror *, struct
local_binding *);
> >   static void sync_ovn_mirror(struct ovn_mirror *, struct ovsdb_idl_txn
*,
> > -                            const struct ovsrec_bridge *);
> > +                            const struct ovsrec_bridge *,
> > +                            struct shash *ovs_mirror_ports);
> >
> >   static void create_ovs_mirror(struct ovn_mirror *, struct
ovsdb_idl_txn *,
> > -                              const struct ovsrec_bridge *);
> > +                              const struct ovsrec_bridge *,
> > +                              struct shash *ovs_mirror_ports);
> >   static void sync_ovs_mirror_ports(struct ovn_mirror *,
> >                                     const struct ovsrec_bridge *);
> >   static void delete_ovs_mirror(struct ovn_mirror *,
> > @@ -69,6 +71,8 @@ static void set_mirror_iface_options(struct
ovsrec_interface *,
> >   static const struct ovsrec_port *get_iface_port(
> >       const struct ovsrec_interface *, const struct ovsrec_bridge *);
> >
> > +static void build_ovs_mirror_ports(const struct ovsrec_bridge *,
> > +                                   struct shash *ovs_mirror_ports);
> >
> >   void
> >   mirror_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > @@ -105,7 +109,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> >       }
> >
> >       struct shash ovn_mirrors = SHASH_INITIALIZER(&ovn_mirrors);
> > -    struct shash tmp_mirrors = SHASH_INITIALIZER(&tmp_mirrors);
> > +    struct shash ovs_local_mirror_ports =
> > +        SHASH_INITIALIZER(&ovs_local_mirror_ports);
> >
> >       /* Iterate through sb mirrors and build the 'ovn_mirrors'. */
> >       const struct sbrec_mirror *sb_mirror;
> > @@ -137,6 +142,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> >           return;
> >       }
> >
> > +    build_ovs_mirror_ports(br_int, &ovs_local_mirror_ports);
> > +
> >       /* Iterate through the local bindings and if the local binding's
'pb' has
> >        * mirrors associated, add it to the ovn_mirror. */
> >       struct shash_node *node;
> > @@ -161,7 +168,7 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> >        * create/update or delete the ovsrec mirror(s). */
> >        SHASH_FOR_EACH (node, &ovn_mirrors) {
> >           struct ovn_mirror *m = node->data;
> > -        sync_ovn_mirror(m, ovs_idl_txn, br_int);
> > +        sync_ovn_mirror(m, ovs_idl_txn, br_int,
&ovs_local_mirror_ports);
> >        }
> >
> >       SHASH_FOR_EACH_SAFE (node, &ovn_mirrors) {
> > @@ -170,9 +177,52 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> >       }
> >
> >       shash_destroy(&ovn_mirrors);
> > +    shash_destroy(&ovs_local_mirror_ports);
> >   }
> >
> >   /* Static functions. */
> > +
> > +/* Builds mapping from mirror-id to ovsrec_port.
> > + */
> > +static void
> > +build_ovs_mirror_ports(const struct ovsrec_bridge *br_int,
> > +                       struct shash *ovs_mirror_ports)
> > +{
> > +    int i;
> > +    for (i = 0; i < br_int->n_ports; i++) {
> > +        const struct ovsrec_port *port_rec = br_int->ports[i];
> > +        int j;
> > +
> > +        if (!strcmp(port_rec->name, br_int->name)) {
> > +            continue;
> > +        }
> > +
> > +        for (j = 0; j < port_rec->n_interfaces; j++) {
> > +            const struct ovsrec_interface *iface_rec;
> > +
> > +            iface_rec = port_rec->interfaces[j];
> > +            const char *mirror_id = smap_get(&iface_rec->external_ids,
> > +                                             "mirror-id");
> > +            if (mirror_id) {
> > +                const struct ovsrec_port *p =
shash_find_data(ovs_mirror_ports,
> > +
 mirror_id);
> > +                if (!p) {
> > +                    shash_add(ovs_mirror_ports, mirror_id, port_rec);
> > +                } else {
> > +                    static struct vlog_rate_limit rl =
> > +                        VLOG_RATE_LIMIT_INIT(1, 5);
> > +                    VLOG_WARN_RL(
> > +                        &rl,
> > +                        "Invalid configuration: same mirror-id [%s] is
"
> > +                        "configured on interfaces of ports: [%s] and
[%s]. "
> > +                        "Ignoring the configuration on interface [%s]",
> > +                        mirror_id, port_rec->name, p->name,
iface_rec->name);
> > +                }
> > +            }
> > +        }
> > +    }
> > +}
> > +
> >   static struct ovn_mirror *
> >   ovn_mirror_create(char *mirror_name)
> >   {
> > @@ -266,7 +316,8 @@ check_and_update_interface_table(const struct
sbrec_mirror *sb_mirror,
> >
> >   static void
> >   sync_ovn_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn
*ovs_idl_txn,
> > -                const struct ovsrec_bridge *br_int)
> > +                const struct ovsrec_bridge *br_int,
> > +                struct shash *ovs_mirror_ports)
> >   {
> >       if (should_delete_ovs_mirror(m)) {
> >           /* Delete the ovsrec mirror. */
> > @@ -281,9 +332,31 @@ sync_ovn_mirror(struct ovn_mirror *m, struct
ovsdb_idl_txn *ovs_idl_txn,
> >       }
> >
> >       if (m->sb_mirror && !m->ovs_mirror) {
> > -        create_ovs_mirror(m, ovs_idl_txn, br_int);
> > -    } else {
> > +        create_ovs_mirror(m, ovs_idl_txn, br_int, ovs_mirror_ports);
> > +        if (!m->ovs_mirror) {
> > +            return;
> > +        }
> > +    } else if (strcmp(m->sb_mirror->type, "local")) {
> >           check_and_update_interface_table(m->sb_mirror, m->ovs_mirror);
> > +
> > +        /* For upgradability, set the "ovn-owned" in case it was not
set when
> > +         * the port was created. */
> > +        if (!smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> > +                                   "ovn-owned", false)) {
> > +
 ovsrec_port_update_external_ids_setkey(m->ovs_mirror->output_port,
> > +                                                   "ovn-owned",
"true");
> > +        }
> > +    } else {
> > +        /* type is local, check mirror-id */
> > +        const struct ovsrec_port *mirror_port =
> > +            shash_find_data(ovs_mirror_ports, m->sb_mirror->sink);
> > +        if (!mirror_port) {
> > +            delete_ovs_mirror(m, br_int);
> > +            return;
> > +        }
> > +        if (mirror_port != m->ovs_mirror->output_port) {
> > +            ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
> > +        }
> >       }
> >
> >       sync_ovs_mirror_ports(m, br_int);
> > @@ -321,25 +394,39 @@ get_iface_port(const struct ovsrec_interface
*iface,
> >
> >   static void
> >   create_ovs_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn
*ovs_idl_txn,
> > -                  const struct ovsrec_bridge *br_int)
> > +                  const struct ovsrec_bridge *br_int,
> > +                  struct shash *ovs_mirror_ports)
> >   {
> > -    struct ovsrec_interface *iface =
ovsrec_interface_insert(ovs_idl_txn);
> > -    char *port_name = xasprintf("ovn-%s", m->name);
> > +    const struct ovsrec_port *mirror_port;
> > +    if (!strcmp(m->sb_mirror->type, "local")) {
> > +        mirror_port = shash_find_data(ovs_mirror_ports,
m->sb_mirror->sink);
> > +        if (!mirror_port) {
> > +            return;
> > +        }
> > +    } else {
> > +        struct ovsrec_interface *iface =
ovsrec_interface_insert(ovs_idl_txn);
> > +        char *port_name = xasprintf("ovn-%s", m->name);
> >
> > -    ovsrec_interface_set_name(iface, port_name);
> > -    ovsrec_interface_set_type(iface, m->sb_mirror->type);
> > -    set_mirror_iface_options(iface, m->sb_mirror);
> > +        ovsrec_interface_set_name(iface, port_name);
> > +        ovsrec_interface_set_type(iface, m->sb_mirror->type);
> > +        set_mirror_iface_options(iface, m->sb_mirror);
> >
> > -    struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> > -    ovsrec_port_set_name(port, port_name);
> > -    ovsrec_port_set_interfaces(port, &iface, 1);
> > -    ovsrec_bridge_update_ports_addvalue(br_int, port);
> > +        struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> > +        ovsrec_port_set_name(port, port_name);
> > +        ovsrec_port_set_interfaces(port, &iface, 1);
> > +        ovsrec_bridge_update_ports_addvalue(br_int, port);
> >
> > -    free(port_name);
> > +        const struct smap port_external_ids =
> > +            SMAP_CONST1(&port_external_ids, "ovn-owned", "true");
> > +        ovsrec_port_set_external_ids(port, &port_external_ids);
> > +
> > +        free(port_name);
> > +        mirror_port = port;
> > +    }
> >
> >       m->ovs_mirror = ovsrec_mirror_insert(ovs_idl_txn);
> >       ovsrec_mirror_set_name(m->ovs_mirror, m->name);
> > -    ovsrec_mirror_set_output_port(m->ovs_mirror, port);
> > +    ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
> >
> >       const struct smap external_ids =
> >           SMAP_CONST1(&external_ids, "ovn-owned", "true");
> > @@ -393,8 +480,13 @@ sync_ovs_mirror_ports(struct ovn_mirror *m, const
struct ovsrec_bridge *br_int)
> >   static void
> >   delete_ovs_mirror(struct ovn_mirror *m, const struct ovsrec_bridge
*br_int)
> >   {
> > -    ovsrec_bridge_update_ports_delvalue(br_int,
m->ovs_mirror->output_port);
> >       ovsrec_bridge_update_mirrors_delvalue(br_int, m->ovs_mirror);
> > -    ovsrec_port_delete(m->ovs_mirror->output_port);
> > +    bool ovn_owned =
smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> > +                                   "ovn-owned", false);
> > +    if (ovn_owned) {
> > +        ovsrec_bridge_update_ports_delvalue(br_int,
> > +
 m->ovs_mirror->output_port);
> > +        ovsrec_port_delete(m->ovs_mirror->output_port);
> > +    }
> >       ovsrec_mirror_delete(m->ovs_mirror);
> >   }
> > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > index 4836a219f93d..dd5632562578 100644
> > --- a/ovn-nb.ovsschema
> > +++ b/ovn-nb.ovsschema
> > @@ -1,7 +1,7 @@
> >   {
> >       "name": "OVN_Northbound",
> > -    "version": "7.0.0",
> > -    "cksum": "94023179 33468",
> > +    "version": "7.0.1",
> > +    "cksum": "441625132 33536",
> >       "tables": {
> >           "NB_Global": {
> >               "columns": {
> > @@ -315,7 +315,8 @@
> >                   "sink":{"type": "string"},
> >                   "type": {"type": {"key": {"type": "string",
> >                                             "enum": ["set", ["gre",
> > -
"erspan"]]}}},
> > +                                                           "erspan",
> > +
"local"]]}}},
> >                   "index": {"type": "integer"},
> >                   "external_ids": {
> >                       "type": {"key": "string", "value": "string",
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 0552eff199a0..20b59df48f4e 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -2693,14 +2693,19 @@ or
> >       <column name="sink">
> >         <p>
> >           The value of this field represents the destination/sink of
the mirror.
> > -        The value it takes is an IP address of the sink port.
> > +        If the <var>type</var> is <code>gre</code> or
<code>erspan</code>,
> > +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> > +        For a <var>type</var> of <code>local</code>, this field
defines a
> > +        local interface on the OVS integration bridge to be used as the
> > +        mirror destination. The interface must possess
external-ids:mirror-id
> > +        that matches this string.
> >         </p>
> >       </column>
> >
> >       <column name="type">
> >         <p>
> > -        The value of this field represents the type of the tunnel used
for
> > -        sending the mirrored packets.
> > +        The value of this field specifies the mirror type -
<code>gre</code>,
> > +        <code>erspan</code> or <code>local</code>.
> >         </p>
> >       </column>
> >
> > @@ -2710,6 +2715,7 @@ or
> >           tunnel type is <code>gre</code>, this field represents the
> >           <code>GRE</code> key value and if the configured tunnel type
is
> >           <code>erspan</code> it represents the <code>erspan_idx</code>
value.
> > +        It is ignored if the type is <code>local</code>.
> >         </p>
> >       </column>
> >
> > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> > index 79ba6841e53a..d5ac393f9210 100644
> > --- a/ovn-sb.ovsschema
> > +++ b/ovn-sb.ovsschema
> > @@ -1,7 +1,7 @@
> >   {
> >       "name": "OVN_Southbound",
> > -    "version": "20.27.0",
> > -    "cksum": "4078371916 30328",
> > +    "version": "20.27.1",
> > +    "cksum": "1439763681 30400",
> >       "tables": {
> >           "SB_Global": {
> >               "columns": {
> > @@ -151,8 +151,9 @@
> >                                                         "to-lport"]]}}},
> >                   "sink":{"type": "string"},
> >                   "type": {"type": {"key": {"type": "string",
> > -                                            "enum": ["set",
> > -                                                     ["gre",
"erspan"]]}}},
> > +                                          "enum": ["set", ["gre",
> > +                                                           "erspan",
> > +
"local"]]}}},
> >                   "index": {"type": "integer"},
> >                   "external_ids": {
> >                       "type": {"key": "string", "value": "string",
> > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > index a77f8f4efb38..9599e55f44e5 100644
> > --- a/ovn-sb.xml
> > +++ b/ovn-sb.xml
> > @@ -2907,20 +2907,29 @@ tcp.flags = RST;
> >       <column name="sink">
> >         <p>
> >           The value of this field represents the destination/sink of
the mirror.
> > +        If the <var>type</var> is <code>gre</code> or
<code>erspan</code>,
> > +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> > +        For a <var>type</var> of <code>local</code>, this field
defines a
> > +        local interface on the OVS integration bridge to be used as the
> > +        mirror destination. The interface must possess
external-ids:mirror-id
> > +        that matches this string.
> >         </p>
> >       </column>
> >
> >       <column name="type">
> >         <p>
> > -        The value of this field represents the type of the tunnel used
for
> > -        sending the mirrored packets
> > +        The value of this field specifies the mirror type -
<code>gre</code>,
> > +        <code>erspan</code> or <code>local</code>.
> >         </p>
> >       </column>
> >
> >       <column name="index">
> >         <p>
> > -        The value of this field represents the key/idx depending on the
> > -        tunnel type configured
> > +        The value of this field represents the tunnel ID. If the
configured
> > +        tunnel type is <code>gre</code>, this field represents the
> > +        <code>GRE</code> key value and if the configured tunnel type is
> > +        <code>erspan</code> it represents the <code>erspan_idx</code>
value.
> > +        It is ignored if the type is <code>local</code>.
> >         </p>
> >       </column>
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 213ad18fa50b..4c124e0e58d9 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -16427,7 +16427,7 @@ AT_CLEANUP
> >   ])
> >
> >   OVN_FOR_EACH_NORTHD([
> > -AT_SETUP([Mirror])
> > +AT_SETUP([Mirror - remote])
> >   AT_KEYWORDS([Mirror])
> >   ovn_start
> >
> > @@ -16655,6 +16655,101 @@ OVN_CLEANUP([hv1], [hv2])
> >   AT_CLEANUP
> >   ])
> >
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([Mirror - local])
> > +AT_KEYWORDS([Mirror])
> > +ovn_start
> > +
> > +ovn-nbctl ls-add ls1
> > +# Create logical port ls1-lp1 in ls1
> > +ovn-nbctl lsp-add ls1 ls1-lp1 \
> > +-- lsp-set-addresses ls1-lp1 "00:00:00:01:01:02 10.0.0.2"
> > +ovn-nbctl lsp-add ls1 ls1-lp2 \
> > +-- lsp-set-addresses ls1-lp2 "00:00:00:01:01:03 10.0.0.3"
> > +
> > +net_add n1
> > +
> > +sim_add hv1
> > +as hv1
> > +ovs-vsctl add-br br-phys -- set bridge br-phys
other-config:hwaddr=\"00:00:00:01:02:00\"
> > +ovn_attach n1 br-phys 192.168.1.11
> > +ovn-appctl -t ovn-controller vlog/set file:dbg
> > +
> > +ovs-vsctl -- add-port br-int vif1 -- \
> > +    set interface vif1 external-ids:iface-id=ls1-lp1 \
> > +    options:tx_pcap=hv1/vif1-tx.pcap \
> > +    options:rxq_pcap=hv1/vif1-rx.pcap \
> > +    ofport-request=1
> > +ovs-vsctl -- add-port br-int vif2 -- \
> > +    set interface vif2 external-ids:iface-id=ls1-lp2 \
> > +    options:tx_pcap=hv1/vif2-tx.pcap \
> > +    options:rxq_pcap=hv1/vif2-rx.pcap \
> > +    ofport-request=2
> > +
> > +# Add two ports as mirroring target
> > +ovs-vsctl -- add-port br-int mirror1 -- \
> > +    set interface mirror1 external-ids:mirror-id=sink1 \
> > +    options:tx_pcap=hv1/mirror1-tx.pcap \
> > +    options:rxq_pcap=hv1/mirror1-rx.pcap \
> > +    ofport-request=3
> > +ovs-vsctl -- add-port br-int mirror2 -- \
> > +    set interface mirror2 external-ids:mirror-id=sink2 \
> > +    options:tx_pcap=hv1/mirror2-tx.pcap \
> > +    options:rxq_pcap=hv1/mirror2-rx.pcap \
> > +    ofport-request=4
> > +
> > +# Create a NB mirror use DB 'create' command.
> > +uuid1=$(ovn-nbctl create mirror name=mirror-from-lp1 type=local
sink=sink1 filter=from-lport)
> > +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror-from-lp1
> > +
> > +# Create another NB mirror use 'mirror-add' command.
> > +check ovn-nbctl mirror-add mirror-to-lp2 local to-lport sink2
> > +check ovn-nbctl lsp-attach-mirror ls1-lp2 mirror-to-lp2
> > +
> > +wait_for_ports_up
> > +check ovn-nbctl --wait=hv sync
> > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
mirror | grep mirror | sort], [0], [dnl
> > +mirror-from-lp1
> > +mirror-to-lp2
> > +])
> > +
> > +# Update to wrong mirror-id, the mirror should be deleted.
> > +check ovn-nbctl set mirror $uuid1 sink=xxx
> > +check ovn-nbctl --wait=hv sync
> > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
mirror | grep mirror | sort], [0], [dnl
> > +mirror-to-lp2
> > +])
> > +
> > +# Change back, and the mirror should be created back
> > +check ovn-nbctl set mirror $uuid1 sink=sink1
> > +check ovn-nbctl --wait=hv sync
> > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
mirror | grep mirror | sort], [0], [dnl
> > +mirror-from-lp1
> > +mirror-to-lp2
> > +])
> > +
> > +# Send a packet from lp1 to lp2, should be mirrored to both mirror
ports.
> > +packet="inport==\"ls1-lp1\" && eth.src==00:00:00:01:01:02 &&
eth.dst==00:00:00:01:01:03 &&
> > +        ip4 && ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 &&
icmp"
> > +AT_CHECK([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> > +
> > +exp_packet="eth.src==00:00:00:01:01:02 && eth.dst==00:00:00:01:01:03
&& ip4 &&
> > +            ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 &&
icmp"
> > +echo $exp_packet | ovstest test-ovn expr-to-packets | sort > expout
> > +
> > +OVS_WAIT_UNTIL([
> > +    rcv_mirror1=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in"
hv1/mirror1-tx.pcap > mirror1.packets && cat mirror1.packets | wc -l`
> > +    rcv_mirror2=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in"
hv1/mirror2-tx.pcap > mirror2.packets && cat mirror2.packets | wc -l`
> > +    echo $rcv_mirror1 $rcv_mirror2
> > +    test $rcv_mirror1 -eq 1 -a $rcv_mirror2 -eq 1])
> > +
> > +AT_CHECK([cat mirror1.packets | sort], [0], [expout])
> > +AT_CHECK([cat mirror2.packets | sort], [0], [expout])
> > +
> > +OVN_CLEANUP([hv1])
> > +AT_CLEANUP
> > +])
> > +
> >   OVN_FOR_EACH_NORTHD([
> >   AT_SETUP([Mirror test bulk updates])
> >   AT_KEYWORDS([Mirror test bulk updates])
> > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> > index 54dbdb791e52..c450b9a5ba7d 100644
> > --- a/utilities/ovn-nbctl.8.xml
> > +++ b/utilities/ovn-nbctl.8.xml
> > @@ -1546,7 +1546,7 @@
> >       <h2> Mirror commands</h2>
> >       <dl>
> >         <dt><code>mirror-add</code> <var>m</var> <var>type</var>
> > -      <var>index</var> <var>filter</var> <var>dest</var></dt>
> > +      [<var>index</var>] <var>filter</var> <var>dest</var></dt>
> >         <dd>
> >           <p>
> >             Creates a new mirror in the <code>Mirror</code>
> > @@ -1556,12 +1556,13 @@
> >
> >           <p>
> >             <var>type</var> specifies the mirror type - <code>gre</code>
> > -          or <code>erspan</code>.
> > +          , <code>erspan</code> or <code>local</code>.
> >           </p>
> >
> >           <p>
> >             <var>index</var> specifies the tunnel index value (which is
> > -          an integer).
> > +          an integer) if the <var>type</var> is <code>gre</code> or
> > +          <code>erspan</code>.
> >           </p>
> >
> >           <p>
> > @@ -1570,7 +1571,12 @@
> >           </p>
> >
> >           <p>
> > -          <var>dest</var> specifies the mirror destination IP (v4 or
v6).
> > +          <var>dest</var> specifies the mirror destination IP (v4 or
v6)
> > +          if the <var>type</var> is <code>gre</code> or
<code>erspan</code>.
> > +          For a <var>type</var> of <code>local</code>, this field
defines a
> > +          local interface on the OVS integration bridge to be used as
the
> > +          mirror destination. The interface must possess
external-ids:mirror-id
> > +          that matches this string.
> >           </p>
> >         </dd>
> >
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index 9399f9462bb1..5f2fbfecfe5b 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -273,15 +273,17 @@ QoS commands:\n\
> >     qos-list SWITCH           print QoS rules for SWITCH\n\
> >   \n\
> >   Mirror commands:\n\
> > -  mirror-add NAME TYPE INDEX FILTER IP\n\
> > +  mirror-add NAME TYPE [INDEX] FILTER {IP | MIRROR-ID} \n\
> >                               add a mirror with given name\n\
> > -                            specify TYPE 'gre' or 'erspan'\n\
> > +                            specify TYPE 'gre', 'erspan', or 'local'\n\
> >                               specify the tunnel INDEX value\n\
> >                                   (indicates key if GRE\n\
> >                                    erpsan_idx if ERSPAN)\n\
> >                               specify FILTER for mirroring selection\n\
> >                                   'to-lport' / 'from-lport'\n\
> > -                            specify Sink / Destination i.e. Remote
IP\n\
> > +                            specify Sink / Destination i.e. Remote IP,
or a\n\
> > +                                local interface with
external-ids:mirror-id\n\
> > +                                matching MIRROR-ID\n\
> >     mirror-del [NAME]         remove mirrors\n\
> >     mirror-list               print mirrors\n\
> >   \n\
> > @@ -7406,17 +7408,19 @@ parse_mirror_filter(const char *arg, const char
**selection_p)
> >   }
> >
> >   static char * OVS_WARN_UNUSED_RESULT
> > -parse_mirror_tunnel_type(const char *arg, const char **type_p)
> > +parse_mirror_type(const char *arg, const char **type_p)
> >   {
> >       /* Validate type.  Only require the first letter. */
> >       if (arg[0] == 'g') {
> >           *type_p = "gre";
> >       } else if (arg[0] == 'e') {
> >           *type_p = "erspan";
> > +    } else if (arg[0] == 'l') {
> > +        *type_p = "local";
> >       } else {
> >           *type_p = NULL;
> > -        return xasprintf("%s: type must be \"gre\" or "
> > -                         "\"erspan\"", arg);
> > +        return xasprintf("%s: type must be \"gre\", "
> > +                         "\"erspan\", or \"local\"", arg);
> >       }
> >       return NULL;
> >   }
> > @@ -7435,16 +7439,16 @@ static void
> >   nbctl_mirror_add(struct ctl_context *ctx)
> >   {
> >       const char *filter = NULL;
> > -    const char *sink_ip = NULL;
> > +    const char *sink = NULL;
> >       const char *type = NULL;
> >       const char *name = NULL;
> > -    char *new_sink_ip = NULL;
> >       int64_t index;
> >       char *error = NULL;
> >       const struct nbrec_mirror *mirror_check = NULL;
> > +    int pos = 1;
> >
> >       /* Mirror Name */
> > -    name = ctx->argv[1];
> > +    name = ctx->argv[pos++];
> >       NBREC_MIRROR_FOR_EACH (mirror_check, ctx->idl) {
> >           if (!strcmp(mirror_check->name, name)) {
> >               ctl_error(ctx, "Mirror with %s name already exists.",
> > @@ -7453,40 +7457,44 @@ nbctl_mirror_add(struct ctl_context *ctx)
> >           }
> >       }
> >
> > -    /* Tunnel Type - GRE/ERSPAN */
> > -    error = parse_mirror_tunnel_type(ctx->argv[2], &type);
> > +    /* Type - gre/erspan/local */
> > +    error = parse_mirror_type(ctx->argv[pos++], &type);
> >       if (error) {
> >           ctx->error = error;
> >           return;
> >       }
> >
> > -    /* tunnel index / GRE key / ERSPAN idx */
> > -    if (!str_to_long(ctx->argv[3], 10, (long int *) &index)) {
> > -        ctl_error(ctx, "Invalid Index");
> > -        return;
> > +    if (strcmp(type, "local")) {
> > +        /* tunnel index / GRE key / ERSPAN idx */
> > +        if (!str_to_long(ctx->argv[pos++], 10, (long int *) &index)) {
> > +            ctl_error(ctx, "Invalid Index");
> > +            return;
> > +        }
> >       }
> >
> >       /* Filter for mirroring */
> > -    error = parse_mirror_filter(ctx->argv[4], &filter);
> > +    error = parse_mirror_filter(ctx->argv[pos++], &filter);
> >       if (error) {
> >           ctx->error = error;
> >           return;
> >       }
> >
> >       /* Destination / Sink details */
> > -    sink_ip = ctx->argv[5];
> > +    sink = ctx->argv[pos++];
> >
> > -    /* check if it is a valid ip */
> > -    new_sink_ip = normalize_ipv4_addr_str(sink_ip);
> > -    if (!new_sink_ip) {
> > -        new_sink_ip = normalize_ipv6_addr_str(sink_ip);
> > -    }
> > +    /* check if it is a valid ip unless it is type 'local' */
> > +    if (strcmp(type, "local")) {
> > +        char *new_sink_ip = normalize_ipv4_addr_str(sink);
> > +        if (!new_sink_ip) {
> > +            new_sink_ip = normalize_ipv6_addr_str(sink);
> > +        }
> >
> > -    if (!new_sink_ip) {
> > -        ctl_error(ctx, "Invalid sink ip: %s", sink_ip);
> > -        return;
> > +        if (!new_sink_ip) {
> > +            ctl_error(ctx, "Invalid sink ip: %s", sink);
> > +            return;
> > +        }
> > +        free(new_sink_ip);
> >       }
> > -    free(new_sink_ip);
> >
> >       /* Create the mirror. */
> >       struct nbrec_mirror *mirror = nbrec_mirror_insert(ctx->txn);
> > @@ -7494,7 +7502,7 @@ nbctl_mirror_add(struct ctl_context *ctx)
> >       nbrec_mirror_set_index(mirror, index);
> >       nbrec_mirror_set_filter(mirror, filter);
> >       nbrec_mirror_set_type(mirror, type);
> > -    nbrec_mirror_set_sink(mirror, sink_ip);
> > +    nbrec_mirror_set_sink(mirror, sink);
> >
> >   }
> >
> > @@ -7672,7 +7680,7 @@ static const struct ctl_command_syntax
nbctl_commands[] = {
> >         NULL, "", RO },
> >
> >       /* mirror commands. */
> > -    { "mirror-add", 5, 5,
> > +    { "mirror-add", 4, 5,
> >         "NAME TYPE INDEX FILTER IP",
> >         nbctl_pre_mirror_add, nbctl_mirror_add, NULL, "--may-exist", RW
},
> >       { "mirror-del", 0, 1, "[NAME]",
>

On Wed, May 10, 2023 at 12:25 PM Han Zhou <hzhou@ovn.org> wrote:
>
> On Mon, May 8, 2023 at 11:06 AM Mark Michelson <mmichels@redhat.com> wrote:
> >
> > Hi Han,
> >
> > For both patches in the series,
> >
> > Acked-by: Mark Michelson <mmichels@redhat.com>
> >
> Thanks Mark. I applied to main.
>
> > I want to put a reply to patch 2 directly, but unfortunately I can't
> > find it in my email. Sometimes patches get sent to spam, but in this
> > case, it's not there either.
> That's weird. At least I see both emails in my inbox.

Hi Han,

When the mirror feature was added we did consider supporting both the
directions but there was one limitation and perhaps it requires some
fix in OVS.

Please take a look at this bz -
https://bugzilla.redhat.com/show_bug.cgi?id=2155579

Basically any packet which OVN transforms to a reply packet, it
doesn't get mirrored for both directions.
Now that we support it, I think it's better we document it ?  What do
you think?

Numan

>
> Regards,
> Han
>
> >
> > On 5/5/23 02:32, Han Zhou wrote:
> > > Today the mirror feature in OVN supports only tunnel to a remote
> > > destinations. This patch adds the support for mirroring to a local OVS
> > > port. It is particularly useful for monitoring traffic that is offloaded
> > > thus not possible to be intercepted by regular tools such as tcpdump.
> > > With this feature, traffic to/from a virtual function that is offloaded
> > > to hardware can be mirrored to a dedicated (pre-configured) OVS port
> > > (which can be another virtual function that is dedicated for
> > > interception purposes).
> > >
> > > Signed-off-by: Han Zhou <hzhou@ovn.org>
> > > ---
> > >   NEWS                      |   1 +
> > >   controller/mirror.c       | 134 ++++++++++++++++++++++++++++++++------
> > >   ovn-nb.ovsschema          |   7 +-
> > >   ovn-nb.xml                |  12 +++-
> > >   ovn-sb.ovsschema          |   9 +--
> > >   ovn-sb.xml                |  17 +++--
> > >   tests/ovn.at              |  97 ++++++++++++++++++++++++++-
> > >   utilities/ovn-nbctl.8.xml |  14 ++--
> > >   utilities/ovn-nbctl.c     |  64 ++++++++++--------
> > >   9 files changed, 287 insertions(+), 68 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 60467581a1ba..f49d4049c332 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -14,6 +14,7 @@ Post v23.03.0
> > >       existing behaviour of flooding these arp requests to all attached
> Ports.
> > >     - Always allow IPv6 Router Discovery, Neighbor Discovery, and
> Multicast
> > >       Listener Discovery protocols, regardless of ACLs defined.
> > > +  - Support using local OVS port as port-mirroring target.
> > >
> > >   OVN v23.03.0 - 03 Mar 2023
> > >   --------------------------
> > > diff --git a/controller/mirror.c b/controller/mirror.c
> > > index 6657369665fe..fd36c7650c41 100644
> > > --- a/controller/mirror.c
> > > +++ b/controller/mirror.c
> > > @@ -54,10 +54,12 @@ static struct ovn_mirror *ovn_mirror_find(struct
> shash *ovn_mirrors,
> > >   static void ovn_mirror_delete(struct ovn_mirror *);
> > >   static void ovn_mirror_add_lport(struct ovn_mirror *, struct
> local_binding *);
> > >   static void sync_ovn_mirror(struct ovn_mirror *, struct ovsdb_idl_txn
> *,
> > > -                            const struct ovsrec_bridge *);
> > > +                            const struct ovsrec_bridge *,
> > > +                            struct shash *ovs_mirror_ports);
> > >
> > >   static void create_ovs_mirror(struct ovn_mirror *, struct
> ovsdb_idl_txn *,
> > > -                              const struct ovsrec_bridge *);
> > > +                              const struct ovsrec_bridge *,
> > > +                              struct shash *ovs_mirror_ports);
> > >   static void sync_ovs_mirror_ports(struct ovn_mirror *,
> > >                                     const struct ovsrec_bridge *);
> > >   static void delete_ovs_mirror(struct ovn_mirror *,
> > > @@ -69,6 +71,8 @@ static void set_mirror_iface_options(struct
> ovsrec_interface *,
> > >   static const struct ovsrec_port *get_iface_port(
> > >       const struct ovsrec_interface *, const struct ovsrec_bridge *);
> > >
> > > +static void build_ovs_mirror_ports(const struct ovsrec_bridge *,
> > > +                                   struct shash *ovs_mirror_ports);
> > >
> > >   void
> > >   mirror_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > > @@ -105,7 +109,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > >       }
> > >
> > >       struct shash ovn_mirrors = SHASH_INITIALIZER(&ovn_mirrors);
> > > -    struct shash tmp_mirrors = SHASH_INITIALIZER(&tmp_mirrors);
> > > +    struct shash ovs_local_mirror_ports =
> > > +        SHASH_INITIALIZER(&ovs_local_mirror_ports);
> > >
> > >       /* Iterate through sb mirrors and build the 'ovn_mirrors'. */
> > >       const struct sbrec_mirror *sb_mirror;
> > > @@ -137,6 +142,8 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > >           return;
> > >       }
> > >
> > > +    build_ovs_mirror_ports(br_int, &ovs_local_mirror_ports);
> > > +
> > >       /* Iterate through the local bindings and if the local binding's
> 'pb' has
> > >        * mirrors associated, add it to the ovn_mirror. */
> > >       struct shash_node *node;
> > > @@ -161,7 +168,7 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > >        * create/update or delete the ovsrec mirror(s). */
> > >        SHASH_FOR_EACH (node, &ovn_mirrors) {
> > >           struct ovn_mirror *m = node->data;
> > > -        sync_ovn_mirror(m, ovs_idl_txn, br_int);
> > > +        sync_ovn_mirror(m, ovs_idl_txn, br_int,
> &ovs_local_mirror_ports);
> > >        }
> > >
> > >       SHASH_FOR_EACH_SAFE (node, &ovn_mirrors) {
> > > @@ -170,9 +177,52 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> > >       }
> > >
> > >       shash_destroy(&ovn_mirrors);
> > > +    shash_destroy(&ovs_local_mirror_ports);
> > >   }
> > >
> > >   /* Static functions. */
> > > +
> > > +/* Builds mapping from mirror-id to ovsrec_port.
> > > + */
> > > +static void
> > > +build_ovs_mirror_ports(const struct ovsrec_bridge *br_int,
> > > +                       struct shash *ovs_mirror_ports)
> > > +{
> > > +    int i;
> > > +    for (i = 0; i < br_int->n_ports; i++) {
> > > +        const struct ovsrec_port *port_rec = br_int->ports[i];
> > > +        int j;
> > > +
> > > +        if (!strcmp(port_rec->name, br_int->name)) {
> > > +            continue;
> > > +        }
> > > +
> > > +        for (j = 0; j < port_rec->n_interfaces; j++) {
> > > +            const struct ovsrec_interface *iface_rec;
> > > +
> > > +            iface_rec = port_rec->interfaces[j];
> > > +            const char *mirror_id = smap_get(&iface_rec->external_ids,
> > > +                                             "mirror-id");
> > > +            if (mirror_id) {
> > > +                const struct ovsrec_port *p =
> shash_find_data(ovs_mirror_ports,
> > > +
>  mirror_id);
> > > +                if (!p) {
> > > +                    shash_add(ovs_mirror_ports, mirror_id, port_rec);
> > > +                } else {
> > > +                    static struct vlog_rate_limit rl =
> > > +                        VLOG_RATE_LIMIT_INIT(1, 5);
> > > +                    VLOG_WARN_RL(
> > > +                        &rl,
> > > +                        "Invalid configuration: same mirror-id [%s] is
> "
> > > +                        "configured on interfaces of ports: [%s] and
> [%s]. "
> > > +                        "Ignoring the configuration on interface [%s]",
> > > +                        mirror_id, port_rec->name, p->name,
> iface_rec->name);
> > > +                }
> > > +            }
> > > +        }
> > > +    }
> > > +}
> > > +
> > >   static struct ovn_mirror *
> > >   ovn_mirror_create(char *mirror_name)
> > >   {
> > > @@ -266,7 +316,8 @@ check_and_update_interface_table(const struct
> sbrec_mirror *sb_mirror,
> > >
> > >   static void
> > >   sync_ovn_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn
> *ovs_idl_txn,
> > > -                const struct ovsrec_bridge *br_int)
> > > +                const struct ovsrec_bridge *br_int,
> > > +                struct shash *ovs_mirror_ports)
> > >   {
> > >       if (should_delete_ovs_mirror(m)) {
> > >           /* Delete the ovsrec mirror. */
> > > @@ -281,9 +332,31 @@ sync_ovn_mirror(struct ovn_mirror *m, struct
> ovsdb_idl_txn *ovs_idl_txn,
> > >       }
> > >
> > >       if (m->sb_mirror && !m->ovs_mirror) {
> > > -        create_ovs_mirror(m, ovs_idl_txn, br_int);
> > > -    } else {
> > > +        create_ovs_mirror(m, ovs_idl_txn, br_int, ovs_mirror_ports);
> > > +        if (!m->ovs_mirror) {
> > > +            return;
> > > +        }
> > > +    } else if (strcmp(m->sb_mirror->type, "local")) {
> > >           check_and_update_interface_table(m->sb_mirror, m->ovs_mirror);
> > > +
> > > +        /* For upgradability, set the "ovn-owned" in case it was not
> set when
> > > +         * the port was created. */
> > > +        if (!smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> > > +                                   "ovn-owned", false)) {
> > > +
>  ovsrec_port_update_external_ids_setkey(m->ovs_mirror->output_port,
> > > +                                                   "ovn-owned",
> "true");
> > > +        }
> > > +    } else {
> > > +        /* type is local, check mirror-id */
> > > +        const struct ovsrec_port *mirror_port =
> > > +            shash_find_data(ovs_mirror_ports, m->sb_mirror->sink);
> > > +        if (!mirror_port) {
> > > +            delete_ovs_mirror(m, br_int);
> > > +            return;
> > > +        }
> > > +        if (mirror_port != m->ovs_mirror->output_port) {
> > > +            ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
> > > +        }
> > >       }
> > >
> > >       sync_ovs_mirror_ports(m, br_int);
> > > @@ -321,25 +394,39 @@ get_iface_port(const struct ovsrec_interface
> *iface,
> > >
> > >   static void
> > >   create_ovs_mirror(struct ovn_mirror *m, struct ovsdb_idl_txn
> *ovs_idl_txn,
> > > -                  const struct ovsrec_bridge *br_int)
> > > +                  const struct ovsrec_bridge *br_int,
> > > +                  struct shash *ovs_mirror_ports)
> > >   {
> > > -    struct ovsrec_interface *iface =
> ovsrec_interface_insert(ovs_idl_txn);
> > > -    char *port_name = xasprintf("ovn-%s", m->name);
> > > +    const struct ovsrec_port *mirror_port;
> > > +    if (!strcmp(m->sb_mirror->type, "local")) {
> > > +        mirror_port = shash_find_data(ovs_mirror_ports,
> m->sb_mirror->sink);
> > > +        if (!mirror_port) {
> > > +            return;
> > > +        }
> > > +    } else {
> > > +        struct ovsrec_interface *iface =
> ovsrec_interface_insert(ovs_idl_txn);
> > > +        char *port_name = xasprintf("ovn-%s", m->name);
> > >
> > > -    ovsrec_interface_set_name(iface, port_name);
> > > -    ovsrec_interface_set_type(iface, m->sb_mirror->type);
> > > -    set_mirror_iface_options(iface, m->sb_mirror);
> > > +        ovsrec_interface_set_name(iface, port_name);
> > > +        ovsrec_interface_set_type(iface, m->sb_mirror->type);
> > > +        set_mirror_iface_options(iface, m->sb_mirror);
> > >
> > > -    struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> > > -    ovsrec_port_set_name(port, port_name);
> > > -    ovsrec_port_set_interfaces(port, &iface, 1);
> > > -    ovsrec_bridge_update_ports_addvalue(br_int, port);
> > > +        struct ovsrec_port *port = ovsrec_port_insert(ovs_idl_txn);
> > > +        ovsrec_port_set_name(port, port_name);
> > > +        ovsrec_port_set_interfaces(port, &iface, 1);
> > > +        ovsrec_bridge_update_ports_addvalue(br_int, port);
> > >
> > > -    free(port_name);
> > > +        const struct smap port_external_ids =
> > > +            SMAP_CONST1(&port_external_ids, "ovn-owned", "true");
> > > +        ovsrec_port_set_external_ids(port, &port_external_ids);
> > > +
> > > +        free(port_name);
> > > +        mirror_port = port;
> > > +    }
> > >
> > >       m->ovs_mirror = ovsrec_mirror_insert(ovs_idl_txn);
> > >       ovsrec_mirror_set_name(m->ovs_mirror, m->name);
> > > -    ovsrec_mirror_set_output_port(m->ovs_mirror, port);
> > > +    ovsrec_mirror_set_output_port(m->ovs_mirror, mirror_port);
> > >
> > >       const struct smap external_ids =
> > >           SMAP_CONST1(&external_ids, "ovn-owned", "true");
> > > @@ -393,8 +480,13 @@ sync_ovs_mirror_ports(struct ovn_mirror *m, const
> struct ovsrec_bridge *br_int)
> > >   static void
> > >   delete_ovs_mirror(struct ovn_mirror *m, const struct ovsrec_bridge
> *br_int)
> > >   {
> > > -    ovsrec_bridge_update_ports_delvalue(br_int,
> m->ovs_mirror->output_port);
> > >       ovsrec_bridge_update_mirrors_delvalue(br_int, m->ovs_mirror);
> > > -    ovsrec_port_delete(m->ovs_mirror->output_port);
> > > +    bool ovn_owned =
> smap_get_bool(&m->ovs_mirror->output_port->external_ids,
> > > +                                   "ovn-owned", false);
> > > +    if (ovn_owned) {
> > > +        ovsrec_bridge_update_ports_delvalue(br_int,
> > > +
>  m->ovs_mirror->output_port);
> > > +        ovsrec_port_delete(m->ovs_mirror->output_port);
> > > +    }
> > >       ovsrec_mirror_delete(m->ovs_mirror);
> > >   }
> > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> > > index 4836a219f93d..dd5632562578 100644
> > > --- a/ovn-nb.ovsschema
> > > +++ b/ovn-nb.ovsschema
> > > @@ -1,7 +1,7 @@
> > >   {
> > >       "name": "OVN_Northbound",
> > > -    "version": "7.0.0",
> > > -    "cksum": "94023179 33468",
> > > +    "version": "7.0.1",
> > > +    "cksum": "441625132 33536",
> > >       "tables": {
> > >           "NB_Global": {
> > >               "columns": {
> > > @@ -315,7 +315,8 @@
> > >                   "sink":{"type": "string"},
> > >                   "type": {"type": {"key": {"type": "string",
> > >                                             "enum": ["set", ["gre",
> > > -
> "erspan"]]}}},
> > > +                                                           "erspan",
> > > +
> "local"]]}}},
> > >                   "index": {"type": "integer"},
> > >                   "external_ids": {
> > >                       "type": {"key": "string", "value": "string",
> > > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > > index 0552eff199a0..20b59df48f4e 100644
> > > --- a/ovn-nb.xml
> > > +++ b/ovn-nb.xml
> > > @@ -2693,14 +2693,19 @@ or
> > >       <column name="sink">
> > >         <p>
> > >           The value of this field represents the destination/sink of
> the mirror.
> > > -        The value it takes is an IP address of the sink port.
> > > +        If the <var>type</var> is <code>gre</code> or
> <code>erspan</code>,
> > > +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> > > +        For a <var>type</var> of <code>local</code>, this field
> defines a
> > > +        local interface on the OVS integration bridge to be used as the
> > > +        mirror destination. The interface must possess
> external-ids:mirror-id
> > > +        that matches this string.
> > >         </p>
> > >       </column>
> > >
> > >       <column name="type">
> > >         <p>
> > > -        The value of this field represents the type of the tunnel used
> for
> > > -        sending the mirrored packets.
> > > +        The value of this field specifies the mirror type -
> <code>gre</code>,
> > > +        <code>erspan</code> or <code>local</code>.
> > >         </p>
> > >       </column>
> > >
> > > @@ -2710,6 +2715,7 @@ or
> > >           tunnel type is <code>gre</code>, this field represents the
> > >           <code>GRE</code> key value and if the configured tunnel type
> is
> > >           <code>erspan</code> it represents the <code>erspan_idx</code>
> value.
> > > +        It is ignored if the type is <code>local</code>.
> > >         </p>
> > >       </column>
> > >
> > > diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> > > index 79ba6841e53a..d5ac393f9210 100644
> > > --- a/ovn-sb.ovsschema
> > > +++ b/ovn-sb.ovsschema
> > > @@ -1,7 +1,7 @@
> > >   {
> > >       "name": "OVN_Southbound",
> > > -    "version": "20.27.0",
> > > -    "cksum": "4078371916 30328",
> > > +    "version": "20.27.1",
> > > +    "cksum": "1439763681 30400",
> > >       "tables": {
> > >           "SB_Global": {
> > >               "columns": {
> > > @@ -151,8 +151,9 @@
> > >                                                         "to-lport"]]}}},
> > >                   "sink":{"type": "string"},
> > >                   "type": {"type": {"key": {"type": "string",
> > > -                                            "enum": ["set",
> > > -                                                     ["gre",
> "erspan"]]}}},
> > > +                                          "enum": ["set", ["gre",
> > > +                                                           "erspan",
> > > +
> "local"]]}}},
> > >                   "index": {"type": "integer"},
> > >                   "external_ids": {
> > >                       "type": {"key": "string", "value": "string",
> > > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > > index a77f8f4efb38..9599e55f44e5 100644
> > > --- a/ovn-sb.xml
> > > +++ b/ovn-sb.xml
> > > @@ -2907,20 +2907,29 @@ tcp.flags = RST;
> > >       <column name="sink">
> > >         <p>
> > >           The value of this field represents the destination/sink of
> the mirror.
> > > +        If the <var>type</var> is <code>gre</code> or
> <code>erspan</code>,
> > > +        the value indicates the tunnel remote IP (either IPv4 or IPv6).
> > > +        For a <var>type</var> of <code>local</code>, this field
> defines a
> > > +        local interface on the OVS integration bridge to be used as the
> > > +        mirror destination. The interface must possess
> external-ids:mirror-id
> > > +        that matches this string.
> > >         </p>
> > >       </column>
> > >
> > >       <column name="type">
> > >         <p>
> > > -        The value of this field represents the type of the tunnel used
> for
> > > -        sending the mirrored packets
> > > +        The value of this field specifies the mirror type -
> <code>gre</code>,
> > > +        <code>erspan</code> or <code>local</code>.
> > >         </p>
> > >       </column>
> > >
> > >       <column name="index">
> > >         <p>
> > > -        The value of this field represents the key/idx depending on the
> > > -        tunnel type configured
> > > +        The value of this field represents the tunnel ID. If the
> configured
> > > +        tunnel type is <code>gre</code>, this field represents the
> > > +        <code>GRE</code> key value and if the configured tunnel type is
> > > +        <code>erspan</code> it represents the <code>erspan_idx</code>
> value.
> > > +        It is ignored if the type is <code>local</code>.
> > >         </p>
> > >       </column>
> > >
> > > diff --git a/tests/ovn.at b/tests/ovn.at
> > > index 213ad18fa50b..4c124e0e58d9 100644
> > > --- a/tests/ovn.at
> > > +++ b/tests/ovn.at
> > > @@ -16427,7 +16427,7 @@ AT_CLEANUP
> > >   ])
> > >
> > >   OVN_FOR_EACH_NORTHD([
> > > -AT_SETUP([Mirror])
> > > +AT_SETUP([Mirror - remote])
> > >   AT_KEYWORDS([Mirror])
> > >   ovn_start
> > >
> > > @@ -16655,6 +16655,101 @@ OVN_CLEANUP([hv1], [hv2])
> > >   AT_CLEANUP
> > >   ])
> > >
> > > +OVN_FOR_EACH_NORTHD([
> > > +AT_SETUP([Mirror - local])
> > > +AT_KEYWORDS([Mirror])
> > > +ovn_start
> > > +
> > > +ovn-nbctl ls-add ls1
> > > +# Create logical port ls1-lp1 in ls1
> > > +ovn-nbctl lsp-add ls1 ls1-lp1 \
> > > +-- lsp-set-addresses ls1-lp1 "00:00:00:01:01:02 10.0.0.2"
> > > +ovn-nbctl lsp-add ls1 ls1-lp2 \
> > > +-- lsp-set-addresses ls1-lp2 "00:00:00:01:01:03 10.0.0.3"
> > > +
> > > +net_add n1
> > > +
> > > +sim_add hv1
> > > +as hv1
> > > +ovs-vsctl add-br br-phys -- set bridge br-phys
> other-config:hwaddr=\"00:00:00:01:02:00\"
> > > +ovn_attach n1 br-phys 192.168.1.11
> > > +ovn-appctl -t ovn-controller vlog/set file:dbg
> > > +
> > > +ovs-vsctl -- add-port br-int vif1 -- \
> > > +    set interface vif1 external-ids:iface-id=ls1-lp1 \
> > > +    options:tx_pcap=hv1/vif1-tx.pcap \
> > > +    options:rxq_pcap=hv1/vif1-rx.pcap \
> > > +    ofport-request=1
> > > +ovs-vsctl -- add-port br-int vif2 -- \
> > > +    set interface vif2 external-ids:iface-id=ls1-lp2 \
> > > +    options:tx_pcap=hv1/vif2-tx.pcap \
> > > +    options:rxq_pcap=hv1/vif2-rx.pcap \
> > > +    ofport-request=2
> > > +
> > > +# Add two ports as mirroring target
> > > +ovs-vsctl -- add-port br-int mirror1 -- \
> > > +    set interface mirror1 external-ids:mirror-id=sink1 \
> > > +    options:tx_pcap=hv1/mirror1-tx.pcap \
> > > +    options:rxq_pcap=hv1/mirror1-rx.pcap \
> > > +    ofport-request=3
> > > +ovs-vsctl -- add-port br-int mirror2 -- \
> > > +    set interface mirror2 external-ids:mirror-id=sink2 \
> > > +    options:tx_pcap=hv1/mirror2-tx.pcap \
> > > +    options:rxq_pcap=hv1/mirror2-rx.pcap \
> > > +    ofport-request=4
> > > +
> > > +# Create a NB mirror use DB 'create' command.
> > > +uuid1=$(ovn-nbctl create mirror name=mirror-from-lp1 type=local
> sink=sink1 filter=from-lport)
> > > +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror-from-lp1
> > > +
> > > +# Create another NB mirror use 'mirror-add' command.
> > > +check ovn-nbctl mirror-add mirror-to-lp2 local to-lport sink2
> > > +check ovn-nbctl lsp-attach-mirror ls1-lp2 mirror-to-lp2
> > > +
> > > +wait_for_ports_up
> > > +check ovn-nbctl --wait=hv sync
> > > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
> mirror | grep mirror | sort], [0], [dnl
> > > +mirror-from-lp1
> > > +mirror-to-lp2
> > > +])
> > > +
> > > +# Update to wrong mirror-id, the mirror should be deleted.
> > > +check ovn-nbctl set mirror $uuid1 sink=xxx
> > > +check ovn-nbctl --wait=hv sync
> > > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
> mirror | grep mirror | sort], [0], [dnl
> > > +mirror-to-lp2
> > > +])
> > > +
> > > +# Change back, and the mirror should be created back
> > > +check ovn-nbctl set mirror $uuid1 sink=sink1
> > > +check ovn-nbctl --wait=hv sync
> > > +AT_CHECK([ovs-vsctl --data=bare --no-heading --columns=name list
> mirror | grep mirror | sort], [0], [dnl
> > > +mirror-from-lp1
> > > +mirror-to-lp2
> > > +])
> > > +
> > > +# Send a packet from lp1 to lp2, should be mirrored to both mirror
> ports.
> > > +packet="inport==\"ls1-lp1\" && eth.src==00:00:00:01:01:02 &&
> eth.dst==00:00:00:01:01:03 &&
> > > +        ip4 && ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 &&
> icmp"
> > > +AT_CHECK([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> > > +
> > > +exp_packet="eth.src==00:00:00:01:01:02 && eth.dst==00:00:00:01:01:03
> && ip4 &&
> > > +            ip.ttl==64 && ip4.src==10.0.0.2 && ip4.dst==10.0.0.3 &&
> icmp"
> > > +echo $exp_packet | ovstest test-ovn expr-to-packets | sort > expout
> > > +
> > > +OVS_WAIT_UNTIL([
> > > +    rcv_mirror1=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in"
> hv1/mirror1-tx.pcap > mirror1.packets && cat mirror1.packets | wc -l`
> > > +    rcv_mirror2=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in"
> hv1/mirror2-tx.pcap > mirror2.packets && cat mirror2.packets | wc -l`
> > > +    echo $rcv_mirror1 $rcv_mirror2
> > > +    test $rcv_mirror1 -eq 1 -a $rcv_mirror2 -eq 1])
> > > +
> > > +AT_CHECK([cat mirror1.packets | sort], [0], [expout])
> > > +AT_CHECK([cat mirror2.packets | sort], [0], [expout])
> > > +
> > > +OVN_CLEANUP([hv1])
> > > +AT_CLEANUP
> > > +])
> > > +
> > >   OVN_FOR_EACH_NORTHD([
> > >   AT_SETUP([Mirror test bulk updates])
> > >   AT_KEYWORDS([Mirror test bulk updates])
> > > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml
> > > index 54dbdb791e52..c450b9a5ba7d 100644
> > > --- a/utilities/ovn-nbctl.8.xml
> > > +++ b/utilities/ovn-nbctl.8.xml
> > > @@ -1546,7 +1546,7 @@
> > >       <h2> Mirror commands</h2>
> > >       <dl>
> > >         <dt><code>mirror-add</code> <var>m</var> <var>type</var>
> > > -      <var>index</var> <var>filter</var> <var>dest</var></dt>
> > > +      [<var>index</var>] <var>filter</var> <var>dest</var></dt>
> > >         <dd>
> > >           <p>
> > >             Creates a new mirror in the <code>Mirror</code>
> > > @@ -1556,12 +1556,13 @@
> > >
> > >           <p>
> > >             <var>type</var> specifies the mirror type - <code>gre</code>
> > > -          or <code>erspan</code>.
> > > +          , <code>erspan</code> or <code>local</code>.
> > >           </p>
> > >
> > >           <p>
> > >             <var>index</var> specifies the tunnel index value (which is
> > > -          an integer).
> > > +          an integer) if the <var>type</var> is <code>gre</code> or
> > > +          <code>erspan</code>.
> > >           </p>
> > >
> > >           <p>
> > > @@ -1570,7 +1571,12 @@
> > >           </p>
> > >
> > >           <p>
> > > -          <var>dest</var> specifies the mirror destination IP (v4 or
> v6).
> > > +          <var>dest</var> specifies the mirror destination IP (v4 or
> v6)
> > > +          if the <var>type</var> is <code>gre</code> or
> <code>erspan</code>.
> > > +          For a <var>type</var> of <code>local</code>, this field
> defines a
> > > +          local interface on the OVS integration bridge to be used as
> the
> > > +          mirror destination. The interface must possess
> external-ids:mirror-id
> > > +          that matches this string.
> > >           </p>
> > >         </dd>
> > >
> > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > > index 9399f9462bb1..5f2fbfecfe5b 100644
> > > --- a/utilities/ovn-nbctl.c
> > > +++ b/utilities/ovn-nbctl.c
> > > @@ -273,15 +273,17 @@ QoS commands:\n\
> > >     qos-list SWITCH           print QoS rules for SWITCH\n\
> > >   \n\
> > >   Mirror commands:\n\
> > > -  mirror-add NAME TYPE INDEX FILTER IP\n\
> > > +  mirror-add NAME TYPE [INDEX] FILTER {IP | MIRROR-ID} \n\
> > >                               add a mirror with given name\n\
> > > -                            specify TYPE 'gre' or 'erspan'\n\
> > > +                            specify TYPE 'gre', 'erspan', or 'local'\n\
> > >                               specify the tunnel INDEX value\n\
> > >                                   (indicates key if GRE\n\
> > >                                    erpsan_idx if ERSPAN)\n\
> > >                               specify FILTER for mirroring selection\n\
> > >                                   'to-lport' / 'from-lport'\n\
> > > -                            specify Sink / Destination i.e. Remote
> IP\n\
> > > +                            specify Sink / Destination i.e. Remote IP,
> or a\n\
> > > +                                local interface with
> external-ids:mirror-id\n\
> > > +                                matching MIRROR-ID\n\
> > >     mirror-del [NAME]         remove mirrors\n\
> > >     mirror-list               print mirrors\n\
> > >   \n\
> > > @@ -7406,17 +7408,19 @@ parse_mirror_filter(const char *arg, const char
> **selection_p)
> > >   }
> > >
> > >   static char * OVS_WARN_UNUSED_RESULT
> > > -parse_mirror_tunnel_type(const char *arg, const char **type_p)
> > > +parse_mirror_type(const char *arg, const char **type_p)
> > >   {
> > >       /* Validate type.  Only require the first letter. */
> > >       if (arg[0] == 'g') {
> > >           *type_p = "gre";
> > >       } else if (arg[0] == 'e') {
> > >           *type_p = "erspan";
> > > +    } else if (arg[0] == 'l') {
> > > +        *type_p = "local";
> > >       } else {
> > >           *type_p = NULL;
> > > -        return xasprintf("%s: type must be \"gre\" or "
> > > -                         "\"erspan\"", arg);
> > > +        return xasprintf("%s: type must be \"gre\", "
> > > +                         "\"erspan\", or \"local\"", arg);
> > >       }
> > >       return NULL;
> > >   }
> > > @@ -7435,16 +7439,16 @@ static void
> > >   nbctl_mirror_add(struct ctl_context *ctx)
> > >   {
> > >       const char *filter = NULL;
> > > -    const char *sink_ip = NULL;
> > > +    const char *sink = NULL;
> > >       const char *type = NULL;
> > >       const char *name = NULL;
> > > -    char *new_sink_ip = NULL;
> > >       int64_t index;
> > >       char *error = NULL;
> > >       const struct nbrec_mirror *mirror_check = NULL;
> > > +    int pos = 1;
> > >
> > >       /* Mirror Name */
> > > -    name = ctx->argv[1];
> > > +    name = ctx->argv[pos++];
> > >       NBREC_MIRROR_FOR_EACH (mirror_check, ctx->idl) {
> > >           if (!strcmp(mirror_check->name, name)) {
> > >               ctl_error(ctx, "Mirror with %s name already exists.",
> > > @@ -7453,40 +7457,44 @@ nbctl_mirror_add(struct ctl_context *ctx)
> > >           }
> > >       }
> > >
> > > -    /* Tunnel Type - GRE/ERSPAN */
> > > -    error = parse_mirror_tunnel_type(ctx->argv[2], &type);
> > > +    /* Type - gre/erspan/local */
> > > +    error = parse_mirror_type(ctx->argv[pos++], &type);
> > >       if (error) {
> > >           ctx->error = error;
> > >           return;
> > >       }
> > >
> > > -    /* tunnel index / GRE key / ERSPAN idx */
> > > -    if (!str_to_long(ctx->argv[3], 10, (long int *) &index)) {
> > > -        ctl_error(ctx, "Invalid Index");
> > > -        return;
> > > +    if (strcmp(type, "local")) {
> > > +        /* tunnel index / GRE key / ERSPAN idx */
> > > +        if (!str_to_long(ctx->argv[pos++], 10, (long int *) &index)) {
> > > +            ctl_error(ctx, "Invalid Index");
> > > +            return;
> > > +        }
> > >       }
> > >
> > >       /* Filter for mirroring */
> > > -    error = parse_mirror_filter(ctx->argv[4], &filter);
> > > +    error = parse_mirror_filter(ctx->argv[pos++], &filter);
> > >       if (error) {
> > >           ctx->error = error;
> > >           return;
> > >       }
> > >
> > >       /* Destination / Sink details */
> > > -    sink_ip = ctx->argv[5];
> > > +    sink = ctx->argv[pos++];
> > >
> > > -    /* check if it is a valid ip */
> > > -    new_sink_ip = normalize_ipv4_addr_str(sink_ip);
> > > -    if (!new_sink_ip) {
> > > -        new_sink_ip = normalize_ipv6_addr_str(sink_ip);
> > > -    }
> > > +    /* check if it is a valid ip unless it is type 'local' */
> > > +    if (strcmp(type, "local")) {
> > > +        char *new_sink_ip = normalize_ipv4_addr_str(sink);
> > > +        if (!new_sink_ip) {
> > > +            new_sink_ip = normalize_ipv6_addr_str(sink);
> > > +        }
> > >
> > > -    if (!new_sink_ip) {
> > > -        ctl_error(ctx, "Invalid sink ip: %s", sink_ip);
> > > -        return;
> > > +        if (!new_sink_ip) {
> > > +            ctl_error(ctx, "Invalid sink ip: %s", sink);
> > > +            return;
> > > +        }
> > > +        free(new_sink_ip);
> > >       }
> > > -    free(new_sink_ip);
> > >
> > >       /* Create the mirror. */
> > >       struct nbrec_mirror *mirror = nbrec_mirror_insert(ctx->txn);
> > > @@ -7494,7 +7502,7 @@ nbctl_mirror_add(struct ctl_context *ctx)
> > >       nbrec_mirror_set_index(mirror, index);
> > >       nbrec_mirror_set_filter(mirror, filter);
> > >       nbrec_mirror_set_type(mirror, type);
> > > -    nbrec_mirror_set_sink(mirror, sink_ip);
> > > +    nbrec_mirror_set_sink(mirror, sink);
> > >
> > >   }
> > >
> > > @@ -7672,7 +7680,7 @@ static const struct ctl_command_syntax
> nbctl_commands[] = {
> > >         NULL, "", RO },
> > >
> > >       /* mirror commands. */
> > > -    { "mirror-add", 5, 5,
> > > +    { "mirror-add", 4, 5,
> > >         "NAME TYPE INDEX FILTER IP",
> > >         nbctl_pre_mirror_add, nbctl_mirror_add, NULL, "--may-exist", RW
> },
> > >       { "mirror-del", 0, 1, "[NAME]",
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev