On Thu, Feb 20, 2020 at 11:19:25AM +0800, Lang Cheng wrote:
>
>
> On 2020/2/20 5:01, Jason Gunthorpe wrote:
> > On Tue, Feb 04, 2020 at 04:24:02PM +0800, Weihang Li wrote:
> > > From: Lang Cheng <chenglang@xxxxxxxxxx>
> > >
> > > Add attribute inactive to mark bonding backup port.
> > >
> > > Signed-off-by: Lang Cheng <chenglang@xxxxxxxxxx>
> > > drivers/infiniband/core/cache.c | 16 +++++++++++++++-
> > > include/rdma/ib_cache.h | 10 ++++++++++
> > > include/rdma/ib_verbs.h | 2 ++
> > > 3 files changed, 27 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> > > index d535995..7a7ef0e 100644
> > > +++ b/drivers/infiniband/core/cache.c
> > > @@ -1175,6 +1175,19 @@ int ib_get_cached_port_state(struct ib_device *device,
> > > }
> > > EXPORT_SYMBOL(ib_get_cached_port_state);
> > > +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num)
> > > +{
> > > + unsigned long flags;
> > > + u8 inactive;
> > > +
> > > + read_lock_irqsave(&device->cache.lock, flags);
> > > + inactive = device->port_data[port_num].cache.inactive;
> > > + read_unlock_irqrestore(&device->cache.lock, flags);
> > > +
> > > + return inactive;
> > > +}
> > > +EXPORT_SYMBOL(ib_get_cached_port_inactive_status);
> > > +
> > > /**
> > > * rdma_get_gid_attr - Returns GID attributes for a port of a device
> > > * at a requested gid_index, if a valid GID entry exists.
> > > @@ -1393,7 +1406,7 @@ static void ib_cache_update(struct ib_device *device,
> > > if (!rdma_is_port_valid(device, port))
> > > return;
> > > - tprops = kmalloc(sizeof *tprops, GFP_KERNEL);
> > > + tprops = kzalloc(sizeof *tprops, GFP_KERNEL);
> > > if (!tprops)
> > > return;
> > > @@ -1435,6 +1448,7 @@ static void ib_cache_update(struct ib_device *device,
> > > device->port_data[port].cache.pkey = pkey_cache;
> > > device->port_data[port].cache.lmc = tprops->lmc;
> > > device->port_data[port].cache.port_state = tprops->state;
> > > + device->port_data[port].cache.inactive = tprops->inactive;
> > > device->port_data[port].cache.subnet_prefix = tprops->subnet_prefix;
> > > write_unlock_irq(&device->cache.lock);
> > > diff --git a/include/rdma/ib_cache.h b/include/rdma/ib_cache.h
> > > index 870b5e6..63b2dd6 100644
> > > +++ b/include/rdma/ib_cache.h
> > > @@ -131,6 +131,16 @@ int ib_get_cached_port_state(struct ib_device *device,
> > > u8 port_num,
> > > enum ib_port_state *port_active);
> > > +/**
> > > + * ib_get_cached_port_inactive_status - Returns a cached port inactive status
> > > + * @device: The device to query.
> > > + * @port_num: The port number of the device to query.
> > > + *
> > > + * ib_get_cached_port_inactive_status() fetches the specified event inactive
> > > + * status stored in the local software cache.
> > > + */
> > > +u8 ib_get_cached_port_inactive_status(struct ib_device *device, u8 port_num);
> > > +
> >
> > kdocs belong with the implementation, not in the header file
>
> All of ib_get_cached_XXX() should remove rdma_is_port_valid() and move kdocs
> from ib_cache.h to cache.c.
> I can send this first, independently.
>
> >
> > > bool rdma_is_zero_gid(const union ib_gid *gid);
> > > const struct ib_gid_attr *rdma_get_gid_attr(struct ib_device *device,
> > > u8 port_num, int index);
> > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> > > index 5608e14..e17d846 100644
> > > +++ b/include/rdma/ib_verbs.h
> > > @@ -665,6 +665,7 @@ struct ib_port_attr {
> > > u8 active_speed;
> > > u8 phys_state;
> > > u16 port_cap_flags2;
> > > + u8 inactive;
> > > };
> >
> > Why is a major structure being changed for this?
> >
> > If LAG is to be brought up to the core code then the core should know
> > what leg is active or not, not the driver.
>
> "LAG is to be brought up to the core", is this under planning?
It is a development direction, sooner or later that will happen.
Thanks
