Re: [PATCH v7 5/5] rdma_rxe: use netlink messages to add/delete links

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 14, 2018 at 08:05:57AM -0800, Steve Wise wrote:
> Add support for the RDMA_NLDEV_CMD_NEWLINK/DELLINK messages which allow
> dynamically adding new RXE links.  Deprecate the old module options
> for now.
> 
> Cc: Moni Shoua <monis@xxxxxxxxxxxx>
> Reviewed-by: Yanjun Zhu <yanjun.zhu@xxxxxxxxxx>
> Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
>  drivers/infiniband/sw/rxe/rxe.c       | 39 +++++++++++++++++++++++++++++++++--
>  drivers/infiniband/sw/rxe/rxe.h       |  2 +-
>  drivers/infiniband/sw/rxe/rxe_net.c   |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_net.h   |  2 +-
>  drivers/infiniband/sw/rxe/rxe_param.h |  3 ++-
>  drivers/infiniband/sw/rxe/rxe_sysfs.c |  6 +++---
>  drivers/infiniband/sw/rxe/rxe_verbs.c |  4 ++--
>  drivers/infiniband/sw/rxe/rxe_verbs.h |  2 +-
>  8 files changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
> index 91ad339fd6e1..07fea762f0ea 100644
> +++ b/drivers/infiniband/sw/rxe/rxe.c
> @@ -31,6 +31,7 @@
>   * SOFTWARE.
>   */
>  
> +#include <rdma/rdma_netlink.h>
>  #include <net/addrconf.h>
>  #include "rxe.h"
>  #include "rxe_loc.h"
> @@ -301,7 +302,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
>  /* called by ifc layer to create new rxe device.
>   * The caller should allocate memory for rxe by calling ib_alloc_device.
>   */
> -int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
> +int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name)
>  {
>  	int err;
>  
> @@ -311,9 +312,39 @@ int rxe_add(struct rxe_dev *rxe, unsigned int mtu)
>  
>  	rxe_set_mtu(rxe, mtu);
>  
> -	return rxe_register_device(rxe);
> +	return rxe_register_device(rxe, ibdev_name);
>  }
>  
> +static int rxe_newlink(const char *ibdev_name, struct net_device *ndev)
> +{
> +	struct rxe_dev *rxe = rxe_get_dev_from_net(ndev);
> +	int err = 0;
> +
> +	if (rxe) {
> +		pr_err("already configured on %s\n", ndev->name);
> +		err = -EEXIST;
> +		ib_device_put(&rxe->ib_dev);
> +		goto err;
> +	}
> +
> +	rxe = rxe_net_add(ibdev_name, ndev);
> +	if (!rxe) {
> +		pr_err("failed to add %s\n", ndev->name);
> +		err = -EINVAL;
> +		goto err;
> +	}
> +
> +	rxe_set_port_state(ndev, rxe);

Hurm. So there is another pre-existing problem here..

ib_register_device creates a device and that immediately becomes
available to be unregistered - ie this can race with a netdev notifier
tearing down the netdev.

So, rxe_register_device/net_add can't return a pointer and we can't
call rxe_set_port_state like this.

rxe_set_port_state has to be done before registration or as part of
some kind of core code callback setting up the device, or we have to
do more stuff with refcounting.

Otherwise I think the two netlink patches are looking great now. It is
unfortunate Parav discovered my patch is all busted up. :(

Jason




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux