[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 Wed, 2010-05-26 at 15:45 +0200, Arnd Bergmann wrote:
> On Wednesday 26 May 2010, Stefan Berger wrote:
> 
> > +static int
> > +doPortProfileOpSetLink(bool nltarget_kernel,
> > +                       int ifindex,
> > +                       const char *profileId,
> > +                       struct ifla_port_vsi *portVsi,
> > +                       const unsigned char *instanceId,
> > +                       const unsigned char *hostUUID,
> > +                       int32_t vf,
> > +                       uint8_t op)
> > +{
> > +    int rc = 0;
> > +    char nlmsgbuf[NLMSGBUF_SIZE];
> > +    struct nlmsghdr *nlm = (struct nlmsghdr *)nlmsgbuf, *resp;
> > +    struct nlmsgerr *err;
> > +    char rtattbuf[RATTBUF_SIZE];
> > +    struct rtattr *rta, *vfports = NULL, *vfport;
> > +    struct ifinfomsg ifinfo = {
> > +        .ifi_family = AF_UNSPEC,
> > +        .ifi_index  = ifindex,
> > +    };
> > +    char *recvbuf = NULL;
> > +    unsigned int recvbuflen = 0;
> > +
> > +    memset(&nlmsgbuf, 0, sizeof(nlmsgbuf));
> > +
> > +    nlInit(nlm, NLM_F_REQUEST, RTM_SETLINK);
> > +
> > +    if (!nlAppend(nlm, sizeof(nlmsgbuf), &ifinfo, sizeof(ifinfo)))
> > +        goto buffer_too_small;
> > +
> > +    if (vf == PORT_SELF_VF) {
> > +        rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_SELF, NULL, 0);
> > +    } else {
> > +        rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORTS, NULL, 0);
> > +        if (!rta ||
> > +            !(vfports = nlAppend(nlm, sizeof(nlmsgbuf),
> > +                                 rtattbuf, rta->rta_len)))
> > +            goto buffer_too_small;
> > +
> > +        /* begin nesting vfports */
> > +        rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORT, NULL, 0);
> > +    }
> 
> 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.

> 
> 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.


> 
> > +    int op = PORT_REQUEST_ASSOCIATE;
> > +    struct ifla_port_vsi portVsi = {
> > +        .vsi_mgr_id       = virtPort->u.virtPort8021Qbg.managerID,
> > +        .vsi_type_version = virtPort->u.virtPort8021Qbg.typeIDVersion,
> > +    };
> > +    bool nltarget_kernel = false;
> 
> Right, "nltarget_kernel = false" is probably a reasonable setting for
> now, we can always extend this if we should need more options.
> 
> > +    int ifindex;
> > +
> > +    if (ifaceGetIndex(true, ifname, &ifindex) != 0) {
> > +        rc = 1;
> > +        goto err_exit;
> > +    }
> 
> 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.

> I guess we need to have access to both here, the underlying
> device to pass as ifindex, and the macvlan device so we
> can figure out the vlan and mac address settings. I did not
> follow the discussion on the libvir-list so far, maybe you
> have already discussed this in depth.
> 

My suggestion previously was to convey the top level device, i.e, the
name of the macvtap device, and let lldpad figure out the parameters in
the path towards the root physical device.

> > +    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?

   Stefan

> 
> 	Arnd



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