[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v8] add 802.1Qbh and 802.1Qbg handling



On Wednesday 26 May 2010, Stefan Berger wrote:
> On Wed, 2010-05-26 at 15:45 +0200, Arnd Bergmann wrote:

> > 
> > I think we need to distinguish three cases here, not two: PORT_SELF_VF
> > (configuring an enic static function), a specific VF (configuring an
> > enic dynamic function) and no VF at all (using lldpad). We cannot use
> > IFLA_PORT_SELF in lldpad because that would imply that we do the
> > communication on the macvtap link itself, which is not allowed to talk
> > VDP to the switch.
> > 
> > We also need a way to communicate the MAC and VLAN address to lldpad
> > in this case.
> 
> Why is that? If we tell lldpad the macvtap device name, it can walk the
> links backwards to the root physical device and determine the vlan id on
> the way. I think it is the most generic solution to let lldpad start
> from the 'top', no? It would do just the same as libvirt would do
> internally.

We should never need to tell lldpad the macvtap name, because lldpad
should not need to care about the implementation details of macvtap.
The problem is that the link may be on anything, including but not limited
to
- tap on a bridge
- raw socket on vhost-net
- macvlan
- macvtap
- sr-iov VF

Some of these are not visible to lldpad, or may not have been created
yet, or may already be in a different namespace, so it's impossible
for lldpad to cover this in general, only the macvtap/macvlan case
without namespaces would work reliably.

libvirt however does know both the mac address and the vlan id,
because it knows what it has created (e.g. macvlan) or configured
(e.g. sr-iov).

> > I would suggest leaving out the IFLA_PORT_VF attribute in that case,
> > and transmitting IFLA_VF_MAC and IFLA_VF_VLAN attributes with their
> > vf field set to zero or maybe (__u32)-1. lldpad would then choose
> > a vf number for its internal housekeeping which gets returned when
> > querying the device, so you have an identifier to refer to.
> 
> So, please modify my dummy server so I can test this.

patch attached.

> > What is ifname here, the macvtap interface or the underlying
> > interface that lldpad talks to?
> 
> It's the macvtap device and lldpad would determine what the underlying 
> physical device is.

Ok, so we need to fix that as well. In case of enic VFs, we also
need to pass the physical device, so the information should be there
somewhere in libvirt. Passing the macvtap device won't work because
of the reasons mentioned above.

> > > +    rc = doPortProfileOpCommon(nltarget_kernel, ifindex,
> > > +                               NULL,
> > > +                               &portVsi,
> > > +                               virtPort->u.virtPort8021Qbg.instanceID,
> > > +                               NULL,
> > > +                               PORT_SELF_VF,
> > > +                               op);
> > 
> > Maybe just pass vf=0 here?
> 
> What are the semantics of
> < 0: ?
> = 0: ?
> > 0: ?
> 
> in case of 802.1Qbg?

This is independent of Qbg but depends on whether we're talking to
the kernel or to a user application like lldpad. The VF number does
not have a meaning to lldpad, so we can define whatever we see fit
here.
My idea was to always pass VF=0 on a request in the IFLA_VF_VLAN
and IFLA_VF_MAC attributes, which unfortunately require putting
something in there. lldpad can reject anything that has a nonzero
number in there, to let us extend the protocol in the future.

	Arnd
diff --git a/hacks/Makefile b/hacks/Makefile
index ba07ec7..e524b64 100644
--- a/hacks/Makefile
+++ b/hacks/Makefile
@@ -2,4 +2,4 @@
 all : nlserver
 
 nlserver : netlinkserver.c
-	gcc -o $@ $^ -lnl
+	gcc -Wall -o $@ $^ -lnl
diff --git a/hacks/netlinkserver.c b/hacks/netlinkserver.c
index a7b9dce..44f130d 100755
--- a/hacks/netlinkserver.c
+++ b/hacks/netlinkserver.c
@@ -2,6 +2,8 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 #include <sys/socket.h>
 #include <linux/netlink.h>
@@ -11,12 +13,21 @@
 #include <netlink/msg.h>
 //#include <netlink/attr.h>
 
-#define USE_IFLA_PORT_SELF
+//#define USE_IFLA_PORT_SELF
 
 static struct nla_policy ifla_policy[IFLA_MAX + 1] =
 {
-  [IFLA_IFNAME]   = { .type = NLA_STRING },
-  [IFLA_VF_PORTS] = { .type = NLA_NESTED },
+  [IFLA_VFINFO_LIST]    = {. type = NLA_NESTED },
+  [IFLA_IFNAME]   	= { .type = NLA_STRING },
+  [IFLA_VF_PORTS]       = { .type = NLA_NESTED },
+};
+
+static struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] =
+{
+  [IFLA_VF_MAC]           = { .minlen = sizeof(struct ifla_vf_mac),
+                              .maxlen = sizeof(struct ifla_vf_mac) },
+  [IFLA_VF_VLAN]          = { .minlen = sizeof(struct ifla_vf_vlan),
+                              .maxlen = sizeof(struct ifla_vf_vlan) },
 };
 
 static struct nla_policy ifla_vf_ports_policy[IFLA_VF_PORT_MAX + 1] =
@@ -41,7 +52,10 @@ static void
 parseMsg(struct nlmsghdr *nlh) {
     struct nlattr *tb[IFLA_MAX+1],
                   *tb2[IFLA_VF_PORT_MAX + 1],
-                  *tb3[IFLA_PORT_MAX+1];
+                  *tb3[IFLA_PORT_MAX+1],
+		  *tb_vfinfo[IFLA_VF_MAX+1],
+		  *tb_vfinfo_list;
+    int rem;
 
     if (nlmsg_parse(nlh, sizeof(struct ifinfomsg),
                     (struct nlattr **)&tb, IFLA_MAX, ifla_policy)) {
@@ -55,6 +69,32 @@ parseMsg(struct nlmsghdr *nlh) {
         printf("interface index: %d\n", ifinfo->ifi_index);
     }
 #ifndef USE_IFLA_PORT_SELF
+    if (!tb[IFLA_VFINFO_LIST]) {
+	printf("IFLA_VFINFO_LIST missing.\n");
+	return;
+    }
+    nla_for_each_nested(tb_vfinfo_list, tb[IFLA_VFINFO_LIST], rem) {
+        if (nla_type(tb_vfinfo_list) != IFLA_VF_INFO) {
+	    printf("nested parsing of IFLA_VFINFO_LIST failed.\n");
+	    return;
+        }
+        if (nla_parse_nested(tb_vfinfo, IFLA_VF_MAX, tb_vfinfo_list,
+				ifla_vf_policy)) {
+	    printf("nested parsing of IFLA_VF_INFO failed.\n");
+	    return;
+        }
+    }
+
+    if (tb_vfinfo[IFLA_VF_MAC]) {
+	struct ifla_vf_mac *mac = RTA_DATA(tb_vfinfo[IFLA_VF_MAC]);
+	__u8 *m = mac->mac;
+	printf("IFLA_VF_MAC=%2x:%2x:%2x:%2x:%2x:%2x\n", m[0], m[1], m[2], m[3], m[4], m[5]);
+    }
+
+    if (tb_vfinfo[IFLA_VF_VLAN]) {
+	struct ifla_vf_vlan *vlan = RTA_DATA(tb_vfinfo[IFLA_VF_VLAN]);
+	printf("IFLA_VF_VLAN=%d\n", vlan->vlan);
+    }
     if (tb[IFLA_VF_PORTS]) {
         if (nla_parse_nested(tb2, IFLA_VF_PORT_MAX, tb[IFLA_VF_PORTS],
                              ifla_vf_ports_policy)) {
@@ -111,11 +151,11 @@ parseMsg(struct nlmsghdr *nlh) {
             }
             if (tb3[IFLA_PORT_REQUEST]) {
                 printf("IFLA_PORT_REQUEST=%d\n",
-                       *(unsigned long*)RTA_DATA(tb3[IFLA_PORT_REQUEST]));
+                       *(unsigned int*)RTA_DATA(tb3[IFLA_PORT_REQUEST]));
             }
             if (tb3[IFLA_PORT_RESPONSE]) {
                 printf("IFLA_PORT_RESPONSE=%d\n",
-                       *(unsigned long*)RTA_DATA(tb3[IFLA_PORT_RESPONSE]));
+                       *(unsigned int*)RTA_DATA(tb3[IFLA_PORT_RESPONSE]));
             }
         }
     }
@@ -192,9 +232,10 @@ err_exit:
 }
 
 
-void main() {
+int main()
+{
 	ssize_t r;
-	int fd, n;
+	int n;
 	#define MAX_PAYLOAD 1024  /* maximum payload size*/
 	struct sockaddr_nl src_addr, dest_addr;
 	struct nlmsghdr *nlh = NULL;
@@ -231,7 +272,7 @@ void main() {
 	while (1) {
 		n = recvmsg(sock_fd, &msg, 0);
 		printf(" Received message payload: %s\n",
-		       NLMSG_DATA(nlh));
+		       (char *)NLMSG_DATA(nlh));
 		printf(" Received a total of %d bytes\n",n);
 
 		fprintf(stderr,"recvd %d bytes in netlink message: \n",nlh->nlmsg_len); 
@@ -283,7 +324,7 @@ void main() {
 		if (r < 0) {
 		       fprintf(stderr,"Error sending on netlink socket : %s\n", strerror(errno));
 		} else {
-                       fprintf(stderr,"sent %d bytes!\n",r);
+                       fprintf(stderr,"sent %zd bytes!\n",r);
 		}
 
 		nlmsg_free(nl_msg);
@@ -291,4 +332,5 @@ void main() {
 	}
 	close(sock_fd);
 
+	return 0;
 }

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]