adding ethhdr type for linux/android for proper packet filtering. #4239
Conversation
|
Some changes occurred in the Android module cc @maurer |
devnexen
force-pushed
the
ethhdr_linux
branch
5 times, most recently
from
55f537d to
d478bbb
Compare
devnexen
force-pushed
the
ethhdr_linux
branch
9 times, most recently
from
da428da to
4f428e0
Compare
There was a problem hiding this comment.
The main thing I see missing here is a use of ethhdr. A struct type with no usage by libc might not actually belong here, even though it is on the UAPI interface - this is a bindings crate for libc, not for the Linux kernel.
Is there a function in libc that you intend to call that uses the ethhdr struct? If so, layering on a commit binding that on top of this one would make a better PR IMO.
src/unix/linux_like/android/mod.rs
Outdated
| @@ -517,6 +517,11 @@ s! { | |||
| pub ifr6_prefixlen: u32, | |||
| pub ifr6_ifindex: c_int, | |||
| } | |||
|
|
|||
| // is __be16 __bitwise __u16 | |||
| pub struct __c_anonymous___be16 { | |||
There was a problem hiding this comment.
Why __c_anonymous_? This isn't an anonymous type, it's name is __be16.
Unless I'm mistaken, __c_anonymous_fieldname is normally used for inline structs/unions attached to a field of that name - this is a named type, and that's not the field name.
|
|
||
| impl Eq for ethhdr {} | ||
|
|
||
| impl PartialEq for ethhdr { |
There was a problem hiding this comment.
This PartialEq definition appears to lack a check on the h_proto field.
There was a problem hiding this comment.
I did not forget but until I solve the alignment issue in some platforms and as I was playing with the h_proto type I do not bother with it just yet (still a draft).
| self.h_dest | ||
| .iter() | ||
| .zip(other.h_dest.iter()) | ||
| .all(|(a, b)| a == b) |
There was a problem hiding this comment.
Do you not want them to compare equal if they have the same CStr value, but different null trailing bytes?
There was a problem hiding this comment.
note that those fields represent mac addresses not as strings (note they re unsigned char).
src/unix/linux_like/android/mod.rs
Outdated
| @@ -517,6 +517,11 @@ s! { | |||
| pub ifr6_prefixlen: u32, | |||
| pub ifr6_ifindex: c_int, | |||
| } | |||
|
|
|||
| // is __be16 __bitwise __u16 | |||
| pub struct __c_anonymous___be16 { | |||
There was a problem hiding this comment.
Consider deriving Eq/PartialEq, adding a method to access/set it as host u16, and adding a Debug based on the host representation?
If you do that, your later stuff on ethhdr can just be a derive.
src/unix/linux_like/android/mod.rs
Outdated
|
|
||
| // linux/if_ether.h | ||
|
|
||
| #[repr(C, align(1))] |
There was a problem hiding this comment.
I know. was trying to solve few architectures build failures.
| f.debug_struct("ethhdr") | ||
| .field("h_dest", &self.h_dest) | ||
| .field("h_source", &self.h_source) | ||
| .finish() |
src/unix/linux_like/linux/mod.rs
Outdated
| @@ -1319,6 +1319,11 @@ s! { | |||
| pub propagation: crate::__u64, | |||
| pub userns_fd: crate::__u64, | |||
| } | |||
|
|
|||
| // is __be16 __bitwise __u16 | |||
| pub struct __c_anonymous___be16 { | |||
There was a problem hiding this comment.
As above re: __c_anonymous - it also might be helpful just to have a type like this higher up in the hierarchy to be shared, I find the idea that only Linux/Android have bigendian u16s unlikely.
There is, for packet filters purpose. sendto/recvfrom can use any buffer e.g. ethhdr for this case. since AF_PACKET family and ETH_P* constants are already there, I m just trying to finish it off. |
devnexen
force-pushed
the
ethhdr_linux
branch
from
9bb6cb5 to
a8ed135
Compare
devnexen
force-pushed
the
ethhdr_linux
branch
from
a8ed135 to
930a40c
Compare
devnexen
force-pushed
the
ethhdr_linux
branch
from
930a40c to
9944b71
Compare
No description provided.