-
-
Notifications
You must be signed in to change notification settings - Fork 105
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
Allow user to specify socket type per ip-addres #210
base: master
Are you sure you want to change the base?
Conversation
Listening on both UDP and TCP is likely unwanted behavior when support for XDP lands. Allow the user to configure listening on UDP, TCP or both per ip-address option in preparation. Specifying no socket at all ensures both UDP and TCP are opened, which is the current default behavior, thereby remaining backwards compatible configuration wise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look nice, it allows for more expressive configuration.
setup_socket_servers(socket, ip); | ||
|
||
if (ip && ip->dev) { | ||
socket->flags = NSD_BIND_DEVICE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
socket->flags = NSD_BIND_DEVICE; | |
socket->flags |= NSD_BIND_DEVICE; |
Should this 'or' in the flag, because another flag exists, for OPTIONAL?
size_t namelen = 0; | ||
unsigned short udp_port = 0, tcp_port = 0; | ||
|
||
/* not an interface if length exceeds size restrictions */ | ||
if (!namelen || namelen >= IFNAMSIZ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
namelen is always 0, because initialized at 0 on line 421 above.
if (!setup_if_sockets(ip, ifaddrs)) | ||
setup_ip_sockets(ip, ifaddrs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the user can only specify all interface names, or all numeric ip addresses, but not mix them on individual interface config lines? Is that a correct reading of this part, not sure if users would need to do that.
Listening on both UDP and TCP is likely unwanted behavior when support for XDP lands. Allow the user to configure listening on UDP, TCP or both per ip-address option in preparation. Specifying no socket type at all ensures both UDP and TCP are opened, which is the current default behavior, thereby remaining backwards compatible configuration wise.