[lldp-devel] [PATCH 1/3] vdp22 filter interface support

John Fastabend john.r.fastabend at intel.com
Wed Nov 27 07:05:18 UTC 2013


On 11/26/2013 1:23 AM, Thomas-Mich Richter wrote:
> On 11/25/2013 10:55 PM, John Fastabend wrote:
>> On 11/21/2013 01:26 AM, Thomas Richter wrote:
>>> The VDP22 protocol allows the return of changed filter


[...]

>>> +enum {                    /* 802.1Qbg VDP ratified standard */
>>> +    IFLA_PORT_VSI_TYPE22 = IFLA_PORT_MAX,
>>> +    IFLA_PORT_VSI_FILTER,
>>> +    __IFLA_PORT_MAX_NEW
>>> +};
>>> +
>>
>> The problem with pushing the defines to the end like this is you get a
>> dependency between lldpad and the application pushing the message to
>> it. For example if you build both libvirt and lldpad at some point in
>> time then upgrade lldpad or libvirt they may no longer be in sync and
>> these messages will fail.
>>
>> We've been going down this path of using the kernel API because it
>> existed and we presumed at some point there would be a kernel consumer
>> for this. As it is not a single driver has implemented this API as best
>> I can tell.
>>
>> At this point I'm trying to decide how to handle this I really don't
>> like creating dependencies between separate applications like this.
>> Any thoughts/ideas? I see you submitted a kernel patch which would
>> I think would resolve this by making these APIs part of the kernel
>> kABI. Once Dave Miller opens the net-next merge window again maybe
>> we need to revisit that patch.
>>
>>
> John,
>
> I understand your concerns. You might have noticed:
> I tried to submit a patch to update if_link.h file to include the new
> structures and attributes IFLA_PORT_VSI_TYPE22 and IFLA_PORT_VSI_FILTER.
> Dave Miller turned it down, he does not accept a define without usage.
> I can follow him (a bit, after all its just a structure definition)
> and that is the reason why I did not push it further.
> IFLA_PORT_VSI_TYPE is not used either and he might decide to drop it
> from the kernel if_link.h and then libvirtd vs lldpad communication
> is broken.
>
> However as the code stands now, the only user of the new defines for the
> attributes IFLA_PORT_VSI_TYPE22 and IFLA_PORT_VSI_FILTER is my test
> program vdptest.c. Libvirtd is not updated to use VDP22 protocols (at the
> moment, that will come later).
> And both programs vdptest and lldpad will be built together, different
> builds of lldpad and libvirtd does not cause problems.
h> Libvirtd does not use the new
> IFLA_PORT_VSI_TYPE22 and IFLA_PORT_VSI_FILTER attributes. Libvirtd
> uses IFLA_VFINFO_LIST to transfer the mac/vlan information. (This design
> decision was not mine, it was done before I joint this team).
>
> My ideas:
> 1. Try my patch again. But I am unsure if we succeed. In fact Qbg is
>     not very active (dead might be a better word). And why should kernel
>     define something it does not use. This is a very strong point.
>     And we do not know any hardware which supports Qbg22 to actually know
>     if we have done it right.

Agreed this is a non-starter for me, really it just makes things worse.
Adding interfaces to the kernel without users is bad practice. Further
running the protocol in user space works just fine for most of the
cases.

> 2. I work on a command line interface for vdp22. It idea is to use the
>     lldptool to trigger commands to associate/de-associate VSIs. Once this
>     works the customers (libvirtd, vdptest) could use this interface defined
>     by lldpad. This would eliminate the external dependency on the kernel.

This seems the best path. If libvirtd, vdptest use this interface then
we don't need to use the kernel netlink interface at all. We can support
the draft version over netlink (at least until all users drop it) and
the standard version over the lldpad interface. Any idea how much work
this would be? My hope is not too much.

> 3. Use the current libvirtd scheme of IFLA_VFINFO_LIST and extend it with
>     a suitable structure to pass the 4 byte group id. For example (mis)use
>     attribute ifla_vf_tx_rate. BTW we have discussed this before.

Yeah I remember. Misusing fields like this also seems like a hack
though.

> 4. Or we could use the ifla_vf_mac, which contains space for 32 byte mac
>     address and use the remaining bytes to store the group id. This is a
>     real hack.

Yep, its a hack.

>
> I will work on the remaining findings and will submit my patch again.
> As said before, this gives me a chance to test it (in fact I have another
> big patch waiting for you: ~100 automated test cases for evb22, ecp22, vdp22).
> It should not break the currently released libvirtd/lldpad communication.
>

Great! I like test code. I should fix up the tests I have here for
various things and commit them as well.

Thanks,
John



More information about the lldp-devel mailing list