sys::socket adding Linux packet filtering on ipv4/ipv6/loopback traffics #2581
Conversation
src/sys/socket/mod.rs
Outdated
| @@ -241,6 +247,12 @@ impl SockProtocol { | |||
| #[cfg(apple_targets)] | |||
| #[allow(non_upper_case_globals)] | |||
| pub const KextEvent: SockProtocol = SockProtocol::Icmp; // Matches libc::SYSPROTO_EVENT | |||
|
|
|||
| /// Packet filter on IPv4 traffic | |||
| /// NOTE: placed here due to conflict (little endian arch) with SockProtocol::NetLinkISCI | |||
There was a problem hiding this comment.
@SteveLauC let me know which direction you prefer me to take here. e.g. using this for all these Eth* flags to avoid future conflicts, not using it all ...
There was a problem hiding this comment.
The current approach looks good, i.e., add them via associated constants when we see conflicts
src/sys/socket/mod.rs
Outdated
| @@ -241,6 +247,12 @@ impl SockProtocol { | |||
| #[cfg(apple_targets)] | |||
| #[allow(non_upper_case_globals)] | |||
| pub const KextEvent: SockProtocol = SockProtocol::Icmp; // Matches libc::SYSPROTO_EVENT | |||
|
|
|||
| /// Packet filter on IPv4 traffic | |||
| /// NOTE: placed here due to conflict (little endian arch) with SockProtocol::NetLinkISCI | |||
There was a problem hiding this comment.
| /// NOTE: placed here due to conflict (little endian arch) with SockProtocol::NetLinkISCI | |
| // NOTE: placed here due to conflict (little endian arch) with SockProtocol::NetLinkISCI |
This should be a code comment, not a doc comment.
conflict (little endian arch)
BTW, wouldn't this conflict always occur regardless of the byte order since they have the same value
There was a problem hiding this comment.
BTW, wouldn't this conflict always occur regardless of the byte order since they have the same value
I was wrong, they only have the same value on Little-endian machines. With big-endian, EthIp is 2048
|
Looks like we need to do conditional compilation to fix the UB issue that fails our CI: /// Packet filter on IPv4 traffic
// NOTE: placed here due to conflict (little endian arch) with `SockProtocol::NetLinkISCI`
#[cfg(linux_android)]
#[allow(non_upper_case_globals)]
#[cfg(target_endian = "little")]
pub const EthIp: SockProtocol = unsafe { std::mem::transmute::<i32, SockProtocol>((libc::ETH_P_IP as u16).to_be() as i32) };
/// Packet filter on IPv4 traffic
#[cfg(linux_android)]
#[cfg(target_endian = "big")]
EthIp = (libc::ETH_P_IP as u16).to_be() as i32, |
devnexen
force-pushed
the
linux_ethernet_proto_af_packet
branch
from
5e91463 to
9a7b5b2
Compare
src/sys/socket/mod.rs
Outdated
| /// Packet filter on IPv4 traffic | ||
| #[cfg(linux_android)] | ||
| #[allow(non_upper_case_globals)] | ||
| #[cfg(target_endian = "big")] | ||
| pub const EthIp: i32 = (libc::ETH_P_IP as u16).to_be() as i32; |
There was a problem hiding this comment.
Sorry for being unclear, this should be an enum variant
There was a problem hiding this comment.
On big-endian targets, no enum variant has value 2048, so that std::mem::transmute() is UB, which is the reason why I said we should make it an enum variant
There was a problem hiding this comment.
yes it should work now
devnexen
force-pushed
the
linux_ethernet_proto_af_packet
branch
5 times, most recently
from
0615efd to
fb48acb
Compare
Respectively `sys::socket::SockProtocol::EthIp`, `sys::socket::SockProtocol::EthIpv6` and `sys::socket::SockProtocol::EthLoop` if we want more refined filtering as opposed to the existing `sys::socket::SockProtocol::EthAll` which captures everything.
devnexen
force-pushed
the
linux_ethernet_proto_af_packet
branch
from
fb48acb to
fbe50dc
Compare
Respectively
sys::socket::SockProtocol::EthIp,sys::socket::SockProtocol::EthIpv6andsys::socket::SockProtocol::EthLoopif we want more refined filtering as opposed to the existingsys::socket::SockProtocol::EthAllwhich captures everything.