On Tue, Apr 23, 2019 at 07:44:51PM +0300, Kamal Heib wrote:
> >> dealloc_driver() is something that introduced recently and I'm interested in
> >> backport this fix to RHEL versions that don't include the dealloc_driver(),
> >> that's why I'm targeting this fix to for-rc.
> >
> > Distro constraints are not really a reason to do something sub optimal
> > upstream...
>
> OK, What about -stable upstream versions?
Greg takes dependent patches generally if asked
> >>>> I think that we need to define a new interface in the core (iwcm.c)
> >>>> for the IWARP devices to use when {allocate, setting ops to,
> >>>> release} iwcm, what do you think?
> >>>
> >>> Why?
> >>>
> >>
> >> 1- Avoid bugs like this.
> >> 2- Reduce code duplication in the providers.
> >> 3- Make the providers code more cleaner.
> >>
> >> I'm suggesting to introduce the following two functions and modify the IWARP
> >> providers to use them.
> >>
> >> int iw_cm_register_device(struct ib_device *ibdev, char *ifname,
> >> const struct iw_cm_ops *ops)
> >> {
> >> ibdev->iw_cm_dev = kzalloc(sizeof(*ibdev->iw_cm_dev), GFP_KERNEL);
> >> if (!ibdev->iw_cm_dev)
> >> return -ENOMEM;
> >>
> >> ibdev->iw_cm_dev->ops = ops;
> >> memcpy(ibdev->iw_cm_dev->ifname, ifname,
> >> sizeof(ibdev->iw_cm_dev->ifname));
> >
> > Why not just have this as part of the normal register process and pass
> > the iw_cm_ops through the existing ops structure?
> >
>
> Do you mean to pass the ifname as a parameter to
> ib_register_device()
No, drivers can set it before calling register_device - not really
sure what it is for.
> and modify the ib_device_ops to include the following callbacks from
> iw_cm_verbs?
That or a pointer to a struct with them, yes.
> If that is the case then we can add the "ifname" & "driver_flags" to
> the ib_device struct and remove the iw_cm_verbs struct, which will
> mean that no need to implement the dealloc_driver() callback,
> because the iwcm pointer isn't allocated.
Yes, this makes more sense since there is really nothing in this
struct except ops function pointers which belong in the ops struct anyhow.
Jason
