| Message ID | 20250422091024.21582-3-arukomoinikova@k2.cloud |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/3,v11] utilities: Cli commands for overlay port mirroring. | expand |
Hi Numan, Dumitru and Ales, I sent the acked-by version from Numan, when will it be possible to get into upstream? Sorry for disturbing you) On 22.04.2025 12:10, Alexandra Rukomoinikova 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> > Acked-by: Numan Siddique <numans@ovn.org> > --- > v10 --> v11: added acked-by, fixed Numan comments. > --- > controller/binding.c | 110 +++++++++++----- > controller/mirror.c | 4 + > controller/physical.c | 28 ++-- > lib/ovn-util.c | 5 + > lib/ovn-util.h | 1 + > tests/ovn.at | 291 ++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 396 insertions(+), 43 deletions(-) > > diff --git a/controller/binding.c b/controller/binding.c > index fdb0ad124..f7535051f 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; > } > } > @@ -1705,12 +1706,16 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > > static bool > consider_container_lport(const struct sbrec_port_binding *pb, > + enum en_lport_type type, > struct binding_ctx_in *b_ctx_in, > struct binding_ctx_out *b_ctx_out) > { > 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); > + const char *binding_port_name = (type == LP_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,25 @@ 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; > + > + if (type == LP_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, pb->parent_port); > + 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 +1770,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 +1797,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 = > @@ -2209,7 +2223,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > break; > > case LP_CONTAINER: > - consider_container_lport(pb, b_ctx_in, b_ctx_out); > + consider_container_lport(pb, LP_CONTAINER, b_ctx_in, b_ctx_out); > + break; > + > + case LP_MIRROR: > + consider_container_lport(pb, LP_MIRROR, b_ctx_in, b_ctx_out); > break; > > case LP_VIRTUAL: > @@ -2485,8 +2503,9 @@ 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 (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out)) { > + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { > + if (!consider_container_lport(b_lport->pb, b_lport->type, > + 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,8 +2931,10 @@ 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) { > - handled = consider_container_lport(pb, b_ctx_in, b_ctx_out); > + } else if (lport_type == LP_CONTAINER || > + lport_type == LP_MIRROR) { > + handled = consider_container_lport(pb, lport_type, b_ctx_in, > + b_ctx_out); > } else { > handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL); > } > @@ -2925,8 +2946,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,9 +2965,9 @@ 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) { > - handled = consider_container_lport(b_lport->pb, b_ctx_in, > - b_ctx_out); > + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { > + handled = consider_container_lport(b_lport->pb, b_lport->type, > + b_ctx_in, b_ctx_out); > if (!handled) { > return false; > } > @@ -3069,6 +3090,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 +3206,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 +3249,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 +3272,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 +3312,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,14 +3578,16 @@ 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 > * port (with iface set to NULL). */ > - handled = consider_container_lport(b_lport->pb, b_ctx_in, > - b_ctx_out); > + handled = consider_container_lport(b_lport->pb, b_lport->type, > + b_ctx_in, b_ctx_out); > } else { > /* This can happen when the lport type changes from one type > * to another. Eg. from normal lport to external. Release the > @@ -3762,9 +3800,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 +3837,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..b26d12d3c 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 = (type == LP_MIRROR) ? true : false; > + if ((binding->parent_port && *binding->parent_port) || is_mirror) { > + if (!binding->tag && !is_mirror) { > 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/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 > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([Port Groups]) > AT_KEYWORDS([ovnpg])
On Thu, Apr 24, 2025 at 8:55 AM Rukomoinikova Aleksandra <ARukomoinikova@k2.cloud> wrote: > > Hi Numan, Dumitru and Ales, > > I sent the acked-by version from Numan, when will it be possible to get > into upstream? > > Sorry for disturbing you) Thanks for v11. I added a NEWS item entry for this in patch 3 and applied the whole series to main. Numan > > On 22.04.2025 12:10, Alexandra Rukomoinikova 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> > > Acked-by: Numan Siddique <numans@ovn.org> > > --- > > v10 --> v11: added acked-by, fixed Numan comments. > > --- > > controller/binding.c | 110 +++++++++++----- > > controller/mirror.c | 4 + > > controller/physical.c | 28 ++-- > > lib/ovn-util.c | 5 + > > lib/ovn-util.h | 1 + > > tests/ovn.at | 291 ++++++++++++++++++++++++++++++++++++++++++ > > 6 files changed, 396 insertions(+), 43 deletions(-) > > > > diff --git a/controller/binding.c b/controller/binding.c > > index fdb0ad124..f7535051f 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; > > } > > } > > @@ -1705,12 +1706,16 @@ consider_vif_lport(const struct sbrec_port_binding *pb, > > > > static bool > > consider_container_lport(const struct sbrec_port_binding *pb, > > + enum en_lport_type type, > > struct binding_ctx_in *b_ctx_in, > > struct binding_ctx_out *b_ctx_out) > > { > > 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); > > + const char *binding_port_name = (type == LP_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,25 @@ 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; > > + > > + if (type == LP_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, pb->parent_port); > > + 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 +1770,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 +1797,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 = > > @@ -2209,7 +2223,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) > > break; > > > > case LP_CONTAINER: > > - consider_container_lport(pb, b_ctx_in, b_ctx_out); > > + consider_container_lport(pb, LP_CONTAINER, b_ctx_in, b_ctx_out); > > + break; > > + > > + case LP_MIRROR: > > + consider_container_lport(pb, LP_MIRROR, b_ctx_in, b_ctx_out); > > break; > > > > case LP_VIRTUAL: > > @@ -2485,8 +2503,9 @@ 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 (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out)) { > > + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { > > + if (!consider_container_lport(b_lport->pb, b_lport->type, > > + 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,8 +2931,10 @@ 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) { > > - handled = consider_container_lport(pb, b_ctx_in, b_ctx_out); > > + } else if (lport_type == LP_CONTAINER || > > + lport_type == LP_MIRROR) { > > + handled = consider_container_lport(pb, lport_type, b_ctx_in, > > + b_ctx_out); > > } else { > > handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL); > > } > > @@ -2925,8 +2946,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,9 +2965,9 @@ 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) { > > - handled = consider_container_lport(b_lport->pb, b_ctx_in, > > - b_ctx_out); > > + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { > > + handled = consider_container_lport(b_lport->pb, b_lport->type, > > + b_ctx_in, b_ctx_out); > > if (!handled) { > > return false; > > } > > @@ -3069,6 +3090,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 +3206,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 +3249,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 +3272,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 +3312,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,14 +3578,16 @@ 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 > > * port (with iface set to NULL). */ > > - handled = consider_container_lport(b_lport->pb, b_ctx_in, > > - b_ctx_out); > > + handled = consider_container_lport(b_lport->pb, b_lport->type, > > + b_ctx_in, b_ctx_out); > > } else { > > /* This can happen when the lport type changes from one type > > * to another. Eg. from normal lport to external. Release the > > @@ -3762,9 +3800,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 +3837,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..b26d12d3c 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 = (type == LP_MIRROR) ? true : false; > > + if ((binding->parent_port && *binding->parent_port) || is_mirror) { > > + if (!binding->tag && !is_mirror) { > > 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/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 > > +]) > > + > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([Port Groups]) > > AT_KEYWORDS([ovnpg]) > > > -- > regards, > Alexandra. >
Hi Numan, Thanks a lot) On 30.04.2025 07:26, Numan Siddique wrote: > On Thu, Apr 24, 2025 at 8:55 AM Rukomoinikova Aleksandra > <ARukomoinikova@k2.cloud> wrote: >> Hi Numan, Dumitru and Ales, >> >> I sent the acked-by version from Numan, when will it be possible to get >> into upstream? >> >> Sorry for disturbing you) > > Thanks for v11. I added a NEWS item entry for this in patch 3 and > applied the whole series to main. > > Numan > >> On 22.04.2025 12:10, Alexandra Rukomoinikova 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> >>> Acked-by: Numan Siddique <numans@ovn.org> >>> --- >>> v10 --> v11: added acked-by, fixed Numan comments. >>> --- >>> controller/binding.c | 110 +++++++++++----- >>> controller/mirror.c | 4 + >>> controller/physical.c | 28 ++-- >>> lib/ovn-util.c | 5 + >>> lib/ovn-util.h | 1 + >>> tests/ovn.at | 291 ++++++++++++++++++++++++++++++++++++++++++ >>> 6 files changed, 396 insertions(+), 43 deletions(-) >>> >>> diff --git a/controller/binding.c b/controller/binding.c >>> index fdb0ad124..f7535051f 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; >>> } >>> } >>> @@ -1705,12 +1706,16 @@ consider_vif_lport(const struct sbrec_port_binding *pb, >>> >>> static bool >>> consider_container_lport(const struct sbrec_port_binding *pb, >>> + enum en_lport_type type, >>> struct binding_ctx_in *b_ctx_in, >>> struct binding_ctx_out *b_ctx_out) >>> { >>> 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); >>> + const char *binding_port_name = (type == LP_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,25 @@ 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; >>> + >>> + if (type == LP_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, pb->parent_port); >>> + 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 +1770,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 +1797,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 = >>> @@ -2209,7 +2223,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) >>> break; >>> >>> case LP_CONTAINER: >>> - consider_container_lport(pb, b_ctx_in, b_ctx_out); >>> + consider_container_lport(pb, LP_CONTAINER, b_ctx_in, b_ctx_out); >>> + break; >>> + >>> + case LP_MIRROR: >>> + consider_container_lport(pb, LP_MIRROR, b_ctx_in, b_ctx_out); >>> break; >>> >>> case LP_VIRTUAL: >>> @@ -2485,8 +2503,9 @@ 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 (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out)) { >>> + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { >>> + if (!consider_container_lport(b_lport->pb, b_lport->type, >>> + 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,8 +2931,10 @@ 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) { >>> - handled = consider_container_lport(pb, b_ctx_in, b_ctx_out); >>> + } else if (lport_type == LP_CONTAINER || >>> + lport_type == LP_MIRROR) { >>> + handled = consider_container_lport(pb, lport_type, b_ctx_in, >>> + b_ctx_out); >>> } else { >>> handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL); >>> } >>> @@ -2925,8 +2946,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,9 +2965,9 @@ 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) { >>> - handled = consider_container_lport(b_lport->pb, b_ctx_in, >>> - b_ctx_out); >>> + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { >>> + handled = consider_container_lport(b_lport->pb, b_lport->type, >>> + b_ctx_in, b_ctx_out); >>> if (!handled) { >>> return false; >>> } >>> @@ -3069,6 +3090,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 +3206,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 +3249,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 +3272,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 +3312,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,14 +3578,16 @@ 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 >>> * port (with iface set to NULL). */ >>> - handled = consider_container_lport(b_lport->pb, b_ctx_in, >>> - b_ctx_out); >>> + handled = consider_container_lport(b_lport->pb, b_lport->type, >>> + b_ctx_in, b_ctx_out); >>> } else { >>> /* This can happen when the lport type changes from one type >>> * to another. Eg. from normal lport to external. Release the >>> @@ -3762,9 +3800,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 +3837,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..b26d12d3c 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 = (type == LP_MIRROR) ? true : false; >>> + if ((binding->parent_port && *binding->parent_port) || is_mirror) { >>> + if (!binding->tag && !is_mirror) { >>> 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/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 >>> +]) >>> + >>> OVN_FOR_EACH_NORTHD([ >>> AT_SETUP([Port Groups]) >>> AT_KEYWORDS([ovnpg]) >> >> -- >> regards, >> Alexandra. >>
diff --git a/controller/binding.c b/controller/binding.c index fdb0ad124..f7535051f 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; } } @@ -1705,12 +1706,16 @@ consider_vif_lport(const struct sbrec_port_binding *pb, static bool consider_container_lport(const struct sbrec_port_binding *pb, + enum en_lport_type type, struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) { 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); + const char *binding_port_name = (type == LP_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,25 @@ 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; + + if (type == LP_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, pb->parent_port); + 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 +1770,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 +1797,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 = @@ -2209,7 +2223,11 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct binding_ctx_out *b_ctx_out) break; case LP_CONTAINER: - consider_container_lport(pb, b_ctx_in, b_ctx_out); + consider_container_lport(pb, LP_CONTAINER, b_ctx_in, b_ctx_out); + break; + + case LP_MIRROR: + consider_container_lport(pb, LP_MIRROR, b_ctx_in, b_ctx_out); break; case LP_VIRTUAL: @@ -2485,8 +2503,9 @@ 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 (!consider_container_lport(b_lport->pb, b_ctx_in, b_ctx_out)) { + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { + if (!consider_container_lport(b_lport->pb, b_lport->type, + 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,8 +2931,10 @@ 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) { - handled = consider_container_lport(pb, b_ctx_in, b_ctx_out); + } else if (lport_type == LP_CONTAINER || + lport_type == LP_MIRROR) { + handled = consider_container_lport(pb, lport_type, b_ctx_in, + b_ctx_out); } else { handled = consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL); } @@ -2925,8 +2946,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,9 +2965,9 @@ 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) { - handled = consider_container_lport(b_lport->pb, b_ctx_in, - b_ctx_out); + if (b_lport->type == LP_CONTAINER || b_lport->type == LP_MIRROR) { + handled = consider_container_lport(b_lport->pb, b_lport->type, + b_ctx_in, b_ctx_out); if (!handled) { return false; } @@ -3069,6 +3090,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 +3206,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 +3249,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 +3272,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 +3312,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,14 +3578,16 @@ 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 * port (with iface set to NULL). */ - handled = consider_container_lport(b_lport->pb, b_ctx_in, - b_ctx_out); + handled = consider_container_lport(b_lport->pb, b_lport->type, + b_ctx_in, b_ctx_out); } else { /* This can happen when the lport type changes from one type * to another. Eg. from normal lport to external. Release the @@ -3762,9 +3800,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 +3837,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..b26d12d3c 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 = (type == LP_MIRROR) ? true : false; + if ((binding->parent_port && *binding->parent_port) || is_mirror) { + if (!binding->tag && !is_mirror) { 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/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 +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([Port Groups]) AT_KEYWORDS([ovnpg])