> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Thursday, August 9, 2018 5:27 PM
> To: Leon Romanovsky <leon@xxxxxxxxxx>
> Cc: Doug Ledford <dledford@xxxxxxxxxx>; Leon Romanovsky
> <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux-rdma@xxxxxxxxxxxxxxx>;
> Parav Pandit <parav@xxxxxxxxxxxx>
> Subject: Re: [PATCH rdma-next 3/7] IB/core: Delete lower netdevice default GID
> entries in bonding scenario
>
> On Mon, Aug 06, 2018 at 08:46:00AM +0300, Leon Romanovsky wrote:
> > From: Parav Pandit <parav@xxxxxxxxxxxx>
> >
> > When NETDEV_CHANGEUPPER event occurs, lower device is not yet
> > established as slave of the master, and when upper device is bond
> > device, default GID entries are fail to delete.
> > Due to this, when bond device is fully configured, default GID entries
> > of bond device cannot be added as default GID entries are occupied by
> > the lower netdevice. This is incorrect. Default GID entries should
> > really be of bond netdevice because in all RoCE GIDs (default or IP),
> > MAC address of the bond device will be used.
> > It is confusing to have default GID of netdevice which is not really
> > used for any purpose.
> >
> > Therefore, as first step, implement
> > (a) filter function which filters if a CHANGEUPPER event netdevice and
> > associated upper device is master device or not.
> > (b) callback function which deletes the default GIDs of lower (event
> > netdevice).
> >
> > Signed-off-by: Parav Pandit <parav@xxxxxxxxxxxx>
> > Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> > drivers/infiniband/core/roce_gid_mgmt.c | 69
> > ++++++++++++++++++++++++++++-----
> > 1 file changed, 60 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/roce_gid_mgmt.c
> > b/drivers/infiniband/core/roce_gid_mgmt.c
> > index c7cf3c7f38a6..6fcd7aeee059 100644
> > +++ b/drivers/infiniband/core/roce_gid_mgmt.c
> > @@ -208,6 +208,33 @@ static int upper_device_filter(struct ib_device
> *ib_dev, u8 port,
> > return res;
> > }
> >
> > +/**
> > + * is_upper_ndev_bond_master_filter - Check if a given netdevice
> > + * is bond master device of netdevice of the the RDMA device of port.
> > + * @ib_dev: IB device to check
> > + * @port: Port to consider for adding default GID
> > + * @rdma_ndev: Pointer to rdma netdevice
> > + * @cookie_ndev: Netdevice to consider to form a default GID
> > + *
> > + * is_upper_ndev_bond_master_filter() returns true if a cookie_netdev
> > + * is bond master device and rdma_ndev is its lower netdevice. It
> > +might
> > + * not have been established as slave device yet.
> > + */
> > +static int
> > +is_upper_ndev_bond_master_filter(struct ib_device *ib_dev, u8 port,
> > + struct net_device *rdma_ndev,
> > + void *cookie_ndev)
> > +{
> > + bool match = false;
> > +
> > + rcu_read_lock();
> > + if (netif_is_bond_master(cookie_ndev) &&
> > + rdma_is_upper_dev_rcu(rdma_ndev, cookie_ndev))
>
> Can we please not rely on implicit casts from void * in function parameters. This
> makes it hard to change code - if someday we rework
> rdma_is_upper_dev_rcu() the compiler no longer protects us against wrong
> argument types.
Sure, yes. Will send v1.
>
> Add
>
> struct net_device *ndev = cookie_ndev;
>
> And use that.
>
> Same comment across all these patches
>
> > +/**
> > + * del_default_gids - Delete default GIDs of the event/cookie netdevice
> > + * @ib_dev: RDMA device pointer
> > + * @port: Port of the RDMA device whose GID table to consider
> > + * @rdma_ndev: Unused rdma netdevice
> > + * @cookie: Pointer to event netdevice
> > + *
> > + * del_default_gids() deletes the default GIDs of the event/cookie netdevice.
> > + */
> > +static void del_default_gids(struct ib_device *ib_dev, u8 port,
> > + struct net_device *rdma_ndev, void *cookie) {
> > + unsigned long gid_type_mask;
> > +
> > + gid_type_mask = roce_gid_type_mask_support(ib_dev, port);
> > +
> > + ib_cache_gid_set_default_gid(ib_dev, port, cookie, gid_type_mask,
> > + IB_CACHE_GID_DEFAULT_MODE_DELETE);
>
> Here too
>
> Jason
