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.

Sign up for GitHub

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

Jump to bottom

setsockopt ZMQ_TOS doesn't work for IPv6 #2208

Closed
saifhhasan opened this issue Nov 14, 2016 · 4 comments
Closed

setsockopt ZMQ_TOS doesn't work for IPv6 #2208

saifhhasan opened this issue Nov 14, 2016 · 4 comments

Comments

@saifhhasan
Copy link
Contributor

saifhhasan commented Nov 14, 2016 •
edited
Loading

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));

Ask for this issue is to extend ZMQ_TOS to support ipv6 traffic class as well.
@bluca
Copy link
Member

bluca commented Nov 14, 2016

Do you have a use case or a way to test that it would work properly if implemented?

@saifhhasan
Copy link
Contributor Author

saifhhasan commented Nov 14, 2016 •
edited
Loading

Yes. Here at Facebook we built Link State Routing Protocol on the top of ZeroMQ transport (IPv6 only communication). It is very critical for us to distinguish control plane traffic from ordinary traffic to not hammer communications between routing daemon under heavy network load.

We have internally carried out the patch and it is working well. Trying to send it upstream. Happy to take back your feedback and iterate as per the requirements.

OpenR: https://code.facebook.com/posts/1142111519143652/introducing-open-r-a-new-modular-routing-platform/

saifhhasan added a commit to saifhhasan/libzmq that referenced this issue Nov 14, 2016
@saifhhasan
Make ZMQ_TOS work with IPv6 sockets
3a13078 
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
saifhhasan added a commit to saifhhasan/libzmq that referenced this issue Nov 14, 2016
@saifhhasan
Make ZMQ_TOS work with IPv6 sockets
fc8007e 
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
Copy link
Member

bluca commented Nov 14, 2016

Great! If you have already done the work, then please do send a PR and we'll merge it. Your diff looks good, the only thing you'll want to make sure is that if IPv6 is not enabled it doesn't cause a failure. Thanks!

@saifhhasan saifhhasan mentioned this issue Nov 14, 2016
Merged
@bluca
Copy link
Member

bluca commented Nov 14, 2016

Fixed by #2209

@bluca bluca closed this as completed Nov 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bluca @saifhhasan