Re: [PATCH RFC 0/3] Support standard SRIOV configuration for IB VFs

Or Gerlitz
2015-05-27T21:58:59Z

On Wed, May 27, 2015 at 8:53 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> On Wed, 2015-05-27 at 11:11 -0600, Jason Gunthorpe wrote:
> Well, let's just be clear: netlink/iproute2 screwed the pooch on their
> implementation of this stuff. Any solution that doesn't include fixing
> this up in some way is not really a good solution.

>> For instance iproute2 would need IB specific code to format
>> the 'ip link show' (review print_vfinfo in iproute2) and to length
>> check 'ip link set vf mac'

>> If we do use 8, then it would be ideal (and my strong preference) to
>> also fix the IFLA_VF_MAC message to have a working length. I think
>> that could be done compatibly with a bit of work. At least that way
>> iproute2 can be kept clean when it learns to do IB, and we could have
>> the option again of using 20 someday if we need.
>>

> Sounds like a reasonable plan.
> Or, this is your patch set, are you going to pick up these action items?

Let see

>> So to be clear, to go with the 8 byte option I suggest:

>> - Engage netdev/iproute and confirm they are philosophically OK
>> with IFLA_VF_MAC != IFLA_ADDRESS

the last thing netdev are is having philosophical views, they are very
practical (and non-perfect and happy and lively community), the one
thing before the last netdev are is caring for the rdma subsystem. If
something has to change @ their direct part, we should come with
patches.

>> - Make a kernel patch to properly size the IFLA_VF_MAC message

You mean the below structure which is expected by the kernel after
they see the IFLA_VF_MAC NL attribute

struct ifla_vf_mac {
__u32 vf;
__u8 mac[32]; /* MAX_ADDR_LEN */
};

How you thought to patch things such that the size of the address
provided by user-space will propagate into the kernel w.o breaking the
NL ABI here?

Why not just take the eight lower bytes and set them NL --> ipoib --> PF driver

>> - Make a iproute patch to use the IFLA_VF_MAC size in print_vfinfo
>> instead of hardcoded ETH_ALEN (using len == 32 mean len 6 for compat)

I was thinking to patch iproute to sense the link type: if eth print
six bytes, if ipoib print 8 bytes, simple.

>> - Drop in the IB patch

sounds good.