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

Enable ipv4 multicast packet types when using zbeacon #2064

Merged
merged 2 commits into from
Mar 10, 2020
Merged

Enable ipv4 multicast packet types when using zbeacon #2064

merged 2 commits into from
Mar 10, 2020

Conversation

Dysl3xik
Copy link
Contributor

@Dysl3xik Dysl3xik commented Mar 6, 2020

Problem: ipv4 multicast packet types when using zbeacon are not implemented - but some notes about it already exist in the code

Solution: Enable ipv4 multicast packet types when using zbeacon

Open Items:
I could not find the files used to gen czmq_selftest or czmq_private_selftest, thefore I could not update the auto code generator files for these. Do they exist?

I intentionally did not modify any behavior for ipv6 with this change zsys_udp_new() has the code to configure the socket differently for IPv6 but its not enabled.

I have not tested this code on any OS aside from windows. There I tested it going through atleast 1 router and verified TTL and addresses were set correctly with wire shark. Additionally I copied the existing beacon test and added the calls to configure it with multicast. These tests pass as well.

Please advise how we can best move this code forward and hopefully have it added into official codebase.

src/zbeacon.c Outdated Show resolved Hide resolved
@bluca
Copy link
Member

bluca commented Mar 7, 2020

I could not find the files used to gen czmq_selftest or czmq_private_selftest, thefore I could not update the auto code generator files for these. Do they exist?

It's zproject via gsl
https://github.com/zeromq/zproject
https://github.com/zeromq/gsl

@sappo
Copy link
Member

sappo commented Mar 7, 2020

@Dysl3xik thanks for this contribution. Its okay with our contribution guidelines if you only tested on one OS. You can check travis to see if it builds on linux and macos if you like though. Currently the build fails on travis but works on my linux machine so that's not a blocker for me.

For further contributions please use the Problem/Solution statement in your commit messages.

Don't worry about the auto-generated files I'll take care of it once the PR is merged.

src/zsys.c Outdated Show resolved Hide resolved
src/zbeacon.c Outdated Show resolved Hide resolved
src/zbeacon.c Outdated Show resolved Hide resolved
src/zbeacon.c Outdated Show resolved Hide resolved
@Dysl3xik
Copy link
Contributor Author

Dysl3xik commented Mar 9, 2020

Failures now seem limited to "implicit declaration of function 'zsys_set_ipv4_mcast_address'" which I do not understand as this function is defined in zsys.h and the api file....

Additionally the windows tests are randomly failing on poller. I see this behavior randomly on my box as well. I also see random failures of the ztimer test on my PC also randomly.....

@bluca
Copy link
Member

bluca commented Mar 9, 2020

After updating the API files, the project needs to be regenerated with gsl, like every other zproject repo. I've done it for you. You should really get that set up.

@bluca bluca merged commit 282b272 into zeromq:master Mar 10, 2020
@sappo
Copy link
Member

sappo commented Mar 14, 2020

@Dysl3xik I build a Docker image for zproject to re-generate generated files in czmq.

It would be great if you could try it on windows and report if it works for you:
https://github.com/zeromq/czmq#code-generation

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