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

Assume the address family based on the destination address #15

Merged
merged 1 commit into from
Apr 27, 2020

Conversation

andrewsykim
Copy link
Collaborator

@andrewsykim andrewsykim commented Mar 31, 2020

Signed-off-by: Andrew Sy Kim kim.andrewsy@gmail.com

In older Linux kernels (< 3.18), the IPVS_DEST_ATTR_ADDR_FAMILY
attribute does not exist. We should assume the address family based
on the destination address if the attribute doesn't exist.

Users running the Kubernetes IPVS proxy on older kernel versions are seeing issues because parseIP returns an error since the address family is not returned from the kernel, see kubernetes/kubernetes#89520 for more details.

…t available

In older Linux kernels (< 3.18), the IPVS_DEST_ATTR_ADDR_FAMILY
attribute does not exist so the address family should be assumed based
on the destination address

Signed-off-by: Andrew Sy Kim <kim.andrewsy@gmail.com>
@@ -351,6 +351,17 @@ func assembleService(attrs []syscall.NetlinkRouteAttr) (*Service, error) {

}

// in older kernels (< 3.18), the svc address family attribute may not exist so we must
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

AFAICT this is only an issue for destination addresses since IPVS_DEST_ATTR_ADDR_FAMILY was added later while IPVS_SVC_ATTR_AF had always existed, but this extra check doesn't seem to hurt.

@andrewsykim andrewsykim changed the title [WIP] Assume the address family based on the destination address Assume the address family based on the destination address Apr 2, 2020
@andrewsykim
Copy link
Collaborator Author

cc @justincormack @lbernail

@justincormack
Copy link
Contributor

What kernels that anyone is using does this apply to? 3.18 was released in 2014...

@niorg
Copy link

niorg commented Apr 19, 2020

CentOS / RHEL 7 is using a kernel based on 3.10

@andrewsykim
Copy link
Collaborator Author

CentOS / RHEL 7 is using a kernel based on 3.10

Yeah and bugs being reported from Kubernetes users are usually users running on CentOS 7. More details in this issue kubernetes/kubernetes#89520. I agree it's not ideal trying to support such old kernels but there seems to be a fairly large amount of users trying to run IPVS on CentOS 7.

@lbernail
Copy link
Contributor

@justincormack I agree it's not great but we still see a lot of users running CentOS / RHEL 7

We introduced the problem in kube-proxy when adding support for dual-stack which required updating libnetwork/ipvs to get moby/libnetwork#2416 and moby/libnetwork#2433

The current impact for Kubernetes users is that they can't run recent kube-proxy versions on this kernel (@andrewsykim disabling dual stack doesn't help with this right? We won't start the v6 proxier but address parsing for the v4 proxier will be wrong)

@justincormack justincormack merged commit a47391a into moby:master Apr 27, 2020
// assume it based on the svc address provided.
if s.AddressFamily == 0 {
addr := (net.IP)(addressBytes)
if addr.To4() != nil {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In running a few more tests, I realized this won't work because To4() checks the last 4 bytes for v4 addresses and leading 0 but netlink returns IPv4 addresses in the first 4 bytes.

Will follow-up with a fix.

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.

4 participants