Unclear how to work with LinkAddr / sockaddr_ll

[internetionals]

Currently I can't seem to find any way to create a new LinkAddr or modify a LinkAddr.

The only way I'm able to get a LinkAddr is through getifaddrs(). But after that I'm unable to change eg. sll_protocol to some specific protocol. I can turn LinkAddr into a sockaddr_ll (as_ref().to_owned()) and change it that way, but then I can't turn it back into a LinkAddr without using unsafe.

Is there a good reason why one shouldn't be able to get mutable access to the sockaddr_ll inside an owned LinkAddr? Or should that be done through mutators on the LinkAddr (I'll make an MR in a heartbeat :-))

Or would adding a constructor or impl From<sockaddr_ll> for LinkAddr be more idiomatic to the way these types are meant to be used?

[asomers]

What are you trying to do anyway? Change an interface's mac address? There is a good reason why the inner sockaddr_ll cannot be exposed; changing its sll_family field could lead to UB, since that field functions as the discriminator for the SockaddrStorage union. Mutators would be possible. But currently, the only way to custom craft a LinkAddr would be to craft a libc::sockaddr_ll, then cast it to a const *libc::sockaddr and use <LinkAddr as SockaddrLike>::from_raw.

[internetionals]

Working with packet(7) sockets basically needs options to at least modify sll_protocol as one can bind on an interface and choose which physical layer protocol one is interested in.

From the manpage:

By  default, all packets of the specified protocol type are passed to a packet socket.  To get packets only from
a specific interface use bind(2) specifying an address in a struct sockaddr_ll to bind the packet socket  to  an
interface.  Fields used for binding are sll_family (should be AF_PACKET), sll_protocol, and sll_ifindex.

When sending this structure is used to specify destination infromation when using SOCK_DGRAM (when using SOCK_RAW you have to craft the l2-layer header yourself). This involves being able to specify sll_protocol, sll_halen and sll_addr. All other fields should be cleared according to the documentation, though I recall it not really matter too much.

From the manpage:

When  you  send packets, it is enough to specify sll_family, sll_addr, sll_halen, sll_ifindex, and sll_protocol.
The other fields should be 0.  sll_hatype and sll_pkttype are set on received packets for your information.

In both cases sll_ifindex is also a field that one might want to change. In the past I crafted sockaddr_ll when needed, though a case can be made that you shouldn't be pulling these out of thin air and getifaddrs() should be used to get the initial references.

So what I basically would need would be either:

A way to derive a LinkAddr from getifaddrs() where I can specify either sll_protocol (for binding) or sll_protocol and sll_addr/sll_halen (for sending).

Or:

A way to construct a LinkAddr with a given sll_protocol and sll_ifindex (for binding) or with a given sll_protocol, sll_ifindex and sll_addr/sll_halen (for sending).

[internetionals]

Thinking about this a little more. As one could get information about new interfaces becoming available through eg. netlink sockets, so I think I would prefer the latter.

So I was thinking along the lines of:

impl LinkAddr {
   pub fn new(protocol: u16, ifindex: usize) -> LinkAddr;
   pub fn new_with_addr(protocol: u16, ifindex: usize, addr: &[u8; 6]) -> LinkAddr;
}

Note: I just noticed you're using [u8; 6] for addr() in your interface. But these links can have a length of 8 bytes in the case of eg. Fibre Channel. I personally don't have a use case for these at the moment, but this might cause issues for other users who do use physical link layers that have EUI-64 addresses. So perhaps using addr: &[u8] in the constructor would be more future proof.

[asomers]

I don't like the proposed constructors because they're potentially too limited. On Linux and OSX, the libc structure has 7 fields. On the BSDs, it has between 8 and 10. We don't want to define a new constructor for every combination of fields that the user might want to initialize. I think it would be better to either use the Builder pattern, or else rely on getifaddrs to initialize the structure, and then provide mutators as necessary.

Regarding [u8; 6], you're right. A better API would be fn addr(&self) -> &[u8], since the libc structure knows the length of valid data in the sdl_data field. It would be better for backwards compatibility, though, to return Option<&[u8]>.

[jonatanzeidler]

We a use case with a raw socket (EtherCAT) where we currently do

        let sll = &libc::sockaddr_ll {
            sll_family: libc::AF_PACKET as u16,
            sll_ifindex: index.try_into()?,
            sll_protocol: ethertype.to_be(),
            sll_addr: [0; LENGTH_PHYSICAL_LAYER_ADDR],
            sll_halen: 0,
            sll_hatype: 0,
            sll_pkttype: 0,
        };
        let ssl_len = std::mem::size_of_val(sll).try_into()?;

        let laddr = unsafe {
            LinkAddr::from_raw(
                sll as *const libc::sockaddr_ll as *const sockaddr,
                Some(ssl_len),
            )
        }
        .ok_or(Error::Configuration("sockaddr_ll is invalid".to_string()))?;

Is there anything preventing you from providing a safe constructor that just takes in the sockaddr_ll by move or a similar From<libc::sockaddr_ll> implementation?

impl LinkAddr {
    pub fn new(sll: libc::sockaddr_ll) -> Self {
        Self(sll)
    }
}

[asomers]

Is there anything preventing you from providing a safe constructor that just takes in the sockaddr_ll by move or a similar From<libc::sockaddr_ll> implementation?

Yes. We can't provide a safe From<libc::sockaddr_ll> implementation because the SockaddrLike implementation requires the sdl_len and sdl_family fields to be correct. Otherwise UB could result. So we can provide TryFrom, but not From.