On Mon, May 8, 2023 at 11:06 AM Mark Michelson <[email protected]> wrote:
>
> Hi Han,
>
> For both patches in the series,
>
> Acked-by: Mark Michelson <[email protected]>
>
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 <[email protected]>
> > ---
> >   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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to