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

Allow user to specify socket type per ip-addres #210

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

k0ekk0ek
Copy link

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.

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.
@k0ekk0ek k0ekk0ek requested a review from wcawijngaards March 24, 2022 15:30
Copy link
Member

@wcawijngaards wcawijngaards left a 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;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
socket->flags = NSD_BIND_DEVICE;
socket->flags |= NSD_BIND_DEVICE;

Should this 'or' in the flag, because another flag exists, for OPTIONAL?

Comment on lines +421 to +425
size_t namelen = 0;
unsigned short udp_port = 0, tcp_port = 0;

/* not an interface if length exceeds size restrictions */
if (!namelen || namelen >= IFNAMSIZ)
Copy link
Member

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.

Comment on lines +621 to +622
if (!setup_if_sockets(ip, ifaddrs))
setup_ip_sockets(ip, ifaddrs);
Copy link
Member

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.

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