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

Change the MAC addresses to be generated based on IP #14685

Conversation

knobunc
Copy link
Contributor

@knobunc knobunc commented Jun 15, 2017

The ARP cache seems to misbehave in some clusters and if an IP address
is reused then the MAC address can get out of sync (he root cause has
not been identified). When that happens the target pod looks at the
MAC address and drops the packets on the floor despite the IP address
being correct.

This patch changes the code to generate the MAC address based on the
IP address so that when an IP is reused, it will have the same MAC
address and the packets will flow.

This behavior is consistent with what Docker does when allocating MACs
and what Kubenet does too. It may change in the future, but this
fixes our CNI plugin now.

Fixes bug 1451854 (https://bugzilla.redhat.com/show_bug.cgi?id=1451854)

@knobunc
Copy link
Contributor Author

knobunc commented Jun 16, 2017

@openshift/networking PTAL

The ARP cache seems to misbehave in some clusters and if an IP address
is reused then the MAC address can get out of sync (he root cause has
not been identified).  When that happens the target pod looks at the
MAC address and drops the packets on the floor despite the IP address
being correct.

This patch changes the code to generate the MAC address based on the
IP address so that when an IP is reused, it will have the same MAC
address and the packets will flow.

This behavior is consistent with what Docker does when allocating MACs
and what Kubenet does too.  It may change in the future, but this
fixes our CNI plugin now.

Fixes bug 1451854 (https://bugzilla.redhat.com/show_bug.cgi?id=1451854)
@knobunc knobunc force-pushed the bug/bz1451854-make-consistent-mac-addrs branch from 2312e94 to 256ddc5 Compare June 16, 2017 14:47
Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

lgtm i guess

@knobunc
Copy link
Contributor Author

knobunc commented Jun 16, 2017

[test][testextended][extended:networking]

@openshift-bot
Copy link
Contributor

Evaluated for origin testextended up to 256ddc5

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/testextended SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended/642/) (Base Commit: ee67917) (PR Branch Commit: 256ddc5) (Extended Tests: networking)

@knobunc
Copy link
Contributor Author

knobunc commented Jun 19, 2017

[test] last flaked with "[Fail] deploymentconfigs [AfterEach] when run iteratively [Conformance] should immediately start a new deployment " see https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin_extended_conformance_gce/3264/consoleFull#-169259343956c60d7be4b02b88ae8c268b.

Based on #12350 that's "Probably slow docker start bug"

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 256ddc5

@knobunc
Copy link
Contributor Author

knobunc commented Jun 19, 2017

@dcbw I slightly rearranged where we set the MAC. Before it was doing it after it read interface info... it seemed better to do it before that. Thoughts?

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2393/) (Base Commit: 5dce9b9) (PR Branch Commit: 256ddc5)

Copy link
Contributor

@dcbw dcbw left a comment

Choose a reason for hiding this comment

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

lgtm

@knobunc
Copy link
Contributor Author

knobunc commented Jun 19, 2017

[merge][severity: blocker]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 256ddc5

Copy link

@pravisankar pravisankar left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-bot
Copy link
Contributor

openshift-bot commented Jun 19, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/1047/) (Base Commit: e4eae07) (PR Branch Commit: 256ddc5) (Extended Tests: blocker) (Image: devenv-rhel7_6379)

@openshift-bot openshift-bot merged commit 1aa6666 into openshift:master Jun 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants