Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to handle OS disabled IPv6, issue #714. #717

Merged
merged 2 commits into from
Apr 25, 2017

Conversation

MichaelSweden
Copy link
Contributor

  • Changes made only in the os_calls.c file (keep the libcommon interface intact)
  • Exported functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect
  • Support three network configurations:
    1. Normal network, with IPv6
    2. Partly disabled IPv6 via sysctl.conf
    3. Total disabled IPv6 via grub

Please see #714

- Changes made only in the os_calls.c file.
- Exported functions changed: g_tcp_bind g_tcp_bind_address g_tcp_connect
- Support three network configurations:
  1) Normal network, with IPv6
  2) Partly disabled IPv6 via sysctl.conf
  3) Total disabled IPv6 via grub
@metalefty
Copy link
Member

I'm testing this.

@metalefty
Copy link
Member

Works fine for me.

@metalefty
Copy link
Member

I'll merge this in 24 hours if no objections.

@MichaelSweden
Copy link
Contributor Author

Okay, great.

I have now tested on Xubuntu 17.04 Beta2 and it works OK.

PS. One thing I notice was that if IPv6 isn't available and the g_tcp_socket function will falling back to IPv4. It will be an ERROR message in log files "g_tcp_socket: Address family not supported by protocol". This isn't so nice! After this it will be an INFO message "IPv6 not supported, falling back to IPv4". I haven't changed this function, so it is as it was before.

@MichaelSweden
Copy link
Contributor Author

Sorry, I'm a bit annoyed that I missed that old error message. This error message will always come at start if the system have IPv6 disabled. I think it should be removed. I have looked at it and it is easy fixed by moving the log_message line at row 422 little bit down to the default case. Like this:

diff --git a/common/os_calls.c b/common/os_calls.c
index ca8ab42..524d819 100644
--- a/common/os_calls.c
+++ b/common/os_calls.c
@@ -419,8 +419,6 @@ g_tcp_socket(void)
     rv = (int)socket(AF_INET6, SOCK_STREAM, 0);
     if (rv < 0)
     {
-        log_message(LOG_LEVEL_ERROR, "g_tcp_socket: %s", g_get_strerror());
-
         switch (errno)
         {
             case EAFNOSUPPORT: /* if IPv6 not supported, retry IPv4 */
@@ -429,6 +427,7 @@ g_tcp_socket(void)
                 break;
 
             default:
+                log_message(LOG_LEVEL_ERROR, "g_tcp_socket: %s", g_get_strerror());
                 return -1;
         }
     }

@metalefty Is it too late to commit a fix to this pull request? You wrote that you will merge this and I don't want to mess things up now.

@metalefty
Copy link
Member

@MichaelSweden No problem. I'm waiting for your fix.

@moobyfr
Copy link
Contributor

moobyfr commented Apr 22, 2017

@MichaelSweden : just make another commit in your branch, and gitlab will add it into this pull request.

@MichaelSweden
Copy link
Contributor Author

Since the diff/system split the change in two. I show you this picture that more clearly show the change I did.
commitfixerrmsg

@metalefty
Copy link
Member

LGTM.

@metalefty metalefty self-requested a review April 24, 2017 01:40
Copy link
Member

@metalefty metalefty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine for me, again.

@metalefty metalefty merged commit 5c668dc into neutrinolabs:devel Apr 25, 2017
@metalefty metalefty mentioned this pull request May 8, 2017
@metalefty metalefty added the IPv6 label May 8, 2017
@MichaelSweden MichaelSweden deleted the support-disabled-ipv6 branch May 11, 2017 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants