Re: [PATCH rdma-next 5/5] IB: Query port through the core instead of directly calling the driver handler

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4179 bytes --]

On Tue, Jan 24, 2017 at 02:59:38PM -0600, Shiraz Saleem wrote:
> On Thu, Jan 19, 2017 at 07:41:02AM +0200, Leon Romanovsky wrote:
> > From: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> >
> > Change the drivers to call ib_query_port in their get port
> > immutable handler instead of their own query port handler.
> >
> > Doing this required to set the core cap flags of this device
> > before the ib_query_port call is made, since the IB core might
> > need these caps to serve the port query.
> >
> > Drivers are ensured by the IB core that the port attributes passed
> > to the port query verb implementation are zero, and hence we
> > removed the zeroing from the drivers.
> >
> > This patch doesn't add any new functionality.
> >
> > Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Reviewed-by: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > ---

<snip>

> > --- a/drivers/infiniband/hw/mlx4/alias_GUID.c
> > +++ b/drivers/infiniband/hw/mlx4/alias_GUID.c
> > @@ -499,6 +499,7 @@ static int set_guid_rec(struct ib_device *ibdev,
> >  	struct list_head *head =
> >  		&dev->sriov.alias_guid.ports_guid[port - 1].cb_list;
> >
> > +	memset(&attr, 0, sizeof(struct ib_port_attr));
> >  	err = __mlx4_ib_query_port(ibdev, port, &attr, 1);
>
> So why not call ib_query_port here as opposed to driver handler? As stated in your subject line.

This is call to mlx4 internal implementation of ib_query_port. This
implementation takes an additional parameter. In this case it is "1",
while call to ib_query_port will call to this function with "0".

>
> >  	if (err) {
> >  		pr_debug("mlx4_ib_query_port failed (err: %d), port: %d\n",
> > diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
> > index 7019b94..646dcbf 100644
> > --- a/drivers/infiniband/hw/mlx4/main.c
> > +++ b/drivers/infiniband/hw/mlx4/main.c
> > @@ -741,7 +741,7 @@ int __mlx4_ib_query_port(struct ib_device *ibdev, u8 port,
> >  {
> >  	int err;
> >
> > -	memset(props, 0, sizeof *props);
> > +	/* props being zeroed by the caller, avoid zeroing it here */
> >
> >  	err = mlx4_ib_port_link_layer(ibdev, port) == IB_LINK_LAYER_INFINIBAND ?
> >  		ib_link_query_port(ibdev, port, props, netw_view) :
> > @@ -1014,7 +1014,7 @@ static int mlx4_ib_modify_port(struct ib_device *ibdev, u8 port, int mask,
> >
> >  	mutex_lock(&mdev->cap_mask_mutex);
> >
> > -	err = mlx4_ib_query_port(ibdev, port, &attr);
> > +	err = ib_query_port(ibdev, port, &attr);
> >  	if (err)
> >  		goto out;
> >
> > @@ -2527,13 +2527,6 @@ static int mlx4_port_immutable(struct ib_device *ibdev, u8 port_num,
> >  	struct mlx4_ib_dev *mdev = to_mdev(ibdev);
> >  	int err;
> >
> > -	err = mlx4_ib_query_port(ibdev, port_num, &attr);
> > -	if (err)
> > -		return err;
> > -
> > -	immutable->pkey_tbl_len = attr.pkey_tbl_len;
> > -	immutable->gid_tbl_len = attr.gid_tbl_len;
> > -
> >  	if (mlx4_ib_port_link_layer(ibdev, port_num) == IB_LINK_LAYER_INFINIBAND) {
> >  		immutable->core_cap_flags = RDMA_CORE_PORT_IBA_IB;
> >  		immutable->max_mad_size = IB_MGMT_MAD_SIZE;
> > @@ -2549,6 +2542,13 @@ static int mlx4_port_immutable(struct ib_device *ibdev, u8 port_num,
> >  			immutable->max_mad_size = IB_MGMT_MAD_SIZE;
> >  	}
> >
> > +	err = ib_query_port(ibdev, port_num, &attr);
> > +	if (err)
> > +		return err;
> > +
> > +	immutable->pkey_tbl_len = attr.pkey_tbl_len;
> > +	immutable->gid_tbl_len = attr.gid_tbl_len;
> > +
> >  	return 0;
> >  }
> >
> > diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
> > index 69fb5ba..5835165 100644
> > --- a/drivers/infiniband/hw/mlx4/sysfs.c
> > +++ b/drivers/infiniband/hw/mlx4/sysfs.c
> > @@ -226,6 +226,7 @@ static int add_port_entries(struct mlx4_ib_dev *device, int port_num)
> >  	int ret = 0 ;
> >  	struct ib_port_attr attr;
> >
> > +	memset(&attr, 0, sizeof(struct ib_port_attr));
> >  	/* get the physical gid and pkey table sizes.*/
> >  	ret = __mlx4_ib_query_port(&device->ib_dev, port_num, &attr, 1);
>
> Ditto as previous comment.

The same answer as before.

>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]