You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Several types are recurring in networking code in general.
For example,
IP addresses (v4 and v6 come from std)
MAC
Vlan ID
MPLS label
TCP / UDP Port number
...
Do we have a policy on when to create wrapper types to describe networking constructs?
For example, netlink messages often use u16 to represent VLAN IDs.
However, vlan IDs are actually only 12 bits long.
My first instinct is to write:
#[derive(Debug,PartialEq,Eq,Clone,Copy,PartialOrd,Ord,Hash)]#[repr(transparent)]pubstructVlanId(u16);implVlanId{pubfntry_new(id:u16) -> Result<Self,Error>{if id >= 4096{returnErr(anyhow::anyhow!("VLAN ID must be less than 4096"));}Ok(Self(id))}}implFrom<VlanId>foru16{fnfrom(val:VlanId) -> Self{
val.0}}implTryFrom<u16>forVlanId{typeError = Error;fntry_from(id:u16) -> Result<Self,Self::Error>{Self::try_new(id)}}implAsRef<u16>forVlanId{fnas_ref(&self) -> &u16{&self.0}}
This is a bit of boilerplate, but it does make the API a bit clearer and safer IMO.
It improves error handling.
It attaches "units" to otherwise non-descript u16 values.
You can use traits or inherent impls to add methods to the type (e.g. Mac::is_multicast()).
That said, if the purpose of the library is to provide a mapping to the netlink API, then it might be better to just use u16 directly as has already been done elsewhere.
After all, a netlink message containing vlan 7000 is a syntactically legal message even if it is semantically nonsensical. Is it the job of this library to provide guard rails against such nonsense messages?
I personally lean towards the use of wrapper types (although they should be implemented in a different crate and used only in the higher level libraries).
I looked at the rest of the code base for guidance on this and found that we seem to prefer the raw types unless the type is compound or more complex.
We seem to have consistently used the ip address types from std, but we don't seem to have wrapped types like MAC or Vlan ID into validated and reusable types.
MAC is represented as [u8; 6] in several places.
In link/link_info/vlan.rs we have:
pubenumInfoVlan{Id(u16),// ...}
and in link/sriov/vf_vlan.rs we have:
pubstructVfVlanInfo{// ...pubvlan_id:u32,// ...}
I'm happy with most approaches, but I do think it would be ideal to have guidelines here.
The text was updated successfully, but these errors were encountered:
Several types are recurring in networking code in general.
For example,
Do we have a policy on when to create wrapper types to describe networking constructs?
For example, netlink messages often use
u16
to represent VLAN IDs.However, vlan IDs are actually only 12 bits long.
My first instinct is to write:
This is a bit of boilerplate, but it does make the API a bit clearer and safer IMO.
u16
values.Mac::is_multicast()
).That said, if the purpose of the library is to provide a mapping to the netlink API, then it might be better to just use
u16
directly as has already been done elsewhere.After all, a netlink message containing vlan 7000 is a syntactically legal message even if it is semantically nonsensical.
Is it the job of this library to provide guard rails against such nonsense messages?
I personally lean towards the use of wrapper types (although they should be implemented in a different crate and used only in the higher level libraries).
I looked at the rest of the code base for guidance on this and found that we seem to prefer the raw types unless the type is compound or more complex.
We seem to have consistently used the ip address types from std, but we don't seem to have wrapped types like MAC or Vlan ID into validated and reusable types.
MAC is represented as
[u8; 6]
in several places.In
link/link_info/vlan.rs
we have:and in
link/sriov/vf_vlan.rs
we have:I'm happy with most approaches, but I do think it would be ideal to have guidelines here.
The text was updated successfully, but these errors were encountered: