On Thu, May 21, 2015 at 11:05:32PM +0300, Or Gerlitz wrote:
> On Thu, May 21, 2015, Jason Gunthorpe wrote:
>
> >> +static int ipoib_set_vf_mac(struct net_device *dev, int queue, u8 *mac)
> >> +{
> >> + char *raw_guid;
> >> + u64 guid = 0;
> >
> > This doesn't seem right at all.
> >
> > It makes no sense that a IPoIB interface with a 20 byte LLADDR would
> > accept an 8 byte LLADDR only for 'ip link set vf mac'
> >
> > The definition of the netlink struct seems to confirm this:
> >
> > struct ifla_vf_mac {
> > __u32 vf;
> > __u8 mac[32]; /* MAX_ADDR_LEN */
> > };
> >
> > If it was really just ever a mac it would really only be 6 bytes.
> > [Honestly, this whole feature seems very inconistent with the rest of
> > the design of ip net link, so who knows]
> >
> > If the ifla_vf_mac had been variable-sized (like every other address
> > related attribute) then sure, auto detect the length and do the right
> > thing.
> >
> > But with this API, I think you have no choice, 'ip set vf mac LLADDR'
> > can only be the 20 byte address.
>
> You can't enforce 20 byte address on ipoib instance b/c the driver
> can't dictate the QPN of their UD QP.
I can think of several ways off the top of my head to deal with
that. Don't see it as a problem.
> Also, you don't need to force that, b/c what the virtualization
> system want to provision relates to a VM ID which is their mac (-->
> guid).
No, I do need to force that because that is how netlink works: each
device has a defined hw address length, and all netlink messages
dealing with LLADDR must use an identical format for the hw
address. For IPoIB that is 20 byte.
It is a simple matter of UAPI sanity, it is not the kernel's problem
that some userspace tools assume a 6 byte LLADDR for all devices -
they are broken.
> (mac), 64bits (guid) or even 128bits (whole gid), but it doesn't and I
> would like to either use the lowest common denominator (48bits) or
> just take always 64bits which could have two zero bytes (e.g when
> libvirt calls into that api through netlink).
Then get the iproute and netdev people to sign off on using
ifla_vf_mac with something other than the defined 20 byte IPoIB HW
address. Maybe you can work out some scheme with them. Making the
IFLA_VF_MAC properly variable sized might be a good angle.
NACK otherwise.
The rest of the series's idea seems sound.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
