On Mon, Mar 13, 2017 at 08:31:15PM +0200, Erez Shitrit wrote:
quoted hunk
diff --git a/include/rdma/ib_ipoib_accel_ops.h b/include/rdma/ib_ipoib_accel_ops.h
new file mode 100644
index 000000000000..148a5529a559
+++ b/include/rdma/ib_ipoib_accel_ops.h
Both patches need a better naming scheme for this file..
rn_opa_vnic.h
rn_ipoib.h
Maybe?
+struct rdma_netdev {
+ void *clnt_priv;
+
+ /* control functions */
+ void (*set_id)(struct net_device *netdev, int id);
+ /* IB resource allocation function, returns new UD QP */
+ int (*ib_dev_init)(struct net_device *dev, struct ib_device *hca,
+ int *qp_num);
Why can't some combination of alloc_rdma_netdev and ndo.open do this stuff?
+ void (*ib_dev_cleanup)(struct net_device *dev, struct ib_device *hca);
Ditto
+ /* send packet */
+ void (*send)(struct net_device *dev, struct sk_buff *skb,
+ struct ipoib_ah *address, u32 dqpn, u32 dqkey);
+ /* multicast */
+ int (*attach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid, int set_qkey);
+ int (*detach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid);
It would make more sense to store the struct ib_device pointer in the
struct rdma_netdev.
Should 'lid' be 'mlid'?
+ int qp_num;
This one probably belongs in ipoib_rdma_netdev
+ void *context;
What is this? Why is something other than ipoib_priv or ipoib_dev_priv
needed?
struct ib_wq_attr *attr,
u32 wq_attr_mask,
struct ib_udata *udata);
+ struct ib_ipoib_accel_ops * (*get_ipoib_accel_ops)(struct ib_device *device);
rebase error? Not sure how this compiles
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
On Mon, Mar 13, 2017 at 02:01:36PM -0600, Jason Gunthorpe wrote:
quoted
+ /* multicast */
+ int (*attach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid, int set_qkey);
+ int (*detach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid);
It would make more sense to store the struct ib_device pointer in the
struct rdma_netdev.
Agree that it shouldn't be a function parameters.
For opa_vnic, I found it convenient to store ib_device pointer in client and
device private structures as those will be available in most places anyhow.
Niranjana
On Tue, Mar 14, 2017 at 9:01 AM, Vishwanathapura, Niranjana
[off-list ref] wrote:
On Mon, Mar 13, 2017 at 02:01:36PM -0600, Jason Gunthorpe wrote:
quoted
quoted
+ /* multicast */
+ int (*attach_mcast)(struct net_device *dev, struct ib_device
*hca,
+ union ib_gid *gid, u16 lid, int set_qkey);
+ int (*detach_mcast)(struct net_device *dev, struct ib_device
*hca,
+ union ib_gid *gid, u16 lid);
It would make more sense to store the struct ib_device pointer in the
struct rdma_netdev.
Agree that it shouldn't be a function parameters.
For opa_vnic, I found it convenient to store ib_device pointer in client and
device private structures as those will be available in most places anyhow.
Will add it to the rdma_netdev obj, as Jason suggested.
Thanks,
Niranjana
On Tue, Mar 14, 2017 at 12:01:09AM -0700, Vishwanathapura, Niranjana wrote:
On Mon, Mar 13, 2017 at 02:01:36PM -0600, Jason Gunthorpe wrote:
quoted
quoted
+ /* multicast */
+ int (*attach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid, int set_qkey);
+ int (*detach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid);
It would make more sense to store the struct ib_device pointer in the
struct rdma_netdev.
Agree that it shouldn't be a function parameters.
For opa_vnic, I found it convenient to store ib_device pointer in client and
device private structures as those will be available in most places anyhow.
If vnic uses it too, then lets add the ib_device and port num to
rdma_netdev itself?
Jason
On Mon, Mar 13, 2017 at 10:01 PM, Jason Gunthorpe
[off-list ref] wrote:
On Mon, Mar 13, 2017 at 08:31:15PM +0200, Erez Shitrit wrote:
quoted
diff --git a/include/rdma/ib_ipoib_accel_ops.h b/include/rdma/ib_ipoib_accel_ops.h
new file mode 100644
index 000000000000..148a5529a559
+++ b/include/rdma/ib_ipoib_accel_ops.h
Both patches need a better naming scheme for this file..
rn_opa_vnic.h
rn_ipoib.h
Maybe?
quoted
+struct rdma_netdev {
+ void *clnt_priv;
+
+ /* control functions */
+ void (*set_id)(struct net_device *netdev, int id);
quoted
+ /* IB resource allocation function, returns new UD QP */
+ int (*ib_dev_init)(struct net_device *dev, struct ib_device *hca,
+ int *qp_num);
Why can't some combination of alloc_rdma_netdev and ndo.open do this stuff?
quoted
+ void (*ib_dev_cleanup)(struct net_device *dev, struct ib_device *hca);
Ditto
quoted
+ /* send packet */
+ void (*send)(struct net_device *dev, struct sk_buff *skb,
+ struct ipoib_ah *address, u32 dqpn, u32 dqkey);
quoted
+ /* multicast */
+ int (*attach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid, int set_qkey);
+ int (*detach_mcast)(struct net_device *dev, struct ib_device *hca,
+ union ib_gid *gid, u16 lid);
It would make more sense to store the struct ib_device pointer in the
struct rdma_netdev.
Should 'lid' be 'mlid'?
quoted
+ int qp_num;
This one probably belongs in ipoib_rdma_netdev
quoted
+ void *context;
The QP as a part of the HW resources, it is created in the low-level
driver, and used by the upper ipoib for few reasons, (for example the
mac of the ipoib interface includes from the qp_num)
Now, if we want to use the ndo's init/uninit i need to store member
variables (qp_num and context) in the rdma_netdev, that will let me
use the ndos as is.
rdma_netdev is the one who belongs to both layers, ipoib and the low-level.
What is this? Why is something other than ipoib_priv or ipoib_dev_priv
needed?
quoted
struct ib_wq_attr *attr,
u32 wq_attr_mask,
struct ib_udata *udata);
+ struct ib_ipoib_accel_ops * (*get_ipoib_accel_ops)(struct ib_device *device);
rebase error? Not sure how this compiles
Jason
On Thu, Mar 16, 2017 at 05:17:55PM +0200, Erez Shitrit wrote:
The QP as a part of the HW resources, it is created in the low-level
driver, and used by the upper ipoib for few reasons, (for example the
mac of the ipoib interface includes from the qp_num)
Now, if we want to use the ndo's init/uninit i need to store member
variables (qp_num and context) in the rdma_netdev, that will let me
use the ndos as is.
rdma_netdev is the one who belongs to both layers, ipoib and the low-level.
IPOIB rdma_netdev's can be casted to ipoib_rdma_netdev, so it is where
ipoib specific cross-layer stuff should live
Jason