[ovs-dev] [PATCH v2] Don't raise an Exception on failure to connect via SSL

Terry Wilson twilson at redhat.com
Tue Oct 13 22:23:20 UTC 2020


On Mon, Oct 12, 2020 at 2:51 PM Thomas Neuman <thomas.neuman at nutanix.com>
wrote:

> > With other socket types, trying to connect and failing will return
> > an error code, but if an SSL Stream is used, then when
> > check_connection_completion(sock) is called, SSL will raise an
> > exception that doesn't derive from socket.error which is handled.
> >
> > This adds handling for SSL.SysCallError which has the same
> > arguments as socket.error (errno, string). A future enhancement
> > could be to go through SSLStream class and implement error
> > checking for all of the possible exceptions similar to how
> > lib/stream-ssl.c's interpret_ssl_error() works across the various
> > methods that are implemented.
> >
> > Signed-off-by: Terry Wilson <twilson at redhat.com  <
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev>>
> > ---
> >  python/ovs/stream.py | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/python/ovs/stream.py b/python/ovs/stream.py
> > index e9bb0c854..f5a520862 100644
> > --- a/python/ovs/stream.py
> > +++ b/python/ovs/stream.py
> > @@ -132,6 +132,10 @@ class Stream(object):
> >      IPTOS_PREC_INTERNETCONTROL = 0xc0
> >      DSCP_DEFAULT = IPTOS_PREC_INTERNETCONTROL >> 2
> >
> > +    @staticmethod
> > +    def check_connection_completion(sock):
> > +        return ovs.socket_util.check_connection_completion(sock)
> > +
>
> Only thing I might question is making this a static method instead of,
> say, a class
> method. Especially considering its usage in "open()" below, and the fact
> that we're
> overriding it in the SSLStream subclass, it seems like that would be
> clearer to me.
> But not a huge deal to me either way, so I'll defer to others' judgement.
>

I get why it seems a little weird, but the reason I chose @staticmethod is
because @classmethod would imply that we are going to actually reference
the class that is passed in the first argument, and we won't. The only
thing this method depends on is the socket, so it seemed a staticmethod
that could be overridden in a subclass with a special need was the way to
go.


> >      @staticmethod
> >      def open(name, dscp=DSCP_DEFAULT):
> >          """Attempts to connect a stream to a remote peer.  'name' is a
> > @@ -189,7 +193,7 @@ class Stream(object):
> >          if error:
> >              return error, None
> >          else:
> > -            err = ovs.socket_util.check_connection_completion(sock)
> > +            err = cls.check_connection_completion(sock)
> >              if err == errno.EAGAIN or err == errno.EINPROGRESS:
> >                  status = errno.EAGAIN
> >                  err = 0
> > @@ -261,7 +265,7 @@ class Stream(object):
> >
> >      def __scs_connecting(self):
> >          if self.socket is not None:
> > -            retval =
> ovs.socket_util.check_connection_completion(self.socket)
> > +            retval = self.check_connection_completion(self.socket)
> >              assert retval != errno.EINPROGRESS
> >          elif sys.platform == 'win32':
> >              if self.retry_connect:
> > @@ -761,6 +765,13 @@ Stream.register_method("tcp", TCPStream)
> >
> >
> >  class SSLStream(Stream):
> > +    @staticmethod
> > +    def check_connection_completion(sock):
> > +        try:
> > +            return Stream.check_connection_completion(sock)
> > +        except SSL.SysCallError as e:
> > +            return ovs.socket_util.get_exception_errno(e)
> > +
> >      @staticmethod
> >      def needs_probes():
> >          return True
> >
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>


More information about the dev mailing list