[ovs-dev,v3,1/2] socket-util: split inet_open_active function and use connect_ex

In an upcoming patch, PyOpenSSL will be replaced with Python ssl module,
but in order to do an async connection with Python ssl module the ssl
socket must be created when the socket is created, but before the
socket is connected.

So, inet_open_active function is splitted in 3 parts:
- inet_create_socket_active: creates the socket and returns the family and
  the socket, or (error, None) if some error needs to be returned.
- inet_connect_active: connect the socket and returns the errno (it
  returns 0 if errno is EINPROGRESS or EWOULDBLOCK).

connect is replaced by connect_ex, since Python suggest to use it for
asynchronous connects and it's also cleaner since inet_connect_active
returns errno that connect_ex already returns, moreover due to a Python
limitation connect cannot not be used with ssl module.

inet_open_active function is changed in order to use the new functions
inet_create_socket_active and inet_connect_active.

Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
---
 python/ovs/socket_util.py | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

On Mon, Oct 25, 2021 at 8:46 AM Timothy Redaelli <tredaelli@redhat.com> wrote:
>
> In an upcoming patch, PyOpenSSL will be replaced with Python ssl module,
> but in order to do an async connection with Python ssl module the ssl
> socket must be created when the socket is created, but before the
> socket is connected.
>
> So, inet_open_active function is splitted in 3 parts:
> - inet_create_socket_active: creates the socket and returns the family and
>   the socket, or (error, None) if some error needs to be returned.
> - inet_connect_active: connect the socket and returns the errno (it
>   returns 0 if errno is EINPROGRESS or EWOULDBLOCK).
>
> connect is replaced by connect_ex, since Python suggest to use it for
> asynchronous connects and it's also cleaner since inet_connect_active
> returns errno that connect_ex already returns, moreover due to a Python
> limitation connect cannot not be used with ssl module.
>
> inet_open_active function is changed in order to use the new functions
> inet_create_socket_active and inet_connect_active.
>
> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
> ---
>  python/ovs/socket_util.py | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
> index 3faa64e9d..651012bf0 100644
> --- a/python/ovs/socket_util.py
> +++ b/python/ovs/socket_util.py
> @@ -222,8 +222,7 @@ def inet_parse_active(target, default_port):
>      return (host_name, port)
>
>
> -def inet_open_active(style, target, default_port, dscp):
> -    address = inet_parse_active(target, default_port)
> +def inet_create_socket_active(style, address):
>      try:
>          is_addr_inet = is_valid_ipv4_address(address[0])
>          if is_addr_inet:
> @@ -235,23 +234,32 @@ def inet_open_active(style, target, default_port, dscp):
>      except socket.error as e:
>          return get_exception_errno(e), None
>
> +    return family, sock
> +
> +
> +def inet_connect_active(sock, address, family, dscp):
>      try:
>          set_nonblocking(sock)
>          set_dscp(sock, family, dscp)
> -        try:
> -            sock.connect(address)
> -        except socket.error as e:
> -            error = get_exception_errno(e)
> -            if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
> -                # WSAEWOULDBLOCK would be the equivalent on Windows
> -                # for EINPROGRESS on Unix.
> -                error = errno.EINPROGRESS
> -            if error != errno.EINPROGRESS:
> -                raise
> -        return 0, sock
> +        error = sock.connect_ex(address)
> +        if error not in (0, errno.EINPROGRESS, errno.EWOULDBLOCK):
> +            sock.close()
> +            return error
> +        return 0
>      except socket.error as e:
>          sock.close()
> -        return get_exception_errno(e), None
> +        return get_exception_errno(e)
> +
> +
> +def inet_open_active(style, target, default_port, dscp):
> +    address = inet_parse_active(target, default_port)
> +    family, sock = inet_create_socket_active(style, address)
> +    if sock is None:
> +        return family, sock
> +    error = inet_connect_active(sock, address, family, dscp)
> +    if error:
> +        return error, None
> +    return 0, sock
>
>
>  def get_exception_errno(e):
> --
> 2.31.1
>

Tested against neutron "OverSsl" tests, and everything worked as it should.

Acked-by: Terry Wilson <twilson@redhat.com>
Tested-by: Terry Wilson <twilson@redhat.com>

On 10/27/21 22:57, Terry Wilson wrote:
> On Mon, Oct 25, 2021 at 8:46 AM Timothy Redaelli <tredaelli@redhat.com> wrote:
>>
>> In an upcoming patch, PyOpenSSL will be replaced with Python ssl module,
>> but in order to do an async connection with Python ssl module the ssl
>> socket must be created when the socket is created, but before the
>> socket is connected.
>>
>> So, inet_open_active function is splitted in 3 parts:
>> - inet_create_socket_active: creates the socket and returns the family and
>>   the socket, or (error, None) if some error needs to be returned.
>> - inet_connect_active: connect the socket and returns the errno (it
>>   returns 0 if errno is EINPROGRESS or EWOULDBLOCK).
>>
>> connect is replaced by connect_ex, since Python suggest to use it for
>> asynchronous connects and it's also cleaner since inet_connect_active
>> returns errno that connect_ex already returns, moreover due to a Python
>> limitation connect cannot not be used with ssl module.
>>
>> inet_open_active function is changed in order to use the new functions
>> inet_create_socket_active and inet_connect_active.
>>
>> Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
>> ---
>>  python/ovs/socket_util.py | 36 ++++++++++++++++++++++--------------
>>  1 file changed, 22 insertions(+), 14 deletions(-)
>>
>> diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
>> index 3faa64e9d..651012bf0 100644
>> --- a/python/ovs/socket_util.py
>> +++ b/python/ovs/socket_util.py
>> @@ -222,8 +222,7 @@ def inet_parse_active(target, default_port):
>>      return (host_name, port)
>>
>>
>> -def inet_open_active(style, target, default_port, dscp):
>> -    address = inet_parse_active(target, default_port)
>> +def inet_create_socket_active(style, address):
>>      try:
>>          is_addr_inet = is_valid_ipv4_address(address[0])
>>          if is_addr_inet:
>> @@ -235,23 +234,32 @@ def inet_open_active(style, target, default_port, dscp):
>>      except socket.error as e:
>>          return get_exception_errno(e), None
>>
>> +    return family, sock
>> +
>> +
>> +def inet_connect_active(sock, address, family, dscp):
>>      try:
>>          set_nonblocking(sock)
>>          set_dscp(sock, family, dscp)
>> -        try:
>> -            sock.connect(address)
>> -        except socket.error as e:
>> -            error = get_exception_errno(e)
>> -            if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
>> -                # WSAEWOULDBLOCK would be the equivalent on Windows
>> -                # for EINPROGRESS on Unix.
>> -                error = errno.EINPROGRESS
>> -            if error != errno.EINPROGRESS:
>> -                raise
>> -        return 0, sock
>> +        error = sock.connect_ex(address)
>> +        if error not in (0, errno.EINPROGRESS, errno.EWOULDBLOCK):
>> +            sock.close()
>> +            return error
>> +        return 0
>>      except socket.error as e:
>>          sock.close()
>> -        return get_exception_errno(e), None
>> +        return get_exception_errno(e)
>> +
>> +
>> +def inet_open_active(style, target, default_port, dscp):
>> +    address = inet_parse_active(target, default_port)
>> +    family, sock = inet_create_socket_active(style, address)
>> +    if sock is None:
>> +        return family, sock
>> +    error = inet_connect_active(sock, address, family, dscp)
>> +    if error:
>> +        return error, None
>> +    return 0, sock
>>
>>
>>  def get_exception_errno(e):
>> --
>> 2.31.1
>>
> 
> Tested against neutron "OverSsl" tests, and everything worked as it should.
> 
> Acked-by: Terry Wilson <twilson@redhat.com>
> Tested-by: Terry Wilson <twilson@redhat.com>
> 

Thanks, Terry and Timothy!  Applied.

Best regards, Ilya Maximets.