On 12 Mar 2025, at 18:01, Aaron Conole wrote:
> When the 'mirror-to' option was conceived, it was intended to allow users
> the ability to arbitrarily build their own ports to use the mirror port
> rather than always using an algorithmically defined mirror port. However,
> the mirror port code never accommodated a port that the user defined as
> part of an OVS bridge. This could be useful for running against all kinds
> of OVS specific ports.
>
> Adjust the 'mirror-to' option so that when the user specifies a port, the
> utility searches through system interfaces, as well as the OVS DB. This
> means that we need to drop the port_exists() check for the mirror port,
> but that should be okay.
>
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-discuss/2025-March/053508.html
> Reported-by: Jun Wang <junwan...@cestc.cn>
> Signed-off-by: Aaron Conole <acon...@redhat.com>
Some style comment below. The rest looks good to me.
Cheers,
Eelco
> ---
> utilities/ovs-tcpdump.in | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
> index 187eafdf25..2070678b3a 100755
> --- a/utilities/ovs-tcpdump.in
> +++ b/utilities/ovs-tcpdump.in
> @@ -421,13 +421,16 @@ def py_which(executable):
> for path in os.environ["PATH"].split(os.pathsep))
>
>
> -def teardown(db_sock, interface, mirror_interface, tap_created):
> +def teardown(db_sock, interface, mirror_interface, tap_created,
> + created_mirror):
> def cleanup_mirror():
> try:
> ovsdb = OVSDB(db_sock)
> ovsdb.destroy_mirror(interface, ovsdb.port_bridge(interface))
> - ovsdb.destroy_port(mirror_interface,
> ovsdb.port_bridge(interface))
> - if tap_created is True:
> + if created_mirror:
> + ovsdb.destroy_port(mirror_interface,
> + ovsdb.port_bridge(interface))
> + if tap_created:
> _del_taps[sys.platform](mirror_interface)
> except Exception:
> print("Unable to tear down the ports and mirrors.")
> @@ -505,15 +508,21 @@ def main():
> if sys.platform in _make_mirror_name:
> mirror_interface = _make_mirror_name[sys.platform](interface)
>
> + created_mirror = True
> if sys.platform in _make_taps and \
> - mirror_interface not in interfaces():
> + mirror_interface not in interfaces() and \
> + not ovsdb.port_exists(mirror_interface):
> _make_taps[sys.platform](mirror_interface,
> ovsdb.interface_mtu(interface))
> tap_created = True
> else:
> + if mirror_interface in interfaces() or \
> + ovsdb.port_exists(mirror_interface):
> + created_mirror = False
> tap_created = False
>
> - if mirror_interface not in interfaces():
> + if mirror_interface not in interfaces() and \
> + not ovsdb.port_exists(mirror_interface):
> print("ERROR: Please create an interface called `%s`" %
> mirror_interface)
> print("See your OS guide for how to do this.")
> @@ -524,15 +533,12 @@ def main():
> if not ovsdb.port_exists(interface):
> print("ERROR: Port %s does not exist." % interface)
> sys.exit(1)
> - if ovsdb.port_exists(mirror_interface):
> - print("ERROR: Mirror port (%s) exists for port %s." %
> - (mirror_interface, interface))
> - sys.exit(1)
>
> - teardown(db_sock, interface, mirror_interface, tap_created)
> + teardown(db_sock, interface, mirror_interface, tap_created,
> created_mirror)
Move the two mirror-related arguments together, and use consistent naming for
the created boolean.
So for example:
teardown(db_sock, interface, mirror_interface, mirror_created,
tap_created)
>
> try:
> - ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
> + if created_mirror:
> + ovsdb.make_port(mirror_interface, ovsdb.port_bridge(interface))
> ovsdb.bridge_mirror(interface, mirror_interface,
> ovsdb.port_bridge(interface),
> mirror_select_all, mirror_filter=mirror_filter)
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
