Enhanced mode for IPoIB driver

include/rdma/ib_verbs.h struct ib_port_immutable { · line 1897

Quoted 4 lines · Leon Romanovsky

struct ib_device {

include/rdma/ib_verbs.h +3 -0

+	/* Do not access @dma_device directly from ULP nor from HW drivers. */
+	struct device                *dma_device;
+

Why is this hunk in this patch? I thought Bart was taking care of this topic?

include/rdma/ib_verbs.h struct ib_port_immutable { · line 1897

Quoted 4 lines · Leon Romanovsky

struct ib_device {

include/rdma/ib_verbs.h +3 -0

+	/* Do not access @dma_device directly from ULP nor from HW drivers. */
+	struct device                *dma_device;
+

Why is this hunk in this patch? I thought Bart was taking care of this topic?

Jason

Hi Erez,

[auto build test ERROR on rdma/master]
[also build test ERROR on v4.11-rc5 next-20170404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

Quoted 5 lines · kbuild test robot

Hi Erez,

[auto build test ERROR on rdma/master]
[also build test ERROR on v4.11-rc5 next-20170404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

It should be applied to v4.11-rc3 and not to -rc5.

Quoted 7 lines · Leon Romanovsky

Quoted 5 lines · kbuild test robot

Hi Erez,

[auto build test ERROR on rdma/master]
[also build test ERROR on v4.11-rc5 next-20170404]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

Quoted 1 line

It should be applied to v4.11-rc3 and not to -rc5.

Hello Leon,

Will it be possible for Doug to apply this series to his tree without rebasing this patch series on rc5 instead of rc3?

Thanks,

Bart.

drivers/infiniband/ulp/ipoib/ipoib.h static inline void skb_add_pseudo_hdr(struct sk_buff *skb) · line 160

Quoted 8 lines · Leon Romanovsky

drivers/infiniband/ulp/ipoib/ipoib.h +6 -2

-/* Keep the refactoring compile able */
-#define ipoib_priv netdev_priv
+static inline void *ipoib_priv(const struct net_device *dev)
+{
+	struct rdma_netdev *rn = netdev_priv(dev);
+
+	return rn->clnt_priv;
+}

This should return struct ipoib_dev_priv *

drivers/infiniband/ulp/ipoib/ipoib_main.c void ipoib_setup(struct net_device *dev) · line 1994

Quoted 6 lines

static ssize_t show_pkey(struct device *dev,

drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -1

 			 struct device_attribute *attr, char *buf)
 {
-	struct ipoib_dev_priv *priv = netdev_priv(to_net_dev(dev));
+	struct ipoib_dev_priv *priv = ipoib_priv(to_net_dev(dev));

Several of these seem to be in the wrong patch

Jason

drivers/infiniband/ulp/ipoib/ipoib.h static inline void skb_add_pseudo_hdr(struct sk_buff *skb) · line 160

Quoted 8 lines · Leon Romanovsky

drivers/infiniband/ulp/ipoib/ipoib.h +6 -2

-/* Keep the refactoring compile able */
-#define ipoib_priv netdev_priv
+static inline void *ipoib_priv(const struct net_device *dev)
+{
+     struct rdma_netdev *rn = netdev_priv(dev);
+
+     return rn->clnt_priv;
+}

This should return struct ipoib_dev_priv *

drivers/infiniband/ulp/ipoib/ipoib_main.c void ipoib_setup(struct net_device *dev) · line 1994

Quoted 6 lines

static ssize_t show_pkey(struct device *dev,
struct device_attribute *attr, char *buf)
{

drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -1

-     struct ipoib_dev_priv *priv = netdev_priv(to_net_dev(dev));
+     struct ipoib_dev_priv *priv = ipoib_priv(to_net_dev(dev));

Several of these seem to be in the wrong patch

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

drivers/infiniband/ulp/ipoib/ipoib.h static inline void skb_add_pseudo_hdr(struct sk_buff *skb) · line 160

Quoted 8 lines · Leon Romanovsky

drivers/infiniband/ulp/ipoib/ipoib.h +6 -2

-/* Keep the refactoring compile able */
-#define ipoib_priv netdev_priv
+static inline void *ipoib_priv(const struct net_device *dev)
+{
+     struct rdma_netdev *rn = netdev_priv(dev);
+
+     return rn->clnt_priv;
+}

This should return struct ipoib_dev_priv *

drivers/infiniband/ulp/ipoib/ipoib_main.c void ipoib_setup(struct net_device *dev) · line 1994

Quoted 5 lines

static ssize_t show_pkey(struct device *dev,
struct device_attribute *attr, char *buf)
{

drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -1

-     struct ipoib_dev_priv *priv = netdev_priv(to_net_dev(dev));
+     struct ipoib_dev_priv *priv = ipoib_priv(to_net_dev(dev));

Several of these seem to be in the wrong patch

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

drivers/infiniband/ulp/ipoib/ipoib_main.c +7 -2

+static struct net_device *ipoib_create_netdev_default(struct ib_device *hca,
+						      const char *name,
+						      unsigned char name_assign_type,
+						      void (*setup)(struct net_device *))
 {
 	struct net_device *dev;
+	struct rdma_netdev *rn;
 
-	dev = alloc_netdev((int)sizeof(struct ipoib_dev_priv), name,
-			   NET_NAME_UNKNOWN, ipoib_setup);
+	dev = alloc_netdev((int)sizeof(struct rdma_netdev),
+			   name,

Quoted 16 lines · Leon Romanovsky

drivers/infiniband/ulp/ipoib/ipoib_main.c +8 -2

+static struct net_device *ipoib_create_netdev_default(struct ib_device *hca,
+                                                   const char *name,
+                                                   unsigned char name_assign_type,
+                                                   void (*setup)(struct net_device *))
 {
      struct net_device *dev;
+     struct rdma_netdev *rn;
 
-     dev = alloc_netdev((int)sizeof(struct ipoib_dev_priv), name,
-                        NET_NAME_UNKNOWN, ipoib_setup);
+     dev = alloc_netdev((int)sizeof(struct rdma_netdev),
+                        name,
+                        name_assign_type, setup);
      if (!dev)
              return NULL;

drivers/infiniband/ulp/ipoib/ipoib_main.c void ipoib_setup(struct net_device *dev) · line 1930

Quoted 6 lines

drivers/infiniband/ulp/ipoib/ipoib_main.c +5 -1

-     return netdev_priv(dev);
+     rn = netdev_priv(dev);
+
+     rn->send = ipoib_send;
+     rn->attach_mcast = ipoib_mcast_attach;
+     rn->detach_mcast = ipoib_mcast_detach;

This should really set rn->hca as well, don't forget the kref

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

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line. I hope that it doesn't really matter and it won't stop you from merging it.

Per-your request, I based this patch set on v4.11-rc3.

Quoted 7 lines · Leon Romanovsky

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

Quoted 6 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line. I hope that it doesn't really matter and it won't stop you from merging it.

Per-your request, I based this patch set on v4.11-rc3.

I didn't dig into the details of the ipoib part, but I think the core change and the overall look is now what I would expect. modulo the little comments

It would still have been nicer to isolate the 'default' implementation so those functions did not touch ipoib_priv, but that is also something that could be done in a followup..

Jason

Quoted 3 lines

It would still have been nicer to isolate the 'default' implementation so those functions did not touch ipoib_priv, but that is also something that could be done in a followup..

Thanks, Jason

Quoted 5 lines · Leon Romanovsky

Quoted 3 lines

It would still have been nicer to isolate the 'default' implementation so those functions did not touch ipoib_priv, but that is also something that could be done in a followup..

Thanks, Jason

I think before merging it would also be good to see the benchmark numbers - we all assume this is making things a lot faster..

Does it now exceed RC performance?

Jason

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of the prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++------
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

--
2.12.0

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

Quoted 5 lines

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

Quoted 8 lines

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

I'm confused by how you expect Niranjana's patch to be merged in this situation?

Doug, will you take care of the conflict between these 2 submitted patches?

[PATCH 02/11] IB/opa-vnic: Virtual Network Interface Controller (VNIC) interface And
[PATCH rdma-next 1/6] IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Ira

Quoted 89 lines

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of the prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++------
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

Quoted 7 lines

--
2.12.0

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

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

Quoted 7 lines

Quoted 7 lines

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB

support.

Quoted 8 lines

Quoted 6 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

I'm confused by how you expect Niranjana's patch to be merged in this

situation?

Quoted 6 lines

Doug, will you take care of the conflict between these 2 submitted patches?

[PATCH 02/11] IB/opa-vnic: Virtual Network Interface Controller (VNIC) interface And [PATCH rdma-next 1/6] IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Ira,

The IPoIB and HFI-VNIC are independent series, so topics should be self- contained. If the merge conflict worries you, Niranjana or me could easily base our patch sets on another topic, but we need to know the acceptance (yes/no) and order between them.

Thanks

Quoted 20 lines

Ira

Quoted 17 lines

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of

the

Quoted 59 lines

Quoted 59 lines

prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 6 lines

Quoted 6 lines

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++---

---

Quoted 18 lines

Quoted 7 lines

 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

Quoted 7 lines

--
2.12.0

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

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

Quoted 26 lines · Ira

Quoted 7 lines

Quoted 7 lines

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB

support.

Quoted 8 lines

Quoted 6 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

I'm confused by how you expect Niranjana's patch to be merged in this

situation?

Quoted 6 lines

Doug, will you take care of the conflict between these 2 submitted patches?

[PATCH 02/11] IB/opa-vnic: Virtual Network Interface Controller (VNIC) interface And [PATCH rdma-next 1/6] IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 6 lines

Ira,

The IPoIB and HFI-VNIC are independent series, so topics should be self- contained. If the merge conflict worries you, Niranjana or me could easily base our patch sets on another topic, but we need to know the acceptance (yes/no) and order between them.

I guess my expectation was that upon your agreement of this new interface, Doug would accept Niranjanas change and you could build on top of that rather than just taking part of Niranjanas patch and expecting Doug to see there are common hunks. Perhaps I'm missing something and git is smart enough to deal with it?

Does this mean you have reviewed and are comfortable with Niranjanas new rdma_netdev changes? If so it seems Doug could accept them.

Ira

Quoted 83 lines

Thanks

Quoted 20 lines

Ira

Quoted 17 lines

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of

the

Quoted 59 lines

Quoted 59 lines

prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 25 lines

Quoted 6 lines

Quoted 6 lines

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++---

---

Quoted 18 lines

Quoted 7 lines

 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

Quoted 7 lines

--
2.12.0

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

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

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

Quoted 26 lines · Ira

Quoted 26 lines · Ira

Quoted 7 lines

Quoted 7 lines

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB

support.

Quoted 8 lines

Quoted 6 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

I'm confused by how you expect Niranjana's patch to be merged in this

situation?

Quoted 6 lines

Doug, will you take care of the conflict between these 2 submitted patches?

[PATCH 02/11] IB/opa-vnic: Virtual Network Interface Controller (VNIC) interface And [PATCH rdma-next 1/6] IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 8 lines

Quoted 6 lines

Ira,

The IPoIB and HFI-VNIC are independent series, so topics should be self- contained. If the merge conflict worries you, Niranjana or me could easily base our patch sets on another topic, but we need to know the acceptance (yes/no) and order between them.

I guess my expectation was that upon your agreement of this new interface, Doug would accept Niranjanas change and you could build on top of that rather than just taking part of Niranjanas patch and expecting Doug to see there are common hunks. Perhaps I'm missing something and git is smart enough to deal with it?

It is not related to smart git. Parallel submissions should be independent, this is the why I'm submitting all our topics based on one common ground (usually -rcX), despite the fact that sometimes there is merge conflict between them.

As long Doug works in this mode, we are obligated to continue.

Quoted 2 lines

Does this mean you have reviewed and are comfortable with Niranjanas new rdma_netdev changes? If so it seems Doug could accept them.

We already discussed it in email and privately, I'm agree with you that IPoIB and HFI-VNIC are better to be separated, due to conceptually difference and future code maintenance. The proposed rdma_netdev suits well both patch sets, so I'm very comfortable with them.

However, I'm not comfortable with ULP location, for the code which is not interoperable between different systems (nodes). After OFA, I realized that it wouldn't work in Susan's LANL mixed cluster, which is pretty awful for the common ULP.

The code itself needs proprietary software (ethernet manager) to be operable and it placed in ULP based on some promise that future drivers will be added to hfi family. Right now, it is not the case.

You asked, I answered :)

Quoted 107 lines

Ira

Quoted 104 lines

Thanks

Quoted 20 lines

Ira

Quoted 17 lines

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of

the

Quoted 59 lines

Quoted 59 lines

prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 6 lines

Quoted 6 lines

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++---

---

Quoted 14 lines

Quoted 14 lines

 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

--
2.12.0

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

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

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

Quoted 40 lines · Leon Romanovsky

Quoted 26 lines · Ira

Quoted 26 lines · Ira

Quoted 7 lines

Quoted 7 lines

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB

support.

Quoted 8 lines

Quoted 6 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line.
I hope that it doesn't really matter and it won't stop you from merging it.

I'm confused by how you expect Niranjana's patch to be merged in this

situation?

Quoted 6 lines

Doug, will you take care of the conflict between these 2 submitted patches?

[PATCH 02/11] IB/opa-vnic: Virtual Network Interface Controller (VNIC) interface And [PATCH rdma-next 1/6] IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 8 lines

Quoted 6 lines

Ira,

The IPoIB and HFI-VNIC are independent series, so topics should be self- contained. If the merge conflict worries you, Niranjana or me could easily base our patch sets on another topic, but we need to know the acceptance (yes/no) and order between them.

I guess my expectation was that upon your agreement of this new interface, Doug would accept Niranjanas change and you could build on top of that rather than just taking part of Niranjanas patch and expecting Doug to see there are common hunks. Perhaps I'm missing something and git is smart enough to deal with it?

It is not related to smart git. Parallel submissions should be independent, this is the why I'm submitting all our topics based on one common ground (usually -rcX), despite the fact that sometimes there is merge conflict between them.

Quoted 9 lines

As long Doug works in this mode, we are obligated to continue.

Quoted 2 lines

Does this mean you have reviewed and are comfortable with Niranjanas new rdma_netdev changes? If so it seems Doug could accept them.

We already discussed it in email and privately, I'm agree with you that IPoIB and HFI-VNIC are better to be separated, due to conceptually difference and future code maintenance. The proposed rdma_netdev suits well both patch sets, so I'm very comfortable with them.

Thanks.

Quoted 10 lines

However, I'm not comfortable with ULP location, for the code which is not interoperable between different systems (nodes). After OFA, I realized that it wouldn't work in Susan's LANL mixed cluster, which is pretty awful for the common ULP.

The code itself needs proprietary software (ethernet manager) to be operable and it placed in ULP based on some promise that future drivers will be added to hfi family. Right now, it is not the case.

You asked, I answered :)

These have been already discussed out before.
That is not what Ira is asking at all.
We have been patiently waiting for a month and a half to ensure this IPoIB patches can fit will with the rdma netdev interface as Jason and Doug wanted.

Now that IPoIB is comfortable with the rdma netdev interface, I think OPA VNIC patch can move forward.

Niranjana

Quoted 115 lines

Quoted 115 lines

Ira

Quoted 83 lines

Thanks

Quoted 20 lines

Ira

Quoted 17 lines

Per-your request, I based this patch set on v4.11-rc3.

Thanks,
Leon

CC Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

CC Alex Vesker <valex-VPRAkNaXOzVWk0Htik3J/[email protected]>

---

The rest comes from Erez:

The IPoIB protocol encapsulates IP packets over Infiniband datagrams. As a direct RDMA Upper Layer Protocol (ULP), IPoIB cannot support HW features that are specific to the IP protocol stack.

Nevertheless, RDMA interfaces have been extended to support some of

the

Quoted 59 lines

Quoted 59 lines

prominent IP offload features, such as TCP/UDP checksum and TSO. This provided reasonable performance gain for IPoIB but is still insufficient to cope with the increasing network bandwidth demand.

However, New features are exisiting in common network interfaces that are very hard to implement in IPoIB interfaces while it uses the RDMA layer, examples include TSS and RSS, tunneling offloads, and XDP. Rather than continuously porting IP network interface developments into the RDMA stack, we propose adding an abstract network data-path interfaces to RDMA devices.

In order to present a consistent interface to users, the IPoIB ULP continues to represent the network device to the IP stack. The common code also manages the IPoIB control plane, such as resolving path queries and registering to multicast groups.
Data path operations are forwarded to devices that implement the new API, or fallback to the standard implementation otherwise. Using the forgoing approach, we show how IPoIB closes the performance gap compared to state-of-the-art Ethernet network interfaces.

The implementation idea is to use the api of alloc_rdma_netdev/free_rdma_netdev to expose a struct that has data members and set of functions that are used for IB network interfaces, like attach/detach multicast to qp, and send IB packet.

The functions are specific for IB operations and are not part of the common api the the netdev struct exposes via the ndo functions.
1. multicast handling - attach/detach
2. send operation - the ndo start_xmit has only 2 parameters and the IB send needs the destination qp and the ah object, there were few options to handle it via the netdev ndo, but they don't make more sense than using a specific send function (we are rdma_netdev after all)

The IPoIB code will be adapted to enable the option of accelerating the network interface, but the code will work as before if the HW below doesn't support the acceleration.
That means that in the default mode of ipoib I tried to keep it as much as it was before, not to force it to adopt the new api, where there is no code sharing between the ipoib and the vnic/hfi.
The default code uses the controll and the data, the accelerator uses only the control flows andstructors.
The changes of the default ipoib can be made on top of that series.

Each HW vendor can supply the acceleration for the IPoIB or to leave IPoIB to work as before.

Thanks,
Erez

Erez Shitrit (5):
IB/IPoIB: Separate control and data related initializations IB/IPoIB: Separate control from HW operation on ipoib_open/stop ndo IB/IPoIB: Rename qpn to be dqpn in ipoib_send and post_send functions IB/IPoIB: Formatting before adding accelerator to driver IB/IPoIB: Support acceleration options callbacks

Leon Romanovsky (1):
IB/IPoIB: Introduce RDMA netdev interface and IPoIB structs

Quoted 25 lines

Quoted 6 lines

Quoted 6 lines

 drivers/infiniband/ulp/ipoib/ipoib.h           |  40 +--
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  66 ++---
 drivers/infiniband/ulp/ipoib/ipoib_ethtool.c   |   6 +-
 drivers/infiniband/ulp/ipoib/ipoib_fs.c        |   4 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        | 339 +++++++++++++------------
 drivers/infiniband/ulp/ipoib/ipoib_main.c      | 312 +++++++++++++++++---

---

Quoted 18 lines

Quoted 7 lines

 drivers/infiniband/ulp/ipoib/ipoib_multicast.c |  39 +--
 drivers/infiniband/ulp/ipoib/ipoib_netlink.c   |  12 +-
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  64 ++---
 drivers/infiniband/ulp/ipoib/ipoib_vlan.c      |   9 +-
 include/rdma/ib_verbs.h                        |  41 +++
 11 files changed, 565 insertions(+), 367 deletions(-)

Quoted 7 lines

--
2.12.0

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

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

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

I just want to be sure that once any vendor in this susbsystem will use the same magic sentence "it gives best performance for our hardware and future devices will come". There will be no questions about interoperability between various devices, no more questions about standards, no more requests to implement for other drivers (hardware and software).

Quoted 6 lines · Leon Romanovsky

I just want to be sure that once any vendor in this susbsystem will use the same magic sentence "it gives best performance for our hardware and future devices will come". There will be no questions about interoperability between various devices, no more questions about standards, no more requests to implement for other drivers (hardware and software).

Don't be absurd..

I repeatedly chewed out intel for having their driver act like a subsystem in of itself, keeping user-facing code that is not linked to hardware out of driver is the right thing to do.

Jason

Yes, you did, however for many reasons hfi ended to be subsystem on its own, and now we are moving this subsystem one layer up. I really failed to understand your point of view, why hfi-vnic is not linked to specific hardware.

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line. I hope that it doesn't really matter and it won't stop you from merging it.

Quoted 7 lines · Leon Romanovsky

Hi Doug,

This patchset mostly comes from Erez with one exception in the first patch.

That patch origins from two different commits, first from Niranjana who added RDMA netdev interface and second from Erez who added IPoIB support.

Quoted 4 lines

During the preparation to submission, I squashed their commits into one and refactored code to allow submission as a standalone topic without creating dependecies between different submissions. This caused to change in author line. I hope that it doesn't really matter and it won't stop you from merging it.

I think that it does indeed matter. The patch was authored by Niranjana it should reflect as such. Why not take Niranjana's patch(s), the fact that you folded them in seems to mean you at least agree with the patch.

-Denny