Please add "Fixes #787" to the commit message.
Thanks, the patch looks mostly correct, but one question below.
On Wed, Oct 26, 2016 at 2:33 PM, Benoît Canet <
benoit.canet.cont...@gmail.com> wrote:
> Add these structure add the end of the list after the
> INET and INET6 addresses are filled.
>
> Signed-off-by: Benoît Canet <ben...@scylladb.com>
> ---
> libc/network/getifaddrs.c | 70 ++++++++++++++++++++++++++++++
> ++++++++++++++---
> 1 file changed, 66 insertions(+), 4 deletions(-)
>
> diff --git a/libc/network/getifaddrs.c b/libc/network/getifaddrs.c
> index 7bab121..bfce1e9 100644
> --- a/libc/network/getifaddrs.c
> +++ b/libc/network/getifaddrs.c
> @@ -11,6 +11,8 @@
> #include <arpa/inet.h> /* inet_pton */
> #include <unistd.h>
> #include <sys/ioctl.h>
> +#include <netpacket/packet.h>
> +#include <net/if_arp.h>
>
> typedef union {
> struct sockaddr_in6 v6;
> @@ -124,15 +126,70 @@ int allocate_and_add_ifaddrs(stor **list, stor
> **head, struct if_nameindex *ii)
> return i;
> }
>
> +int fill_mac_addrs(int sock, stor *list, struct if_nameindex *ii)
> +{
> + stor *head;
> + size_t i;
> +
> + for(head = list, i = 0;
> + head; head = (stor*)(head->next)) {
> + struct sockaddr_ll *hw_addr = (struct sockaddr_ll *)
> &head->addr;
> + head->ifa.ifa_addr = (struct sockaddr*) &head->addr;
> + struct ifreq req;
> + int ret;
> +
> + if (!ii[i].if_name) {
> + continue;
> + }
>
Shouldn't we also skip non-AF_INET interfaces, like loopback?
I think your code makes two copies of the loopback interface, and then
tries to find the hw address for that interface (I don't know what it
finds). Would have been nicer to have just one copy.
Is it really necessary for you to call allocate_and_add_ifaddrs() twice?
Couldn't you fill the normal interfaces just once (as in the original
code), and then loop over them adding *additional* AF_PACKET interface for
each AF_INET interface?
(Another possible problem with the way you did it which I don't fully
understand how it worked for you: sockaddr_ll has a different length than
sockaddr_in. So how could you allocate the structure with the normal code
which allocated sockaddr_in, and then put a sockaddr_ll in there?)
> +
> + /* zero the ifreq structure */
> + memset(&req, 0, sizeof(req));
> +
> + /* fill in the interface name */
> + strlcpy(req.ifr_name, ii[i].if_name, IFNAMSIZ);
> +
> + /* Get the hardware address */
> + ret = ioctl(sock, SIOCGIFHWADDR, &req);
> + if (ret == -1) {
> + return ret;
> + }
> +
> + /* Fill address, index, type, length, familly */
> + memcpy(hw_addr->sll_addr, req.ifr_hwaddr.sa_data,
> IFHWADDRLEN);
> + hw_addr->sll_ifindex = ii[i].if_index;
> + hw_addr->sll_hatype = ARPHRD_ETHER;
> + hw_addr->sll_halen = IFHWADDRLEN;
> + hw_addr->sll_family = AF_PACKET;
> +
> + /* Get and fill the address flags */
> + ret = ioctl(sock, SIOCGIFFLAGS, &req);
> + if (ret == - 1) {
> + return ret;
> + }
> + head->ifa.ifa_flags = req.ifr_flags;
> + i++;
> + }
> +
> + return 0;
> +}
> +
> int getifaddrs(struct ifaddrs **ifap)
> {
> stor *list = 0, *head = 0;
> struct if_nameindex* ii = if_nameindex();
> if(!ii) return -1;
> + int addr_count;
> size_t i;
> - if (!allocate_and_add_ifaddrs(&list, &head, ii)) {
> - if_freenameindex(ii);
> - goto err2;
> + // allocate and add to the list twice the number of
> + // interface of ifaddr storage in order to have enough
> + // for MAC HW addresses that require their own location.
> + for (i = 0; i < 2; i++) {
> + addr_count =
> + allocate_and_add_ifaddrs(&list, &head, ii);
> + if (!addr_count) {
> + if_freenameindex(ii);
> + goto err2;
> + }
> }
> if_freenameindex(ii);
>
> @@ -142,7 +199,9 @@ int getifaddrs(struct ifaddrs **ifap)
> struct ifconf conf = {.ifc_len = sizeof reqs, .ifc_req = reqs};
> if(-1 == ioctl(sock, SIOCGIFCONF, &conf)) goto err;
> size_t reqitems = conf.ifc_len / sizeof(struct ifreq);
> - for(head = list; head; head = (stor*)head->next) {
> + int j;
> + // loop and fill the INET addrs
> + for(head = list, j = 0; head && j < addr_count; head =
> (stor*)head->next, j++) {
> for(i = 0; i < reqitems; i++) {
> // get SIOCGIFADDR of active interfaces.
> if(!strcmp(reqs[i].ifr_name, head->name)) {
> @@ -173,6 +232,9 @@ int getifaddrs(struct ifaddrs **ifap)
> head->ifa.ifa_ifu.ifu_dstaddr = (struct sockaddr*)
> &head->dst;
> }
> }
> + if (-1 == fill_mac_addrs(sock, head, ii)) {
> + goto err;
>
+ }
> close(sock);
> void* last = 0;
> for(head = list; head; head=(stor*)head->next) last=head;
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
