Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; --- src/util/virnetlink.c | 25 +++++++++++++++++++++++++ src/util/virnetlink.h | 1 + 2 files changed, 26 insertions(+) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index fdd3a6a4..e191f63b 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -41,6 +41,18 @@ VIR_LOG_INIT("util.netlink"); #define NETLINK_ACK_TIMEOUT_S (2*1000) #if defined(WITH_LIBNL) + +/* + * VETH_INFO_PEER is defined in libnl, but it isn't exposed. + * We include it just like what iproute2 has done.

+ */ +enum { + VETH_INFO_UNSPEC, + VETH_INFO_PEER, + + __VETH_INFO_MAX +}; + /* State for a single netlink event handle */ struct virNetlinkEventHandle { int watch; @@ -535,6 +547,19 @@ virNetlinkNewLink(const char *ifname, NETLINK_MSG_NEST_END(nl_msg, infodata); } + if (STREQ(type, "veth") && extra_args && extra_args->veth_peer) { + struct nlattr *infoveth = NULL; + + NETLINK_MSG_NEST_START(nl_msg, infodata, IFLA_INFO_DATA); + NETLINK_MSG_NEST_START(nl_msg, infoveth, VETH_INFO_PEER); + nlmsg_reserve(nl_msg, sizeof(struct ifinfomsg), 0); + NETLINK_MSG_PUT(nl_msg, IFLA_IFNAME, + (strlen(extra_args->veth_peer) + 1), + extra_args->veth_peer); + NETLINK_MSG_NEST_END(nl_msg, infoveth); + NETLINK_MSG_NEST_END(nl_msg, infodata); + } + NETLINK_MSG_NEST_END(nl_msg, linkinfo); if (extra_args) { diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 7121eac4..7c4ed202 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -84,6 +84,7 @@ struct _virNetlinkNewLinkData { const int *ifindex; /* The index for the 'link' device */ const virMacAddr *mac; /* The MAC address of the device */ const uint32_t *macvlan_mode; /* The mode of macvlan */ + const char *veth_peer; /* The peer name for veth */ }; int virNetlinkNewLink(const char *ifname,

On 11/17/20 1:48 AM, Shi Lei wrote: > When netlink is supported, use netlink to create veth device pair > rather than 'ip link' command. > > Signed-off-by: Shi Lei <shi_lei(a)massclouds.com&gt; > --- >   src/util/virnetdevveth.c | 39 ++++++++++++++++++++++++++++++--------- >   1 file changed, 30 insertions(+), 9 deletions(-) > > diff --git a/src/util/virnetdevveth.c b/src/util/virnetdevveth.c > index b3eee1af..996bf5dd 100644 > --- a/src/util/virnetdevveth.c > +++ b/src/util/virnetdevveth.c > @@ -27,6 +27,7 @@ >   #include "virfile.h" >   #include "virstring.h" >   #include "virnetdev.h" > +#include "virnetlink.h" >   >   #define VIR_FROM_THIS VIR_FROM_NONE >   > @@ -116,7 +117,6 @@ int virNetDevVethCreate(char** veth1, char** veth2) >       for (i = 0; i < MAX_VETH_RETRIES; i++) { >           g_autofree char *veth1auto = NULL; >           g_autofree char *veth2auto = NULL; > -        g_autoptr(virCommand) cmd = NULL; >   >           int status; >           if (!*veth1) { > @@ -136,15 +136,32 @@ int virNetDevVethCreate(char** veth1, char** veth2) >               vethNum = veth2num + 1; >           } >   > -        cmd = virCommandNew("ip"); > -        virCommandAddArgList(cmd, "link", "add", > -                             *veth1 ? *veth1 : veth1auto, > -                             "type", "veth", "peer", "name", > -                             *veth2 ? *veth2 : veth2auto, > -                             NULL); > +#if defined(WITH_LIBNL) > +        { > +            int error = 0; > +            virNetlinkNewLinkData data = { > +                .veth_peer = *veth2 ? *veth2 : veth2auto, > +            }; >   > -        if (virCommandRun(cmd, &status) < 0) > -            goto cleanup; > +            status = virNetlinkNewLink(*veth1 ? *veth1 : veth1auto, > +                                       "veth", &data, &error); > +            if (status < 0) > +                goto cleanup; > +        } > +#else > +        { > +            g_autoptr(virCommand) cmd = NULL; > +            cmd = virCommandNew("ip"); > +            virCommandAddArgList(cmd, "link", "add", > +                                 *veth1 ? *veth1 : veth1auto, > +                                 "type", "veth", "peer", "name", > +                                 *veth2 ? *veth2 : veth2auto, > +                                 NULL); > + > +            if (virCommandRun(cmd, &status) < 0) > +                goto cleanup; > +        } > +#endif /* WITH_LIBNL */ Although it isn't a hard/fast rule, we generally prefer to have complete functions within an #ifdefs rather then putting #ifdefs in the middle of a function. Could you put these bits into smaller functions, patterned like below, and then call the vir*Internal() functions from the existing functions? (other than that this looks fine, although I haven't actually tried *running* it yet :-))

#if defined(WITH_LIBNL) int virNetDevVethCreateInternal(char *veth1, char *veth2) {     int error;     virNetlinkNewLinkData data = { .veth_peer = veth2 };     return virNetlinkNewLink(veth1, "veth", &data, &error); } int virNetDevVethDeleteInternal(char *veth) {     return virNetlinkDelLink(veth, NULL); } #else int virNetDevVethCreateInternal(char *veth1, char *veth2) {     g_autoptr(virCommand) cmd = virCommandNew("ip");     virCommandAddArgList(cmd, "link", "add", veth1, veth2);     return virCommandRun(cmd, NULL); } int virNetDevVethDeleteInternal(char *veth) {     int status;     g_autoptr(virCommand) cmd = virCommandNewArgList("ip", "link", "del", veth, NULL);     if (virCommandRun(cmd, &status) < 0)         return -1;     if (status != 0) {         if (!virNetDevExists(veth)) {             VIR_DEBUG("Device %s already deleted (by kernel namespace cleanup)", veth);             return 0;         }         virReportError(VIR_ERR_INTERNAL_ERROR,                        _("Failed to delete veth device %s"), veth);         return -1;     }     return 0; } >   >           if (status == 0) { >               if (veth1auto) { > @@ -188,6 +205,9 @@ int virNetDevVethCreate(char** veth1, char** veth2) >    */ >   int virNetDevVethDelete(const char *veth) >   { > +#if defined(WITH_LIBNL) > +    return virNetlinkDelLink(veth, NULL); > +#else >       int status; >       g_autoptr(virCommand) cmd = virCommandNewArgList("ip", "link", >                                                          "del", veth, NULL); > @@ -206,4 +226,5 @@ int virNetDevVethDelete(const char *veth) >       } >   >       return 0; > +#endif /* WITH_LIBNL */ >   } (Aside from this, it looks like veth interface naming could benefit from the same simplifications that I did to tap and macvtap awhile back (basically changing the auto-generated names to never re-use a name - commit 95089f481 and commit d7f38beb2e). But that isn't something for this series, just another thing to add to the to-do list :-))