Re: [PATCH for-next 3/4] RDMA/core: Add common iWARP query port

[Jason Gunthorpe <jgg@ziepe.ca>]

On Wed, Jul 31, 2019 at 02:15:02PM +0300, Kamal Heib wrote:
> Add support for a common iWARP query port function, the new function
> includes a common code that is used by the iWARP devices to update the
> port attributes like max_mtu, active_mtu, state, and phys_state, the
> function also includes a call for the driver-specific query_port callback
> to query the device-specific port attributes.
> 
> Signed-off-by: Kamal Heib <kamalheib1@gmail.com>
>  drivers/infiniband/core/device.c | 85 ++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 9773145dee09..8db632a35a30 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -1940,6 +1940,72 @@ void ib_dispatch_event(struct ib_event *event)
>  }
>  EXPORT_SYMBOL(ib_dispatch_event);
>  
> +static int __iw_query_port(struct ib_device *device,
> +			   u8 port_num,
> +			   struct ib_port_attr *port_attr)

Not sure the __ prefixes make sense here

> +{
> +	struct in_device *inetdev;
> +	struct net_device *netdev;
> +	int err;
> +
> +	memset(port_attr, 0, sizeof(*port_attr));
> +
> +	netdev = ib_device_get_netdev(device, port_num);
> +	if (!netdev)
> +		return -ENODEV;

missing dev_put(netdev)

> +	port_attr->max_mtu = IB_MTU_4096;
> +	port_attr->active_mtu = ib_mtu_int_to_enum(netdev->mtu);
> +
> +	if (!netif_carrier_ok(netdev)) {
> +		port_attr->state = IB_PORT_DOWN;
> +		port_attr->phys_state = IB_PORT_PHYS_STATE_DISABLED;
> +	} else {
> +		inetdev = in_dev_get(netdev);
> +
> +		if (inetdev && inetdev->ifa_list) {
> +			port_attr->state = IB_PORT_ACTIVE;
> +			port_attr->phys_state = IB_PORT_PHYS_STATE_LINK_UP;
> +			in_dev_put(inetdev);
> +		} else {
> +			port_attr->state = IB_PORT_INIT;
> +			port_attr->phys_state =
> +				IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING;
> +		}
> +	}
> +
> +	err = device->ops.query_port(device, port_num, port_attr);
> +	if (err)
> +		return err;
> +
> +	return 0;
> +}
> +
> +static int __ib_query_port(struct ib_device *device,
> +			   u8 port_num,
> +			   struct ib_port_attr *port_attr)
> +{
> +	union ib_gid gid;
> +	int err;
> +
> +	memset(port_attr, 0, sizeof(*port_attr));

gid = {}

> +
> +	err = device->ops.query_port(device, port_num, port_attr);
> +	if (err || port_attr->subnet_prefix)
> +		return err;
> +
> +	if (rdma_port_get_link_layer(device, port_num) !=
> +	    IB_LINK_LAYER_INFINIBAND)
> +		return 0;
> +
> +	err = device->ops.query_gid(device, port_num, 0, &gid);
> +	if (err)
> +		return err;
> +
> +	port_attr->subnet_prefix = be64_to_cpu(gid.global.subnet_prefix);
> +	return 0;
> +}
> +
>  /**
>   * ib_query_port - Query IB port attributes
>   * @device:Device to query
> @@ -1953,26 +2019,17 @@ int ib_query_port(struct ib_device *device,
>  		  u8 port_num,
>  		  struct ib_port_attr *port_attr)
>  {
> -	union ib_gid gid;
>  	int err;
>  
>  	if (!rdma_is_port_valid(device, port_num))
>  		return -EINVAL;
>  
> -	memset(port_attr, 0, sizeof(*port_attr));
> -	err = device->ops.query_port(device, port_num, port_attr);
> -	if (err || port_attr->subnet_prefix)
> -		return err;
> -
> -	if (rdma_port_get_link_layer(device, port_num) != IB_LINK_LAYER_INFINIBAND)
> -		return 0;
> -
> -	err = device->ops.query_gid(device, port_num, 0, &gid);
> -	if (err)
> -		return err;
> +	if (rdma_node_get_transport(device->node_type) == RDMA_TRANSPORT_IWARP)
> +		err = __iw_query_port(device, port_num, port_attr);
> +	else
> +		err = __ib_query_port(device, port_num, port_attr);

Just return, no need to have err

Jason