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

Make ZMQ_TOS work with IPv6 sockets #2209

Merged
merged 1 commit into from
Nov 14, 2016
Merged

Make ZMQ_TOS work with IPv6 sockets #2209

merged 1 commit into from
Nov 14, 2016

Conversation

saifhhasan
Copy link
Contributor

Summary:
To set Type Of Service for IP layer packets ZMQ provides ZMQ_TOS socket
option. However this only works for v4 sockets. Considering things are moving to
IPv6 heavily (especially within enterprise networks), ZMQ should support setting
traffic class for v6 based on ZMQ_TOS.

There is a subtle difference between v4 and v6 in terms of the positioning of
field but TOS has same meaning in both v4 and v6. Linux provides following APIs
for v4/v6 to set TOS field value.

// For v4
setsockopt(fd, IPPROTO_IP, IP_TOS, tos, sizeof(tos));

// For v6
setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, tos, sizeof(tos));

Test Plan:
Make sure Cmake works fine and all tests passes.
Imported this change to OpenR project and tested on our local testbed.
Captured some packets exchanged between PUB/SUB and ROUTER/ROUTER sockets
pairs. Verify that TCLASS value is set as per expectation.

Tasks: #2208

Summary:
To set `Type Of Service` for IP layer packets ZMQ provides `ZMQ_TOS` socket
option. However this only works for v4 sockets. Considering things are moving to
IPv6 heavily (especially within enterprise networks), ZMQ should support setting
`traffic class` for v6 based on `ZMQ_TOS`.

There is a subtle difference between v4 and v6 in terms of the positioning of
field but TOS has same meaning in both v4 and v6. Linux provides following APIs
for v4/v6 to set TOS field value.
```
// For v4
setsockopt(fd, IPPROTO_IP, IP_TOS, tos, sizeof(tos));

// For v6
setsockopt(fd, IPPROTO_IPV6, IPV6_TCLASS, tos, sizeof(tos));
```

Test Plan:
Make sure Cmake works fine and all tests passes.
Imported this change to `OpenR` project and tested on our local testbed.
Captured some packets exchanged between PUB/SUB and ROUTER/ROUTER sockets
pairs. Verify that `TCLASS` value is set as per expectation.

Tasks: zeromq#2208
@bluca bluca merged commit bc18604 into zeromq:master Nov 14, 2016
@bluca
Copy link
Member

bluca commented Nov 14, 2016

Thanks! Will it handle gracefully the case where IPv6 is not enabled? If not we can fix it up.

@bluca
Copy link
Member

bluca commented Nov 14, 2016

Also, for your next contributions, could you please follow the contributing guidelines for the git commit message?

Problem: X is broken

Solution: do Y and Z to fix X

http://zeromq.org/docs:contributing

@saifhhasan
Copy link
Contributor Author

Thanks @bluca for quick response. Amazed by the speed of zeromq community :) I will follow the guide for future contributions (somehow missed that part).

I have not tested with IPv4 only system yet. Not sure what will be the behaviour (it might need masking off in case if IPv6 is not enabled).

@bluca
Copy link
Member

bluca commented Nov 14, 2016

Welcome to the community :-) What you've just seen in action is our C4.1 process: https://rfc.zeromq.org/spec:42/C4/ - IE. merge first, ask questions later :-)

I'll give it a go for the v4 case and fixup if there's a fallout, no worries.

@bluca
Copy link
Member

bluca commented Nov 14, 2016

ENOPROTOOPT is returned if IPv6 is not available, so it's a simple fix: #2210

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.

2 participants