[{"id":3404433,"web_url":"http://patchwork.ozlabs.org/comment/3404433/","msgid":"<7bf3ff88-70ee-4003-bc9e-2f810087fae8@nvidia.com>","list_archive_url":null,"date":"2024-10-29T12:16:00","subject":"Re: [ovs-dev] [PATCH 1/9] ipsec: Add a helper function to run\n commands from the monitor.","submitter":{"id":80349,"url":"http://patchwork.ozlabs.org/api/people/80349/","name":"Roi Dayan","email":"roid@nvidia.com"},"content":"On 29/10/2024 12:14, Ilya Maximets wrote:\n> Until now, functions that needed to call external programs like openssl\n> or ipsec commands were using subprocess commands directly.  Most of\n> these calls had no failure checks or any logging making it hard to\n> understand what is happening inside the daemon when something doesn't\n> work as intended.\n> \n> Some commands also had a chance to not read the command output in full.\n> That might sound like not a big problem, but in practice it causes\n> ovs-monitor-ipsec to deadlock pluto and itself with certain versions of\n> Libreswan (mainly Libreswan 5+).  The order of events is following:\n> \n>  1. ovs-monitor-ipsec calls ipsec status redirecting the output\n>     to a pipe.\n>  2. ipsec status calls ipsec whack.\n>  3. ipsec whack connects to pluto and asks for status.\n>  4. ovs-monitor-ipsec doesn't read the pipe in full.\n>  5. ipsec whack blocks on write to the other side of the pipe\n>     when it runs out of buffer space.\n>  6. pluto blocks on sendmsg to ipsec whack for the same reason.\n>  7. ovs-monitor-ipsec calls another ipsec command and blocks\n>     on connection to pluto.\n> \n> In this scenario the running process is at the mercy of garbage\n> collector and it doesn't run because we're blocked on calling another\n> ipsec command.  All the processes are completely blocked and will not\n> do any work until ipsec whack is killed.\n> \n> With this change we're introducing a new function that will be used\n> for all the external process execution commands and will read the full\n> output before returning, avoiding the deadlock.  It will also log all\n> the failures as warnings and the commands themselves at the debug level.\n> \n> We'll be adding more logic into this function in later commits as well,\n> so it will not stay that simple.\n> \n> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>\n> ---\n>  ipsec/ovs-monitor-ipsec.in | 296 +++++++++++++++++--------------------\n>  1 file changed, 134 insertions(+), 162 deletions(-)\n> \n> diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in\n> index 37c509ac6..19a401609 100755\n> --- a/ipsec/ovs-monitor-ipsec.in\n> +++ b/ipsec/ovs-monitor-ipsec.in\n> @@ -84,6 +84,28 @@ monitor = None\n>  xfrm = None\n>  \n>  \n> +def run_command(args, description=None):\n> +    \"\"\" This function runs the process args[0] with args[1:] arguments\n> +    and returns a tuple: return-code, stdout, stderr. \"\"\"\n> +\n> +    if not description:\n\nthe last arg as a description might not give much or confuse?\nmaybe it's better to use the first arg i.e. the command ?\n\n> +        description = args[-1]\n> +\n> +    vlog.dbg(\"Running %s\" % args)\n> +    proc = subprocess.Popen(args, stdout=subprocess.PIPE,\n> +                            stderr=subprocess.PIPE)\n> +    pout, perr = proc.communicate()\n> +\n\nyou can skip call to len() in the if statement.\npout and perr should always be a string empty or not.\nthe if statement will work well on perr itself.\n\n> +    if proc.returncode or len(perr):\n> +        vlog.warn(\"Failed to %s; exit code: %d\"\n> +                  % (description, proc.returncode))\n> +        vlog.warn(\"cmdline: %s\" % proc.args)\n> +        vlog.warn(\"stderr: %s\" % perr)\n> +        vlog.warn(\"stdout: %s\" % pout)\n> +\n\nSince pout and perr are always strings you don't need\nthe or statement.\n\nthis should work the same\n\n    return proc.returncode, pout, perr\n\nalso every caller that uses pout and/or perr\nwill do decode().strip().\nso why not return it like so by default?\n\n    return proc.returncode, pout.decode().strip(), perr.decode()\n\n\n> +    return proc.returncode, pout or b'', perr or b''\n> +\n> +\n>  class XFRM(object):\n>      \"\"\"This class is a simple wrapper around ip-xfrm (8) command line\n>      utility.  We are using this class only for informational purposes\n> @@ -99,13 +121,14 @@ class XFRM(object):\n>          where <key> is destination IPv4 address and <value> is SELECTOR of\n>          the IPsec policy.\"\"\"\n>          policies = {}\n> -        proc = subprocess.Popen([self.IP, 'xfrm', 'policy'],\n> -                                stdout=subprocess.PIPE)\n> -        while True:\n> -            line = proc.stdout.readline().strip().decode()\n> -            if line == '':\n> -                break\n> -            a = line.split(\" \")\n> +\n> +        ret, pout, perr = run_command([self.IP, 'xfrm', 'policy'],\n> +                                      \"get XFRM policies\")\n> +        if ret:\n> +            return policies\n> +\n> +        for line in pout.decode().splitlines():\n> +            a = line.strip().split(\" \")\n>              if len(a) >= 4 and a[0] == \"src\" and a[2] == \"dst\":\n>                  dst = (a[3].split(\"/\"))[0]\n>                  if dst not in policies:\n> @@ -122,13 +145,14 @@ class XFRM(object):\n>          in a dictionary where <key> is destination IPv4 address and <value>\n>          is SELECTOR.\"\"\"\n>          securities = {}\n> -        proc = subprocess.Popen([self.IP, 'xfrm', 'state'],\n> -                                stdout=subprocess.PIPE)\n> -        while True:\n> -            line = proc.stdout.readline().strip().decode()\n> -            if line == '':\n> -                break\n> -            a = line.split(\" \")\n> +\n> +        ret, pout, perr = run_command([self.IP, 'xfrm', 'state'],\n> +                                      \"get XFRM state\")\n> +        if ret:\n> +            return securities\n> +\n> +        for line in pout.decode().splitlines():\n> +            a = line.strip().split(\" \")\n>              if len(a) >= 4 and a[0] == \"sel\" \\\n>                      and a[1] == \"src\" and a[3] == \"dst\":\n>                  remote_ip = a[4].rstrip().split(\"/\")[0]\n> @@ -242,7 +266,7 @@ conn prevent_unencrypted_vxlan\n>          f.close()\n>  \n>          vlog.info(\"Restarting StrongSwan\")\n> -        subprocess.call([self.IPSEC, \"restart\"])\n> +        run_command([self.IPSEC, \"restart\"], \"restart StrongSwan\")\n>  \n>      def get_active_conns(self):\n>          \"\"\"This function parses output from 'ipsec status' command.\n> @@ -252,13 +276,13 @@ conn prevent_unencrypted_vxlan\n>          sample line from the parsed outpus as <value>. \"\"\"\n>  \n>          conns = {}\n> -        proc = subprocess.Popen([self.IPSEC, 'status'], stdout=subprocess.PIPE)\n> +        ret, pout, perr = run_command([self.IPSEC, 'status'],\n> +                                      \"get active connections\")\n> +        if ret:\n> +            return conns\n>  \n> -        while True:\n> -            line = proc.stdout.readline().strip().decode()\n> -            if line == '':\n> -                break\n> -            tunnel_name = line.split(\":\")\n> +        for line in pout.decode().splitlines():\n> +            tunnel_name = line.strip().split(\":\")\n>              if len(tunnel_name) < 2:\n>                  continue\n>              m = re.match(r\"(.*)(-in-\\d+|-out-\\d+|-\\d+).*\", tunnel_name[0])\n> @@ -341,15 +365,11 @@ conn prevent_unencrypted_vxlan\n>          Once strongSwan vici bindings will be distributed with major\n>          Linux distributions this function could be simplified.\"\"\"\n>          vlog.info(\"Refreshing StrongSwan configuration\")\n> -        proc = subprocess.Popen([self.IPSEC, \"update\"],\n> -                        stdout=subprocess.PIPE,\n> -                        stderr=subprocess.PIPE)\n> -        outs, errs = proc.communicate()\n> -        if proc.returncode != 0:\n> -            vlog.err(\"StrongSwan failed to update configuration:\\n\"\n> -                           \"%s \\n %s\" % (str(outs), str(errs)))\n> -\n> -        subprocess.call([self.IPSEC, \"rereadsecrets\"])\n> +\n> +        run_command([self.IPSEC, \"update\"],\n> +                    \"update StrongSwan's configuration\")\n> +        run_command([self.IPSEC, \"rereadsecrets\"], \"re-read secrets\")\n> +\n>          # \"ipsec update\" command does not remove those tunnels that were\n>          # updated or that disappeared from the ipsec.conf file.  So, we have\n>          # to manually remove them by calling \"ipsec stroke down-nb <tunnel>\"\n> @@ -382,7 +402,8 @@ conn prevent_unencrypted_vxlan\n>  \n>                  if not tunnel or tunnel.version != ver:\n>                      vlog.info(\"%s is outdated %u\" % (conn, ver))\n> -                    subprocess.call([self.IPSEC, \"stroke\", \"down-nb\", conn])\n> +                    run_command([self.IPSEC, \"stroke\", \"down-nb\", conn],\n> +                                \"stroke the outdated %s\" % conn)\n>  \n>  \n>  class LibreSwanHelper(object):\n> @@ -460,13 +481,11 @@ conn prevent_unencrypted_vxlan\n>          # Collect version infromation\n>          self.IPSEC = libreswan_root_prefix + \"/usr/sbin/ipsec\"\n>          self.IPSEC_AUTO = [self.IPSEC]\n> -        proc = subprocess.Popen([self.IPSEC, \"--version\"],\n> -                                stdout=subprocess.PIPE,\n> -                                encoding=\"latin1\")\n> -        pout, perr = proc.communicate()\n>  \n> -        v = re.match(\"^Libreswan v?(.*)$\", pout)\n> +        ret, pout, perr = run_command([self.IPSEC, \"--version\"],\n> +                                      \"get Libreswan's version\")\n>          try:\n> +            v = re.match(\"^Libreswan v?(.*)$\", pout.decode().strip())\n>              version = int(v.group(1).split(\".\")[0])\n>          except:\n>              version = 0\n> @@ -513,7 +532,7 @@ conn prevent_unencrypted_vxlan\n>          f.close()\n>  \n>          vlog.info(\"Restarting LibreSwan\")\n> -        subprocess.call([self.IPSEC, \"restart\"])\n> +        run_command([self.IPSEC, \"restart\"], \"restart Libreswan\")\n>  \n>      def config_init(self):\n>          self.conf_file = open(self.IPSEC_CONF, \"w\")\n> @@ -599,8 +618,10 @@ conn prevent_unencrypted_vxlan\n>  \n>      def refresh(self, monitor):\n>          vlog.info(\"Refreshing LibreSwan configuration\")\n> -        subprocess.call(self.IPSEC_AUTO + [\"--ctlsocket\", self.IPSEC_CTL,\n> -                        \"--config\", self.IPSEC_CONF, \"--rereadsecrets\"])\n> +        run_command(self.IPSEC_AUTO + [\"--ctlsocket\", self.IPSEC_CTL,\n> +                                       \"--config\", self.IPSEC_CONF,\n> +                                       \"--rereadsecrets\"],\n> +                    \"re-read secrets\")\n>          tunnels = set(monitor.tunnels.keys())\n>  \n>          # Delete old connections\n> @@ -627,9 +648,10 @@ conn prevent_unencrypted_vxlan\n>  \n>                  if not tunnel or tunnel.version != ver:\n>                      vlog.info(\"%s is outdated %u\" % (conn, ver))\n> -                    subprocess.call(self.IPSEC_AUTO + [\"--ctlsocket\",\n> -                                    self.IPSEC_CTL, \"--config\",\n> -                                    self.IPSEC_CONF, \"--delete\", conn])\n> +                    run_command(self.IPSEC_AUTO +\n> +                                [\"--ctlsocket\", self.IPSEC_CTL,\n> +                                 \"--config\", self.IPSEC_CONF,\n> +                                 \"--delete\", conn], \"delete %s\" % conn)\n>                  elif ifname in tunnels:\n>                      tunnels.remove(ifname)\n>  \n> @@ -649,43 +671,43 @@ conn prevent_unencrypted_vxlan\n>          # Update shunt policy if changed\n>          if monitor.conf_in_use[\"skb_mark\"] != monitor.conf[\"skb_mark\"]:\n>              if monitor.conf[\"skb_mark\"]:\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--add\",\n>                              \"--asynchronous\", \"prevent_unencrypted_gre\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--add\",\n>                              \"--asynchronous\", \"prevent_unencrypted_geneve\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--add\",\n>                              \"--asynchronous\", \"prevent_unencrypted_stt\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--add\",\n>                              \"--asynchronous\", \"prevent_unencrypted_vxlan\"])\n>              else:\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--delete\",\n>                              \"--asynchronous\", \"prevent_unencrypted_gre\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--delete\",\n>                              \"--asynchronous\", \"prevent_unencrypted_geneve\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--delete\",\n>                              \"--asynchronous\", \"prevent_unencrypted_stt\"])\n> -                subprocess.call(self.IPSEC_AUTO +\n> +                run_command(self.IPSEC_AUTO +\n>                              [\"--config\", self.IPSEC_CONF,\n>                              \"--ctlsocket\", self.IPSEC_CTL,\n>                              \"--delete\",\n> @@ -700,14 +722,13 @@ conn prevent_unencrypted_vxlan\n>          sample line from the parsed outpus as <value>. \"\"\"\n>  \n>          conns = {}\n> -        proc = subprocess.Popen([self.IPSEC, 'status', '--ctlsocket',\n> -                                self.IPSEC_CTL], stdout=subprocess.PIPE)\n> -\n> -        while True:\n> -            line = proc.stdout.readline().strip().decode()\n> -            if line == '':\n> -                break\n> +        ret, pout, perr = run_command([self.IPSEC, 'status',\n> +                                      '--ctlsocket', self.IPSEC_CTL],\n> +                                      \"get active connections\")\n> +        if ret:\n> +            return conns\n>  \n> +        for line in pout.decode().splitlines():\n>              m = re.search(r\"#\\d+: \\\"(.*)\\\".*\", line)\n>              if not m:\n>                  continue\n> @@ -732,117 +753,72 @@ conn prevent_unencrypted_vxlan\n>          # the \"ipsec auto --start\" command is lost. Just retry to make sure\n>          # the command is received by LibreSwan.\n>          while True:\n> -            proc = subprocess.Popen(self.IPSEC_AUTO +\n> -                                    [\"--config\", self.IPSEC_CONF,\n> -                                    \"--ctlsocket\", self.IPSEC_CTL,\n> -                                    \"--start\",\n> -                                    \"--asynchronous\", conn],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            perr = str(proc.stderr.read())\n> -            pout = str(proc.stdout.read())\n> -            if not re.match(r\".*Connection refused.*\", perr) and \\\n> -                    not re.match(r\".*need --listen.*\", pout):\n> +            ret, pout, perr = run_command(self.IPSEC_AUTO +\n> +                                          [\"--config\", self.IPSEC_CONF,\n> +                                          \"--ctlsocket\", self.IPSEC_CTL,\n> +                                          \"--start\",\n> +                                          \"--asynchronous\", conn],\n> +                                          \"start %s\" % conn)\n> +            if not re.match(r\".*Connection refused.*\", perr.decode()) and \\\n> +                    not re.match(r\".*need --listen.*\", pout.decode()):\n>                  break\n>  \n> -        if re.match(r\".*[F|f]ailed to initiate connection.*\", pout):\n> +        if re.match(r\".*[F|f]ailed to initiate connection.*\", pout.decode()):\n>              vlog.err('Failed to initiate connection through'\n>                      ' Interface %s.\\n' % (conn.split('-')[0]))\n> -            vlog.err(pout)\n> +            vlog.err(\"stdout: %s\" % pout)\n>  \n>      def _nss_clear_database(self):\n>          \"\"\"Remove all OVS IPsec related state from the NSS database\"\"\"\n> -        try:\n> -            proc = subprocess.Popen(['certutil', '-L', '-d',\n> -                                    self.IPSEC_D],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE,\n> -                                    universal_newlines=True)\n> -            lines = proc.stdout.readlines()\n> -\n> -            for line in lines:\n> -                s = line.strip().split()\n> -                if len(s) < 1:\n> -                    continue\n> -                name = s[0]\n> -                if name.startswith(self.CERT_PREFIX):\n> -                    self._nss_delete_cert(name)\n> -                elif name.startswith(self.CERTKEY_PREFIX):\n> -                    self._nss_delete_cert_and_key(name)\n> +        ret, pout, perr = run_command(['certutil', '-L', '-d', self.IPSEC_D],\n> +                                      \"clear NSS database\")\n> +        if ret:\n> +            return\n>  \n> -        except Exception as e:\n> -            vlog.err(\"Failed to clear NSS database.\\n\" + str(e))\n> +        for line in pout.decode().splitlines():\n> +            s = line.strip().split()\n> +            if len(s) < 1:\n> +                continue\n> +            name = s[0]\n> +            if name.startswith(self.CERT_PREFIX):\n> +                self._nss_delete_cert(name)\n> +            elif name.startswith(self.CERTKEY_PREFIX):\n> +                self._nss_delete_cert_and_key(name)\n>  \n>      def _nss_import_cert(self, cert, name, cert_type):\n>          \"\"\"Cert_type is 'CT,,' for the CA certificate and 'P,P,P' for the\n>          normal certificate.\"\"\"\n> -        try:\n> -            proc = subprocess.Popen(['certutil', '-A', '-a', '-i', cert,\n> -                                    '-d', self.IPSEC_D, '-n',\n> -                                    name, '-t', cert_type],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -        except Exception as e:\n> -            vlog.err(\"Failed to import certificate into NSS.\\n\" + str(e))\n> +        run_command(['certutil', '-A', '-a', '-i', cert, '-d', self.IPSEC_D,\n> +                     '-n', name, '-t', cert_type],\n> +                    \"import certificate %s into NSS\" % name)\n>  \n>      def _nss_delete_cert(self, name):\n> -        try:\n> -            proc = subprocess.Popen(['certutil', '-D', '-d',\n> -                                    self.IPSEC_D, '-n', name],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -        except Exception as e:\n> -            vlog.err(\"Failed to delete certificate from NSS.\\n\" + str(e))\n> +        run_command(['certutil', '-D', '-d', self.IPSEC_D, '-n', name],\n> +                    \"delete certificate %s from NSS\" % name)\n>  \n>      def _nss_import_cert_and_key(self, cert, key, name):\n> -        try:\n> -            # Avoid deleting other files\n> -            path = os.path.abspath('/tmp/%s.p12' % name)\n> -            if not path.startswith('/tmp/'):\n> -                raise Exception(\"Illegal certificate name!\")\n> -\n> -            # Create p12 file from pem files\n> -            proc = subprocess.Popen(['openssl', 'pkcs12', '-export',\n> -                                    '-in', cert, '-inkey', key, '-out',\n> -                                    path, '-name', name, '-passout', 'pass:'],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -\n> -            # Load p12 file to the database\n> -            proc = subprocess.Popen(['pk12util', '-i', path, '-d',\n> -                                    self.IPSEC_D, '-W', ''],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -\n> -        except Exception as e:\n> -            vlog.err(\"Import cert and key failed.\\n\" + str(e))\n> +        # Avoid deleting other files\n> +        path = os.path.abspath('/tmp/%s.p12' % name)\n> +        if not path.startswith('/tmp/'):\n> +            vlog.err(\"Illegal certificate name '%s'!\" % name)\n> +            return\n> +\n> +        if run_command(['openssl', 'pkcs12', '-export',\n> +                       '-in', cert, '-inkey', key,\n> +                       '-out', path, '-name', name,\n> +                       '-passout', 'pass:'],\n> +                       \"create p12 file from pem files\")[0]:\n> +            return\n> +\n> +        # Load p12 file to the database\n> +        run_command(['pk12util', '-i', path, '-d', self.IPSEC_D, '-W', ''],\n> +                    \"load p12 file to the NSS database\")\n>          os.remove(path)\n>  \n>      def _nss_delete_cert_and_key(self, name):\n> -        try:\n> -            # Delete certificate and private key\n> -            proc = subprocess.Popen(['certutil', '-F', '-d',\n> -                                    self.IPSEC_D, '-n', name],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -\n> -        except Exception as e:\n> -            vlog.err(\"Delete cert and key failed.\\n\" + str(e))\n> +        # Delete certificate and private key\n> +        run_command(['certutil', '-F', '-d', self.IPSEC_D, '-n', name],\n> +                    \"delete certificate and private key for %s\" % name)\n>  \n>  \n>  class IPsecTunnel(object):\n> @@ -1220,19 +1196,15 @@ class IPsecMonitor(object):\n>              self.ike_helper.refresh(self)\n>  \n>      def _get_cn_from_cert(self, cert):\n> -        try:\n> -            proc = subprocess.Popen(['openssl', 'x509', '-noout', '-subject',\n> -                                    '-nameopt', 'RFC2253', '-in', cert],\n> -                                    stdout=subprocess.PIPE,\n> -                                    stderr=subprocess.PIPE)\n> -            proc.wait()\n> -            if proc.returncode:\n> -                raise Exception(proc.stderr.read())\n> -            m = re.search(r\"CN=(.+?),\", proc.stdout.readline().decode())\n> -            if not m:\n> -                raise Exception(\"No CN in the certificate subject.\")\n> -        except Exception as e:\n> -            vlog.warn(str(e))\n> +        ret, pout, perr = run_command(['openssl', 'x509', '-noout', '-subject',\n> +                                       '-nameopt', 'RFC2253', '-in', cert],\n> +                                       \"get certificate %s options\" % cert)\n> +        if ret:\n> +            return None\n> +\n> +        m = re.search(r\"CN=(.+?),\", pout.decode().strip())\n> +        if not m:\n> +            vlog.warn(\"No CN in the certificate subject (%s).\" % cert)\n>              return None\n>  \n>          return m.group(1)","headers":{"Return-Path":"<ovs-dev-bounces@openvswitch.org>","X-Original-To":["incoming@patchwork.ozlabs.org","ovs-dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n unprotected) header.d=Nvidia.com header.i=@Nvidia.com header.a=rsa-sha256\n header.s=selector2 header.b=oS5fu4nf;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp3.osuosl.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key,\n unprotected) header.d=Nvidia.com header.i=@Nvidia.com header.a=rsa-sha256\n header.s=selector2 header.b=oS5fu4nf","smtp4.osuosl.org;\n dmarc=pass (p=reject dis=none) header.from=nvidia.com","smtp4.osuosl.org; dkim=pass (2048-bit key,\n unprotected) header.d=Nvidia.com header.i=@Nvidia.com header.a=rsa-sha256\n header.s=selector2 header.b=oS5fu4nf","dkim=none (message not signed)\n header.d=none;dmarc=none action=none header.from=nvidia.com;"],"Received":["from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4Xd8PH0y5Kz1xxC\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 29 Oct 2024 23:16:19 +1100 (AEDT)","from localhost (localhost [127.0.0.1])\n\tby smtp3.osuosl.org (Postfix) with ESMTP id 39A4360862;\n\tTue, 29 Oct 2024 12:16:17 +0000 (UTC)","from smtp3.osuosl.org ([127.0.0.1])\n by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id 1tNnE3GwcZIR; Tue, 29 Oct 2024 12:16:15 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56])\n\tby smtp3.osuosl.org (Postfix) with ESMTPS id 17C75600C4;\n\tTue, 29 Oct 2024 12:16:15 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id B1991C08A6;\n\tTue, 29 Oct 2024 12:16:14 +0000 (UTC)","from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 03A4AC08A3\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:16:12 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp4.osuosl.org (Postfix) with ESMTP id CF8D3404D4\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:16:12 +0000 (UTC)","from smtp4.osuosl.org ([127.0.0.1])\n by localhost (smtp4.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id jqM0hGcNPxwt for <ovs-dev@openvswitch.org>;\n Tue, 29 Oct 2024 12:16:10 +0000 (UTC)","from NAM04-DM6-obe.outbound.protection.outlook.com\n (mail-dm6nam04on20631.outbound.protection.outlook.com\n [IPv6:2a01:111:f403:2409::631])\n by smtp4.osuosl.org (Postfix) with ESMTPS id 7381C404C8\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:16:10 +0000 (UTC)","from SJ1PR12MB6027.namprd12.prod.outlook.com (2603:10b6:a03:48a::7)\n by CY8PR12MB7562.namprd12.prod.outlook.com (2603:10b6:930:95::16)\n with Microsoft SMTP Server (version=TLS1_2,\n cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8093.25; Tue, 29 Oct\n 2024 12:16:06 +0000","from SJ1PR12MB6027.namprd12.prod.outlook.com\n ([fe80::c5a7:d01b:ebc8:fae3]) by SJ1PR12MB6027.namprd12.prod.outlook.com\n ([fe80::c5a7:d01b:ebc8:fae3%3]) with mapi id 15.20.8093.023; Tue, 29 Oct 2024\n 12:16:06 +0000"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections - client-ip=140.211.9.56;\n helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=<UNKNOWN> ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp3.osuosl.org 17C75600C4","OpenDKIM Filter v2.11.0 smtp4.osuosl.org 7381C404C8"],"Received-SPF":"Pass (mailfrom) identity=mailfrom;\n client-ip=2a01:111:f403:2409::631;\n helo=nam04-dm6-obe.outbound.protection.outlook.com;\n envelope-from=roid@nvidia.com; receiver=<UNKNOWN>","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp4.osuosl.org 7381C404C8","ARC-Seal":"i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none;\n b=rjeRIsvBuZBHC8/pF9lwJdXYCPzDEaY4BDPO55KDv9boHNTSeEBIka6wAmNYPf6blOyd7rZaGGrxy9kaCzJ9RkcyRI6yb3b8SKoAFzvNwtnF3GyCh3eVteZofwV+gq085cWLU0Y1q0syAOpIGftUr5iCp205H6V/+7wjyhtuhsMMYHu5iA0dTrWUsunwC6yWJfS3SpbBLN4qWoBImO88fNOOvERFa97XTmZV0UyTJRFfgHQ1cPI57XhIfp+tKgAeFRwZhPvTLfdTTl27yjGJVOcCJDQOw3gT6wF4zE7cy+gPI7sHw07ZLzTeIDCLH1nhsRYqHVnyhmwJQLTxxeFeMA==","ARC-Message-Signature":"i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com;\n s=arcselector10001;\n h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;\n bh=1xqtWB8u0Thk6sUzrgT7GgFhndG1OdWSHFatingWZo0=;\n b=KarijjUirypcJaqhhCpFBpv5CfJ97X1PlqkmJtqRD7ikbxeiQw8aOSUtOr6iGvv1HU0fQebCDaXIDVVBXoCzNr86N7LS6r8tzwYqh92uuFAaZbdXG/wbsItDpBw+dOvmwqDXYB+kY4HWEUPQ+sGYnhwL1PbaIyxqHX6ngr4swazpfVORbiXYYZx+9xqHJTiCK2Xh9rRQ9EyNNDt9SMxMJ3mzPv6ps3cMpY9arj2nzSWA22ZscsxY7fdQKXq0NBsQqXjGsj8tCdzaLocO/n6Pe3hrGNlv3u2U2dEx8LnfmnGRhn8fgfPd/ONxSVq3LZWYXCaWC1kAs/6iFk9JbJqlGQ==","ARC-Authentication-Results":"i=1; mx.microsoft.com 1; spf=pass\n smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com;\n dkim=pass header.d=nvidia.com; arc=none","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com;\n s=selector2;\n h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;\n bh=1xqtWB8u0Thk6sUzrgT7GgFhndG1OdWSHFatingWZo0=;\n b=oS5fu4nfjqJ4gD8BZVEAokllz++pT+KSkGp/l8UAGROJLfNqmu+O1ujNvz6nLsFcZO+nVdDl+BM40Jk6Km2ICAtJzES68dMmBMp1vCjOhu13jRtk/Z16MWmpvZPBinzgNcOtYYpzeSErjywe6j5NHqSP/K/SpYs0TUBLjIeCD94Csje3lk76mKFrr6QzeTriNaYskwGFcvwXljjq5swzRJAuyahANQ3SHnme7V4hpayTUDIiWS5L9BB4O+xmjNzR6NR013eSRWujDWkE7vDQOwuL4t8mviSHYlY78suRDfAqMubDeaLJVGYgl5bedAbh/AaFxsQEG/wKwWw0UUGlLQ==","Message-ID":"<7bf3ff88-70ee-4003-bc9e-2f810087fae8@nvidia.com>","Date":"Tue, 29 Oct 2024 14:16:00 +0200","User-Agent":"Mozilla Thunderbird Beta","To":"Ilya Maximets <i.maximets@ovn.org>, ovs-dev@openvswitch.org","References":"<20241029101608.2991596-1-i.maximets@ovn.org>\n <20241029101608.2991596-2-i.maximets@ovn.org>","Content-Language":"en-US","In-Reply-To":"<20241029101608.2991596-2-i.maximets@ovn.org>","X-ClientProxiedBy":"LO6P265CA0001.GBRP265.PROD.OUTLOOK.COM\n (2603:10a6:600:339::9) To SJ1PR12MB6027.namprd12.prod.outlook.com\n (2603:10b6:a03:48a::7)","MIME-Version":"1.0","X-MS-PublicTrafficType":"Email","X-MS-TrafficTypeDiagnostic":"SJ1PR12MB6027:EE_|CY8PR12MB7562:EE_","X-MS-Office365-Filtering-Correlation-Id":"aaa53270-ed72-4e66-cb56-08dcf81376c7","X-MS-Exchange-SenderADCheck":"1","X-MS-Exchange-AntiSpam-Relay":"0","X-Microsoft-Antispam":"BCL:0;ARA:13230040|1800799024|366016|376014;","X-Microsoft-Antispam-Message-Info":"=?utf-8?q?jPtI794h0CBiuX6fcaI4smAA+1j+QT5?=\n\t=?utf-8?q?WXQJqdyV0uNWwRaeim/WdettLNQ5PAtu8dfqprYAtMH1kv3qKQC15mmWV+wyPawDQ?=\n\t=?utf-8?q?KmW1dvcZ/lDIhdsnWncP9e+X1RQc/nOVhqmOCcyCm5rkL6vNciIJriLUho41T6va+?=\n\t=?utf-8?q?/2CsWOg3vfniWyaBX2xkKalZlPCoNaCI417wJblJqv2H4lgJXenrKt39y7vHLxZ+I?=\n\t=?utf-8?q?js7/xQh2MEg1L1bUM5ACeK8BonF+IO/FHCYqqVE6f/mfMAgqAF3pDGAVr6zihjeMP?=\n\t=?utf-8?q?6NoKnOHGi4f/aoQWQSBkRKZxvL2WHINQPvuDPt9zlObc/KDlTnB2ZAKo/i19Rir4A?=\n\t=?utf-8?q?O+AWqwGQriVmBzWZCjLoynfCCsIT+MlJdw752IXZJea+l250GY0lpLE4Eg3gIiBPT?=\n\t=?utf-8?q?1ENE91tMgzWe+BP6jFlmIVX+FK8gNPVW0zJH1RkOQXBZFTgrx21vqlLC3cZOAA7Oj?=\n\t=?utf-8?q?dEq7TJmQE3oTUgS0nt8ZWHkG4bd4kB8M1bWCoIEcbAKbnW7xWXlRpbq3ifxKAyWjr?=\n\t=?utf-8?q?pjChk2CgjosD5qkMfrwS5tuEU43YzyvWaZ6/crwhCDGpKZ5NxLCWzuiBXgbZRQk1F?=\n\t=?utf-8?q?SBszlT4naHMUHQEH+4ZlVOyXoBpVJGitNILScveR+3edMJtITPwNOetJuSdrQCEvf?=\n\t=?utf-8?q?2nYNok7ccEDDVbuiAFhRJ5lnr/YyMs4Mk80BzPykft2rEo3tQXSmCKx1+bmNqvkZR?=\n\t=?utf-8?q?MbSgl9kyKEytMkxgj33iT5gm4yGE8fXsAHaOTUknoj8O3KsrF7sTijPffCos8mzl3?=\n\t=?utf-8?q?W1jcm8q/ua+Ar4Ibv4DEqm+nzewWNb0g2WY4RlVZxA84BvzVRIszjexz8E+BAGln/?=\n\t=?utf-8?q?Zec3BZxGCsrBPOs7yllLLxxOCPesJuPA6mB2d4Ru4cBoB4Qa9Namjlx7/PlRouqJv?=\n\t=?utf-8?q?lgR/NJFB7PZjwow86+bOzUR5ZGLgAvWUEZLQ+Xbhw8+6RBbcbl9JiMImg4qTZpVXK?=\n\t=?utf-8?q?ykDUS/+803bfPcPWwfviO1M6H2Z8/1A4KbPv4J0Qvdy8Qekaq+/4Ef1brLH89txmH?=\n\t=?utf-8?q?aC+cAoAUvD7uT8ulgny3jJaCkD2l00HTq1j7q2DYcAzmZ5j+Ba0AMi/DC2PAy/2gY?=\n\t=?utf-8?q?0c5nvCSF5tzPKuIv760WnETR/9/9Pv6qORAM3Qml2oRtLYkZ0nOZIvbLqrQNtXLdn?=\n\t=?utf-8?q?m4E211U+oWx509ac55mcF0/02QW5my9zZW60gO+TGOe0umymIl/v/439Aqins76LW?=\n\t=?utf-8?q?fF7E1Inuq6N8J4jvQd21jvjqXu3VEUrBtSE9NlgK75tsk3IAWI1LMQ3x4F+IuD+MZ?=\n\t=?utf-8?q?DWVFqgOqlk7n7?=","X-Forefront-Antispam-Report":"CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;\n IPV:NLI; SFV:NSPM; H:SJ1PR12MB6027.namprd12.prod.outlook.com; PTR:; CAT:NONE;\n SFS:(13230040)(1800799024)(366016)(376014); DIR:OUT; SFP:1101;","X-MS-Exchange-AntiSpam-MessageData-ChunkCount":"1","X-MS-Exchange-AntiSpam-MessageData-0":"=?utf-8?q?rrjy1hGh/4bgAuA+tEn22b6mAu2J?=\n\t=?utf-8?q?dT0/LoC0r7ywqP790Cysd6Mw0IyzrGxAWBpXA14xWYnI/PV3duP9bij9FfrZFocIV?=\n\t=?utf-8?q?80/KFkA/o12DsXdiyBQp9/5+7ICz0Ze8acfwge1+aN521U/he74m8QfxQSk/4syKT?=\n\t=?utf-8?q?doLEvSN7xnx5dgF7ax/Oipg8KKV00Bazh8NxNOOWu5NgSKAwlGe1ZTAE+5XzJThng?=\n\t=?utf-8?q?uVS3WUW/zEUTsmDEYi49A9Nkp9euidGPEqNBEYHeBUuGXAULGqOI2kqlunKb9cPQz?=\n\t=?utf-8?q?jm3Ss/+d+rq6AhWqx0Oo75oH3T9ZBDin18PEmpMdiXLhbbyioq6qLUOTV0JqTD23a?=\n\t=?utf-8?q?5IfzVMiPkf2K8KUSqV7qaE7n7q/gEfM/DgM1PrJVEyElq5SZwgKWjHxQuZrcqgbMa?=\n\t=?utf-8?q?+ZSu0XXTMXqBvHfRNr1DF5zmISeyWkQlX+DfNO163IA8q5/Jgdi39B4ggl/5Aguyq?=\n\t=?utf-8?q?EVMADZffaPjPpSaG+MlAMK2OB3l3t+vhE7BscSEiOhiq8fSF2y/4aAIQpg9Zu2s6S?=\n\t=?utf-8?q?Blw2jfqWxkxAkjFaAzW7HMMBlD4Ka88DBXBiYwpsEof4PJL4QbkagN4NmQ79Wvgez?=\n\t=?utf-8?q?K8UF19i1XOX6yvdCMYR1KeMQnZlsLUR62RFOcw7OHEN/d5x6cw1yJEuDfuVxMg8sh?=\n\t=?utf-8?q?p4D02QKpafai1hf3o16xPlGKZPlbp9UebQVj+MIb7ir0EvJ4p4r9P6DFI+LPf+ue/?=\n\t=?utf-8?q?uH7jU1UJlWDsYQ/3LGCGhLzv7hEVsdT1QhoPIQNJSj3tTXDSGVG9BrMP5E8JJBvDa?=\n\t=?utf-8?q?M8fnD6rikk/IfEfYSA7AbcnmiPgEExyX0aRaJkwa+AMQlwjbLnAfsQdzI92PvIpA/?=\n\t=?utf-8?q?RZkHrL3dZiS6gCI+XjrnkqY+BWvWXNLu6ayIFk0W0wZWBVyhWXRpnOboCaUNYdtEH?=\n\t=?utf-8?q?iAAQu1W/w/mxjgOoKYo2SvGZhoaHQriYIBflcjYZGDpj9iCYA5n9Ptoc7ka0HwOx3?=\n\t=?utf-8?q?9lagV0UeOclh3N7T9rwk1z8OkL4kkoJV8lsGr+D7dOVVuj5/msxdR9KSvjM6lAfh5?=\n\t=?utf-8?q?73ZO13rB0h5bDvTUTzgJ8Xa+gMegYxp+TkxSPXiSnljnkiPpOeKmr9H3+B3d/OVj9?=\n\t=?utf-8?q?SEt9lW5NK3dUomHeHIPQNZHJt/oJUyjQVkQaTkXRV5Ho/UvIVc2tgKXoOUPwFtHhW?=\n\t=?utf-8?q?xC7CQSOcZ3n+/a0c6b0BV90rjIToUFQ7Ik2J+UzYs313v4ympn0xAfgu/9/DKBGG4?=\n\t=?utf-8?q?0L/BoXkIGWpQFGwLrZYO4441DLyOcJFTw7L98TsRQGKwpRYTEGLJ2igb0BxDEL/6/?=\n\t=?utf-8?q?afbqsNUgMbohQmpr+R7JywQ9kHOz5xVKCKr9D8JiXxI8/+jv5jB+xVi3M16IQORnZ?=\n\t=?utf-8?q?XVjwkE697vMH/ugRIT3dOV9FT1XXCJbk0t856SDPFmbubTsWVVVvYvEcPg+GAhor8?=\n\t=?utf-8?q?YGgLC6ux7ZwaXIWBPMMLpYdiAlzFK7tfRlEl9qH5osk7gHNh1USpnUb9ooAcYl41z?=\n\t=?utf-8?q?+terqzNJhytnLfljMwexr6tBUMR3E2g51do97mYja1GBAoT/zoA4s6siEB7CVM7RD?=\n\t=?utf-8?q?aixyOyexXEdt?=","X-OriginatorOrg":"Nvidia.com","X-MS-Exchange-CrossTenant-Network-Message-Id":"\n aaa53270-ed72-4e66-cb56-08dcf81376c7","X-MS-Exchange-CrossTenant-AuthSource":"SJ1PR12MB6027.namprd12.prod.outlook.com","X-MS-Exchange-CrossTenant-AuthAs":"Internal","X-MS-Exchange-CrossTenant-OriginalArrivalTime":"29 Oct 2024 12:16:06.3741 (UTC)","X-MS-Exchange-CrossTenant-FromEntityHeader":"Hosted","X-MS-Exchange-CrossTenant-Id":"43083d15-7273-40c1-b7db-39efd9ccc17a","X-MS-Exchange-CrossTenant-MailboxType":"HOSTED","X-MS-Exchange-CrossTenant-UserPrincipalName":"\n amN4f9msdu62Nm8EJDGCKSSjyucYZ2XbMSx60oy08M4Hq8GTD/j7hoer8Iiij5f3","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"CY8PR12MB7562","Subject":"Re: [ovs-dev] [PATCH 1/9] ipsec: Add a helper function to run\n commands from the monitor.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"<ovs-dev.openvswitch.org>","List-Unsubscribe":"<https://mail.openvswitch.org/mailman/options/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>","List-Archive":"<http://mail.openvswitch.org/pipermail/ovs-dev/>","List-Post":"<mailto:ovs-dev@openvswitch.org>","List-Help":"<mailto:ovs-dev-request@openvswitch.org?subject=help>","List-Subscribe":"<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=subscribe>","From":"Roi Dayan via dev <ovs-dev@openvswitch.org>","Reply-To":"Roi Dayan <roid@nvidia.com>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" <ovs-dev-bounces@openvswitch.org>"}},{"id":3404470,"web_url":"http://patchwork.ozlabs.org/comment/3404470/","msgid":"<8e8f56f3-3ddb-4b75-9f76-bcf6351f4b97@ovn.org>","list_archive_url":null,"date":"2024-10-29T12:50:27","subject":"Re: [ovs-dev] [PATCH 1/9] ipsec: Add a helper function to run\n commands from the monitor.","submitter":{"id":76798,"url":"http://patchwork.ozlabs.org/api/people/76798/","name":"Ilya Maximets","email":"i.maximets@ovn.org"},"content":"On 10/29/24 13:16, Roi Dayan wrote:\n> \n> \n> On 29/10/2024 12:14, Ilya Maximets wrote:\n>> Until now, functions that needed to call external programs like openssl\n>> or ipsec commands were using subprocess commands directly.  Most of\n>> these calls had no failure checks or any logging making it hard to\n>> understand what is happening inside the daemon when something doesn't\n>> work as intended.\n>>\n>> Some commands also had a chance to not read the command output in full.\n>> That might sound like not a big problem, but in practice it causes\n>> ovs-monitor-ipsec to deadlock pluto and itself with certain versions of\n>> Libreswan (mainly Libreswan 5+).  The order of events is following:\n>>\n>>  1. ovs-monitor-ipsec calls ipsec status redirecting the output\n>>     to a pipe.\n>>  2. ipsec status calls ipsec whack.\n>>  3. ipsec whack connects to pluto and asks for status.\n>>  4. ovs-monitor-ipsec doesn't read the pipe in full.\n>>  5. ipsec whack blocks on write to the other side of the pipe\n>>     when it runs out of buffer space.\n>>  6. pluto blocks on sendmsg to ipsec whack for the same reason.\n>>  7. ovs-monitor-ipsec calls another ipsec command and blocks\n>>     on connection to pluto.\n>>\n>> In this scenario the running process is at the mercy of garbage\n>> collector and it doesn't run because we're blocked on calling another\n>> ipsec command.  All the processes are completely blocked and will not\n>> do any work until ipsec whack is killed.\n>>\n>> With this change we're introducing a new function that will be used\n>> for all the external process execution commands and will read the full\n>> output before returning, avoiding the deadlock.  It will also log all\n>> the failures as warnings and the commands themselves at the debug level.\n>>\n>> We'll be adding more logic into this function in later commits as well,\n>> so it will not stay that simple.\n>>\n>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>\n>> ---\n>>  ipsec/ovs-monitor-ipsec.in | 296 +++++++++++++++++--------------------\n>>  1 file changed, 134 insertions(+), 162 deletions(-)\n>>\n>> diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in\n>> index 37c509ac6..19a401609 100755\n>> --- a/ipsec/ovs-monitor-ipsec.in\n>> +++ b/ipsec/ovs-monitor-ipsec.in\n>> @@ -84,6 +84,28 @@ monitor = None\n>>  xfrm = None\n>>  \n>>  \n>> +def run_command(args, description=None):\n>> +    \"\"\" This function runs the process args[0] with args[1:] arguments\n>> +    and returns a tuple: return-code, stdout, stderr. \"\"\"\n>> +\n>> +    if not description:\n> \n> the last arg as a description might not give much or confuse?\n> maybe it's better to use the first arg i.e. the command ?\n\nThanks, Roi, for taking a look!\n\nThe first argument in most cases is 'ipsec', so it doesn't give\na lot of context.  The last argument is used, just because I was\na little lazy and didn't want to add repeating descriptions to\nall the calls like this:\n\n    run_command(self.IPSEC_AUTO +\n                [\"--config\", self.IPSEC_CONF,\n                \"--ctlsocket\", self.IPSEC_CTL,\n                \"--add\",\n                \"--asynchronous\", \"prevent_unencrypted_gre\"])\n\nThese ones have a recongnizeable last argument, which is an actual\ncommand name.  Most other types of calls do have their description\nspecified.\n\nSo, we could either keep as is, or add \"prevent unencrypted gre\"\nto the commands like the one above, which seems a little repetitive.\nBut using the first argument doesn't seem to be very useful.\n\nWDYT?\n\n> \n>> +        description = args[-1]\n>> +\n>> +    vlog.dbg(\"Running %s\" % args)\n>> +    proc = subprocess.Popen(args, stdout=subprocess.PIPE,\n>> +                            stderr=subprocess.PIPE)\n>> +    pout, perr = proc.communicate()\n>> +\n> \n> you can skip call to len() in the if statement.\n> pout and perr should always be a string empty or not.\n> the if statement will work well on perr itself.\n\nYeah, good point, I'll change that.\n\n> \n>> +    if proc.returncode or len(perr):\n>> +        vlog.warn(\"Failed to %s; exit code: %d\"\n>> +                  % (description, proc.returncode))\n>> +        vlog.warn(\"cmdline: %s\" % proc.args)\n>> +        vlog.warn(\"stderr: %s\" % perr)\n>> +        vlog.warn(\"stdout: %s\" % pout)\n>> +\n> \n> Since pout and perr are always strings you don't need\n> the or statement.\n> \n> this should work the same\n> \n>     return proc.returncode, pout, perr\n\nI guess, there were too many verions of this code and they were not\ninitialized at some point during timeout exception handling in the\npatch #5, but they are now, so I should fix this, agreed.\n\n> \n> also every caller that uses pout and/or perr\n> will do decode().strip().\n> so why not return it like so by default?\n> \n>     return proc.returncode, pout.decode().strip(), perr.decode()\n\nGood point.  I'd leave the strip() for the caller, but otherwise\nit makes sense to decode here, true.\n\n> \n> \n>> +    return proc.returncode, pout or b'', perr or b''\n>> +\n>> +\n\nBest regards, Ilya Maximets.","headers":{"Return-Path":"<ovs-dev-bounces@openvswitch.org>","X-Original-To":["incoming@patchwork.ozlabs.org","ovs-dev@openvswitch.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","ovs-dev@lists.linuxfoundation.org"],"Authentication-Results":["legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org\n (client-ip=140.211.166.136; helo=smtp3.osuosl.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org)","smtp3.osuosl.org;\n dmarc=none (p=none dis=none) header.from=ovn.org"],"Received":["from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4Xd98r4zKzz1xwF\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 29 Oct 2024 23:50:36 +1100 (AEDT)","from localhost (localhost [127.0.0.1])\n\tby smtp3.osuosl.org (Postfix) with ESMTP id 4476560869;\n\tTue, 29 Oct 2024 12:50:34 +0000 (UTC)","from smtp3.osuosl.org ([127.0.0.1])\n by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id 99hKyiBqXTEw; Tue, 29 Oct 2024 12:50:33 +0000 (UTC)","from lists.linuxfoundation.org (lf-lists.osuosl.org\n [IPv6:2605:bc80:3010:104::8cd3:938])\n\tby smtp3.osuosl.org (Postfix) with ESMTPS id 0E26B60851;\n\tTue, 29 Oct 2024 12:50:33 +0000 (UTC)","from lf-lists.osuosl.org (localhost [127.0.0.1])\n\tby lists.linuxfoundation.org (Postfix) with ESMTP id BC405C08A6;\n\tTue, 29 Oct 2024 12:50:32 +0000 (UTC)","from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136])\n by lists.linuxfoundation.org (Postfix) with ESMTP id 4F222C08A3\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:50:32 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp3.osuosl.org (Postfix) with ESMTP id 3DEF5607CA\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:50:32 +0000 (UTC)","from smtp3.osuosl.org ([127.0.0.1])\n by localhost (smtp3.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id Hn4Pg0l6O4LD for <ovs-dev@openvswitch.org>;\n Tue, 29 Oct 2024 12:50:31 +0000 (UTC)","from mail-wm1-f66.google.com (mail-wm1-f66.google.com\n [209.85.128.66])\n by smtp3.osuosl.org (Postfix) with ESMTPS id 0BAE060782\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 12:50:30 +0000 (UTC)","by mail-wm1-f66.google.com with SMTP id\n 5b1f17b1804b1-4316e9f4a40so54651975e9.2\n for <ovs-dev@openvswitch.org>; Tue, 29 Oct 2024 05:50:30 -0700 (PDT)","from [192.168.0.13] (ip-86-49-44-151.bb.vodafone.cz. [86.49.44.151])\n by smtp.gmail.com with ESMTPSA id\n ffacd0b85a97d-38058b3c236sm12393986f8f.35.2024.10.29.05.50.27\n (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n Tue, 29 Oct 2024 05:50:28 -0700 (PDT)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections -\n client-ip=2605:bc80:3010:104::8cd3:938; helo=lists.linuxfoundation.org;\n envelope-from=ovs-dev-bounces@openvswitch.org; receiver=<UNKNOWN> ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp3.osuosl.org 0E26B60851","OpenDKIM Filter v2.11.0 smtp3.osuosl.org 0BAE060782"],"Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=209.85.128.66;\n helo=mail-wm1-f66.google.com; envelope-from=i.maximets.ovn@gmail.com;\n receiver=<UNKNOWN>","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp3.osuosl.org 0BAE060782","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=1e100.net; s=20230601; t=1730206229; x=1730811029;\n h=content-transfer-encoding:in-reply-to:autocrypt:from\n :content-language:references:to:subject:cc:user-agent:mime-version\n :date:message-id:x-gm-message-state:from:to:cc:subject:date\n :message-id:reply-to;\n bh=H+VJKX85Wj0gy61dG/PYuL86tcrBkZdy1FhdEF0wzuA=;\n b=xAsn2szrxprddncpf+jk0wvRm90Il9eJPz/PRkSSnM3OWb0wAOtYM7GJf4rGsprlDQ\n 8CkPjUX3RIJs04H95Mk1mA3ZmsJPHGwMdEPhh/TwAGmCsizi/lOmudfxcYPE7LZUudYx\n WGJlfBrhpzFwqIaItwIz5qznmMDm5zIyM3GZP3e2mt4k7tD5CEyohrIV1h5LrR/tbS95\n Vkgx1L1UP7bsEKdNwEL+D48X/DUU5E/aU87Tozj10Rcstm++bGh0Z/QCV5r2zSgT+TCA\n 5DMep+zDl8rKKDYGmTRDQKP/RgUFfDfDGrYC/A6owFe0GJpotqQ+6iePFyxp89tCMHmu\n 61Vw==","X-Forwarded-Encrypted":"i=1;\n AJvYcCUJy+n3Zow0WzYcHpbyhEFerqjgF/i4ONDdnjgtjM/ppxn4rEuJeSzRUUKOJaQmckCjZEQamC+j@openvswitch.org","X-Gm-Message-State":"AOJu0Yy0JxHI0x2LlzeY+HNRdhRH06msrb4WNJkwQaK9d5FGylli5C+y\n QsMGiUMHXg4E6ce74sVQgZBYt3jmuokEGxbr8xz8O526fkSbp2x+","X-Google-Smtp-Source":"\n AGHT+IHnQhCBA88yfFLfn+bHhfiKbQIn7iUx2RnGYYhlSXa9w+awJjet+/J1It9itqhcz88JHKoAGg==","X-Received":"by 2002:a05:600c:468b:b0:42c:af06:703 with SMTP id\n 5b1f17b1804b1-4319ad34be1mr90568395e9.31.1730206228510;\n Tue, 29 Oct 2024 05:50:28 -0700 (PDT)","Message-ID":"<8e8f56f3-3ddb-4b75-9f76-bcf6351f4b97@ovn.org>","Date":"Tue, 29 Oct 2024 13:50:27 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","To":"Roi Dayan <roid@nvidia.com>, ovs-dev@openvswitch.org","References":"<20241029101608.2991596-1-i.maximets@ovn.org>\n <20241029101608.2991596-2-i.maximets@ovn.org>\n <7bf3ff88-70ee-4003-bc9e-2f810087fae8@nvidia.com>","Content-Language":"en-US","From":"Ilya Maximets <i.maximets@ovn.org>","Autocrypt":"addr=i.maximets@ovn.org; keydata=\n xsFNBF77bOMBEADVZQ4iajIECGfH3hpQMQjhIQlyKX4hIB3OccKl5XvB/JqVPJWuZQRuqNQG\n /B70MP6km95KnWLZ4H1/5YOJK2l7VN7nO+tyF+I+srcKq8Ai6S3vyiP9zPCrZkYvhqChNOCF\n pNqdWBEmTvLZeVPmfdrjmzCLXVLi5De9HpIZQFg/Ztgj1AZENNQjYjtDdObMHuJQNJ6ubPIW\n cvOOn4WBr8NsP4a2OuHSTdVyAJwcDhu+WrS/Bj3KlQXIdPv3Zm5x9u/56NmCn1tSkLrEgi0i\n /nJNeH5QhPdYGtNzPixKgPmCKz54/LDxU61AmBvyRve+U80ukS+5vWk8zvnCGvL0ms7kx5sA\n tETpbKEV3d7CB3sQEym8B8gl0Ux9KzGp5lbhxxO995KWzZWWokVUcevGBKsAx4a/C0wTVOpP\n FbQsq6xEpTKBZwlCpxyJi3/PbZQJ95T8Uw6tlJkPmNx8CasiqNy2872gD1nN/WOP8m+cIQNu\n o6NOiz6VzNcowhEihE8Nkw9V+zfCxC8SzSBuYCiVX6FpgKzY/Tx+v2uO4f/8FoZj2trzXdLk\n BaIiyqnE0mtmTQE8jRa29qdh+s5DNArYAchJdeKuLQYnxy+9U1SMMzJoNUX5uRy6/3KrMoC/\n 7zhn44x77gSoe7XVM6mr/mK+ViVB7v9JfqlZuiHDkJnS3yxKPwARAQABzSJJbHlhIE1heGlt\n ZXRzIDxpLm1heGltZXRzQG92bi5vcmc+wsGUBBMBCAA+AhsDBQsJCAcCBhUKCQgLAgQWAgMB\n Ah4BAheAFiEEh+ma1RKWrHCY821auffsd8gpv5YFAmP+Y/MFCQjFXhAACgkQuffsd8gpv5Yg\n OA//eEakvE7xTHNIMdLW5r3XnWSEY44dFDEWTLnS7FbZLLHxPNFXN0GSAA8ZsJ3fE26O5Pxe\n EEFTf7R/W6hHcSXNK4c6S8wR4CkTJC3XOFJchXCdgSc7xS040fLZwGBuO55WT2ZhQvZj1PzT\n 8Fco8QKvUXr07saHUaYk2Lv2mRhEPP9zsyy7C2T9zUzG04a3SGdP55tB5Adi0r/Ea+6VJoLI\n ctN8OaF6BwXpag8s76WAyDx8uCCNBF3cnNkQrCsfKrSE2jrvrJBmvlR3/lJ0OYv6bbzfkKvo\n 0W383EdxevzAO6OBaI2w+wxBK92SMKQB3R0ZI8/gqCokrAFKI7gtnyPGEKz6jtvLgS3PeOtf\n 5D7PTz+76F/X6rJGTOxR3bup+w1bP/TPHEPa2s7RyJISC07XDe24n9ZUlpG5ijRvfjbCCHb6\n pOEijIj2evcIsniTKER2pL+nkYtx0bp7dZEK1trbcfglzte31ZSOsfme74u5HDxq8/rUHT01\n 51k/vvUAZ1KOdkPrVEl56AYUEsFLlwF1/j9mkd7rUyY3ZV6oyqxV1NKQw4qnO83XiaiVjQus\n K96X5Ea+XoNEjV4RdxTxOXdDcXqXtDJBC6fmNPzj4QcxxyzxQUVHJv67kJOkF4E+tJza+dNs\n 8SF0LHnPfHaSPBFrc7yQI9vpk1XBxQWhw6oJgy3OwU0EXvts4wEQANCXyDOic0j2QKeyj/ga\n OD1oKl44JQfOgcyLVDZGYyEnyl6b/tV1mNb57y/YQYr33fwMS1hMj9eqY6tlMTNz+ciGZZWV\n YkPNHA+aFuPTzCLrapLiz829M5LctB2448bsgxFq0TPrr5KYx6AkuWzOVq/X5wYEM6djbWLc\n VWgJ3o0QBOI4/uB89xTf7mgcIcbwEf6yb/86Cs+jaHcUtJcLsVuzW5RVMVf9F+Sf/b98Lzrr\n 2/mIB7clOXZJSgtV79Alxym4H0cEZabwiXnigjjsLsp4ojhGgakgCwftLkhAnQT3oBLH/6ix\n 87ahawG3qlyIB8ZZKHsvTxbWte6c6xE5dmmLIDN44SajAdmjt1i7SbAwFIFjuFJGpsnfdQv1\n OiIVzJ44kdRJG8kQWPPua/k+AtwJt/gjCxv5p8sKVXTNtIP/sd3EMs2xwbF8McebLE9JCDQ1\n RXVHceAmPWVCq3WrFuX9dSlgf3RWTqNiWZC0a8Hn6fNDp26TzLbdo9mnxbU4I/3BbcAJZI9p\n 9ELaE9rw3LU8esKqRIfaZqPtrdm1C+e5gZa2gkmEzG+WEsS0MKtJyOFnuglGl1ZBxR1uFvbU\n VXhewCNoviXxkkPk/DanIgYB1nUtkPC+BHkJJYCyf9Kfl33s/bai34aaxkGXqpKv+CInARg3\n fCikcHzYYWKaXS6HABEBAAHCwXwEGAEIACYCGwwWIQSH6ZrVEpascJjzbVq59+x3yCm/lgUC\n Y/5kJAUJCMVeQQAKCRC59+x3yCm/lpF7D/9Lolx00uxqXz2vt/u9flvQvLsOWa+UBmWPGX9u\n oWhQ26GjtbVvIf6SECcnNWlu/y+MHhmYkz+h2VLhWYVGJ0q03XkktFCNwUvHp3bTXG3IcPIC\n eDJUVMMIHXFp7TcuRJhrGqnlzqKverlY6+2CqtCpGMEmPVahMDGunwqFfG65QubZySCHVYvX\n T9SNga0Ay/L71+eVwcuGChGyxEWhVkpMVK5cSWVzZe7C+gb6N1aTNrhu2dhpgcwe1Xsg4dYv\n dYzTNu19FRpfc+nVRdVnOto8won1SHGgYSVJA+QPv1x8lMYqKESOHAFE/DJJKU8MRkCeSfqs\n izFVqTxTk3VXOCMUR4t2cbZ9E7Qb/ZZigmmSgilSrOPgDO5TtT811SzheAN0PvgT+L1Gsztc\n Q3BvfofFv3OLF778JyVfpXRHsn9rFqxG/QYWMqJWi+vdPJ5RhDl1QUEFyH7ok/ZY60/85FW3\n o9OQwoMf2+pKNG3J+EMuU4g4ZHGzxI0isyww7PpEHx6sxFEvMhsOp7qnjPsQUcnGIIiqKlTj\n H7i86580VndsKrRK99zJrm4s9Tg/7OFP1SpVvNvSM4TRXSzVF25WVfLgeloN1yHC5Wsqk33X\n XNtNovqA0TLFjhfyyetBsIOgpGakgBNieC9GnY7tC3AG+BqG5jnVuGqSTO+iM/d+lsoa+w==","In-Reply-To":"<7bf3ff88-70ee-4003-bc9e-2f810087fae8@nvidia.com>","Subject":"Re: [ovs-dev] [PATCH 1/9] ipsec: Add a helper function to run\n commands from the monitor.","X-BeenThere":"ovs-dev@openvswitch.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"<ovs-dev.openvswitch.org>","List-Unsubscribe":"<https://mail.openvswitch.org/mailman/options/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=unsubscribe>","List-Archive":"<http://mail.openvswitch.org/pipermail/ovs-dev/>","List-Post":"<mailto:ovs-dev@openvswitch.org>","List-Help":"<mailto:ovs-dev-request@openvswitch.org?subject=help>","List-Subscribe":"<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>,\n <mailto:ovs-dev-request@openvswitch.org?subject=subscribe>","Cc":"i.maximets@ovn.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"ovs-dev-bounces@openvswitch.org","Sender":"\"dev\" <ovs-dev-bounces@openvswitch.org>"}}]