On 03/08/2018 02:11 AM, Chen Hanxiao wrote:
From: Chen Hanxiao <chenhanxiao@gmail.com>
introduce helper to parse RTM_GETNEIGH query message and store it in struct virArpTable.
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- v4-rebase: fit split Makefile.am fit new virMacAddr fields
v4: use netlink query instead of parsing /proc/net/arp
v3: s/virGetArpTable/virArpTableGet alloc virArpTable in virArpTableGet return ENOSUPP on none-Linux platform move helpers to virarptable.[ch]
po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 ++ src/util/Makefile.inc.am | 2 + src/util/virarptable.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virarptable.h | 48 +++++++++++++ 6 files changed, 238 insertions(+) create mode 100644 src/util/virarptable.c create mode 100644 src/util/virarptable.h
Couple of Coverity issues.... [...]
diff --git a/src/util/virarptable.c b/src/util/virarptable.c new file mode 100644 index 000000000..cb56338eb --- /dev/null +++ b/src/util/virarptable.c
[...]
+# define NDA_RTA(r) \ + ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ndmsg)))) + +static int +parse_rtattr(struct rtattr *tb[], int max, struct rtattr *rta, int len) +{ + memset(tb, 0, sizeof(struct rtattr *) * (max + 1)); + while (RTA_OK(rta, len)) { + if ((rta->rta_type <= max) && (!tb[rta->rta_type])) + tb[rta->rta_type] = rta; + rta = RTA_NEXT(rta, len); + } + + if (len) + VIR_WARN("malformed netlink message: Deficit %d, rta_len=%d", + len, rta->rta_len); + return 0; +} + +virArpTablePtr virArpTableGet(void)
As an aside - this format is non standard, should be virArpTablePtr virArpTableGet(void) and there should be 2 blank lines between functions.
+{ + int num = 0; + int msglen; + void *nlData = NULL; + virArpTablePtr table = NULL; + char *ipstr = NULL; + struct nlmsghdr* nh; + struct rtattr * tb[NDA_MAX+1]; + + msglen = virNetlinkGetNeighbor(&nlData, 0, 0); + if (msglen < 0) + return NULL; + + if (VIR_ALLOC(table) < 0) + return NULL; + + nh = (struct nlmsghdr*)nlData; + + while (NLMSG_OK(nh, msglen)) { + struct ndmsg *r = NLMSG_DATA(nh); + int len = nh->nlmsg_len; + void *addr; + + if ((len -= NLMSG_LENGTH(sizeof(*nh))) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("wrong nlmsg len")); + goto cleanup; + } + + if (r->ndm_family && (r->ndm_family != AF_INET)) + goto next_nlmsg; + + /* catch stale and reachalbe arp entry only */ + if (r->ndm_state && + (!(r->ndm_state == NUD_STALE || r->ndm_state == NUD_REACHABLE))) { + nh = NLMSG_NEXT(nh, msglen); + continue; + } + + if (nh->nlmsg_type == NLMSG_DONE) + goto end_of_netlink_messages; + + parse_rtattr(tb, NDA_MAX, NDA_RTA(r), + nh->nlmsg_len - NLMSG_LENGTH(sizeof(*r))); + + if (tb[NDA_DST] == NULL || tb[NDA_LLADDR] == NULL) + goto next_nlmsg; + + if (tb[NDA_DST]) { + virSocketAddr virAddr; + if (VIR_REALLOC_N(table->t, num + 1) < 0) + goto cleanup; + + table->n = num + 1; + + addr = RTA_DATA(tb[NDA_DST]); + bzero(&virAddr, sizeof(virAddr)); + virAddr.len = sizeof(virAddr.data.inet4); + virAddr.data.inet4.sin_family = AF_INET; + virAddr.data.inet4.sin_addr = *(struct in_addr *)addr; + ipstr = virSocketAddrFormat(&virAddr); + + if (VIR_STRDUP(table->t[num].ipaddr, ipstr) < 0) + goto cleanup; + + VIR_FREE(ipstr); + } + + if (tb[NDA_LLADDR]) { + virMacAddr macaddr; + char ifmac[VIR_MAC_STRING_BUFLEN]; + + addr = RTA_DATA(tb[NDA_LLADDR]); + memcpy(macaddr.addr, addr, VIR_MAC_BUFLEN); + + virMacAddrFormat(&macaddr, ifmac); + + if (VIR_STRDUP(table->t[num].mac, ifmac) < 0) + goto cleanup; + + num++; + } + + next_nlmsg: + nh = NLMSG_NEXT(nh, msglen); + } + + end_of_netlink_messages: + VIR_FREE(nlData); + return table; + + cleanup:
If we end up here, then @table (and anything that's allocated into it) is leaked
+ VIR_FREE(ipstr); + VIR_FREE(nlData); + return NULL; +} + +#else + +virArpTablePtr virArpTableGet(void)
Similar comment here about the format...
+{ + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("get arp table not implemented on this platform")); + return NULL; +} + +#endif /* __linux__ */ + +void +virArpTableFree(virArpTablePtr table) +{ + size_t i;
This can be called by qemuARPGetInterfaces when @table == NULL, so it would be good to put in a "if (!table) return;" right here
+ for (i = 0; i < table->n; i++) { + VIR_FREE(table->t[i].ipaddr); + VIR_FREE(table->t[i].mac); + } + VIR_FREE(table); +}
John [...]