On 5/26/19 9:41 AM, Yuval Shaia wrote:
On Fri, May 24, 2019 at 08:24:30AM +0300, Marcel Apfelbaum wrote:
Hi Yuval,

On 5/5/19 1:55 PM, Yuval Shaia wrote:
Any GID change in guest must be propogate to host. This is already done
by firing QMP event to managment system such as libvirt which in turn
will update the host with the relevant change.
Agreed, *any* management software can do that.

When qemu is executed on non-qmp framework (ex from command-line) we
need to update the host instead.
Fix it by adding support to update the RoCE device's Ethernet function
IP list from qemu via netlink.
I am not sure this is the right approach. I don't think QEMU should actively
change
the host network configuration.
As you pointed out yourself, the management software should make such
changes.
I know about few deployments that are not using any management software,
they fires their VMs right from command-line.

Currently those deployments cannot use pvrdma.

I agree you cannot always assume the QEMU instance is managed by libvirt,
what about adding this functionality to rdma-multiplexer? The multiplexer
may
register to the same QMP event.
Two reasons prevent us from doing this:
- rdmacm-mux is a specific MAD multiplexer for CM packets, lets do not add
   management function to it.
- rdmacm-mux might be redundant when MAD multiplexer will be implemented in
   kernel. So what then?

Even if you think the multiplexer is not the right way to do it, even a
simple bash script
disguised  as a systemd service can subscribe to the QMP event and make the
change on the host.

What do you think?
Another contrib app? A lightweight management software??

I would not call a simple bash script "management software".
You can even add a how-to in the pvrdma.txt doc
or add a script to scripts/qmp (I saw a fuse script there, maybe is ok to add another for pvrdma)
If I understand correctly we just need to listen to a qmp event
and issue a bash command, right?


See, i do not have an argument if qemu policy is not allowing qemu process
to do external configuration (ex network). I'm just looking from a narrow
perspective of easy deployment - people sometimes runs qemu without libvirt
(or any other management software for that matter), if they want to use
pvrdma they are forced to install libvirt just for that.


Adding Markus ad Paolo for their take on this. Would be OK if QEMU
would change the networking configuration of the host?


Thanks,
Marcel

Thanks,
Marcel

Signed-off-by: Yuval Shaia <yuval.sh...@oracle.com>
---
   configure              |  6 ++++
   hw/rdma/rdma_backend.c | 74 +++++++++++++++++++++++++++++++++++++++++-
   2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 5b183c2e39..1f707b1a62 100755
--- a/configure
+++ b/configure
@@ -3132,6 +3132,8 @@ fi
   cat > $TMPC <<EOF &&
   #include <sys/mman.h>
+#include <libmnl/libmnl.h>
+#include <linux/rtnetlink.h>
   int
   main(void)
@@ -3144,10 +3146,13 @@ main(void)
   }
   EOF
+pvrdma_libs="-lmnl"
+
   if test "$rdma" = "yes" ; then
       case "$pvrdma" in
       "")
           if compile_prog "" ""; then
+            libs_softmmu="$libs_softmmu $pvrdma_libs"
               pvrdma="yes"
           else
               pvrdma="no"
@@ -3156,6 +3161,7 @@ if test "$rdma" = "yes" ; then
       "yes")
           if ! compile_prog "" ""; then
               error_exit "PVRDMA is not supported since mremap is not 
implemented"
+                        " or libmnl-devel is not installed"
           fi
           pvrdma="yes"
           ;;
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 05f6b03221..bc57b1a624 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -16,6 +16,11 @@
   #include "qemu/osdep.h"
   #include "qapi/qapi-events-rdma.h"
+#include "linux/if_addr.h"
+#include "libmnl/libmnl.h"
+#include "linux/rtnetlink.h"
+#include "net/if.h"
+
   #include <infiniband/verbs.h>
   #include "contrib/rdmacm-mux/rdmacm-mux.h"
@@ -47,6 +52,61 @@ static void dummy_comp_handler(void *ctx, struct ibv_wc *wc)
       rdma_error_report("No completion handler is registered");
   }
+static int netlink_route_update(const char *ifname, union ibv_gid *gid,
+                                __u16 type)
+{
+    char buf[MNL_SOCKET_BUFFER_SIZE];
+    struct nlmsghdr *nlh;
+    struct ifaddrmsg *ifm;
+    struct mnl_socket *nl;
+    int ret;
+    uint32_t ipv4;
+
+    nl = mnl_socket_open(NETLINK_ROUTE);
+    if (!nl) {
+        rdma_error_report("Fail to connect to netlink\n");
+        return -EIO;
+    }
+
+    ret = mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID);
+    if (ret < 0) {
+        rdma_error_report("Fail to bind to netlink\n");
+        goto out;
+    }
+
+    nlh = mnl_nlmsg_put_header(buf);
+    nlh->nlmsg_type = type;
+    nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL;
+    nlh->nlmsg_seq = 1;
+
+    ifm = mnl_nlmsg_put_extra_header(nlh, sizeof(*ifm));
+    ifm->ifa_index = if_nametoindex(ifname);
+    if (gid->global.subnet_prefix) {
+        ifm->ifa_family = AF_INET6;
+        ifm->ifa_prefixlen = 64;
+        ifm->ifa_flags = IFA_F_PERMANENT;
+        ifm->ifa_scope = RT_SCOPE_UNIVERSE;
+        mnl_attr_put(nlh, IFA_ADDRESS, sizeof(*gid), gid);
+    } else {
+        ifm->ifa_family = AF_INET;
+        ifm->ifa_prefixlen = 24;
+        memcpy(&ipv4, (char *)&gid->global.interface_id + 4, sizeof(ipv4));
+        mnl_attr_put(nlh, IFA_LOCAL, 4, &ipv4);
+    }
+
+    ret = mnl_socket_sendto(nl, nlh, nlh->nlmsg_len);
+    if (ret < 0) {
+        rdma_error_report("Fail to send msg to to netlink\n");
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    mnl_socket_close(nl);
+    return ret;
+}
+
   static inline void complete_work(enum ibv_wc_status status, uint32_t 
vendor_err,
                                    void *ctx)
   {
@@ -1123,7 +1183,13 @@ int rdma_backend_add_gid(RdmaBackendDev *backend_dev, 
const char *ifname,
                                               gid->global.subnet_prefix,
                                               gid->global.interface_id);
-    return ret;
+    /*
+     * We ignore return value since operation might completed sucessfully
+     * by the QMP consumer
+     */
+    netlink_route_update(ifname, gid, RTM_NEWADDR);
+
+    return 0;
   }
   int rdma_backend_del_gid(RdmaBackendDev *backend_dev, const char *ifname,
@@ -1149,6 +1215,12 @@ int rdma_backend_del_gid(RdmaBackendDev *backend_dev, 
const char *ifname,
                                               gid->global.subnet_prefix,
                                               gid->global.interface_id);
+    /*
+     * We ignore return value since operation might completed sucessfully
+     * by the QMP consumer
+     */
+    netlink_route_update(ifname, gid, RTM_DELADDR);
+
       return 0;
   }


Reply via email to