Re: [PATCH 2/2] iWARP Core Changes.

[Sean Hefty <mshefty@ichips.intel.com>]

Mainly nits...

Steve Wise wrote:
> -static int copy_addr(struct rdma_dev_addr *dev_addr, struct net_device *dev,
> +int copy_addr(struct rdma_dev_addr *dev_addr, struct net_device *dev,
>  		     unsigned char *dst_dev_addr)

Might want to rename this to something like rdma_copy_addr if you're going to 
export it.

> +static int cma_iw_handler(struct iw_cm_id *iw_id, struct iw_cm_event *iw_event)
> +{
> +	struct rdma_id_private *id_priv = iw_id->context;
> +	enum rdma_cm_event_type event = 0;
> +	struct sockaddr_in *sin;
> +	int ret = 0;
> +
> +	atomic_inc(&id_priv->dev_remove);
> +
> +	switch (iw_event->event) {
> +	case IW_CM_EVENT_CLOSE:
> +		event = RDMA_CM_EVENT_DISCONNECTED;
> +		break;
> +	case IW_CM_EVENT_CONNECT_REPLY:
> +		sin = (struct sockaddr_in*)&id_priv->id.route.addr.src_addr;
> +		*sin = iw_event->local_addr;
> +		sin = (struct sockaddr_in*)&id_priv->id.route.addr.dst_addr;

spacing nit - (struct sockaddr_in *) &id_priv->...

> +struct net_device *ip_dev_find(u32 ip);

Just include header file with definition.

> +	sin = (struct sockaddr_in*)&new_cm_id->route.addr.src_addr;
> +	*sin = iw_event->local_addr;
> +	sin = (struct sockaddr_in*)&new_cm_id->route.addr.dst_addr;

same spacing nit...  appears in a couple other places as well.

> +static inline union ib_gid* iw_addr_get_sgid(struct rdma_dev_addr* rda)
> +{
> +	return (union ib_gid*)rda->src_dev_addr;
> +}
> +
> +static inline union ib_gid* iw_addr_get_dgid(struct rdma_dev_addr* rda)
> +{
> +	return (union ib_gid*)rda->dst_dev_addr;
> +}

spacing nits

> +struct iw_cm_verbs;
>  struct ib_device {
>  	struct device                *dma_device;
>  
> @@ -846,6 +873,8 @@ struct ib_device {
>  
>  	u32                           flags;
>  
> +	struct iw_cm_verbs*           iwcm;
> +

'*' placement nit

- Sean
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/