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 parseIP error when parseIP before get AddressFamily #2433

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

icefed
Copy link
Contributor

@icefed icefed commented Aug 15, 2019

Signed-off-by: Tom Zhao zlwangel@gmail.com

Fix: #2431

@arkodg
Copy link
Contributor

arkodg commented Aug 15, 2019

@icefed thanks for fixing this issue

thinking if something like this would be cleaner

initialize AddressFamily before the loop starts

s.AddressFamily = syscall.AF_INET

and when we receive s.AddressFamily we can recompute s.Address

                case ipvsDestAttrAddressFamily:
                       s.AddressFamily = native.Uint16(attr.Value)
                       // Recompute s.Address
                       fallthrough
		case ipvsSvcAttrAddress:
			ip, err := parseIP(attr.Value, s.AddressFamily)
			if err != nil {
				return nil, err
			}
			s.Address = ip

@arkodg
Copy link
Contributor

arkodg commented Aug 15, 2019

can we also add a test case for this, so we catch it, if its 100% reproducible

@icefed
Copy link
Contributor Author

icefed commented Aug 16, 2019

@icefed thanks for fixing this issue

thinking if something like this would be cleaner

initialize AddressFamily before the loop starts

s.AddressFamily = syscall.AF_INET

and when we receive s.AddressFamily we can recompute s.Address

                case ipvsDestAttrAddressFamily:
                       s.AddressFamily = native.Uint16(attr.Value)
                       // Recompute s.Address
                       fallthrough
		case ipvsSvcAttrAddress:
			ip, err := parseIP(attr.Value, s.AddressFamily)
			if err != nil {
				return nil, err
			}
			s.Address = ip

Hi @arkodg, ipvsSvcAttrAddress Value has 16 bytes, we can not use ipvsDestAttrAddressFamily Value(2 bytes) to recompute Address. So, it will not work.

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM, can we also add a test case please

@icefed
Copy link
Contributor Author

icefed commented Aug 17, 2019

LGTM, can we also add a test case please

Sorry for miss this, I will add test later.

@icefed
Copy link
Contributor Author

icefed commented Aug 19, 2019

hi @arkodg, I added tests for ipv6 and local passed, I don't know circleci: unit-tests failed.

@arkodg
Copy link
Contributor

arkodg commented Aug 19, 2019

@icefed can you please rebase to the latest master, I've taken care of the unit-tests regression by reverting a commit

Signed-off-by: Tom Zhao <zlwangel@gmail.com>
@thaJeztah
Copy link
Member

@arkodg looks like this was rebased; CI is green now

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

Copy link
Collaborator

@selansen selansen left a comment

Choose a reason for hiding this comment

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

LGTM

@selansen selansen merged commit 6edb83e into moby:master Aug 22, 2019
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 3, 2019
full diff: moby/libnetwork@09cdcc8...92d1fbe

relevant changes included (omitting some changes that were added _and_ reverted in this bump):

- moby/libnetwork#2433 Fix parseIP error when parseIP before get AddressFamily
  - fixes moby/libnetwork#2431 parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
  - moby/libnetwork#2289
  - this was a regression introduced in moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2440 Bump hashicorp go-sockaddr v1.0.2, go-multierror v1.0.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 3, 2019
full diff: moby/libnetwork@09cdcc8...92d1fbe

relevant changes included (omitting some changes that were added _and_ reverted in this bump):

- moby/libnetwork#2433 Fix parseIP error when parseIP before get AddressFamily
  - fixes moby/libnetwork#2431 parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
  - moby/libnetwork#2289
  - this was a regression introduced in moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2440 Bump hashicorp go-sockaddr v1.0.2, go-multierror v1.0.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: bab58c19246bfaad9bbff8dd88f1b6a224b8dc22
Component: engine
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 16, 2019
full diff: moby/libnetwork@09cdcc8...92d1fbe

relevant changes included (omitting some changes that were added _and_ reverted in this bump):

- moby/libnetwork#2433 Fix parseIP error when parseIP before get AddressFamily
  - fixes moby/libnetwork#2431 parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
  - moby/libnetwork#2289
  - this was a regression introduced in moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2440 Bump hashicorp go-sockaddr v1.0.2, go-multierror v1.0.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bab58c1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 17, 2019
full diff: moby/libnetwork@09cdcc8...92d1fbe

relevant changes included (omitting some changes that were added _and_ reverted in this bump):

- moby/libnetwork#2433 Fix parseIP error when parseIP before get AddressFamily
  - fixes moby/libnetwork#2431 parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
  - moby/libnetwork#2289
  - this was a regression introduced in moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2440 Bump hashicorp go-sockaddr v1.0.2, go-multierror v1.0.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bab58c19246bfaad9bbff8dd88f1b6a224b8dc22)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 44ca36c7cff269fbf53b98c73b1eea36a17bdf3a
Component: engine
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
full diff: moby/libnetwork@09cdcc8...92d1fbe

relevant changes included (omitting some changes that were added _and_ reverted in this bump):

- moby/libnetwork#2433 Fix parseIP error when parseIP before get AddressFamily
  - fixes moby/libnetwork#2431 parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
  - moby/libnetwork#2289
  - this was a regression introduced in moby/libnetwork#2416 Fix hardcoded AF_INET for IPv6 address handling
- moby/libnetwork#2440 Bump hashicorp go-sockaddr v1.0.2, go-multierror v1.0.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: zach <Zachary.Joyner@linux.com>
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.

parseIP Error ip=[172 17 0 2 0 0 0 0 0 0 0 0 0 0 0 0]
4 participants