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

Makes NetAddress comparable #1569

Merged
merged 1 commit into from
Feb 15, 2017
Merged

Makes NetAddress comparable #1569

merged 1 commit into from
Feb 15, 2017

Conversation

SeanTAllen
Copy link
Member

Provides two types of equality.

1: "basic equality"

Compares all the fields of NetAddress to see if they are equal. This
is the comparison if we use == or eq.

2: "address equality"

Compares based on the ip address portion of NetAddress. Does not
take port, family, length or scope into account. We are using
"address equality" at Sendence to compare, "is this IP for a local
or remote address" which is in turn used to decide whether to turn
on TCP_NODELAY (on Linux, using TCP_NODELAY on local addresses has
a negative impact on our performance/stability. Using it on
remote addresses helps our performance/stability).

@SeanTAllen SeanTAllen added the do not merge This PR should not be merged at this time label Feb 9, 2017
@SeanTAllen
Copy link
Member Author

@sylvanc @jemc i believe this is what we all agreed upon, yes? if yes, i'll add a couple basic tests around equality for this. marking as "do not merge" for now until y'all sign off and i add the tests.

i'm not sure, is length supposed to be part of the equality comparison? it's not clear to me what it is.

Provides two types of equality.

1: "basic equality"

Compares all the fields of NetAddress to see if they are equal. This
is the comparison if we use `==` or `eq`.

2: "address equality"

Compares based on the ip address portion of NetAddress. Does not
take port, family, length or scope into account. We are using
"address equality" at Sendence to compare, "is this IP for a local
or remote address" which is in turn used to decide whether to turn
on TCP_NODELAY (on Linux, using TCP_NODELAY on local addresses has
a negative impact on our performance/stability. Using it on
remote addresses helps our performance/stability).
Copy link
Contributor

@sylvanc sylvanc left a comment

Choose a reason for hiding this comment

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

Length does go in the comparison. It is sin_len, which is a BSD thing:

The length member, sin_len, was added with 4.3BSD-Reno, when support for the OSI protocols was added (Figure 1.15). Before this release, the first member was sin_family, which was historically an unsigned short. Not all vendors support a length field for socket address structures and the POSIX specification does not require this member. The datatype that we show, uint8_t, is typical, and POSIX-compliant systems provide datatypes of this form (Figure 3.2).

http://www.masterraghu.com/subjects/np/introduction/unix_network_programming_v1.3/ch03lev1sec2.html

@sylvanc sylvanc removed the do not merge This PR should not be merged at this time label Feb 15, 2017
@sylvanc sylvanc merged commit c36180a into master Feb 15, 2017
@SeanTAllen SeanTAllen deleted the net-address-equality branch June 7, 2018 20:19
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.

3 participants