> -----Original Message-----
> From: Jason Gunthorpe <jgg@xxxxxxxx>
> Sent: Wednesday, August 15, 2018 4:02 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 v1 0/7] RoCE LAG/Bonding GID table fixes
>
> On Tue, Aug 14, 2018 at 10:36:14AM +0300, Leon Romanovsky wrote:
> > From: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> >
> > Changelog v0->v1:
> > * Don't rely on explicit pointer casting from void to required by
> > functions where it is supplied - Patch 3 and 5.
> >
> > >From Parav,
> >
> > Currently RoCE LAG/Bonding has following issues.
> >
> > 1. When NETDEV_CHANGEUPPER occurs, netdevice_event_changeupper()
> > attempts to delete default GID entries of the upper device, but they
> > are actually added based on lower netdev, so bonding_default_del_cmd()
> > fails to delete such entries.
> >
> > 2. Due to that when master bond device is fully setup, its default GID
> > entries cannot be added because old entries exist. This is incorrect
> > and confusing; when using bonding/lag mode, always add the default GID
> > entries for the master bond device. So implement a filter, callback
> > function to detect the master-slave relationship in bonding mode and
> > delete the lower netdev's default GIDs and add master bond device's default
> GIDs.
> >
> > 3. When FAILOVER event occurs, it cannot delete the master bond
> > device's default GIDs they are of lower netdevice during NETDEV_UP event
> handling.
> > So always add default GIDs of master bond netdevice if bonding is used
> > for rdma device's netdevice.
> >
> > 4. When MAC address is changed, it only updates the default GID entries.
> > IP based GIDs are untouched, but it is incorrect. Some HCAs such as
> > mlx5 GIDs contains the MAC address as well at HCA hardware level.
> > Therefore update (delete/add) IP based GIDs as well.
>
> Applied to for-next, I fixed these compile warnings:
>
> drivers/infiniband/core/roce_gid_mgmt.c:266: warning: Function parameter or
> member 'cookie' not described in 'is_upper_ndev_bond_master_filter'
> drivers/infiniband/core/roce_gid_mgmt.c:266: warning: Excess function
> parameter 'cookie_ndev' description in 'is_upper_ndev_bond_master_filter'
> drivers/infiniband/core/roce_gid_mgmt.c:507: warning: Function parameter or
> member 'ib_dev' not described in 'rdma_roce_rescan_device'
> drivers/infiniband/core/roce_gid_mgmt.c:507: warning: Excess function
> parameter 'device' description in 'rdma_roce_rescan_device'
>
> and some other wonky things with this diff:
>
Thanks Jason,
> diff --git a/drivers/infiniband/core/roce_gid_mgmt.c
> b/drivers/infiniband/core/roce_gid_mgmt.c
> index 9ff2a72379bada..ee366199b169ca 100644
> --- a/drivers/infiniband/core/roce_gid_mgmt.c
> +++ b/drivers/infiniband/core/roce_gid_mgmt.c
> @@ -201,7 +201,7 @@ is_ndev_for_default_gid_filter(struct ib_device *ib_dev,
> u8 port,
> struct net_device *rdma_ndev, void *cookie) {
> struct net_device *cookie_ndev = cookie;
> - int res;
> + bool res;
>
> if (!rdma_ndev)
> return false;
> @@ -232,7 +232,7 @@ static bool pass_all_filter(struct ib_device *ib_dev, u8
> port, static bool upper_device_filter(struct ib_device *ib_dev, u8 port,
> struct net_device *rdma_ndev, void *cookie) {
> - int res;
> + bool res;
>
> if (!rdma_ndev)
> return false;
> @@ -253,7 +253,7 @@ static bool upper_device_filter(struct ib_device *ib_dev,
> u8 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
> + * @cookie: 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 @@ -
> 293,7 +293,7 @@ static void update_gid_ip(enum gid_op_type gid_op, static
> void bond_delete_netdev_default_gids(struct ib_device *ib_dev,
> u8 port,
> struct net_device *rdma_ndev,
> - void *event_ndev)
> + struct net_device *event_ndev)
> {
Thanks Jason. Yes, this should not be void. While correcting the order, somehow I made it void, which is incorrect.
> struct net_device *real_dev = rdma_vlan_dev_real_dev(event_ndev);
> unsigned long gid_type_mask;
>
> Jason
