netdev - Re: [RFC v1 for accelerated IPoIB 04/25] IB/verb: Add ipoib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Mar 2017 14:01:36 -0600
From:   Jason Gunthorpe <jgunthorpe@...idianresearch.com>
To:     Erez Shitrit <erezsh@...lanox.com>
Cc:     dledford@...hat.com, linux-rdma@...r.kernel.org,
        netdev@...r.kernel.org, valex@...lanox.com, leonro@...lanox.com,
        saedm@...lanox.com, erezsh@....mellanox.co.il
Subject: Re: [RFC v1 for accelerated IPoIB 04/25] IB/verb: Add ipoib_options
 struct and API

On Mon, Mar 13, 2017 at 08:31:15PM +0200, Erez Shitrit wrote:

> 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