12:40 a.m.
Add a few variants of RTNL utilities to look up addresses in the
kernel's neighbour table entries and to add new entries. This is
basically the ARP (for IPv4) and NDP (for IPv6) tables, that map IP
addresses to hardware addresses.
---
There are three variants of each method, I currently only have a use
case for the ipv4 and ipv6 variants so I'm happy dropping the struct
l_rtnl_address variant.
ell/ell.sym | 6 ++
ell/rtnl.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++
ell/rtnl.h | 36 ++++++++++
ell/useful.h | 4 ++
4 files changed, 240 insertions(+)
diff --git a/ell/ell.sym b/ell/ell.sym
index b29d98d..8760f2b 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -663,6 +663,12 @@ global:
l_rtnl_ifaddr_extract;
l_rtnl_ifaddr_add;
l_rtnl_ifaddr_delete;
+ l_rtnl_neighbour_get_ipv4_hwaddr;
+ l_rtnl_neighbour_get_ipv6_hwaddr;
+ l_rtnl_neighbour_get_addr_hwaddr;
+ l_rtnl_neighbour_set_ipv4_hwaddr;
+ l_rtnl_neighbour_set_ipv6_hwaddr;
+ l_rtnl_neighbour_set_addr_hwaddr;
/* icmp6 */
l_icmp6_client_new;
l_icmp6_client_free;
diff --git a/ell/rtnl.c b/ell/rtnl.c
index 1061105..4d81c48 100644
--- a/ell/rtnl.c
+++ b/ell/rtnl.c
@@ -27,6 +27,9 @@
#define _GNU_SOURCE
#include <linux/if.h>
#include <linux/icmpv6.h>
+#include <linux/neighbour.h>
+#include <linux/if_ether.h>
+#include <net/if_arp.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <errno.h>
@@ -34,6 +37,7 @@
#include "useful.h"
#include "netlink.h"
#include "log.h"
+#include "util.h"
#include "rtnl.h"
#include "private.h"
@@ -1311,3 +1315,193 @@ LIB_EXPORT uint32_t l_rtnl_route_delete(struct l_netlink *rtnl,
int ifindex,
return _rtnl_route_change(rtnl, RTM_DELROUTE, ifindex, rt,
cb, user_data, destroy);
}
+
+struct rtnl_neighbour_get_data {
+ l_rtnl_neighbour_get_cb_t cb;
+ void *user_data;
+ l_netlink_destroy_func_t destroy;
+};
+
+static void rtnl_neighbour_get_cb(int error, uint16_t type, const void *data,
+ uint32_t len, void *user_data)
+{
+ struct rtnl_neighbour_get_data *cb_data = user_data;
+ const struct ndmsg *ndmsg = data;
+ struct rtattr *attr;
+ const uint8_t *hwaddr = NULL;
+
+ if (error != 0)
+ goto done;
+
+ if (type != RTM_NEWNEIGH || len < NLMSG_ALIGN(sizeof(*ndmsg))) {
+ error = -EIO;
+ goto done;
+ }
+
+ if (!(ndmsg->ndm_state & (NUD_PERMANENT | NUD_NOARP | NUD_REACHABLE))) {
+ error = -ENOENT;
+ goto done;
+ }
+
+ attr = (void *) ndmsg + NLMSG_ALIGN(sizeof(*ndmsg));
+ len -= NLMSG_ALIGN(sizeof(*ndmsg));
+
+ for (; RTA_OK(attr, len); attr = RTA_NEXT(attr, len))
+ switch (attr->rta_type) {
+ case NDA_LLADDR:
+ hwaddr = RTA_DATA(attr);
+ break;
+ }
+
+ if (!hwaddr)
+ error = -EIO;
+
+done:
+ if (cb_data->cb) {
+ cb_data->cb(error, hwaddr, cb_data->user_data);
+ cb_data->cb = NULL;
+ }
+}
+
+static void rtnl_neighbour_get_destroy_cb(void *user_data)
+{
+ struct rtnl_neighbour_get_data *cb_data = user_data;
+
+ if (cb_data->destroy)
+ cb_data->destroy(cb_data->user_data);
+
+ l_free(cb_data);
+}
+
+static uint32_t rtnl_neighbour_get_hwaddr(struct l_netlink *rtnl,
+ int ifindex, uint8_t family,
+ const void *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ size_t bufsize = NLMSG_ALIGN(sizeof(struct ndmsg)) +
+ RTA_SPACE(16); /* NDA_DST */
+ uint8_t buf[bufsize];
+ struct ndmsg *ndmsg = (struct ndmsg *) buf;
+ void *rta_buf = (void *) ndmsg + NLMSG_ALIGN(sizeof(struct ndmsg));
+ __auto_type cb_data = struct_alloc(rtnl_neighbour_get_data,
+ cb, user_data, destroy);
+ uint32_t ret;
+
+ memset(buf, 0, bufsize);
+ ndmsg->ndm_family = family;
+ ndmsg->ndm_ifindex = ifindex;
+ ndmsg->ndm_flags = 0;
+
+ rta_buf += rta_add_address(rta_buf, NDA_DST, family, addr, addr);
+
+ ret = l_netlink_send(rtnl, RTM_GETNEIGH, 0, ndmsg,
+ rta_buf - (void *) ndmsg,
+ rtnl_neighbour_get_cb, cb_data,
+ rtnl_neighbour_get_destroy_cb);
+ if (ret)
+ return ret;
+
+ l_free(cb_data);
+ return 0;
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_get_ipv4_hwaddr(struct l_netlink *rtnl,
+ int ifindex, uint32_t addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ return rtnl_neighbour_get_hwaddr(rtnl, ifindex, AF_INET, &addr,
+ cb, user_data, destroy);
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_get_ipv6_hwaddr(struct l_netlink *rtnl,
+ int ifindex, const uint8_t *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ return rtnl_neighbour_get_hwaddr(rtnl, ifindex, AF_INET6, addr,
+ cb, user_data, destroy);
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl,
+ int ifindex,
+ const struct l_rtnl_address *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ const uint8_t *ip = addr->family == AF_INET ?
+ (const void *) &addr->in_addr : (const void *) &addr->in6_addr;
+
+ return rtnl_neighbour_get_hwaddr(rtnl, ifindex, addr->family, ip,
+ cb, user_data, destroy);
+}
+
+static uint32_t rtnl_neighbour_set_hwaddr(struct l_netlink *rtnl,
+ int ifindex, uint8_t family,
+ const void *addr, const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ size_t bufsize = NLMSG_ALIGN(sizeof(struct ndmsg)) +
+ RTA_SPACE(16) + /* NDA_DST */
+ RTA_SPACE(ETH_ALEN); /* NDA_LLADDR */
+ uint8_t buf[bufsize];
+ struct ndmsg *ndmsg = (struct ndmsg *) buf;
+ void *rta_buf = (void *) ndmsg + NLMSG_ALIGN(sizeof(struct ndmsg));
+
+ memset(buf, 0, bufsize);
+ ndmsg->ndm_family = family;
+ ndmsg->ndm_ifindex = ifindex;
+ ndmsg->ndm_flags = 0;
+ ndmsg->ndm_state = NUD_REACHABLE;
+
+ rta_buf += rta_add_address(rta_buf, NDA_DST, family, addr, addr);
+ rta_buf += rta_add_data(rta_buf, NDA_LLADDR, hwaddr, ETH_ALEN);
+
+ return l_netlink_send(rtnl, RTM_NEWNEIGH, NLM_F_CREATE | NLM_F_REPLACE,
+ ndmsg, rta_buf - (void *) ndmsg,
+ cb, user_data, destroy);
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl,
+ int ifindex, uint32_t addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ return rtnl_neighbour_set_hwaddr(rtnl, ifindex, AF_INET, &addr,
+ hwaddr, cb, user_data, destroy);
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_set_ipv6_hwaddr(struct l_netlink *rtnl,
+ int ifindex, const uint8_t *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ return rtnl_neighbour_set_hwaddr(rtnl, ifindex, AF_INET6, addr,
+ hwaddr, cb, user_data, destroy);
+}
+
+LIB_EXPORT uint32_t l_rtnl_neighbour_set_addr_hwaddr(struct l_netlink *rtnl,
+ int ifindex,
+ const struct l_rtnl_address *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy)
+{
+ const uint8_t *ip = addr->family == AF_INET ?
+ (const void *) &addr->in_addr : (const void *) &addr->in6_addr;
+
+ return rtnl_neighbour_set_hwaddr(rtnl, ifindex, addr->family, ip,
+ hwaddr, cb, user_data, destroy);
+}
diff --git a/ell/rtnl.h b/ell/rtnl.h
index 9ea24f5..834cd14 100644
--- a/ell/rtnl.h
+++ b/ell/rtnl.h
@@ -33,6 +33,9 @@ extern "C" {
struct l_rtnl_address;
struct l_rtnl_route;
+typedef void (*l_rtnl_neighbour_get_cb_t) (int error, const uint8_t *hwaddr,
+ void *user_data);
+
struct l_rtnl_address *l_rtnl_address_new(const char *ip, uint8_t prefix_len);
struct l_rtnl_address *l_rtnl_address_clone(const struct l_rtnl_address *orig);
void l_rtnl_address_free(struct l_rtnl_address *addr);
@@ -191,6 +194,39 @@ uint32_t l_rtnl_route_delete(struct l_netlink *rtnl, int ifindex,
void *user_data,
l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
+ uint32_t addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_ipv6_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const uint8_t *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const struct l_rtnl_address *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
+ uint32_t addr, const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_ipv6_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const uint8_t *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const struct l_rtnl_address *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+
#ifdef __cplusplus
}
#endif
diff --git a/ell/useful.h b/ell/useful.h
index d696bc3..b4783ce 100644
--- a/ell/useful.h
+++ b/ell/useful.h
@@ -83,3 +83,7 @@ static inline int secure_select(int select_left, int l, int r)
return r ^ ((l ^ r) & mask);
}
+
+#define struct_alloc(structname, ...) \
+ (struct structname *) l_memdup(&(struct structname) { __VA_ARGS__ }, \
+ sizeof(struct structname))
--
2.30.2
7:05 p.m.
Hi Andrew,
On 8/24/21 7:40 PM, Andrew Zaborowski wrote:
Add a few variants of RTNL utilities to look up addresses in the
kernel's neighbour table entries and to add new entries. This is
basically the ARP (for IPv4) and NDP (for IPv6) tables, that map IP
addresses to hardware addresses.
---
There are three variants of each method, I currently only have a use
case for the ipv4 and ipv6 variants so I'm happy dropping the struct
l_rtnl_address variant.
ell/ell.sym | 6 ++
ell/rtnl.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++
ell/rtnl.h | 36 ++++++++++
ell/useful.h | 4 ++
4 files changed, 240 insertions(+)
<snip>
diff --git a/ell/rtnl.h b/ell/rtnl.h
index 9ea24f5..834cd14 100644
--- a/ell/rtnl.h
+++ b/ell/rtnl.h
@@ -33,6 +33,9 @@ extern "C" {
struct l_rtnl_address;
struct l_rtnl_route;
+typedef void (*l_rtnl_neighbour_get_cb_t) (int error, const uint8_t *hwaddr,
You probably want a length as well, no?
+ void *user_data);
+
struct l_rtnl_address *l_rtnl_address_new(const char *ip, uint8_t prefix_len);
struct l_rtnl_address *l_rtnl_address_clone(const struct l_rtnl_address *orig);
void l_rtnl_address_free(struct l_rtnl_address *addr);
@@ -191,6 +194,39 @@ uint32_t l_rtnl_route_delete(struct l_netlink *rtnl, int ifindex,
void *user_data,
l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
+ uint32_t addr,
We do not use uint32_t to represent an ipv4 anywhere in the ell API.
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_ipv6_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const uint8_t *addr,
similarly to above, ipv6 is never represented like this.
I was hoping to avoid using in_addr / in6_addr in the public API, but maybe that
would be the lesser evil option than 'const uint8_t *'. Or just use
l_rtnl_address for everything.
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const struct l_rtnl_address *addr,
+ l_rtnl_neighbour_get_cb_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
+ uint32_t addr, const uint8_t *hwaddr,
You're making an assumption that hwaddr is always 6? That will not really be
true long term. Might as well take care of this now.
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_ipv6_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const uint8_t *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+uint32_t l_rtnl_neighbour_set_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
+ const struct l_rtnl_address *addr,
+ const uint8_t *hwaddr,
+ l_netlink_command_func_t cb,
+ void *user_data,
+ l_netlink_destroy_func_t destroy);
+
Regards,
-Denis
10:40 p.m.
Hi Denis,
On Wed, 25 Aug 2021 at 21:09, Denis Kenzior <denkenz(a)gmail.com> wrote:
On 8/24/21 7:40 PM, Andrew Zaborowski wrote:
> Add a few variants of RTNL utilities to look up addresses in the
> kernel's neighbour table entries and to add new entries. This is
> basically the ARP (for IPv4) and NDP (for IPv6) tables, that map IP
> addresses to hardware addresses.
> ---
> There are three variants of each method, I currently only have a use
> case for the ipv4 and ipv6 variants so I'm happy dropping the struct
> l_rtnl_address variant.
>
> ell/ell.sym | 6 ++
> ell/rtnl.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++
> ell/rtnl.h | 36 ++++++++++
> ell/useful.h | 4 ++
> 4 files changed, 240 insertions(+)
>
<snip>
> diff --git a/ell/rtnl.h b/ell/rtnl.h
> index 9ea24f5..834cd14 100644
> --- a/ell/rtnl.h
> +++ b/ell/rtnl.h
> @@ -33,6 +33,9 @@ extern "C" {
> struct l_rtnl_address;
> struct l_rtnl_route;
>
> +typedef void (*l_rtnl_neighbour_get_cb_t) (int error, const uint8_t *hwaddr,
You probably want a length as well, no?
> + void *user_data);
> +
> struct l_rtnl_address *l_rtnl_address_new(const char *ip, uint8_t prefix_len);
> struct l_rtnl_address *l_rtnl_address_clone(const struct l_rtnl_address *orig);
> void l_rtnl_address_free(struct l_rtnl_address *addr);
> @@ -191,6 +194,39 @@ uint32_t l_rtnl_route_delete(struct l_netlink *rtnl, int
ifindex,
> void *user_data,
> l_netlink_destroy_func_t destroy);
>
> +uint32_t l_rtnl_neighbour_get_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
> + uint32_t addr,
We do not use uint32_t to represent an ipv4 anywhere in the ell API.
> + l_rtnl_neighbour_get_cb_t cb,
> + void *user_data,
> + l_netlink_destroy_func_t destroy);
> +uint32_t l_rtnl_neighbour_get_ipv6_hwaddr(struct l_netlink *rtnl, int ifindex,
> + const uint8_t *addr,
similarly to above, ipv6 is never represented like this.
I was hoping to avoid using in_addr / in6_addr in the public API, but maybe that
would be the lesser evil option than 'const uint8_t *'. Or just use
l_rtnl_address for everything.
My preference would be for in_addr / in6_addr, l_rtnl_address seems to
be overkill for a wrapper for a 32-bit value.
The only evil aspect of these that I can see is uint32_t's endianness
ambiguity (something similar to kernel's __be32 would have been
better) but the simplicity is a big advantage.
> + l_rtnl_neighbour_get_cb_t cb,
> + void *user_data,
> + l_netlink_destroy_func_t destroy);
> +uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
> + const struct l_rtnl_address *addr,
> + l_rtnl_neighbour_get_cb_t cb,
> + void *user_data,
> + l_netlink_destroy_func_t destroy);
> +uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
> + uint32_t addr, const uint8_t *hwaddr,
You're making an assumption that hwaddr is always 6? That will not really be
true long term. Might as well take care of this now.
I am, but if the address is not 6 bytes the user will know it based on
the ifindex too. We can be more explicit and include a length though.
Best regards
10:45 p.m.
Hi Andrew,
> I was hoping to avoid using in_addr / in6_addr in the public API,
but maybe that
> would be the lesser evil option than 'const uint8_t *'. Or just use
> l_rtnl_address for everything.
My preference would be for in_addr / in6_addr, l_rtnl_address seems to
be overkill for a wrapper for a 32-bit value.
I wouldn't actually mind seeing more context on how you're using these. In
theory you would need the rtnl address anyway, unless you're using this within
dhcp server classes. In which case you can provide a private API as well.
The only evil aspect of these that I can see is uint32_t's endianness
ambiguity (something similar to kernel's __be32 would have been
better) but the simplicity is a big advantage.
Use of struct in_addr would imply network order is being used. Same as
inet_ntoa/inet_ntop, etc.
>
>> + l_rtnl_neighbour_get_cb_t cb,
>> + void *user_data,
>> + l_netlink_destroy_func_t destroy);
>> +uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl, int ifindex,
>> + const struct l_rtnl_address *addr,
>> + l_rtnl_neighbour_get_cb_t cb,
>> + void *user_data,
>> + l_netlink_destroy_func_t destroy);
>> +uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl, int ifindex,
>> + uint32_t addr, const uint8_t *hwaddr,
>
> You're making an assumption that hwaddr is always 6? That will not really be
> true long term. Might as well take care of this now.
I am, but if the address is not 6 bytes the user will know it based on
the ifindex too. We can be more explicit and include a length though.
User might, but API implementation doesn't. How does the implementation of this
function know that ifindex is for a device that uses 8 byte hardware address?
ifindex is just an int as far as it is concerned. So unless you're going to
introduce a netdev abstraction, hardware length would be needed here at some point.
Regards,
-Denis
11:57 p.m.
On Thu, 26 Aug 2021 at 00:51, Denis Kenzior <denkenz(a)gmail.com> wrote:
>> I was hoping to avoid using in_addr / in6_addr in the public
API, but maybe that
>> would be the lesser evil option than 'const uint8_t *'. Or just use
>> l_rtnl_address for everything.
>
> My preference would be for in_addr / in6_addr, l_rtnl_address seems to
> be overkill for a wrapper for a 32-bit value.
I wouldn't actually mind seeing more context on how you're using these. In
theory you would need the rtnl address anyway, unless you're using this within
dhcp server classes. In which case you can provide a private API as well.
This would be used in IWD like in
https://github.com/balrog-kun/iwd/commit/2da9ad4bb20a85f53fc459b5ec00dd90...
We only build an l_rtnl_address for our own IPs in netconfig.c.
>
> The only evil aspect of these that I can see is uint32_t's endianness
> ambiguity (something similar to kernel's __be32 would have been
> better) but the simplicity is a big advantage.
>
Use of struct in_addr would imply network order is being used. Same as
inet_ntoa/inet_ntop, etc.
Right, and same as __be32. __be32 has the advantage that it's not a
struct so, 1. you know its size and contents and can pass its pointer
directly to, say, an attribute builder without looking up internal
structures, 2. don't need to convert to/from it by assigning cryptic
field names, 3. can pass it as parameter.
>>
>>> + l_rtnl_neighbour_get_cb_t cb,
>>> + void *user_data,
>>> + l_netlink_destroy_func_t destroy);
>>> +uint32_t l_rtnl_neighbour_get_addr_hwaddr(struct l_netlink *rtnl, int
ifindex,
>>> + const struct l_rtnl_address *addr,
>>> + l_rtnl_neighbour_get_cb_t cb,
>>> + void *user_data,
>>> + l_netlink_destroy_func_t destroy);
>>> +uint32_t l_rtnl_neighbour_set_ipv4_hwaddr(struct l_netlink *rtnl, int
ifindex,
>>> + uint32_t addr, const uint8_t *hwaddr,
>>
>> You're making an assumption that hwaddr is always 6? That will not really
be
>> true long term. Might as well take care of this now.
>
> I am, but if the address is not 6 bytes the user will know it based on
> the ifindex too. We can be more explicit and include a length though.
User might, but API implementation doesn't. How does the implementation of this
function know that ifindex is for a device that uses 8 byte hardware address?
ifindex is just an int as far as it is concerned. So unless you're going to
introduce a netdev abstraction, hardware length would be needed here at some point.
True, we don't have netdev data directly in ell and would need to
either get it from the user or query it from the kernel if that is
ever a concern.
Best regards
1:50 p.m.
Hi Andrew,
> Use of struct in_addr would imply network order is being used.
Same as
> inet_ntoa/inet_ntop, etc.
Right, and same as __be32. __be32 has the advantage that it's not a
struct so, 1. you know its size and contents and can pass its pointer
directly to, say, an attribute builder without looking up internal
structures, 2. don't need to convert to/from it by assigning cryptic
field names, 3. can pass it as parameter.
Some of what you say isn't really correct. Look at the signature of inet_ntoa
for example.
You're arguing for use of __be32 because you store ipv4 addresses as uints. I
get that. But that doesn't help you with ipv6 and there is already a well
defined structure to represent an ipv4 address.
Anyhow, given how you're planning to use this, I'd just adopt a signature
similar to inet_ntop. Something like:
l_rtnl_neighbor_set_hwaddr(rtnl, ifindex, int family, const void *, const
uint8_t *hwaddr, size_t hwaddr_len, cb, user_data, destroy)
Also, US spelling of 'neighbor' would be preferred.
Regards,
-Denis
7 p.m.
Hi Denis,
On Thu, 26 Aug 2021 at 15:59, Denis Kenzior <denkenz(a)gmail.com> wrote:
>> Use of struct in_addr would imply network order is being
used. Same as
>> inet_ntoa/inet_ntop, etc.
>
> Right, and same as __be32. __be32 has the advantage that it's not a
> struct so, 1. you know its size and contents and can pass its pointer
> directly to, say, an attribute builder without looking up internal
> structures, 2. don't need to convert to/from it by assigning cryptic
> field names, 3. can pass it as parameter.
Some of what you say isn't really correct. Look at the signature of inet_ntoa
for example.
You're arguing for use of __be32 because you store ipv4 addresses as uints. I
get that. But that doesn't help you with ipv6 and there is already a well
defined structure to represent an ipv4 address.
I'd argue that an array for ipv6 is still more practical than the same
array wrapped in a struct (basically same thing as with IPv4 in_addr)
but ok.
Anyhow, given how you're planning to use this, I'd just adopt a signature
similar to inet_ntop. Something like:
l_rtnl_neighbor_set_hwaddr(rtnl, ifindex, int family, const void *, const
uint8_t *hwaddr, size_t hwaddr_len, cb, user_data, destroy)
Ok.
Also, US spelling of 'neighbor' would be preferred.
The reason I chose neighbour is because that is the name of the kernel
API but ok.
Best regards
357
days inactive
358
days old
6 comments
2 participants
participants (2)
-
Andrew Zaborowski -
Denis Kenzior