Message-ID: <e17ecc0c-a360-a900-7e19-382006af0a9a@huawei.com>
Date:   Mon, 7 Nov 2016 18:04:14 +0800
From:   Shaobo Xu <xushaobo2@...wei.com>
To:     Anurup M <anurup.m@...wei.com>,
        Salil Mehta <salil.mehta@...wei.com>, <dledford@...hat.com>
CC:     <linux-rdma@...r.kernel.org>, <netdev@...r.kernel.org>,
        <mehta.salil.lnk@...il.com>, <linux-kernel@...r.kernel.org>,
        <linuxarm@...wei.com>
Subject: Re: [PATCH for-next 10/11] IB/hns: Implement the add_gid/del_gid and
 optimize the GIDs management



On 2016/11/7 16:17, Anurup M wrote:
> 
> 
> On 11/4/2016 10:06 PM, Salil Mehta wrote:
>> From: Shaobo Xu <xushaobo2@...wei.com>
>>
>> IB core has implemented the calculation of GIDs and the management
>> of GID tables, and it is now responsible to supply query function
>> for GIDs. So the calculation of GIDs and the management of GID
>> tables in the RoCE driver is redundant.
>>
>> The patch is to implement the add_gid/del_gid to set the GIDs in
>> the RoCE driver, remove the redundant calculation and management of
>> GIDs in the notifier call of the net device and the inet, and
>> update the query_gid.
>>
>> Signed-off-by: Shaobo Xu <xushaobo2@...wei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@...wei.com>
>> Signed-off-by: Salil Mehta  <salil.mehta@...wei.com>
>> ---
>>  drivers/infiniband/hw/hns/hns_roce_device.h |    2 -
>>  drivers/infiniband/hw/hns/hns_roce_main.c   |  270 +++++----------------------
>>  2 files changed, 48 insertions(+), 224 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
>> index 593a42a..9ef1cc3 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_device.h
>> +++ b/drivers/infiniband/hw/hns/hns_roce_device.h
>> @@ -429,8 +429,6 @@ struct hns_roce_ib_iboe {
>>  	struct net_device      *netdevs[HNS_ROCE_MAX_PORTS];
>>  	struct notifier_block	nb;
>>  	struct notifier_block	nb_inet;
>> -	/* 16 GID is shared by 6 port in v1 engine. */
>> -	union ib_gid		gid_table[HNS_ROCE_MAX_GID_NUM];
>>  	u8			phy_port[HNS_ROCE_MAX_PORTS];
>>  };
>>  
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
>> index 6770171..795ef97 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_main.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_main.c
>> @@ -35,52 +35,13 @@
>>  #include <rdma/ib_addr.h>
>>  #include <rdma/ib_smi.h>
>>  #include <rdma/ib_user_verbs.h>
>> +#include <rdma/ib_cache.h>
>>  #include "hns_roce_common.h"
>>  #include "hns_roce_device.h"
>>  #include "hns_roce_user.h"
>>  #include "hns_roce_hem.h"
>>  
>>  /**
>> - * hns_roce_addrconf_ifid_eui48 - Get default gid.
>> - * @eui: eui.
>> - * @vlan_id:  gid
>> - * @dev:  net device
>> - * Description:
>> - *    MAC convert to GID
>> - *        gid[0..7] = fe80 0000 0000 0000
>> - *        gid[8] = mac[0] ^ 2
>> - *        gid[9] = mac[1]
>> - *        gid[10] = mac[2]
>> - *        gid[11] = ff        (VLAN ID high byte (4 MS bits))
>> - *        gid[12] = fe        (VLAN ID low byte)
>> - *        gid[13] = mac[3]
>> - *        gid[14] = mac[4]
>> - *        gid[15] = mac[5]
>> - */
>> -static void hns_roce_addrconf_ifid_eui48(u8 *eui, u16 vlan_id,
>> -					 struct net_device *dev)
>> -{
>> -	memcpy(eui, dev->dev_addr, 3);
>> -	memcpy(eui + 5, dev->dev_addr + 3, 3);
>> -	if (vlan_id < 0x1000) {
>> -		eui[3] = vlan_id >> 8;
>> -		eui[4] = vlan_id & 0xff;
>> -	} else {
>> -		eui[3] = 0xff;
>> -		eui[4] = 0xfe;
>> -	}
>> -	eui[0] ^= 2;
>> -}
>> -
>> -static void hns_roce_make_default_gid(struct net_device *dev, union ib_gid *gid)
>> -{
>> -	memset(gid, 0, sizeof(*gid));
>> -	gid->raw[0] = 0xFE;
>> -	gid->raw[1] = 0x80;
>> -	hns_roce_addrconf_ifid_eui48(&gid->raw[8], 0xffff, dev);
>> -}
>> -
>> -/**
>>   * hns_get_gid_index - Get gid index.
>>   * @hr_dev: pointer to structure hns_roce_dev.
>>   * @port:  port, value range: 0 ~ MAX
>> @@ -96,30 +57,6 @@ int hns_get_gid_index(struct hns_roce_dev *hr_dev, u8 port, int gid_index)
>>  	return gid_index * hr_dev->caps.num_ports + port;
>>  }
>>  
>> -static int hns_roce_set_gid(struct hns_roce_dev *hr_dev, u8 port, int gid_index,
>> -		     union ib_gid *gid)
>> -{
>> -	struct device *dev = &hr_dev->pdev->dev;
>> -	u8 gid_idx = 0;
>> -
>> -	if (gid_index >= hr_dev->caps.gid_table_len[port]) {
>> -		dev_err(dev, "gid_index %d illegal, port %d gid range: 0~%d\n",
>> -			gid_index, port, hr_dev->caps.gid_table_len[port] - 1);
>> -		return -EINVAL;
>> -	}
>> -
>> -	gid_idx = hns_get_gid_index(hr_dev, port, gid_index);
>> -
>> -	if (!memcmp(gid, &hr_dev->iboe.gid_table[gid_idx], sizeof(*gid)))
>> -		return -EINVAL;
>> -
>> -	memcpy(&hr_dev->iboe.gid_table[gid_idx], gid, sizeof(*gid));
>> -
>> -	hr_dev->hw->set_gid(hr_dev, port, gid_index, gid);
>> -
>> -	return 0;
>> -}
>> -
>>  static void hns_roce_set_mac(struct hns_roce_dev *hr_dev, u8 port, u8 *addr)
>>  {
>>  	u8 phy_port;
>> @@ -147,15 +84,44 @@ static void hns_roce_set_mtu(struct hns_roce_dev *hr_dev, u8 port, int mtu)
>>  	hr_dev->hw->set_mtu(hr_dev, phy_port, tmp);
>>  }
>>  
>> -static void hns_roce_update_gids(struct hns_roce_dev *hr_dev, int port)
>> +static int hns_roce_add_gid(struct ib_device *device, u8 port_num,
>> +			    unsigned int index, const union ib_gid *gid,
>> +			    const struct ib_gid_attr *attr, void **context)
>> +{
>> +	struct hns_roce_dev *hr_dev = to_hr_dev(device);
>> +	u8 port = port_num - 1;
>> +	unsigned long flags;
>> +
>> +	if (port >= hr_dev->caps.num_ports)
>> +		return -EINVAL;
>> +
>> +	spin_lock_irqsave(&hr_dev->iboe.lock, flags);
>> +
>> +	hr_dev->hw->set_gid(hr_dev, port, index, (union ib_gid *)gid);
>> +
>> +	spin_unlock_irqrestore(&hr_dev->iboe.lock, flags);
>> +
>> +	return 0;
>> +}
>> +
>> +static int hns_roce_del_gid(struct ib_device *device, u8 port_num,
>> +			    unsigned int index, void **context)
>>  {
>> -	struct ib_event event;
>> +	struct hns_roce_dev *hr_dev = to_hr_dev(device);
>> +	union ib_gid zgid = { {0} };
>> +	u8 port = port_num - 1;
>> +	unsigned long flags;
>> +
>> +	if (port >= hr_dev->caps.num_ports)
>> +		return -EINVAL;
>>  
>> -	/* Refresh gid in ib_cache */
>> -	event.device = &hr_dev->ib_dev;
>> -	event.element.port_num = port + 1;
>> -	event.event = IB_EVENT_GID_CHANGE;
>> -	ib_dispatch_event(&event);
>> +	spin_lock_irqsave(&hr_dev->iboe.lock, flags);
>> +
>> +	hr_dev->hw->set_gid(hr_dev, port, index, &zgid);
> zgid has value zero. and after this call, where is zgid used?
While deleting the GIDs of the specified port, it needs to set zgid to the port GID configure register,
which means no GID to this port.

Best Regards,
>> +
>> +	spin_unlock_irqrestore(&hr_dev->iboe.lock, flags);
>> +
>> +	return 0;
>>  }
>>  
>>  static int handle_en_event(struct hns_roce_dev *hr_dev, u8 port,
>> @@ -164,8 +130,6 @@ static int handle_en_event(struct hns_roce_dev *hr_dev, u8 port,
>>  	struct device *dev = &hr_dev->pdev->dev;
>>  	struct net_device *netdev;
>>  	unsigned long flags;
>> -	union ib_gid gid;
>> -	int ret = 0;
>>  
>>  	netdev = hr_dev->iboe.netdevs[port];
>>  	if (!netdev) {
>> @@ -181,10 +145,6 @@ static int handle_en_event(struct hns_roce_dev *hr_dev, u8 port,
>>  	case NETDEV_REGISTER:
>>  	case NETDEV_CHANGEADDR:
>>  		hns_roce_set_mac(hr_dev, port, netdev->dev_addr);
>> -		hns_roce_make_default_gid(netdev, &gid);
>> -		ret = hns_roce_set_gid(hr_dev, port, 0, &gid);
>> -		if (!ret)
>> -			hns_roce_update_gids(hr_dev, port);
>>  		break;
>>  	case NETDEV_DOWN:
>>  		/*
>> @@ -197,7 +157,7 @@ static int handle_en_event(struct hns_roce_dev *hr_dev, u8 port,
>>  	}
>>  
>>  	spin_unlock_irqrestore(&hr_dev->iboe.lock, flags);
>> -	return ret;
>> +	return 0;
>>  }
>>  
>>  static int hns_roce_netdev_event(struct notifier_block *self,
>> @@ -224,118 +184,17 @@ static int hns_roce_netdev_event(struct notifier_block *self,
>>  	return NOTIFY_DONE;
>>  }
>>  
>> -static void hns_roce_addr_event(int event, struct net_device *event_netdev,
>> -				struct hns_roce_dev *hr_dev, union ib_gid *gid)
>> -{
>> -	struct hns_roce_ib_iboe *iboe = NULL;
>> -	int gid_table_len = 0;
>> -	unsigned long flags;
>> -	union ib_gid zgid;
>> -	u8 gid_idx = 0;
>> -	u8 port = 0;
>> -	int i = 0;
>> -	int free;
>> -	struct net_device *real_dev = rdma_vlan_dev_real_dev(event_netdev) ?
>> -				      rdma_vlan_dev_real_dev(event_netdev) :
>> -				      event_netdev;
>> -
>> -	if (event != NETDEV_UP && event != NETDEV_DOWN)
>> -		return;
>> -
>> -	iboe = &hr_dev->iboe;
>> -	while (port < hr_dev->caps.num_ports) {
>> -		if (real_dev == iboe->netdevs[port])
>> -			break;
>> -		port++;
>> -	}
>> -
>> -	if (port >= hr_dev->caps.num_ports) {
>> -		dev_dbg(&hr_dev->pdev->dev, "can't find netdev\n");
>> -		return;
>> -	}
>> -
>> -	memset(zgid.raw, 0, sizeof(zgid.raw));
>> -	free = -1;
>> -	gid_table_len = hr_dev->caps.gid_table_len[port];
>> -
>> -	spin_lock_irqsave(&hr_dev->iboe.lock, flags);
>> -
>> -	for (i = 0; i < gid_table_len; i++) {
>> -		gid_idx = hns_get_gid_index(hr_dev, port, i);
>> -		if (!memcmp(gid->raw, iboe->gid_table[gid_idx].raw,
>> -			    sizeof(gid->raw)))
>> -			break;
>> -		if (free < 0 && !memcmp(zgid.raw,
>> -			iboe->gid_table[gid_idx].raw, sizeof(zgid.raw)))
>> -			free = i;
>> -	}
>> -
>> -	if (i >= gid_table_len) {
>> -		if (free < 0) {
>> -			spin_unlock_irqrestore(&hr_dev->iboe.lock, flags);
>> -			dev_dbg(&hr_dev->pdev->dev,
>> -				"gid_index overflow, port(%d)\n", port);
>> -			return;
>> -		}
>> -		if (!hns_roce_set_gid(hr_dev, port, free, gid))
>> -			hns_roce_update_gids(hr_dev, port);
>> -	} else if (event == NETDEV_DOWN) {
>> -		if (!hns_roce_set_gid(hr_dev, port, i, &zgid))
>> -			hns_roce_update_gids(hr_dev, port);
>> -	}
>> -
>> -	spin_unlock_irqrestore(&hr_dev->iboe.lock, flags);
>> -}
>> -
>> -static int hns_roce_inet_event(struct notifier_block *self, unsigned long event,
>> -			       void *ptr)
>> -{
>> -	struct in_ifaddr *ifa = ptr;
>> -	struct hns_roce_dev *hr_dev;
>> -	struct net_device *dev = ifa->ifa_dev->dev;
>> -	union ib_gid gid;
>> -
>> -	ipv6_addr_set_v4mapped(ifa->ifa_address, (struct in6_addr *)&gid);
>> -
>> -	hr_dev = container_of(self, struct hns_roce_dev, iboe.nb_inet);
>> -
>> -	hns_roce_addr_event(event, dev, hr_dev, &gid);
>> -
>> -	return NOTIFY_DONE;
>> -}
>> -
>> -static int hns_roce_setup_mtu_gids(struct hns_roce_dev *hr_dev)
>> +static int hns_roce_setup_mtu_mac(struct hns_roce_dev *hr_dev)
>>  {
>> -	struct in_ifaddr *ifa_list = NULL;
>> -	union ib_gid gid = {{0} };
>> -	u32 ipaddr = 0;
>> -	int index = 0;
>> -	int ret = 0;
>> -	u8 i = 0;
>> +	u8 i;
>>  
>>  	for (i = 0; i < hr_dev->caps.num_ports; i++) {
>>  		hns_roce_set_mtu(hr_dev, i,
>>  				 ib_mtu_enum_to_int(hr_dev->caps.max_mtu));
>>  		hns_roce_set_mac(hr_dev, i, hr_dev->iboe.netdevs[i]->dev_addr);
>> -
>> -		if (hr_dev->iboe.netdevs[i]->ip_ptr) {
>> -			ifa_list = hr_dev->iboe.netdevs[i]->ip_ptr->ifa_list;
>> -			index = 1;
>> -			while (ifa_list) {
>> -				ipaddr = ifa_list->ifa_address;
>> -				ipv6_addr_set_v4mapped(ipaddr,
>> -						       (struct in6_addr *)&gid);
>> -				ret = hns_roce_set_gid(hr_dev, i, index, &gid);
>> -				if (ret)
>> -					break;
>> -				index++;
>> -				ifa_list = ifa_list->ifa_next;
>> -			}
>> -			hns_roce_update_gids(hr_dev, i);
>> -		}
>>  	}
>>  
>> -	return ret;
>> +	return 0;
>>  }
>>  
>>  static int hns_roce_query_device(struct ib_device *ib_dev,
>> @@ -444,31 +303,6 @@ static enum rdma_link_layer hns_roce_get_link_layer(struct ib_device *device,
>>  static int hns_roce_query_gid(struct ib_device *ib_dev, u8 port_num, int index,
>>  			      union ib_gid *gid)
>>  {
>> -	struct hns_roce_dev *hr_dev = to_hr_dev(ib_dev);
>> -	struct device *dev = &hr_dev->pdev->dev;
>> -	u8 gid_idx = 0;
>> -	u8 port;
>> -
>> -	if (port_num < 1 || port_num > hr_dev->caps.num_ports ||
>> -	    index >= hr_dev->caps.gid_table_len[port_num - 1]) {
>> -		dev_err(dev,
>> -			"port_num %d index %d illegal! correct range: port_num 1~%d index 0~%d!\n",
>> -			port_num, index, hr_dev->caps.num_ports,
>> -			hr_dev->caps.gid_table_len[port_num - 1] - 1);
>> -		return -EINVAL;
>> -	}
>> -
>> -	port = port_num - 1;
>> -	gid_idx = hns_get_gid_index(hr_dev, port, index);
>> -	if (gid_idx >= HNS_ROCE_MAX_GID_NUM) {
>> -		dev_err(dev, "port_num %d index %d illegal! total gid num %d!\n",
>> -			port_num, index, HNS_ROCE_MAX_GID_NUM);
>> -		return -EINVAL;
>> -	}
>> -
>> -	memcpy(gid->raw, hr_dev->iboe.gid_table[gid_idx].raw,
>> -	       HNS_ROCE_GID_SIZE);
>> -
>>  	return 0;
>>  }
>>  
>> @@ -646,6 +480,8 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>>  	ib_dev->get_link_layer		= hns_roce_get_link_layer;
>>  	ib_dev->get_netdev		= hns_roce_get_netdev;
>>  	ib_dev->query_gid		= hns_roce_query_gid;
>> +	ib_dev->add_gid			= hns_roce_add_gid;
>> +	ib_dev->del_gid			= hns_roce_del_gid;
>>  	ib_dev->query_pkey		= hns_roce_query_pkey;
>>  	ib_dev->alloc_ucontext		= hns_roce_alloc_ucontext;
>>  	ib_dev->dealloc_ucontext	= hns_roce_dealloc_ucontext;
>> @@ -688,32 +524,22 @@ static int hns_roce_register_device(struct hns_roce_dev *hr_dev)
>>  		return ret;
>>  	}
>>  
>> -	ret = hns_roce_setup_mtu_gids(hr_dev);
>> +	ret = hns_roce_setup_mtu_mac(hr_dev);
>>  	if (ret) {
>> -		dev_err(dev, "roce_setup_mtu_gids failed!\n");
>> -		goto error_failed_setup_mtu_gids;
>> +		dev_err(dev, "setup_mtu_mac failed!\n");
>> +		goto error_failed_setup_mtu_mac;
>>  	}
>>  
>>  	iboe->nb.notifier_call = hns_roce_netdev_event;
>>  	ret = register_netdevice_notifier(&iboe->nb);
>>  	if (ret) {
>>  		dev_err(dev, "register_netdevice_notifier failed!\n");
>> -		goto error_failed_setup_mtu_gids;
>> -	}
>> -
>> -	iboe->nb_inet.notifier_call = hns_roce_inet_event;
>> -	ret = register_inetaddr_notifier(&iboe->nb_inet);
>> -	if (ret) {
>> -		dev_err(dev, "register inet addr notifier failed!\n");
>> -		goto error_failed_register_inetaddr_notifier;
>> +		goto error_failed_setup_mtu_mac;
>>  	}
>>  
>>  	return 0;
>>  
>> -error_failed_register_inetaddr_notifier:
>> -	unregister_netdevice_notifier(&iboe->nb);
>> -
>> -error_failed_setup_mtu_gids:
>> +error_failed_setup_mtu_mac:
>>  	ib_unregister_device(ib_dev);
>>  
>>  	return ret;
>>
> 
> _______________________________________________
> linuxarm mailing list
> linuxarm@...wei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm
> 
> .
> 

Powered by blists - more mailing lists