Skip to content

Fix parseIP error when parseIP before get AddressFamily#2433

Merged
selansen merged 1 commit into
moby:masterfrom
icefed:fix-ipvs-parseip
Aug 22, 2019
Merged

Fix parseIP error when parseIP before get AddressFamily#2433
selansen merged 1 commit into
moby:masterfrom
icefed:fix-ipvs-parseip

Conversation

@icefed

@icefed icefed commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: Tom Zhao [email protected]

Fix: #2431

@arkodg

arkodg commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

@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

arkodg commented Aug 15, 2019

Copy link
Copy Markdown
Contributor

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

@icefed

icefed commented Aug 16, 2019

Copy link
Copy Markdown
Contributor Author

@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.

arkodg
arkodg approved these changes Aug 16, 2019
View reviewed changes

@arkodg arkodg left a comment •
edited
Loading

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.

LGTM, can we also add a test case please

@icefed

icefed commented Aug 17, 2019

Copy link
Copy Markdown
Contributor Author

LGTM, can we also add a test case please

Sorry for miss this, I will add test later.

@icefed

icefed commented Aug 19, 2019

Copy link
Copy Markdown
Contributor Author

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

@arkodg

arkodg commented Aug 19, 2019

Copy link
Copy Markdown
Contributor

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

@arkodg arkodg mentioned this pull request Aug 22, 2019
Merged
@thaJeztah

thaJeztah commented Aug 22, 2019

Copy link
Copy Markdown
Member

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

arkodg
arkodg approved these changes Aug 22, 2019
View reviewed changes

@arkodg arkodg left a comment

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.

LGTM thanks !

selansen
selansen approved these changes Aug 22, 2019
View reviewed changes

@selansen selansen left a comment

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.

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
@thaJeztah
bump lib network to 92d1fbe1eb0883cf11d283cea8e658275146411d
bab58c1 
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 <[email protected]>
@thaJeztah thaJeztah mentioned this pull request Sep 3, 2019
Merged
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 3, 2019
@thaJeztah
bump lib network to 92d1fbe1eb0883cf11d283cea8e658275146411d
86a524a 
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 <[email protected]>
Upstream-commit: bab58c19246bfaad9bbff8dd88f1b6a224b8dc22
Component: engine
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 16, 2019
@thaJeztah
bump lib network to 92d1fbe1eb0883cf11d283cea8e658275146411d
44ca36c 
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 <[email protected]>
(cherry picked from commit bab58c1)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Sep 17, 2019
@thaJeztah
bump lib network to 92d1fbe1eb0883cf11d283cea8e658275146411d
4a5ccdb 
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 <[email protected]>
(cherry picked from commit bab58c19246bfaad9bbff8dd88f1b6a224b8dc22)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Upstream-commit: 44ca36c7cff269fbf53b98c73b1eea36a17bdf3a
Component: engine
@danwinship danwinship mentioned this pull request Sep 20, 2019
Closed
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
@thaJeztah @burnMyDread
bump lib network to 92d1fbe1eb0883cf11d283cea8e658275146411d
a6903bc 
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 <[email protected]>
Signed-off-by: zach <[email protected]>
@andrewsykim andrewsykim mentioned this pull request Jan 30, 2020
Merged
@alexandreChichmanian alexandreChichmanian mentioned this pull request Feb 14, 2020
Closed
@lbernail lbernail mentioned this pull request Apr 25, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@arkodg arkodg arkodg approved these changes
@selansen selansen selansen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

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

@icefed @arkodg @thaJeztah @selansen