On 4/13/26 10:54 AM, Alexandra Rukomoinikova wrote:
> Currently, ingress logical port mirroring does not take ACL rules into
> account, allowing the receiver interface (the sink in OVN) to observe
> the same traffic that exits the port.
>
> Ingress mirroring should also bypass port security checks. This is
> important because packets sent from a mirrored port that do not originate
> from the port’s MAC address would otherwise be dropped, potentially
> masking issues such as unexpected virtual machine behavior.
>
> Fixes: 2a2fe266d09c ("northd: Added support for port mirroring in OVN
> overlay.")
> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
> ---
Hi Alexandra,
Thanks for the patch, it looks good to me overall.
Just double checking, we're fine with the egress pipeline order right?
Mirroring happens now after to-lport ACLs are evaluated but _before_
port security. So if a packet is dropped by a to-lport ACL we don't
mirror it. But if a packet is dropped by egress port security we do
mirror it.
Please see some more small comments inline.
> lib/ovn-util.c | 4 ++--
> northd/northd.h | 6 +++---
> tests/ovn.at | 18 +++++++++++++++++-
> 3 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index 65fdb3a59..cb2692e0b 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -1026,8 +1026,8 @@ ip_address_and_port_from_lb_key(const char *key, char
> **ip_address,
> *
> * NOTE: If OVN_NORTHD_PIPELINE_CSUM is updated make sure to double check
> * whether an update of OVN_INTERNAL_MINOR_VER is required. */
> -#define OVN_NORTHD_PIPELINE_CSUM "3760014456 11249"
> -#define OVN_INTERNAL_MINOR_VER 13
> +#define OVN_NORTHD_PIPELINE_CSUM "2129825571 11245"
> +#define OVN_INTERNAL_MINOR_VER 14
>
> /* Returns the OVN version. The caller must free the returned value. */
> char *
> diff --git a/northd/northd.h b/northd/northd.h
> index 139519006..8f57b930d 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -509,9 +509,9 @@ ovn_datapath_is_stale(const struct ovn_datapath *od)
> /* Pipeline stages. */
> #define PIPELINE_STAGES \
> /* Logical switch ingress stages. */ \
> - PIPELINE_STAGE(SWITCH, IN, CHECK_PORT_SEC, 0, "ls_in_check_port_sec")
> \
> - PIPELINE_STAGE(SWITCH, IN, APPLY_PORT_SEC, 1, "ls_in_apply_port_sec")
> \
> - PIPELINE_STAGE(SWITCH, IN, MIRROR, 2, "ls_in_mirror") \
> + PIPELINE_STAGE(SWITCH, IN, MIRROR, 0, "ls_in_mirror") \
> + PIPELINE_STAGE(SWITCH, IN, CHECK_PORT_SEC, 1, "ls_in_check_port_sec") \
> + PIPELINE_STAGE(SWITCH, IN, APPLY_PORT_SEC, 2, "ls_in_apply_port_sec") \
> PIPELINE_STAGE(SWITCH, IN, LOOKUP_FDB, 3, "ls_in_lookup_fdb") \
> PIPELINE_STAGE(SWITCH, IN, PUT_FDB, 4, "ls_in_put_fdb") \
> PIPELINE_STAGE(SWITCH, IN, PRE_ACL, 5, "ls_in_pre_acl") \
> diff --git a/tests/ovn.at b/tests/ovn.at
> index cec3bb9a7..6d6481135 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -19122,9 +19122,25 @@ 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
>
> +# Ensure that port security on the source port does not impact mirroring:
> +# send a packet with an unknown MAC and checking it appears on the sink port.
Typo: and check.
> +check ovn-nbctl lsp-set-port-security ls1-lp1 $ls1_lp1_mac
Missing --wait=hv? We risk ovn-controller not having the port security
in place at the moment we're injecting the test packet below.
> +
> +fake_mac="f1:f1:f1:f1:f1:04"
> +packet="inport==\"ls1-lp1\" && eth.src==$fake_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
> +
> +OVN_CHECK_PACKETS([hv3/vif1-tx.pcap], [packet])
> +
> +as hv3 reset_pcap_file hv3-vif1 hv3/vif1
> +
> +# Test mirror filtering.
> check ovn-nbctl mirror-rule-add mirror0 200 '1' skip
> check ovn-nbctl --wait=hv sync
>
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
