Add setter and getter for multicast_ifv4 and multicast_ifv6
#1291
Conversation
| pub fn set_ip_multicast_ifv4<Fd: AsFd>(fd: Fd, value: &Ipv4Addr) -> io::Result<()> { | ||
| backend::net::sockopt::set_ip_multicast_ifv4(fd.as_fd(), value) | ||
| } | ||
|
|
There was a problem hiding this comment.
The Linux docs for IP_MULTICAST_IF say "The argument for setsockopt(2) is an ip_mreqn or (since Linux 3.5) ip_mreq structure similar to IP_ADD_MEMBERSHIP, or an
in_addr structure.". Would it make sense here to also add set_ip_multicast_ifv4_with_ifindex, similar to the existing set_ip_add_membership_with_ifindex, which uses the ip_mreqn form?
src/net/sockopt.rs
Outdated
| /// [module-level documentation]: self#references-for-get_ip_-and-set_ip_-functions | ||
| #[inline] | ||
| #[doc(alias = "IP_MULTICAST_IF")] | ||
| pub fn set_ip_multicast_ifv4<Fd: AsFd>(fd: Fd, value: &Ipv4Addr) -> io::Result<()> { |
There was a problem hiding this comment.
For naming, the *_ip_* names are all specific to IPv4, so we shouldn't need the v4 suffix here and in similar functions.
src/net/sockopt.rs
Outdated
| /// [module-level documentation]: self#references-for-get_ip_-and-set_ip_-functions | ||
| #[inline] | ||
| #[doc(alias = "IPV6_MULTICAST_IF")] | ||
| pub fn set_ip_multicast_ifv6<Fd: AsFd>(fd: Fd, value: u32) -> io::Result<()> { |
There was a problem hiding this comment.
IPv6 sockopt functions should use _ipv6_ instead of _ip_, and then they don't need a v6 suffix.
src/net/sockopt.rs
Outdated
| /// [module-level documentation]: self#references-for-get_ip_-and-set_ip_-functions | ||
| #[inline] | ||
| #[doc(alias = "IPV6_MULTICAST_IF")] | ||
| pub fn set_ip_multicast_ifv6<Fd: AsFd>(fd: Fd, value: u32) -> io::Result<()> { |
There was a problem hiding this comment.
Other sockopts that have an interface index argument use i32, rather than u32. Is there a reason to prefer u32?
There was a problem hiding this comment.
Hi, u32 is just for the compatible with socket2::set_multicast_if_v6.
There was a problem hiding this comment.
Ah. Well, rustix is preparing for a major semver bump soon, so we can change that if we want. Or we can change set_ip_add_membership_with_ifindex and set_ip_drop_membership_with_ifindex to use u32. Which do you think is better?
There was a problem hiding this comment.
IMO, changing it to u32 is better:
- compatible with
socket2'sset_ip_add_membership_with_ifindexandset_ip_drop_membership_with_ifindex - A negative interface index makes no sense and it is an invalid value, so
u32can avoid misusing.
Should I change it in this PR?
There was a problem hiding this comment.
Yes, changing it in this PR would be good. Thanks!
|
Thanks! |
Hi, this is the PR for #1299.