Re: [PATCH v4 3/5] drivers/net/i40e: add Port Representor functionality

[Mohammad Abdul Awal <mohammad.abdul.awal@intel.com>]

On 10/01/2018 12:37, Xing, Beilei wrote:
> -----Original Message-----
>> From: Awal, Mohammad Abdul
>> On 10/01/2018 07:56, Xing, Beilei wrote:
>>> <...>
>>>
>>>> +static const struct ether_addr null_mac_addr;
>>>> +
>>>> +int
>>>> +rte_pmd_i40e_remove_vf_mac_addr(uint8_t port, uint16_t vf_id,
>>>> +	struct ether_addr *mac_addr)
>>>> +{
>>>> +	struct rte_eth_dev *dev;
>>>> +	struct i40e_pf_vf *vf;
>>>> +	struct i40e_vsi *vsi;
>>>> +	struct i40e_pf *pf;
>>>> +
>>>> +	if (i40e_validate_mac_addr((u8 *)mac_addr) != I40E_SUCCESS)
>>>> +		return -EINVAL;
>>>> +
>>>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
>>>> +
>>>> +	dev = &rte_eth_devices[port];
>>>> +
>>>> +	if (!is_i40e_supported(dev))
>>>> +		return -ENOTSUP;
>>>> +
>>>> +	pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>>>> +
>>>> +	if (vf_id >= pf->vf_num || !pf->vfs)
>>>> +		return -EINVAL;
>>>> +
>>>> +	vf = &pf->vfs[vf_id];
>>>> +	vsi = vf->vsi;
>>>> +	if (!vsi) {
>>>> +		PMD_DRV_LOG(ERR, "Invalid VSI.");
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	if (!is_same_ether_addr(mac_addr, &vf->mac_addr)) {
>>>> +		PMD_DRV_LOG(ERR, "Mac address does not match.");
>>>> +		return -EINVAL;
>>>> +	}
>>> Not very understand why only vf->mac_addr can be moved?
>> It it checks if the mac address we want to delete, similar to the function
>> rte_pmd_i40e_set_vf_mac_addr where we set the mac address of a VF.
> But in this way, seems we can't delete the mac address added by rte_pmd_i40e_add_vf_mac_addr.
> set_vf_mac_addr should be used for setting default mac address, right?
OK, I understand what you mean.
I have fixed it now. We reset the vf->mac_addr only if the mac address 
is matched.
Then we delete it from the vsi->mac_list.

Thanks,
Awal.

>
>>> I think any mac address in vsi->mac_list can be removed.
>> Yes, it is removed from the vsi->mac_list in the next line if the mac address is
>> matched in the previous checking.
>>
>>>> +
>>>> +	/* reset the mac with null mac */
>>>> +	ether_addr_copy(&null_mac_addr, &vf->mac_addr);
>> Here we reset the mac address of a VF with null address, reverting the
>> operation of function rte_pmd_i40e_set_vf_mac_addr where we set the
>> mac address of a VF.
>>>> +
>>>> +	/* Remove the mac */
>>>> +	i40e_vsi_delete_mac(vsi, mac_addr);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +