On Wed, Jul 31, 2019 at 10:09:27AM -0300, Jason Gunthorpe wrote:
> 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@xxxxxxxxx>
> > 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
>
OK, I'll remove it in v2.
> > +{
> > + 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)
>
My bad, I'll fix it in v2.
> > + 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 = {}
>
I'll fix it in v2.
> > +
> > + 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
>
I'll fix it in v2.
> Jason
Thanks for your review.
