On 15.04.2026 12:39, Dumitru Ceara wrote:
> 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.
Hi Dumitru! thanks a lot for pointing this out — you’re absolutely
right, it does look asymmetrical, and we can mirror the traffic that
will be drop on security stage in egress. I’ll address this in the v2.
>> 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
