RE: [PATCH v3 1/4] RDMA/siw: Fabricate a GID on tun and loopback devices

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

 




> -----Original Message-----
> From: Chuck Lever <cel@xxxxxxxxxx>
> Sent: Wednesday, 14 June 2023 16:01
> To: jgg@xxxxxxxxxx
> Cc: Tom Talpey <tom@xxxxxxxxxx>; Chuck Lever <chuck.lever@xxxxxxxxxx>;
> linux-rdma@xxxxxxxxxxxxxxx; tom@xxxxxxxxxx; Bernard Metzler
> <BMT@xxxxxxxxxxxxxx>
> Subject: [EXTERNAL] [PATCH v3 1/4] RDMA/siw: Fabricate a GID on tun and
> loopback devices
> 
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
> 
> LOOPBACK and NONE (tunnel) devices have all-zero MAC addresses.
> Currently, siw_device_create() falls back to copying the IB device's
> name in those cases, because an all-zero MAC address breaks the RDMA
> core address resolution mechanism.
> 
> However, at the point when siw_device_create() constructs a GID, the
> ib_device::name field is uninitialized, leaving the MAC address to
> remain in an all-zero state.
> 
> Fabricate a random artificial GID for such devices, and ensure that
> artificial GID is returned for all device query operations.
> 
> Reported-by: Tom Talpey <tom@xxxxxxxxxx>
> Link: INVALID URI REMOVED
> 3A__lore.kernel.org_linux-
> 2Drdma_SA0PR15MB391986C07C4D41E107E79659994FA-
> 40SA0PR15MB3919.namprd15.prod.outlook.com_T_-
> 23t&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=2TaYXQ0T-
> r8ZO1PP1alNwU_QJcRRLfmYTAgd3QCvqSc&m=w7mQNFzAaTSOyd34VFHqKq
> LkkCgAazZP2N4AAIhvYSeGp1Yql6zFl5sRSI5me4ZP&s=h4oUDwpMECpIMGmo
> nuXTcpqpDRIvWBQWq32ALVPSQjk&e=
> Fixes: a2d36b02c15d ("RDMA/siw: Enable siw on tunnel devices")
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>  drivers/infiniband/sw/siw/siw.h       |    1 +
>  drivers/infiniband/sw/siw/siw_main.c  |   22 ++++++++--------------
>  drivers/infiniband/sw/siw/siw_verbs.c |    4 ++--
>  3 files changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw.h
> b/drivers/infiniband/sw/siw/siw.h
> index 2f3a9cda3850..8b4a710b82bc 100644
> --- a/drivers/infiniband/sw/siw/siw.h
> +++ b/drivers/infiniband/sw/siw/siw.h
> @@ -74,6 +74,7 @@ struct siw_device {
> 
>  	u32 vendor_part_id;
>  	int numa_node;
> +	char raw_gid[ETH_ALEN];
> 
>  	/* physical port state (only one port per device) */
>  	enum ib_port_state state;
> diff --git a/drivers/infiniband/sw/siw/siw_main.c
> b/drivers/infiniband/sw/siw/siw_main.c
> index 65b5cda5457b..f45600d169ae 100644
> --- a/drivers/infiniband/sw/siw/siw_main.c
> +++ b/drivers/infiniband/sw/siw/siw_main.c
> @@ -75,8 +75,7 @@ static int siw_device_register(struct siw_device *sdev,
> const char *name)
>  		return rv;
>  	}
> 
> -	siw_dbg(base_dev, "HWaddr=%pM\n", sdev->netdev->dev_addr);
> -
> +	siw_dbg(base_dev, "HWaddr=%pM\n", sdev->raw_gid);
>  	return 0;
>  }
> 
> @@ -313,24 +312,19 @@ static struct siw_device *siw_device_create(struct
> net_device *netdev)
>  		return NULL;
> 
>  	base_dev = &sdev->base_dev;
> -
>  	sdev->netdev = netdev;
> 
> -	if (netdev->type != ARPHRD_LOOPBACK && netdev->type !=
> ARPHRD_NONE) {
> -		addrconf_addr_eui48((unsigned char *)&base_dev-
> >node_guid,
> -				    netdev->dev_addr);
> +	if (netdev->addr_len) {
> +		memcpy(sdev->raw_gid, netdev->dev_addr,
> +		       min_t(unsigned int, netdev->addr_len, ETH_ALEN));
>  	} else {
>  		/*
> -		 * This device does not have a HW address,
> -		 * but connection mangagement lib expects gid != 0
> +		 * This device does not have a HW address, but
> +		 * connection mangagement requires a unique gid.
>  		 */
> -		size_t len = min_t(size_t, strlen(base_dev->name), 6);
> -		char addr[6] = { };
> -
> -		memcpy(addr, base_dev->name, len);
> -		addrconf_addr_eui48((unsigned char *)&base_dev-
> >node_guid,
> -				    addr);
> +		eth_random_addr(sdev->raw_gid);
>  	}
> +	addrconf_addr_eui48((u8 *)&base_dev->node_guid, sdev->raw_gid);
> 
>  	base_dev->uverbs_cmd_mask |=
> BIT_ULL(IB_USER_VERBS_CMD_POST_SEND);
> 
> diff --git a/drivers/infiniband/sw/siw/siw_verbs.c
> b/drivers/infiniband/sw/siw/siw_verbs.c
> index 398ec13db624..32b0befd25e2 100644
> --- a/drivers/infiniband/sw/siw/siw_verbs.c
> +++ b/drivers/infiniband/sw/siw/siw_verbs.c
> @@ -157,7 +157,7 @@ int siw_query_device(struct ib_device *base_dev,
> struct ib_device_attr *attr,
>  	attr->vendor_part_id = sdev->vendor_part_id;
> 
>  	addrconf_addr_eui48((u8 *)&attr->sys_image_guid,
> -			    sdev->netdev->dev_addr);
> +			    sdev->raw_gid);
> 
>  	return 0;
>  }
> @@ -218,7 +218,7 @@ int siw_query_gid(struct ib_device *base_dev, u32
> port, int idx,
> 
>  	/* subnet_prefix == interface_id == 0; */
>  	memset(gid, 0, sizeof(*gid));
> -	memcpy(&gid->raw[0], sdev->netdev->dev_addr, 6);
> +	memcpy(gid->raw, sdev->raw_gid, ETH_ALEN);
> 
>  	return 0;
>  }
> 

Thank you Chuck. Looks good to me!

Reviewed-by: Bernard Metzler <bmt@xxxxxxxxxxxxxx>




[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