[PATCH for-next 00/10] IB SR-IOV support

[PATCH for-next 00/10] IB SR-IOV support

[Eli Cohen]

Hi Doug, Dave 

The following series adds support for managing SRIOV IB VFs in a standard 
way (rtnetlink, iproute2) through IPoIB ndo entries which translate to 
corresponding verbs calls. 

In IB networks, 64 bit GUIDs are used as the primary means of identification. 
To support that for VFs, we added a set_vf_guid ndo which is used to program 
the VF node and port GUID from the PF. 

Those verbs are implemented by the mlx5 driver along with some more changes 
needed in the driver, IPoIB and the IB core to support IB virtualization.

We've copied netdev only on the 1st patch of the series, as the rest of it just 
uses the patch along with the existing ndos in IPoIB plus add things which 
are internal to the IB stack. 

The series is rebased against 4.5-rc6 and Meny's patches:
http://www.spinics.net/lists/linux-rdma/msg33536.html and also assumes
Leon's patch that extends the kernel ib device attr caps field to u64.

Eli 

Eli Cohen (9):
  net/core: Add support for configuring VF GUIDs
  IB/mlx5: Fix decision on using MAD_IFC
  IB/core: Support accessing SA in virtualized environment
  IB/core: Add interfaces to control VF attributes
  IB/ipoib: Add ndo operations for configuring VFs
  net/mlx5_core: Add VF param when querying vport counter
  net/mlx5_core: Implement modify HCA vport command
  IB/mlx5: Implement callbacks for manipulating VFs
  IB/ipoib: Allow mcast packets from other VFs

Or Gerlitz (1):
  IB/core: Use GRH when the path hop-limit > 0

 drivers/infiniband/core/device.c                |   1 +
 drivers/infiniband/core/sa_query.c              |   7 +-
 drivers/infiniband/core/verbs.c                 |  40 +++++
 drivers/infiniband/hw/mlx5/Makefile             |   2 +-
 drivers/infiniband/hw/mlx5/ib_virt.c            | 194 ++++++++++++++++++++++++
 drivers/infiniband/hw/mlx5/mad.c                |   2 +-
 drivers/infiniband/hw/mlx5/main.c               |  12 +-
 drivers/infiniband/hw/mlx5/mlx5_ib.h            |   8 +
 drivers/infiniband/ulp/ipoib/ipoib_ib.c         |  29 +++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c       |  65 +++++++-
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c   |   6 +
 drivers/net/ethernet/mellanox/mlx5/core/vport.c |  72 ++++++++-
 include/linux/mlx5/driver.h                     |   5 +-
 include/linux/mlx5/mlx5_ifc.h                   |   6 +
 include/linux/mlx5/vport.h                      |   7 +-
 include/linux/netdevice.h                       |   3 +
 include/rdma/ib_verbs.h                         |  25 +++
 include/uapi/linux/if_link.h                    |   7 +
 net/core/rtnetlink.c                            |  79 +++++++++-
 19 files changed, 547 insertions(+), 23 deletions(-)
 create mode 100644 drivers/infiniband/hw/mlx5/ib_virt.c

-- 
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Eli Cohen]

Add two new NLAs to support configuration of Infiniband node or port
GUIDs. New applications can choose to use this interface to configure
GUIDs with iproute2 with commands such as:

ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70
ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78

For backwards compatibility, old applications which set the MAC of a VF
may set the VF's port GUID for an infiniband port also via set MAC. The
GUID will be generated from the 6-byte MAC per IETF RFC 7042. Note that
when using set MAC to set a port GUID, the node GUID is set as well (to
the port guid value).

A new ndo, ndo_sef_vf_guid is introduced to notify the net device of the
request to change the GUID.

Signed-off-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 include/linux/netdevice.h    |  3 ++
 include/uapi/linux/if_link.h |  7 ++++
 net/core/rtnetlink.c         | 79 ++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 83 insertions(+), 6 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5440b7b705eb..7b4ae218b90b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1147,6 +1147,9 @@ struct net_device_ops {
 						   struct nlattr *port[]);
 	int			(*ndo_get_vf_port)(struct net_device *dev,
 						   int vf, struct sk_buff *skb);
+	int			(*ndo_set_vf_guid)(struct net_device *dev,
+						   int vf, u64 guid,
+						   int guid_type);
 	int			(*ndo_set_vf_rss_query_en)(
 						   struct net_device *dev,
 						   int vf, bool setting);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index a30b78090594..1d01e8a4e5dd 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -556,6 +556,8 @@ enum {
 				 */
 	IFLA_VF_STATS,		/* network device statistics */
 	IFLA_VF_TRUST,		/* Trust VF */
+	IFLA_VF_IB_NODE_GUID,	/* VF Infiniband node GUID */
+	IFLA_VF_IB_PORT_GUID,	/* VF Infiniband port GUID */
 	__IFLA_VF_MAX,
 };
 
@@ -588,6 +590,11 @@ struct ifla_vf_spoofchk {
 	__u32 setting;
 };
 
+struct ifla_vf_guid {
+	__u32 vf;
+	__u64 guid;
+};
+
 enum {
 	IFLA_VF_LINK_STATE_AUTO,	/* link state of the uplink */
 	IFLA_VF_LINK_STATE_ENABLE,	/* link always up */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d735e854f916..9db6e5bde786 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1387,6 +1387,8 @@ static const struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
 	[IFLA_VF_RSS_QUERY_EN]	= { .len = sizeof(struct ifla_vf_rss_query_en) },
 	[IFLA_VF_STATS]		= { .type = NLA_NESTED },
 	[IFLA_VF_TRUST]		= { .len = sizeof(struct ifla_vf_trust) },
+	[IFLA_VF_IB_NODE_GUID]	= { .len = sizeof(struct ifla_vf_guid) },
+	[IFLA_VF_IB_PORT_GUID]	= { .len = sizeof(struct ifla_vf_guid) },
 };
 
 static const struct nla_policy ifla_vf_stats_policy[IFLA_VF_STATS_MAX + 1] = {
@@ -1534,6 +1536,58 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[])
 	return 0;
 }
 
+static int handle_infiniband_guid(struct net_device *dev, struct ifla_vf_guid *ivt,
+				  int guid_type)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	return ops->ndo_set_vf_guid(dev, ivt->vf, ivt->guid, guid_type);
+}
+
+static int handle_vf_guid(struct net_device *dev, struct ifla_vf_guid *ivt, int guid_type)
+{
+	if (dev->type != ARPHRD_INFINIBAND)
+		return -EOPNOTSUPP;
+
+	return handle_infiniband_guid(dev, ivt, guid_type);
+}
+
+static int handle_vf_mac(struct net_device *dev, struct ifla_vf_mac *ivm)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+	struct ifla_vf_guid ivt;
+	u8 *s = ivm->mac;
+	u8 d[8];
+	int err;
+
+	if (dev->type != ARPHRD_INFINIBAND) {
+		if (!ops->ndo_set_vf_mac)
+			return -EOPNOTSUPP;
+
+		return ops->ndo_set_vf_mac(dev, ivm->vf, ivm->mac);
+	}
+
+	if (!ops->ndo_set_vf_guid)
+		return -EOPNOTSUPP;
+
+	d[0] = s[0];
+	d[1] = s[1];
+	d[2] = s[2];
+	d[3] = 0xff;
+	d[4] = 0xfe;
+	d[5] = s[3];
+	d[6] = s[4];
+	d[7] = s[5];
+
+	ivt.vf = ivm->vf;
+	ivt.guid = be64_to_cpu(*(__be64 *)d);
+	err = handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_NODE_GUID);
+	if (err)
+		return err;
+
+	return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);
+}
+
 static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
@@ -1542,12 +1596,7 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
 	if (tb[IFLA_VF_MAC]) {
 		struct ifla_vf_mac *ivm = nla_data(tb[IFLA_VF_MAC]);
 
-		err = -EOPNOTSUPP;
-		if (ops->ndo_set_vf_mac)
-			err = ops->ndo_set_vf_mac(dev, ivm->vf,
-						  ivm->mac);
-		if (err < 0)
-			return err;
+		return handle_vf_mac(dev, ivm);
 	}
 
 	if (tb[IFLA_VF_VLAN]) {
@@ -1636,6 +1685,24 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr **tb)
 			return err;
 	}
 
+	if (tb[IFLA_VF_IB_NODE_GUID]) {
+		struct ifla_vf_guid *ivt = nla_data(tb[IFLA_VF_IB_NODE_GUID]);
+
+		if (!ops->ndo_set_vf_guid)
+			return -EOPNOTSUPP;
+
+		return handle_vf_guid(dev, ivt, IFLA_VF_IB_NODE_GUID);
+	}
+
+	if (tb[IFLA_VF_IB_PORT_GUID]) {
+		struct ifla_vf_guid *ivt = nla_data(tb[IFLA_VF_IB_PORT_GUID]);
+
+		if (!ops->ndo_set_vf_guid)
+			return -EOPNOTSUPP;
+
+		return handle_vf_guid(dev, ivt, IFLA_VF_IB_PORT_GUID);
+	}
+
 	return err;
 }
 
-- 
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Jason Gunthorpe]

On Tue, Mar 01, 2016 at 06:52:14PM +0200, Eli Cohen wrote:
> Add two new NLAs to support configuration of Infiniband node or port
> GUIDs. New applications can choose to use this interface to configure
> GUIDs with iproute2 with commands such as:
> 
> ip link set dev ib0 vf 0 node_guid 00:02:c9:03:00:21:6e:70
> ip link set dev ib0 vf 0 port_guid 00:02:c9:03:00:21:6e:78

I like this idea better than the last version..

> +static int handle_vf_mac(struct net_device *dev, struct ifla_vf_mac *ivm)
> +{
[..]
> +	return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);

But is this emulation really necessary? It seems dangerous and
continues the bad practice of assuming IFLA_VF_MAC is fixed to 6 bytes
in size, and is not just LLADDR bytes. I'd rather see mac sets fail on
IB.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Eli Cohen]

On Tue, Mar 01, 2016 at 10:37:51AM -0700, Jason Gunthorpe wrote:
> > +	return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);
> 
> But is this emulation really necessary? It seems dangerous and
> continues the bad practice of assuming IFLA_VF_MAC is fixed to 6 bytes
> in size, and is not just LLADDR bytes. I'd rather see mac sets fail on
> IB.
> 
struct ifla_vf_mac  already defines mac as 32 bytes but the idea here
is that applications that configure six byte Ethernet MACs to VFs will
continue to work without any change. 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Jason Gunthorpe]

On Tue, Mar 01, 2016 at 07:49:51PM +0200, Eli Cohen wrote:
> On Tue, Mar 01, 2016 at 10:37:51AM -0700, Jason Gunthorpe wrote:
> > > +	return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);
> > 
> > But is this emulation really necessary? It seems dangerous and
> > continues the bad practice of assuming IFLA_VF_MAC is fixed to 6 bytes
> > in size, and is not just LLADDR bytes. I'd rather see mac sets fail on
> > IB.
> > 
> struct ifla_vf_mac  already defines mac as 32 bytes but the idea here
> is that applications that configure six byte Ethernet MACs to VFs will
> continue to work without any change. 

In my view it is incorrect for an application to try and set a 6 byte
mac on an *infiniband* interface, the kernel should refuse to do it.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Or Gerlitz]

On Tue, Mar 1, 2016 at 8:25 PM, Jason Gunthorpe
<jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> On Tue, Mar 01, 2016 at 07:49:51PM +0200, Eli Cohen wrote:
>> On Tue, Mar 01, 2016 at 10:37:51AM -0700, Jason Gunthorpe wrote:
>> > > + return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);
>> >
>> > But is this emulation really necessary? It seems dangerous and
>> > continues the bad practice of assuming IFLA_VF_MAC is fixed to 6 bytes
>> > in size, and is not just LLADDR bytes. I'd rather see mac sets fail on
>> > IB.
>> >
>> struct ifla_vf_mac  already defines mac as 32 bytes but the idea here
>> is that applications that configure six byte Ethernet MACs to VFs will
>> continue to work without any change.
>
> In my view it is incorrect for an application to try and set a 6 byte
> mac on an *infiniband* interface, the kernel should refuse to do it.

As Eli wrote, there's a well defined way to extend MAC to GUID. With
that in hand, the idea here is to allow staged/evolved support for IB
Virtualization using un-touched provisioning systems which assign VMs
with 6-byte MACs along with the fully IB aware solution where the
upper level does provision IB GUIDs.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Doug Ledford]

[-- Attachment #1.1: Type: text/plain, Size: 1605 bytes --]

On 3/1/2016 4:08 PM, Or Gerlitz wrote:
> On Tue, Mar 1, 2016 at 8:25 PM, Jason Gunthorpe
> <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
>> On Tue, Mar 01, 2016 at 07:49:51PM +0200, Eli Cohen wrote:
>>> On Tue, Mar 01, 2016 at 10:37:51AM -0700, Jason Gunthorpe wrote:
>>>>> + return handle_infiniband_guid(dev, &ivt, IFLA_VF_IB_PORT_GUID);
>>>>
>>>> But is this emulation really necessary? It seems dangerous and
>>>> continues the bad practice of assuming IFLA_VF_MAC is fixed to 6 bytes
>>>> in size, and is not just LLADDR bytes. I'd rather see mac sets fail on
>>>> IB.
>>>>
>>> struct ifla_vf_mac  already defines mac as 32 bytes but the idea here
>>> is that applications that configure six byte Ethernet MACs to VFs will
>>> continue to work without any change.
>>
>> In my view it is incorrect for an application to try and set a 6 byte
>> mac on an *infiniband* interface, the kernel should refuse to do it.
> 
> As Eli wrote, there's a well defined way to extend MAC to GUID. With
> that in hand, the idea here is to allow staged/evolved support for IB
> Virtualization using un-touched provisioning systems which assign VMs
> with 6-byte MACs

Exactly *what* provisioning system tries to set the VF_MAC on an IPoIB
interface and expects it to set the GUID of an underlying IB device?

> along with the fully IB aware solution where the
> upper level does provision IB GUIDs.

There has never been upstream support for this MAC->GUID stuff you refer
to.  I'm not convinced we should add it now versus just doing things
right, period.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Or Gerlitz]

On Wed, Mar 2, 2016 at 6:50 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Exactly *what* provisioning system tries to set the VF_MAC on an IPoIB
> interface and expects it to set the GUID of an underlying IB device?

The provisioning system need not be fully aware in all their
components this is IB here, there's PCI linkage that tells these are
VFs of this PF and they have to be used for these VMs.

>> along with the fully IB aware solution where the
>> upper level does provision IB GUIDs.

> There has never been upstream support for this MAC->GUID stuff you refer
> to.  I'm not convinced we should add it now versus just doing things
> right, period.

We **are** doing things right with the new ndo.

Using the small MAC->GUID addition, people could be using non-modified
(or almost non-modified) provisioning systems that assign SRIOV VMs
with a MACs --- just use these patches on their hosts and get DHCP
server supplying IP addresses based on the derived GUIDs (this is
supported today).
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 3461 bytes --]

On 03/02/2016 01:40 PM, Or Gerlitz wrote:
> On Wed, Mar 2, 2016 at 6:50 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
>> Exactly *what* provisioning system tries to set the VF_MAC on an IPoIB
>> interface and expects it to set the GUID of an underlying IB device?
> 
> The provisioning system need not be fully aware in all their
> components this is IB here, there's PCI linkage that tells these are
> VFs of this PF and they have to be used for these VMs.

If I understand you correctly, then I don't think I agree.

From what I read, I gather you mean:

libvirt can be used to control guests today, and you can list a PCI
device as "managed" and specify a MAC address (which has libvirt
assuming the device is an ethernet device).  In that case, libvirt
automatically detaches the device from the host (if attached), figures
out if it's a PF or VF, sets the MAC address using either PF or VF MAC
setting methods in ethtool, attaches the device to the guest, then
starts the guest.  And you're saying we should put the MAC->GUID
transformation into this code for IB so that libvirt can be blissfully
ignorant and people can tell libvirt it's an ethernet device with a MAC
and libvirt will treat it as such and life will be grand.

Except it won't.  Along with setting the GUID, we also need to set the
P_Keys allowed list (at least using the alias GUIDs method of mlx4 you
do, so unless you add a patch to this series to switch mlx4 to this new
method, that's a valid concern).  And nothing in libvirt can do that as
long libvirt thinks this is ethernet because libvirt doesn't control the
vlans on a guest's ethernet device, the guest does.  In that sense, IB
and ethernet vary greatly.

So, at *best*, the solution you are suggesting for existing setups is a
partial solution that leaves things only half done.

I don't see the justification to clutter up upstream code with a
solution that isn't at least completely functional in its
implementation.  If the solution is only partial, then I would rather
leave it out and tell people to upgrade their libvirt to know about IB
devices.

This is actually further backed up, in my mind, by the fact that you can
have RoCE/iWARP Ethernet devices and regular Ethernet devices, and
libvirt needs to be taught the concept of an RDMA capable device,
whether Ethernet or IB, so that when trying to select a host for
migration it can make sure that the migration target has the same
capabilities as the hardware you are migrating from.  So, to me, doing
this right *requires* a libvirt upgrade, and there is no sense in this
middle ground GUID from MAC hack that you are suggesting.

>>> along with the fully IB aware solution where the
>>> upper level does provision IB GUIDs.
> 
>> There has never been upstream support for this MAC->GUID stuff you refer
>> to.  I'm not convinced we should add it now versus just doing things
>> right, period.
> 
> We **are** doing things right with the new ndo.
> 
> Using the small MAC->GUID addition, people could be using non-modified
> (or almost non-modified) provisioning systems that assign SRIOV VMs
> with a MACs --- just use these patches on their hosts and get DHCP
> server supplying IP addresses based on the derived GUIDs (this is
> supported today).
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Or Gerlitz]

On Fri, Mar 4, 2016 at 4:35 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

[...]
> So, at *best*, the solution you are suggesting for existing setups is a
> partial solution that leaves things only half done.
[...]

Doug,

Point/s taken, justifying the code re-route of libvirt attempting to
issue set_vf_mac on the PF to go through
set_vf_guid needs handling of more aspects. We will remove it from this patch.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 01/10] net/core: Add support for configuring VF GUIDs

[Eugene Syromiatnikov]

On Tue, Mar 01, 2016 at 06:52:14PM +0200, Eli Cohen wrote:
> +struct ifla_vf_guid {
> +	__u32 vf;
> +	__u64 guid;
> +};

This type definition differs in size on 64-bit and (most of) 32-bit
architectures, and it breaks 32-on-64-bit compat applications, as a result.