Support `NlattrType`s for generic unsigned integer types (Only `u16` presently)

[a-gavin]

Would adding this feature cause a breaking change?
I'm not sure, but hopefully no?

Is your feature request related to a problem? Please describe.
NlattrType impl is only supported for u16 serialized type enums. However, there are several non-u16 attributes in the nl80211 generic Netlink specification (all u32 that I can tell from work I've done recently).

While defining these actually u32 attributes as u16 may permit usage for parsing messages received from the kernel module (e.g. nested attributes), this workaround can prevent usage when sending messages to the kernel module.

I have specifically seen this when defining/using the nested enum nl80211_iftype u32 attr enum. When attempting to add a new interface with the u16 workaround, the kernel returns an error message complaining about invalid length. Wireshark netlink capture confirms this and working case using the iw tool shows success with the correct length.

Describe the solution you'd like
Added support for arbitrary unsigned integer type serialized type enums, specifically u32.

Additional context
Example where the actual type is u32, but only defining as u16 will compile:

/// Virtual interface types
// TODO: This is actually a u32, but compiler won't allow anything but u16 for Nlattr
#[neli_enum(serialized_type = "u16")]
pub enum Nl80211IfType {
    /// Unspecified type, driver decides
    Unspecified = nl80211_iftype::NL80211_IFTYPE_UNSPECIFIED as u16,
    ...
    /// NAN device interface type (not a netdev)
    Nan = nl80211_iftype::NL80211_IFTYPE_NAN as u16,
}
impl neli::consts::genl::NlAttrType for Nl80211IfType {}

[jbaublitz]

Hi @a-gavin, thanks for the bug report. Can you clarify in what data structure exactly these u32s are being returned? Part of the reason I've defined it this way is that NlAttrType in the header is always going to be a u16 according to the data structure provided by the kernel headers. Are these somehow being passed back as payloads in the nested attributes? If they are, it may be better to designate them as something other than NlAttrType as those should always be able to be represented by a u16. If they are simply defined as u32s and can fit inside a u16, a conversion between the two may be good enough.

I may be misunderstanding your problem so any additional detail you can give me would be helpful here, specifically referencing what data structures these constants are being passed back in. There is a chance that this is due to non-standard netlink code as it seems not every family always follows the spec.

[a-gavin]

Thanks for your response. I'm amenable to any solution here, as this issue blocks core functionality of the library I'm hacking on.

For a more concrete example of this, see the NL80211_ATTR_IFTYPE nl80211 attribute. It is used both non-nested and nested and contains the enum nl80211_iftype. For non-nested use, nl80211 reports interface type (here in iw) and configures interface type for a new interface (here in iw). For nested use, nl80211 reports supported interface types for a given WiFi radio in the NL80211_ATTR_SUPPORTED_IFTYPES parent attribute (here in iw).

My struggle here using neli is I can either configure and query the NL80211_ATTR_IFTYPE as a u32 but not parse out the NL80211_ATTR_SUPPORTED_IFTYPES, or I can do the opposite.

Example iw CLI to reproduce this is as follows (to perform netlink monitor packet capture):

# Report WiFi interface's type (screenshot below)
iw wlan0 info

# Report supported interfaces for a given WiFi radio (screenshot below)
iw phy0 info | grep -A 7 "Supported interface modes:"

# Create new WiFi interface (monitor here)
sudo iw phy0 interface add moni0 type monitor

If it helps, the nl80211 driver configures the NL80211_ATTR_IFTYPE nl80211 attribute here when requesting interface information, here when creating a new interface, and here (nested) when reporting interface types supported by a WiFi radio.

iw wlan0 info

iw phy0 info | grep -A 7 "Supported interface modes:"

[a-gavin]

Curious what your thoughts on this are. Happy to help with a solution where I can.

I ran into this again with another nl80211 attribute i'd like to parse out, which is a variation on the theme. The NL80211_ATTR_SUPPORTED_COMMANDS returned during a iw phy0 info command contains a nested array of NL80211_CMD_Xs.

[jbaublitz]

Hi @a-gavin, I sincerely apologize for the delay. There have been a lot of different things I need to tend to in neli including cleaning up some code that was submitted that only works on little endian. I've finally found some time to take a look at this and if I'm understanding the wire dump you provided correctly, I think this may be a misunderstanding on your part about what the NlAttrType trait is for. Specifically, I believe that what I'm seeing is that the Nl80211IfType is being limited to a u16. This is by design as it is the first field in this data structure which can only be a u16 by definition. Making it larger than that would cause padding/serialization errors and the kernel would reject it. I think you are confusing the length of the attribute with the length of the payload. The lengths I'm seeing for the nested attributes are 4. This actually means that there is not payload at all as the header is exactly 4 bytes and the length is the total size of the attribute, not the payload. For example, the attribute you highlighted in the screenshot is an example of an attribute of type NL80211_ATTR_SUPPORTED_IFTYPES (size u16) with a nested attribute of type NL80211_ATTR_IFTYPE (again, size u16) and then no payload, totally to a length of 4. Does that make sense?

[a-gavin]

@jbaublitz No worries. I understand that open source projects are a (thankless) labor of love, so I appreciate your efforts no matter the timeline :)

Apologies as I didn't communicate this well, but I understand that netlink attributes must be u16. The issue I'm trying to highlight here is that I am unable to both define an enum that is both a netlink attribute but is also serialized with a non-u16 type payload.

The example Wireshark capture I shared above includes both methods of using the enum nl80211_iftype values. In the first screenshot, it is used as a u32 payload contained within the NL80211_ATTR_IFTYPE attribute (length 8 bytes, payload length 4 bytes). In the second screenshot, it is used as a series of nested attributes contained with the NL80211_ATTR_SUPPORTED_IFTYPES attribute (length 4 bytes, no payload).

Assuming my understanding is correct here, I don't presently see a way to define an enum that can both be a netlink attribute and a non-u16 payload. Please let me know if I have misunderstood something or if any of this is unclear.

[jbaublitz]

@a-gavin Thank you for clarifying! That makes a lot more sense. For the time being I believe that the only workaround is simply converting it to a u32 integer value and that can be serialized easily with the appropriate width. As for a principled way to handle this long term, I'm not entirely sure. I believe that allowing multiple serialization implementations may cause a lot of problems as it would be almost impossible to specify which binary representation you want in the current framework. I'm not really seeing a clean way to make sure that the appropriate integer width is used without some sort of system to specify which implementation to use for the same data type. While this is not ideal, I'm wondering if duplicating the constants and defining an attribute vs. a payload enum would solve the problem. It's not very nice design wise, but I believe that will give you the separation at the type level to essentially define the serialization as a u16 in one case and u32 in another case. The design of neli really hinges on the data type being a 1:1 relationship with the binary serialization so I'm not sure if there's a way to implement exactly what you're looking for without some sort of total rewrite.

[a-gavin]

@jbaublitz Thanks for the quick reply. Understood on design assumptions requiring data type and serialization being 1:1. I'll need to review the number of nl80211 constants that operate in this way, but I believe there are at least a few.

I'll experiment a bit and get back to you. I'm not sure I'd want to upstream a solution that defines enums that are separately payload and attributes, but my opinion on that may change.

What are your thoughts on adjusting the neli_enum annotation to somehow support this use case? As you mention, that would require substantial changes/creating some sort of new system to support this. I'm new to proc macros, but I'd be happy to experiment.

[jbaublitz]

@a-gavin The problem has less to do with proc macros and more the design of the library. For example, when we implement ToBytes for a large struct, the idea is that every single field must also implement ToBytes. ToBytes specifically has exactly one serialized representation and this allows users who know nothing about the serialization to simply call .to_bytes() and get the binary representation that they expect.

Now there are a few angles we could take for this. Because this problem seems limited to attributes, we may be able to pull some type magic to effectively allow serializing a u16 as a u32. For example, we may be able to do something like this:

fn set_payload_as<P, T>(&mut self, payload: &P) -> Result<(), SerError> where T: Size + To Bytes + From<P>

Do you think some sort of solution like this could work? I find it unlikely that the correct answer is to have multiple options for how a data structure is serialized because it seems to be the exception and the not rule that there are multiple binary representations of a data structure, but we may be able to get around this in the case of integers at the very least.

[jbaublitz]

@a-gavin Any thoughts on this?

[a-gavin]

@jbaublitz Hey, sorry holiday season gets busy! Understood on core point being single serialized representation.

I'm not opposed to the solution you proposed, and share your view that integers are (hopefully) the exception and not the rule. From my viewpoint (mostly nl80211), this style of attribute that's also a payload w/ different size seems to be the only special case for differing binary representations.

This will make the usage of these attributes a little less seamless than others, but like you say, that can't be avoided without a much larger rework of things that probably isn't necessary for this special case. I'm still just happy I can write this code in Rust :)