diff mbox series

[ovs-dev] ovs-tcpdump:Solve the problem of residual mirror port

Message ID 2024032120333696815820@chinatelecom.cn
State Under Review
Delegated to: Simon Horman
Headers show
Series [ovs-dev] ovs-tcpdump:Solve the problem of residual mirror port | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

yangchang March 21, 2024, 12:33 p.m. UTC
Before creating an mirror port, it is necessary to first check whether the original port exists.
Otherwise, when the original port does not exist, the created mirror port will remain.

Signed-off-by: yangchang <yangchang@chinatelecom.cn>
---
 utilities/ovs-tcpdump.in | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--
1.8.3.1



yangchang@chinatelecom.cn

Comments

Simon Horman March 27, 2024, 5:49 p.m. UTC | #1
On Thu, Mar 21, 2024 at 08:33:37PM +0800, yangchang wrote:
> Before creating an mirror port, it is necessary to first check whether the original port exists.
> Otherwise, when the original port does not exist, the created mirror port will remain.
> 
> Signed-off-by: yangchang <yangchang@chinatelecom.cn>

Hi,

Thanks for your patch.

As I understand things, this avoids a residual mirror port for an error
case, by checking for the error condition before the port is created.

As such I agree it improves the situation.  However, I see several other
error cases that occur, which result in calls to sys.exit(1), after the
mirror port is created (the call to _make_mirror_name).  And that there are
other resources that seem to dangle, f.e. the call to _make_taps().

I am wondering if it would be netter to take a more robust approach to
cleaning up resources when errors occur.
diff mbox series

Patch

diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in
index a49ec9f..fb2f9b2 100755
--- a/utilities/ovs-tcpdump.in
+++ b/utilities/ovs-tcpdump.in
@@ -487,6 +487,9 @@  def main():
         print("TCPDUMP Args: %s" % ' '.join(tcpdargs))

     ovsdb = OVSDB(db_sock)
+    if not ovsdb.port_exists(interface):
+        print("ERROR: Port %s does not exist." % interface)
+        sys.exit(1)
     if mirror_interface is None:
         mirror_interface = "mi%s" % interface
         if sys.platform in _make_mirror_name:
@@ -508,9 +511,6 @@  def main():
               (mirror_interface, mirror_interface + "2"))
         sys.exit(1)

-    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))