On Fri, Apr 18, 2025 at 8:52 AM Alexandra Rukomoinikova
<arukomoinikova@k2.cloud> wrote:
>
> Added new mirror port type which is the link between the source
> and the target port, its parent is the target port. VLAN tagging
> is not required for mirror ports — packets are transmitted without
> VLAN tag. Also since lport mirror works due to logical flows, we
> don't need to pass information about the mirror to ovs.
>
> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud>
> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
> Co-authored-by: Vladislav Odintsov <vlodintsov@k2.cloud>
> Tested-by: Ivan Burnin <iburnin@k2.cloud>
> ---
> v9 --> v10: noting changed, just re-sent.
Thanks for v10. Please see some comments below.
With those comments addressed:
Acked-by: Numan Siddique <num...@ovn.org>
Numan
> ---
> controller/binding.c | 97 ++++++++++----
> controller/mirror.c | 4 +
> controller/physical.c | 28 ++--
> lib/ovn-util.c | 5 +
> lib/ovn-util.h | 1 +
> ovn-nb.xml | 6 +
> tests/ovn.at | 291 ++++++++++++++++++++++++++++++++++++++++++
> 7 files changed, 395 insertions(+), 37 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index fdb0ad124..6ce02d263 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -1537,14 +1537,15 @@ is_binding_lport_this_chassis(struct binding_lport
> *b_lport,
> || is_postponed_port(b_lport->pb->logical_port)));
> }
>
> -/* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER,
> - * 'false' otherwise. */
> +/* Returns 'true' if the 'lbinding' has binding lports of type
> + * LP_CONTAINER/LP_MIRROR, 'false' otherwise. */
> static bool
> is_lbinding_container_parent(struct local_binding *lbinding)
> {
> struct binding_lport *b_lport;
> LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> - if (b_lport->type == LP_CONTAINER) {
> + if (b_lport->type == LP_CONTAINER ||
> + b_lport->type == LP_MIRROR) {
> return true;
> }
> }
> @@ -1710,7 +1711,11 @@ consider_container_lport(const struct
> sbrec_port_binding *pb,
> {
> struct shash *local_bindings = &b_ctx_out->lbinding_data->bindings;
> struct local_binding *parent_lbinding;
> - parent_lbinding = local_binding_find(local_bindings, pb->parent_port);
> + bool is_mirror = !strcmp(pb->type, "mirror");
No need to use strcmp here. Pass the "lport_type" param to this
function and use it here.
i.,e
--------------
static bool
consider_container_lport(const struct sbrec_port_binding *pb,
enum lport_type type,
struct binding_ctx_in *b_ctx_in,
struct binding_ctx_out *b_ctx_out)
{
...
--------------------------------
> + const char *binding_port_name = is_mirror ? pb->mirror_port :
> + pb->parent_port;
> +
> + parent_lbinding = local_binding_find(local_bindings, binding_port_name);
>
> if (!parent_lbinding) {
> /* There is no local_binding for parent port. Create it
> @@ -1725,7 +1730,7 @@ consider_container_lport(const struct
> sbrec_port_binding *pb,
> * we want the these container ports also be claimed by the
> * chassis.
> * */
> - parent_lbinding = local_binding_create(pb->parent_port, NULL);
> + parent_lbinding = local_binding_create(binding_port_name, NULL);
> local_binding_add(local_bindings, parent_lbinding);
> }
>
> @@ -1739,17 +1744,26 @@ consider_container_lport(const struct
> sbrec_port_binding *pb,
> remove_related_lport(b_lport->pb, b_ctx_out);
> }
>
> - struct binding_lport *container_b_lport =
> - local_binding_add_lport(binding_lports, parent_lbinding, pb,
> - LP_CONTAINER);
> + struct binding_lport *container_b_lport;
>
> - struct binding_lport *parent_b_lport =
> - binding_lport_find(binding_lports, pb->parent_port);
> + if (is_mirror) {
> + container_b_lport = local_binding_add_lport(binding_lports,
> + parent_lbinding,
> + pb, LP_MIRROR);
> + } else {
> + container_b_lport = local_binding_add_lport(binding_lports,
> + parent_lbinding,
> + pb, LP_CONTAINER);
> + }
> +
> + struct binding_lport *parent_b_lport = binding_lport_find(
> + binding_lports,
> + binding_port_name);
Can you please keep the old indentation here ?
i.e
struct binding_lport *parent_b_lport =
binding_lport_find(binding_lports, binding_port_name);
...
>
> bool can_consider_c_lport = true;
> if (!parent_b_lport || !parent_b_lport->pb) {
> const struct sbrec_port_binding *parent_pb = lport_lookup_by_name(
> - b_ctx_in->sbrec_port_binding_by_name, pb->parent_port);
> + b_ctx_in->sbrec_port_binding_by_name, binding_port_name);
>
> if (parent_pb && get_lport_type(parent_pb) == LP_VIF) {
> /* Its possible that the parent lport is not considered yet.
> @@ -1757,7 +1771,7 @@ consider_container_lport(const struct
> sbrec_port_binding *pb,
> consider_vif_lport(parent_pb, b_ctx_in, b_ctx_out,
> parent_lbinding);
> parent_b_lport = binding_lport_find(binding_lports,
> - pb->parent_port);
> + binding_port_name);
> } else {
> /* The parent lport doesn't exist. Cannot consider the container
> * lport for binding. */
> @@ -1784,7 +1798,8 @@ consider_container_lport(const struct
> sbrec_port_binding *pb,
> }
>
> ovs_assert(parent_b_lport && parent_b_lport->pb);
> - /* cannot bind to this chassis if the parent_port cannot be bounded. */
> + /* cannot bind to this chassis if the parent_port/mirror_port
> + * cannot be bounded. */
> /* Do not bind neither if parent is postponed */
>
> enum can_bind can_bind =
> @@ -2212,6 +2227,10 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> binding_ctx_out *b_ctx_out)
> consider_container_lport(pb, b_ctx_in, b_ctx_out);
> break;
>
> + case LP_MIRROR:
> + consider_container_lport(pb, b_ctx_in, b_ctx_out);
> + break;
> +
> case LP_VIRTUAL:
> consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> break;
> @@ -2485,7 +2504,7 @@ consider_iface_claim(const struct ovsrec_interface
> *iface_rec,
> /* Update the child local_binding's iface (if any children) and try to
> * claim the container lbindings. */
> LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> - if (b_lport->type == LP_CONTAINER) {
> + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) {
> if (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out))
> {
> return false;
> }
> @@ -2575,7 +2594,7 @@ consider_iface_release(const struct ovsrec_interface
> *iface_rec,
> remove_related_lport(b_lport->pb, b_ctx_out);
> }
>
> - /* Check if the lbinding has children of type PB_CONTAINER.
> + /* Check if the lbinding has children of type PB_CONTAINER/PB_MIRROR.
> * If so, don't delete the local_binding. */
> if (!is_lbinding_container_parent(lbinding)) {
> local_binding_delete(lbinding, local_bindings, binding_lports,
> @@ -2883,13 +2902,13 @@ handle_deleted_vif_lport(const struct
> sbrec_port_binding *pb,
> }
> }
>
> - /* If its a container lport, then delete its entry from local_lports
> - * if present.
> + /* If its a container or mirror lport, then delete its entry from
> + * local_lports if present.
> * Note: If a normal lport is deleted, we don't want to remove
> * it from local_lports if there is a VIF entry.
> * consider_iface_release() takes care of removing from the local_lports
> * when the interface change happens. */
> - if (lport_type == LP_CONTAINER) {
> + if (lport_type == LP_CONTAINER || lport_type == LP_MIRROR) {
> remove_local_lports(pb->logical_port, b_ctx_out);
> }
>
> @@ -2912,7 +2931,8 @@ handle_updated_vif_lport(const struct
> sbrec_port_binding *pb,
>
> if (lport_type == LP_VIRTUAL) {
> handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> - } else if (lport_type == LP_CONTAINER) {
> + } else if (lport_type == LP_CONTAINER ||
> + lport_type == LP_MIRROR) {
> handled = consider_container_lport(pb, b_ctx_in, b_ctx_out);
> } else {
> handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL);
> @@ -2925,8 +2945,8 @@ handle_updated_vif_lport(const struct
> sbrec_port_binding *pb,
> bool now_claimed = (pb->chassis == b_ctx_in->chassis_rec);
>
> if (lport_type == LP_VIRTUAL || lport_type == LP_CONTAINER ||
> - (claimed == now_claimed &&
> - !is_additional_chassis(pb, b_ctx_in->chassis_rec))) {
> + lport_type == LP_MIRROR || (claimed == now_claimed &&
> + !is_additional_chassis(pb, b_ctx_in->chassis_rec))) {
> return true;
> }
>
> @@ -2944,7 +2964,7 @@ handle_updated_vif_lport(const struct
> sbrec_port_binding *pb,
>
> struct binding_lport *b_lport;
> LIST_FOR_EACH (b_lport, list_node, &lbinding->binding_lports) {
> - if (b_lport->type == LP_CONTAINER) {
> + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) {
> handled = consider_container_lport(b_lport->pb, b_ctx_in,
> b_ctx_out);
> if (!handled) {
> @@ -3069,6 +3089,7 @@ handle_updated_port(struct binding_ctx_in *b_ctx_in,
> switch (lport_type) {
> case LP_VIF:
> case LP_CONTAINER:
> + case LP_MIRROR:
> case LP_VIRTUAL:
> /* If port binding type just changed, port might be a "related_lport"
> * while it should not. Remove it from that set. It will be added
> @@ -3184,6 +3205,8 @@ binding_handle_port_binding_changes(struct
> binding_ctx_in *b_ctx_in,
> */
> struct shash deleted_container_pbs =
> SHASH_INITIALIZER(&deleted_container_pbs);
> + struct shash deleted_mirror_pbs =
> + SHASH_INITIALIZER(&deleted_mirror_pbs);
> struct shash deleted_virtual_pbs =
> SHASH_INITIALIZER(&deleted_virtual_pbs);
> struct shash deleted_vif_pbs =
> @@ -3225,6 +3248,8 @@ binding_handle_port_binding_changes(struct
> binding_ctx_in *b_ctx_in,
> shash_add(&deleted_vif_pbs, pb->logical_port, pb);
> } else if (lport_type == LP_CONTAINER) {
> shash_add(&deleted_container_pbs, pb->logical_port, pb);
> + } else if (lport_type == LP_MIRROR) {
> + shash_add(&deleted_mirror_pbs, pb->logical_port, pb);
> } else if (lport_type == LP_VIRTUAL) {
> shash_add(&deleted_virtual_pbs, pb->logical_port, pb);
> } else if (lport_type == LP_LOCALPORT) {
> @@ -3246,6 +3271,15 @@ binding_handle_port_binding_changes(struct
> binding_ctx_in *b_ctx_in,
> }
> }
>
> + SHASH_FOR_EACH_SAFE (node, &deleted_mirror_pbs) {
> + handled = handle_deleted_vif_lport(node->data, LP_MIRROR, b_ctx_in,
> + b_ctx_out);
> + shash_delete(&deleted_mirror_pbs, node);
> + if (!handled) {
> + goto delete_done;
> + }
> + }
> +
> SHASH_FOR_EACH_SAFE (node, &deleted_virtual_pbs) {
> handled = handle_deleted_vif_lport(node->data, LP_VIRTUAL, b_ctx_in,
> b_ctx_out);
> @@ -3277,6 +3311,7 @@ binding_handle_port_binding_changes(struct
> binding_ctx_in *b_ctx_in,
>
> delete_done:
> shash_destroy(&deleted_container_pbs);
> + shash_destroy(&deleted_mirror_pbs);
> shash_destroy(&deleted_virtual_pbs);
> shash_destroy(&deleted_vif_pbs);
> shash_destroy(&deleted_localport_pbs);
> @@ -3542,8 +3577,10 @@ local_binding_handle_stale_binding_lports(struct
> local_binding *lbinding,
> binding_lport_delete(&b_ctx_out->lbinding_data->lports,
> b_lport);
> handled = consider_virtual_lport(pb, b_ctx_in, b_ctx_out);
> - } else if (b_lport->type == LP_CONTAINER &&
> - pb_lport_type == LP_CONTAINER) {
> + } else if ((b_lport->type == LP_CONTAINER &&
> + pb_lport_type == LP_CONTAINER) ||
> + (b_lport->type == LP_MIRROR &&
> + pb_lport_type == LP_MIRROR)) {
> /* For container lport, binding_lport is preserved so that when
> * the parent port is created, it can be considered.
> * consider_container_lport() creates the binding_lport for the
> parent
> @@ -3762,9 +3799,9 @@ binding_lport_get_parent_pb(struct binding_lport
> *b_lport)
> * If the 'b_lport' type is LP_VIF, then its name and its lbinding->name
> * should match. Otherwise this should be cleaned up.
> *
> - * If the 'b_lport' type is LP_CONTAINER, then its parent_port name should
> - * be the same as its lbinding's name. Otherwise this should be
> - * cleaned up.
> + * If the 'b_lport' type is LP_CONTAINER or LP_MIRROR, then its parent_port
> + * name should be the same as its lbinding's name. Otherwise this should
> + * be cleaned up.
> *
> * If the 'b_lport' type is LP_VIRTUAL, then its virtual parent name
> * should be the same as its lbinding's name. Otherwise this
> @@ -3799,6 +3836,12 @@ binding_lport_check_and_cleanup(struct binding_lport
> *b_lport,
> }
> break;
>
> + case LP_MIRROR:
> + if (strcmp(b_lport->pb->mirror_port, b_lport->lbinding->name)) {
> + cleanup_blport = true;
> + }
> + break;
> +
> case LP_VIRTUAL:
> if (!b_lport->pb->virtual_parent ||
> strcmp(b_lport->pb->virtual_parent, b_lport->lbinding->name)) {
> diff --git a/controller/mirror.c b/controller/mirror.c
> index b557b96da..2f1c811a0 100644
> --- a/controller/mirror.c
> +++ b/controller/mirror.c
> @@ -121,6 +121,10 @@ mirror_run(struct ovsdb_idl_txn *ovs_idl_txn,
> /* Iterate through sb mirrors and build the 'ovn_mirrors'. */
> const struct sbrec_mirror *sb_mirror;
> SBREC_MIRROR_TABLE_FOR_EACH (sb_mirror, sb_mirror_table) {
> + /* We don't need to add mirror to ovs if it is lport mirror. */
> + if (!strcmp(sb_mirror->type, "lport")) {
> + continue;
> + }
> struct ovn_mirror *m = ovn_mirror_create(sb_mirror->name);
> m->sb_mirror = sb_mirror;
> ovn_mirror_add(&ovn_mirrors, m);
> diff --git a/controller/physical.c b/controller/physical.c
> index b4f91b134..ab7ea58da 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -1883,23 +1883,31 @@ consider_port_binding(const struct physical_ctx *ctx,
>
> int tag = 0;
> bool nested_container = false;
> - const struct sbrec_port_binding *parent_port = NULL;
> + const struct sbrec_port_binding *binding_port = NULL;
> ofp_port_t ofport;
> - if (binding->parent_port && *binding->parent_port) {
> - if (!binding->tag) {
> + bool is_mirror = !strcmp(binding->type, "mirror");
I think instead of using strcmp, you can use the 'type' variable
(en_lport_type) here right ?
> + if ((binding->parent_port && *binding->parent_port) || is_mirror) {
This can be
if ((binding->parent_port && *binding->parent_port) || type == LP_MIRROR)
> + if (!binding->tag && !is_mirror) {
Same here.
> return;
> }
> +
> + const char *binding_port_name = is_mirror ? binding->mirror_port :
> + binding->parent_port;
> ofport = local_binding_get_lport_ofport(ctx->local_bindings,
> - binding->parent_port);
> + binding_port_name);
> if (ofport) {
> - tag = *binding->tag;
> + if (!is_mirror) {
> + tag = *binding->tag;
> + }
> nested_container = true;
> - parent_port = lport_lookup_by_name(
> - ctx->sbrec_port_binding_by_name, binding->parent_port);
>
> - if (parent_port
> + binding_port
> + = lport_lookup_by_name(ctx->sbrec_port_binding_by_name,
> + binding_port_name);
> +
> + if (binding_port
> && !lport_can_bind_on_this_chassis(ctx->chassis,
> - parent_port)) {
> + binding_port)) {
> /* Even though there is an ofport for this container
> * parent port, it is requested on different chassis ignore
> * this ofport.
> @@ -1963,7 +1971,7 @@ consider_port_binding(const struct physical_ctx *ctx,
> struct zone_ids zone_ids = get_zone_ids(binding, ctx->ct_zones);
> /* Pass the parent port binding if the port is a nested
> * container. */
> - put_local_common_flows(dp_key, binding, parent_port, &zone_ids,
> + put_local_common_flows(dp_key, binding, binding_port, &zone_ids,
> &ctx->debug, ofpacts_p, flow_table);
>
> /* Table 0, Priority 150 and 100.
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index e2682ead7..af17082a9 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -1206,6 +1206,8 @@ get_lport_type(const struct sbrec_port_binding *pb)
> return LP_REMOTE;
> } else if (!strcmp(pb->type, "vtep")) {
> return LP_VTEP;
> + } else if (!strcmp(pb->type, "mirror")) {
> + return LP_MIRROR;
> }
>
> return LP_UNKNOWN;
> @@ -1219,6 +1221,8 @@ get_lport_type_str(enum en_lport_type lport_type)
> return "VIF";
> case LP_CONTAINER:
> return "CONTAINER";
> + case LP_MIRROR:
> + return "MIRROR";
> case LP_VIRTUAL:
> return "VIRTUAL";
> case LP_PATCH:
> @@ -1261,6 +1265,7 @@ is_pb_router_type(const struct sbrec_port_binding *pb)
>
> case LP_VIF:
> case LP_CONTAINER:
> + case LP_MIRROR:
> case LP_VIRTUAL:
> case LP_LOCALNET:
> case LP_LOCALPORT:
> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> index 3c04ff221..36f8adc9b 100644
> --- a/lib/ovn-util.h
> +++ b/lib/ovn-util.h
> @@ -409,6 +409,7 @@ enum en_lport_type {
> LP_UNKNOWN,
> LP_VIF,
> LP_CONTAINER,
> + LP_MIRROR,
> LP_PATCH,
> LP_L3GATEWAY,
> LP_LOCALNET,
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e416a1fba..44382a2cb 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1136,6 +1136,12 @@
> <code>ovn-ic</code> instead of by CMS. Any change to the port
> will
> be automatically overwritten by <code>ovn-ic</code>.
> </dd>
> +
> + <dt><code>mirror</code></dt>
> + <dd>
> + Service port for lport port mirroring that has a
> + gparent - target port(sink).
> + </dd>
Hmm. Its odd that this patch has this change. We are not adding new
port types in the Northbound db right ?
It should be removed.
> </dl>
> </column>
> </group>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 333068b99..52dd5bbe6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -18772,6 +18772,297 @@ OVN_CLEANUP([hv1],[hv2])
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([Mirror - lport: 2 HVs, 2 LS, 1 lport/LS, 2 peer LRs])
> +ovn_start
> +
> +# logical network:
> +# ls1 192.168.1.0/24 - lr1 - ls2 172.16.1.0/24
> +# ls1_lp1 - 192.168.1.2
> +# ls2_lp1 - 172.16.1.2
> +# ls2_lp2 - 172.16.1.3
> +#
> +# test cases
> +# sending packet from ls1_lp1 to ls2_lp1
> +# after mirror setting on ls1_lp1 expect to see
> +# mirrored packet on ls2_lp2
> +
> +ls1_lp1_mac="f0:00:00:01:02:03"
> +ls2_lp2_mac="f0:00:00:01:02:05"
> +rp_ls1_mac="00:00:00:01:02:03"
> +rp_ls2_mac="00:00:00:01:02:04"
> +ls2_lp1_mac="f0:00:00:01:02:04"
> +
> +ls1_lp1_ip="192.168.1.2"
> +ls2_lp1_ip="172.16.1.2"
> +ls2_lp2_ip="172.16.1.3"
> +
> +check ovn-nbctl lr-add R1
> +check ovn-nbctl lr-add R2
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl ls-add ls2
> +
> +# Connect ls1 to R1
> +check ovn-nbctl lrp-add R1 ls1 $rp_ls1_mac 192.168.1.1/24
> +
> +check ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1
> type=router \
> + options:router-port=ls1 addresses=\"$rp_ls1_mac\"
> +
> +# Connect ls2 to R2
> +check ovn-nbctl lrp-add R2 ls2 $rp_ls2_mac 172.16.1.1/24
> +
> +check ovn-nbctl lsp-add ls2 rp-ls2 -- set Logical_Switch_Port rp-ls2
> type=router \
> + options:router-port=ls2 addresses=\"$rp_ls2_mac\"
> +
> +# Connect R1 to R2
> +check ovn-nbctl lrp-add R1 R1_R2 00:00:00:02:03:04 20.0.0.1/24 peer=R2_R1
> +check ovn-nbctl lrp-add R2 R2_R1 00:00:00:02:03:05 20.0.0.2/24 peer=R1_R2
> +
> +# Add static routes
> +check ovn-nbctl lr-route-add R1 172.16.1.0/24 20.0.0.2
> +check ovn-nbctl lr-route-add R2 192.168.1.0/24 20.0.0.1
> +
> +# Create logical port ls1-lp1 in ls1
> +check ovn-nbctl lsp-add ls1 ls1-lp1 \
> +-- lsp-set-addresses ls1-lp1 "$ls1_lp1_mac $ls1_lp1_ip"
> +
> +# Create logical port ls2-lp1 in ls2
> +check ovn-nbctl lsp-add ls2 ls2-lp1 \
> +-- lsp-set-addresses ls2-lp1 "$ls2_lp1_mac $ls2_lp1_ip"
> +
> +# Create logical port ls2-lp2 in ls2
> +check ovn-nbctl lsp-add ls2 ls2-lp2 \
> +-- lsp-set-addresses ls2-lp2 "$ls2_lp2_mac $ls2_lp2_ip"
> +
> +# Create two hypervisor and create OVS ports corresponding to logical ports.
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.5
> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \
> + options:tx_pcap=hv1/vif1-tx.pcap \
> + options:rxq_pcap=hv1/vif1-rx.pcap \
> + ofport-request=1
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.6
> +ovs-vsctl -- add-port br-int hv2-vif1 -- \
> + set interface hv2-vif1 external-ids:iface-id=ls2-lp1 \
> + options:tx_pcap=hv2/vif1-tx.pcap \
> + options:rxq_pcap=hv2/vif1-rx.pcap \
> + ofport-request=2
> +
> +sim_add hv3
> +as hv3
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.7
> +ovs-vsctl -- add-port br-int hv3-vif1 -- \
> + set interface hv3-vif1 external-ids:iface-id=ls2-lp2 \
> + options:tx_pcap=hv3/vif1-tx.pcap \
> + options:rxq_pcap=hv3/vif1-rx.pcap \
> + ofport-request=3
> +
> +
> +# Pre-populate the hypervisors' ARP tables.
> +OVN_POPULATE_ARP
> +
> +# Allow some time for ovn-northd and ovn-controller to catch up.
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +
> +# Packet to send.
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +expected="eth.src==$rp_ls2_mac && eth.dst==$ls2_lp1_mac &&
> + ip4 && ip.ttl==62 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +echo $expected | ovstest test-ovn expr-to-packets > expected
> +
> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
> +
> +# Expect no packets on target port at the moment.
> +: > expected
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [expected])
> +
> +check ovn-nbctl mirror-add mirror0 lport both ls2-lp2
> +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror0
> +
> +check ovn-nbctl --wait=hv sync
> +
> +check_row_count Port_Binding 1 logical_port=mp-ls1-ls2-lp2
> +
> +hv3_uuid=$(fetch_column sb:Chassis _uuid name=hv3)
> +check_column "$hv3_uuid" sb:Port_Binding chassis logical_port=mp-ls1-ls2-lp2
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep mirror| ovn_strip_lflows], [0],
> [dnl
> + table=??(ls_in_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_mirror ), priority=100 , match=(inport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_out_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_mirror ), priority=100 , match=(outport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> +])
> +
> +as hv2 reset_pcap_file hv2-vif1 hv2/vif1
> +
> +echo "---------------------"
> +ovn-sbctl show
> +
> +echo "---------------------"
> +ovn-sbctl list port_b
> +
> +hv3_uuid=`ovn-sbctl --bare --columns _uuid find Chassis name="hv3"`
> +check_column "$hv3_uuid" sb:Port_Binding chassis logical_port=mp-ls1-ls2-lp2
> +
> +# Packet to send.
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +echo $packet | ovstest test-ovn expr-to-packets > packet
> +
> +expected="eth.src==$rp_ls2_mac && eth.dst==$ls2_lp1_mac &&
> + ip4 && ip.ttl==62 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +echo $expected | ovstest test-ovn expr-to-packets > expected
> +
> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
> +
> +# Expect mirrored packet on target port.
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [packet])
> +
> +as hv2 reset_pcap_file hv2-vif1 hv2/vif1
> +as hv3 reset_pcap_file hv3-vif1 hv3/vif1
> +
> +# Expect no mirrored packet on target port after detaching.
> +check ovn-nbctl lsp-detach-mirror ls1-lp1 mirror0
> +
> +check ovn-nbctl --wait=hv sync
> +
> +echo "---------------------"
> +ovn-sbctl show
> +
> +echo "---------------------"
> +ovn-sbctl list port_b
> +
> +check_row_count Port_Binding 0 logical_port=mp-ls1-ls2-lp2
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep mirror| ovn_strip_lflows], [0],
> [dnl
> + table=??(ls_in_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_mirror ), priority=0 , match=(1), action=(next;)
> +])
> +
> +as hv3 reset_pcap_file hv3-vif1 hv3/vif1
> +
> +# Packet to send.
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +expected="eth.src==$rp_ls2_mac && eth.dst==$ls2_lp1_mac &&
> + ip4 && ip.ttl==62 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +echo $expected | ovstest test-ovn expr-to-packets > expected
> +
> +OVN_CHECK_PACKETS([hv2/vif1-tx.pcap], [expected])
> +
> +# Expect no packets on target port after detaching mirror.
> +: > expected
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [expected])
> +
> +as hv2 reset_pcap_file hv2-vif1 hv2/vif1
> +as hv3 reset_pcap_file hv3-vif1 hv3/vif1
> +
> +# Test mirror filtering.
> +check ovn-nbctl lsp-attach-mirror ls1-lp1 mirror0
> +
> +check ovn-nbctl mirror-rule-add mirror0 200 '1' skip
> +check ovn-nbctl --wait=hv sync
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep mirror | ovn_strip_lflows], [0],
> [dnl
> + table=??(ls_in_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_mirror ), priority=100 , match=(inport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_in_mirror ), priority=300 , match=(inport == "ls1-lp1"
> && (1)), action=(next;)
> + table=??(ls_out_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_mirror ), priority=100 , match=(outport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_out_mirror ), priority=300 , match=(outport == "ls1-lp1"
> && (1)), action=(next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_out_pre_acl |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_pre_acl ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_pre_acl ), priority=110 , match=(eth.src ==
> $svc_monitor_mac), action=(next;)
> + table=??(ls_out_pre_acl ), priority=65535, match=(outport ==
> "mp-ls1-ls2-lp2"), action=(next(pipeline=egress, table=??);)
> +])
> +
> +# Check target port deletion.
> +check ovn-nbctl lsp-del ls2-lp2
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep mirror | ovn_strip_lflows], [0],
> [dnl
> + table=??(ls_in_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_mirror ), priority=0 , match=(1), action=(next;)
> +])
> +
> +check ovn-nbctl lsp-add ls2 ls2-lp2
> +
> +# Packet to send.
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +# Expect no packets on target port after rule added.
> +: > expected
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [expected])
> +
> +check ovn-nbctl mirror-rule-add mirror0 300 'udp.dst == 4369' mirror
> +check ovn-nbctl --wait=hv sync
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep mirror| ovn_strip_lflows], [0],
> [dnl
> + table=??(ls_in_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_in_mirror ), priority=100 , match=(inport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_in_mirror ), priority=300 , match=(inport == "ls1-lp1"
> && (1)), action=(next;)
> + table=??(ls_in_mirror ), priority=400 , match=(inport == "ls1-lp1"
> && (udp.dst == 4369)), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_out_mirror ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_mirror ), priority=100 , match=(outport ==
> "ls1-lp1"), action=(mirror("mp-ls1-ls2-lp2"); next;)
> + table=??(ls_out_mirror ), priority=300 , match=(outport == "ls1-lp1"
> && (1)), action=(next;)
> + table=??(ls_out_mirror ), priority=400 , match=(outport == "ls1-lp1"
> && (udp.dst == 4369)), action=(mirror("mp-ls1-ls2-lp2"); next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list ls1 | grep ls_out_pre_acl |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_pre_acl ), priority=0 , match=(1), action=(next;)
> + table=??(ls_out_pre_acl ), priority=110 , match=(eth.src ==
> $svc_monitor_mac), action=(next;)
> + table=??(ls_out_pre_acl ), priority=65535, match=(outport ==
> "mp-ls1-ls2-lp2"), action=(next(pipeline=egress, table=??);)
> +])
> +
> +# Packet to send.
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4369"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +echo $packet | ovstest test-ovn expr-to-packets > packet
> +
> +packet="inport==\"ls1-lp1\" && eth.src==$ls1_lp1_mac && eth.dst==$rp_ls1_mac
> &&
> + ip4 && ip.ttl==64 && ip4.src==$ls1_lp1_ip && ip4.dst==$ls2_lp1_ip &&
> + udp && udp.src==53 && udp.dst==4368"
> +OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"])
> +
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [packet])
> +
> +check ovn-nbctl mirror-del
> +
> +OVN_CLEANUP([hv1],[hv2],[hv3])
> +AT_CLEANUP
> +])
It would be great if you can add a multinode test for mirror ports.
It can be a follow up patch too,
> +
> OVN_FOR_EACH_NORTHD([
> AT_SETUP([Port Groups])
> AT_KEYWORDS([ovnpg])
> --
> 2.48.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
