Skip to content

halrmt: Allow IPv4 as fallback when IPv6 is disabled. - #4405

Open
BsAtHome wants to merge 1 commit into
LinuxCNC:masterfrom
BsAtHome:fix_halrmt-ipv4
Open

halrmt: Allow IPv4 as fallback when IPv6 is disabled.#4405
BsAtHome wants to merge 1 commit into
LinuxCNC:masterfrom
BsAtHome:fix_halrmt-ipv4

Conversation

@BsAtHome

Copy link
Copy Markdown
Contributor

The new halrmt server uses v4-mapped-on-v6 to have one socket accept both IPv6 and IPv4 connections. However, if IPv6 is disabled at the kernel command line (or not compiled in), then it would fail to create a listening socket. This PR adds an IPv4-only fallback in case IPv6 fails.

Comment thread src/hal/utils/halrmt.cc
Comment thread src/hal/utils/halrmt.cc Outdated
Comment thread src/hal/utils/halrmt.cc Outdated
Comment thread src/hal/utils/halrmt.cc

static int getSocket4(struct sockaddr_in &addr, socklen_t &slen)
{
int sockfd = socket(AF_INET, SOCK_STREAM, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The refactor dropped SOCK_CLOEXEC | SOCK_NONBLOCK from both socket() calls here
and at line 2918. Master has them on the listen socket:

sockfd = socket(AF_INET6, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);

and halrmt has no fcntl fallback anywhere, unlike emcrsh which calls
set_nonblock() after listen(). So the listener is now blocking and inheritable:

  • accept4() after a spurious POLLIN blocks the poll loop indefinitely (accept(2)
    BUGS: the connection can be aborted between poll and accept, leaving nothing
    to take). That is the caveat from linuxcncrsh: IPv6-only listen socket fails to start when IPv6 is disabled #4401.
  • halrmt forks and execs at line 628, so every component it launches inherits
    the listening fd and keeps port 5006 bound past halrmt's exit.

The first push had the flags on the IPv6 socket. Restoring them on both calls
should be all that is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants