Re: [dpdk-dev] [PATCH v3 1/3] kni: support for MAC addr change

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 1/22/2018 5:20 AM, Hemant Agrawal wrote:
> Hi Ferruh,
> 
> On 1/22/2018 3:37 AM, Ferruh Yigit wrote:
>> On 1/18/2018 6:12 AM, Hemant Agrawal wrote:
>>> This patch adds following:
>>> 1. Option to configure the mac address during create. Generate random
>>>    address only if the user has not provided any valid address.
>>> 2. Inform usespace, if mac address is being changed in linux.
>>> 3. Implement default handling of mac address change in the corresponding
>>>    ethernet device.
>>>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>
>> <...>
>>
>>> @@ -530,6 +556,14 @@ rte_kni_handle_request(struct rte_kni *kni)
>>>  			req->result = kni->ops.config_network_if(\
>>>  					kni->ops.port_id, req->if_up);
>>>  		break;
>>> +	case RTE_KNI_REQ_CHANGE_MAC_ADDR: /* Change MAC Address */
>>> +		if (kni->ops.config_mac_address)
>>> +			req->result = kni->ops.config_mac_address(
>>> +					kni->ops.port_id, req->mac_addr);
>>> +		else if (kni->ops.port_id != UINT16_MAX)
>>
>> This won't be enough. rte_kni_alloc() can be called with NULL ops value. For
>> that case m_ctx->ops won't be updated. And by default ops will have all zeros,
>> not sure how to differentiate it from real port_id zero.
>>
> 
> I think, I tried to address that in the first patch.
> 
> rte_kni_alloc(struct rte_mempool *pktmbuf_pool,
>   	memset(ctx, 0, sizeof(struct rte_kni));
>   	if (ops)
>   		memcpy(&ctx->ops, ops, sizeof(struct rte_kni_ops));
> +	else
> +		ctx->ops.port_id = UINT16_MAX;
> 
> Do you still see issue?

You are right, this fixes it, I missed this part. So patch lgtm:

Series,
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

> 
> Regards,
> Hemant
>