Re: [libvirt] [PATCH v2] macvtap: convert nl msg construction to use libnl

Any comments on this one? Stefan On 11/04/2010 06:57 AM, Stefan Berger wrote: > V2: > - forgot to convert two more function that were hidden in #defines > - small nits > > In a first step I am converting the netlink message construction in > macvtap code to use libnl. It's pretty much a 1:1 conversion except > that now the message needs to be allocated and deallocated. > > Signed-off-by: Stefan Berger <stefanb(a)us.ibm.com&gt; > > --- > src/util/macvtap.c | 321 > ++++++++++++++++++++++------------------------------- > 1 file changed, 137 insertions(+), 184 deletions(-) > > Index: libvirt-acl/src/util/macvtap.c > =================================================================== > --- libvirt-acl.orig/src/util/macvtap.c > +++ libvirt-acl/src/util/macvtap.c > @@ -196,51 +196,6 @@ err_exit: > } > > > -static struct rtattr * > -rtattrCreate(char *buffer, int bufsize, int type, > - const void *data, int datalen) > -{ > - struct rtattr *r = (struct rtattr *)buffer; > - r->rta_type = type; > - r->rta_len = RTA_LENGTH(datalen); > - if (r->rta_len > bufsize) > - return NULL; > - memcpy(RTA_DATA(r), data, datalen); > - return r; > -} > - > - > -static void > -nlInit(struct nlmsghdr *nlm, int flags, int type) > -{ > - nlm->nlmsg_len = NLMSG_LENGTH(0); > - nlm->nlmsg_flags = flags; > - nlm->nlmsg_type = type; > -} > - > - > -static void > -nlAlign(struct nlmsghdr *nlm) > -{ > - nlm->nlmsg_len = NLMSG_ALIGN(nlm->nlmsg_len); > -} > - > - > -static void * > -nlAppend(struct nlmsghdr *nlm, int totlen, const void *data, int > datalen) > -{ > - char *pos; > - nlAlign(nlm); > - if (nlm->nlmsg_len + NLMSG_ALIGN(datalen) > totlen) > - return NULL; > - pos = (char *)nlm + nlm->nlmsg_len; > - memcpy(pos, data, datalen); > - nlm->nlmsg_len += datalen; > - nlAlign(nlm); > - return pos; > -} > - > - > # if WITH_MACVTAP > > static int > @@ -252,74 +207,63 @@ link_add(const char *type, > int *retry) > { > int rc = 0; > - char nlmsgbuf[NLMSGBUF_SIZE]; > - struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; > + struct nlmsghdr *resp; > struct nlmsgerr *err; > - char rtattbuf[RATTBUF_SIZE]; > - struct rtattr *rta, *rta1, *li; > struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; > int ifindex; > char *recvbuf = NULL; > unsigned int recvbuflen; > + struct nl_msg *nl_msg; > + struct nlattr *linkinfo, *info_data; > > if (ifaceGetIndex(true, srcdev, &ifindex) != 0) > return -1; > > *retry = 0; > > - memset(&nlmsgbuf, 0, sizeof(nlmsgbuf)); > - > - nlInit(nlm, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL, > RTM_NEWLINK); > + nl_msg = nlmsg_alloc_simple(RTM_NEWLINK, > + NLM_F_REQUEST | NLM_F_CREATE | > NLM_F_EXCL); > + if (!nl_msg) { > + virReportOOMError(); > + return -1; > + } > > - if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) > + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) > < 0) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINK, > - &ifindex, sizeof(ifindex)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put_u32(nl_msg, IFLA_LINK, ifindex) < 0) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_ADDRESS, > - macaddress, macaddrsize); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_ADDRESS, macaddrsize, macaddress) < 0) > goto buffer_too_small; > > - if (ifname) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, > - ifname, strlen(ifname) + 1); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > - goto buffer_too_small; > - } > + if (ifname && > + nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) > + goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_LINKINFO, > NULL, 0); > - if (!rta || > - !(li = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len))) > + if (!(linkinfo = nla_nest_start(nl_msg, IFLA_LINKINFO))) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_KIND, > - type, strlen(type)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_INFO_KIND, strlen(type), type) < 0) > goto buffer_too_small; > > if (macvlan_mode > 0) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_INFO_DATA, > - NULL, 0); > - if (!rta || > - !(rta1 = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len))) > + if (!(info_data = nla_nest_start(nl_msg, IFLA_INFO_DATA))) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_MACVLAN_MODE, > - &macvlan_mode, sizeof(macvlan_mode)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_MACVLAN_MODE, sizeof(macvlan_mode), > + &macvlan_mode) < 0) > goto buffer_too_small; > > - rta1->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)rta1; > + nla_nest_end(nl_msg, info_data); > } > > - li->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)li; > + nla_nest_end(nl_msg, linkinfo); > > - if (nlComm(nlm, &recvbuf, &recvbuflen, 0) < 0) > - return -1; > + if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, 0) < 0) { > + rc = -1; > + goto err_exit; > + } > > if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) > goto malformed_resp; > @@ -357,50 +301,58 @@ link_add(const char *type, > goto malformed_resp; > } > > +err_exit: > + nlmsg_free(nl_msg); > + > VIR_FREE(recvbuf); > > return rc; > > malformed_resp: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > _("malformed netlink response message")); > VIR_FREE(recvbuf); > return -1; > > buffer_too_small: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("internal buffer is too small")); > + _("allocated netlink buffer is too small")); > return -1; > } > > > static int > -link_del(const char *name) > +link_del(const char *ifname) > { > int rc = 0; > - char nlmsgbuf[NLMSGBUF_SIZE]; > - struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; > + struct nlmsghdr *resp; > struct nlmsgerr *err; > - char rtattbuf[RATTBUF_SIZE]; > - struct rtattr *rta; > struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC }; > char *recvbuf = NULL; > unsigned int recvbuflen; > + struct nl_msg *nl_msg; > > - memset(&nlmsgbuf, 0, sizeof(nlmsgbuf)); > - > - nlInit(nlm, NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL, > RTM_DELLINK); > + nl_msg = nlmsg_alloc_simple(RTM_DELLINK, > + NLM_F_REQUEST | NLM_F_CREATE | > NLM_F_EXCL); > + if (!nl_msg) { > + virReportOOMError(); > + return -1; > + } > > - if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) > + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) > < 0) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, > - name, strlen(name)+1); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) > goto buffer_too_small; > > - if (nlComm(nlm, &recvbuf, &recvbuflen, 0) < 0) > - return -1; > + if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, 0) < 0) { > + rc = -1; > + goto err_exit; > + } > > if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) > goto malformed_resp; > @@ -416,7 +368,7 @@ link_del(const char *name) > if (err->error) { > virReportSystemError(-err->error, > _("error destroying %s interface"), > - name); > + ifname); > rc = -1; > } > break; > @@ -428,19 +380,26 @@ link_del(const char *name) > goto malformed_resp; > } > > +err_exit: > + nlmsg_free(nl_msg); > + > VIR_FREE(recvbuf); > > return rc; > > malformed_resp: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > _("malformed netlink response message")); > VIR_FREE(recvbuf); > return -1; > > buffer_too_small: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("internal buffer is too small")); > + _("allocated netlink buffer is too small")); > return -1; > } > > @@ -790,40 +749,44 @@ link_dump(bool nltarget_kernel, const ch > struct nlattr **tb, char **recvbuf) > { > int rc = 0; > - char nlmsgbuf[NLMSGBUF_SIZE] = { 0, }; > - struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; > + struct nlmsghdr *resp; > struct nlmsgerr *err; > - char rtattbuf[RATTBUF_SIZE]; > - struct rtattr *rta; > struct ifinfomsg ifinfo = { > .ifi_family = AF_UNSPEC, > .ifi_index = ifindex > }; > unsigned int recvbuflen; > uint32_t pid = 0; > + struct nl_msg *nl_msg; > > *recvbuf = NULL; > > - nlInit(nlm, NLM_F_REQUEST, RTM_GETLINK); > + nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST); > + if (!nl_msg) { > + virReportOOMError(); > + return -1; > + } > > - if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) > + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) > < 0) > goto buffer_too_small; > > if (ifindex < 0 && ifname) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, > - ifname, strlen(ifname) + 1); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) > goto buffer_too_small; > } > > if (!nltarget_kernel) { > pid = getLldpadPid(); > - if (pid == 0) > - return -1; > + if (pid == 0) { > + rc = -1; > + goto err_exit; > + } > } > > - if (nlComm(nlm, recvbuf, &recvbuflen, pid) < 0) > - return -1; > + if (nlComm(nlmsg_hdr(nl_msg), recvbuf, &recvbuflen, pid) < 0) { > + rc = -1; > + goto err_exit; > + } > > if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL) > goto malformed_resp; > @@ -859,17 +822,24 @@ link_dump(bool nltarget_kernel, const ch > if (rc != 0) > VIR_FREE(*recvbuf); > > +err_exit: > + nlmsg_free(nl_msg); > + > return rc; > > malformed_resp: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > _("malformed netlink response message")); > VIR_FREE(*recvbuf); > return -1; > > buffer_too_small: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("internal buffer is too small")); > + _("allocated netlink buffer is too small")); > return -1; > } > > @@ -1053,11 +1023,8 @@ doPortProfileOpSetLink(bool nltarget_ker > uint8_t op) > { > int rc = 0; > - char nlmsgbuf[NLMSGBUF_SIZE]; > - struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp; > + struct nlmsghdr *resp; > struct nlmsgerr *err; > - char rtattbuf[RATTBUF_SIZE]; > - struct rtattr *rta, *vfports = NULL, *vfport; > struct ifinfomsg ifinfo = { > .ifi_family = AF_UNSPEC, > .ifi_index = ifindex, > @@ -1065,23 +1032,24 @@ doPortProfileOpSetLink(bool nltarget_ker > char *recvbuf = NULL; > unsigned int recvbuflen = 0; > uint32_t pid = 0; > + struct nl_msg *nl_msg; > + struct nlattr *vfports = NULL, *vfport; > > - memset(&nlmsgbuf, 0, sizeof(nlmsgbuf)); > - > - nlInit(nlm, NLM_F_REQUEST, RTM_SETLINK); > + nl_msg = nlmsg_alloc_simple(RTM_SETLINK, NLM_F_REQUEST); > + if (!nl_msg) { > + virReportOOMError(); > + return -1; > + } > > - if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo))) > + if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) > < 0) > goto buffer_too_small; > > - if (ifname) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_IFNAME, > - ifname, strlen(ifname) + 1); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > - goto buffer_too_small; > - } > + if (ifname && > + nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0) > + goto buffer_too_small; > > if (macaddr && vlanid >= 0) { > - struct rtattr *vfinfolist, *vfinfo; > + struct nlattr *vfinfolist, *vfinfo; > struct ifla_vf_mac ifla_vf_mac = { > .vf = vf, > .mac = { 0, }, > @@ -1094,110 +1062,88 @@ doPortProfileOpSetLink(bool nltarget_ker > > memcpy(ifla_vf_mac.mac, macaddr, 6); > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_VFINFO_LIST, > - NULL, 0); > - if (!rta || > - !(vfinfolist = nlAppend(nlm, sizeof(nlmsgbuf), > - rtattbuf, rta->rta_len))) > + if (!(vfinfolist = nla_nest_start(nl_msg, IFLA_VFINFO_LIST))) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_INFO, > - NULL, 0); > - if (!rta || > - !(vfinfo = nlAppend(nlm, sizeof(nlmsgbuf), > - rtattbuf, rta->rta_len))) > + if (!(vfinfo = nla_nest_start(nl_msg, IFLA_VF_INFO))) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_MAC, > - &ifla_vf_mac, sizeof(ifla_vf_mac)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (!nla_put(nl_msg, IFLA_VF_MAC, sizeof(ifla_vf_mac), > + &ifla_vf_mac) < 0) > goto buffer_too_small; > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_VLAN, > - &ifla_vf_vlan, sizeof(ifla_vf_vlan)); > - > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (!nla_put(nl_msg, IFLA_VF_VLAN, sizeof(ifla_vf_vlan), > + &ifla_vf_vlan) < 0) > goto buffer_too_small; > > - vfinfo->rta_len = (char *)nlm + nlm->nlmsg_len - (char > *)vfinfo; > - > - vfinfolist->rta_len = (char *)nlm + nlm->nlmsg_len - > - (char *)vfinfolist; > + nla_nest_end(nl_msg, vfinfo); > + nla_nest_end(nl_msg, vfinfolist); > } > > if (vf == PORT_SELF_VF && nltarget_kernel) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_PORT_SELF, NULL, 0); > + if (!(vfport = nla_nest_start(nl_msg, IFLA_PORT_SELF))) > + goto buffer_too_small; > } else { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_VF_PORTS, NULL, 0); > - if (!rta || > - !(vfports = nlAppend(nlm, sizeof(nlmsgbuf), > - rtattbuf, rta->rta_len))) > + if (!(vfports = nla_nest_start(nl_msg, IFLA_VF_PORTS))) > goto buffer_too_small; > > /* begin nesting vfports */ > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORT, > NULL, 0); > + if (!(vfport = nla_nest_start(nl_msg, IFLA_VF_PORT))) > + goto buffer_too_small; > } > > - if (!rta || > - !(vfport = nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len))) > - goto buffer_too_small; > - > if (profileId) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_PORT_PROFILE, > - profileId, strlen(profileId) + 1); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_PROFILE, strlen(profileId) + 1, > + profileId) < 0) > goto buffer_too_small; > } > > if (portVsi) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_PORT_VSI_TYPE, > - portVsi, sizeof(*portVsi)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_VSI_TYPE, sizeof(*portVsi), > + portVsi) < 0) > goto buffer_too_small; > } > > if (instanceId) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_PORT_INSTANCE_UUID, > - instanceId, VIR_UUID_BUFLEN); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_INSTANCE_UUID, VIR_UUID_BUFLEN, > + instanceId) < 0) > goto buffer_too_small; > } > > if (hostUUID) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), > IFLA_PORT_HOST_UUID, > - hostUUID, VIR_UUID_BUFLEN); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_HOST_UUID, VIR_UUID_BUFLEN, > + hostUUID) < 0) > goto buffer_too_small; > } > > if (vf != PORT_SELF_VF) { > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_VF, > - &vf, sizeof(vf)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_VF, sizeof(vf), &vf) < 0) > goto buffer_too_small; > } > > - rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_REQUEST, > - &op, sizeof(op)); > - if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, > rta->rta_len)) > + if (nla_put(nl_msg, IFLA_PORT_REQUEST, sizeof(op), &op) < 0) > goto buffer_too_small; > > /* end nesting of vport */ > - vfport->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfport; > + nla_nest_end(nl_msg, vfport); > > if (vfports) { > /* end nesting of vfports */ > - vfports->rta_len = (char *)nlm + nlm->nlmsg_len - (char > *)vfports; > + nla_nest_end(nl_msg, vfports); > } > > if (!nltarget_kernel) { > pid = getLldpadPid(); > - if (pid == 0) > - return -1; > + if (pid == 0) { > + rc = -1; > + goto err_exit; > + } > } > > - if (nlComm(nlm, &recvbuf, &recvbuflen, pid) < 0) > - return -1; > + if (nlComm(nlmsg_hdr(nl_msg), &recvbuf, &recvbuflen, pid) < 0) { > + rc = -1; > + goto err_exit; > + } > > if (recvbuflen < NLMSG_LENGTH(0) || recvbuf == NULL) > goto malformed_resp; > @@ -1225,19 +1171,26 @@ doPortProfileOpSetLink(bool nltarget_ker > goto malformed_resp; > } > > +err_exit: > + nlmsg_free(nl_msg); > + > VIR_FREE(recvbuf); > > return rc; > > malformed_resp: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > _("malformed netlink response message")); > VIR_FREE(recvbuf); > return -1; > > buffer_too_small: > + nlmsg_free(nl_msg); > + > macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("internal buffer is too small")); > + _("allocated netlink buffer is too small")); > return -1; > } > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list