[PATCH] net/i40e: no more initial VF MAC address

[PATCH] net/i40e: no more initial VF MAC address

[Qi Zhang]

During PF initialization, PF will generate an initial mac
address for VFs, the purpose is to help VF keep a constant
mac address between startup/shutdown cycles. Now this is not
necessary, since we already provide an API to set VF mac
address from PF side (rte_pmd_i40e_set_vf_addr). Application
can use this API to lock down VF's mac address.
This change also sync DPDK PF's behavior with kernel driver.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/net/i40e/i40e_pf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index ddfc140..4e05dca 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -1053,7 +1053,6 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 		ret = i40e_pf_host_vf_reset(&pf->vfs[i], 0);
 		if (ret != I40E_SUCCESS)
 			goto fail;
-		eth_random_addr(pf->vfs[i].mac_addr.addr_bytes);
 	}
 
 	/* restore irq0 */
-- 
2.7.4

Re: [PATCH] net/i40e: no more initial VF MAC address

[Ferruh Yigit]

On 1/18/2017 1:45 AM, Qi Zhang wrote:
> During PF initialization, PF will generate an initial mac
> address for VFs, the purpose is to help VF keep a constant
> mac address between startup/shutdown cycles. 

I believe below does not provide permanent MAC address, PF assign a
random mac address to its VFs on each PF initialization.

> Now this is not
> necessary, since we already provide an API to set VF mac
> address from PF side (rte_pmd_i40e_set_vf_addr). 

rte_pmd_i40e_set_vf_mac_addr?

> Application
> can use this API to lock down VF's mac address.

As far as I can see how it was (before this patch):

1- During PF init, assign random MAC to VFs.
2- Application can overwrite VF MAC address with above API.
3- During VF init, if there is a valid MAC assigned by PF use it.
4- During VF init, if PF not assigned MAC, assign a random one.

So this patch removes above step 1.

Overall even if application will explicitly assign a MAC to VF or not,
VF will have a valid MAC address. So patch looks good, just please
reword commit log to describe above.

> This change also sync DPDK PF's behavior with kernel driver.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
>  drivers/net/i40e/i40e_pf.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> index ddfc140..4e05dca 100644
> --- a/drivers/net/i40e/i40e_pf.c
> +++ b/drivers/net/i40e/i40e_pf.c
> @@ -1053,7 +1053,6 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
>  		ret = i40e_pf_host_vf_reset(&pf->vfs[i], 0);
>  		if (ret != I40E_SUCCESS)
>  			goto fail;
> -		eth_random_addr(pf->vfs[i].mac_addr.addr_bytes);
>  	}
>  
>  	/* restore irq0 */
> 

[PATCH v2] net/i40e: no more initial VF MAC address

[Qi Zhang]

During PF initialization, PF will generate an initial mac address
for VFs, the purpose is to help VF keep a constant mac address between
its startup/shutdown cycles. Now this is not necessary, since we already
provide an API to set VF's MAC address from PF side
(rte_pmd_i40e_set_vf_mac_addr).
Application can use this API to lock down VF's mac address.(of course this
should happen before VF init)
While without this patch, we still can use rte_pmd_i40e_set_vf_mac_addr
to overwrite the random one, but this patch align DPDK's default behavior
with Kernel PF driver's, and this help to give an identical experience
when work with kernel VF driver.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2: add more explanation in commit log

 drivers/net/i40e/i40e_pf.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index f771dfb..ffa8328 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -1349,7 +1349,6 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
 		ret = i40e_pf_host_vf_reset(&pf->vfs[i], 0);
 		if (ret != I40E_SUCCESS)
 			goto fail;
-		eth_random_addr(pf->vfs[i].mac_addr.addr_bytes);
 	}
 
 	/* restore irq0 */
-- 
2.7.4

Re: [PATCH] net/i40e: no more initial VF MAC address

[Zhang, Qi Z]

Hi Ferruh:
	Sorry to reply this late, I think I missed your email.

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Saturday, January 21, 2017 1:39 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Zhang, Helin <helin.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: no more initial VF MAC address
> 
> On 1/18/2017 1:45 AM, Qi Zhang wrote:
> > During PF initialization, PF will generate an initial mac address for
> > VFs, the purpose is to help VF keep a constant mac address between
> > startup/shutdown cycles.
> 
> I believe below does not provide permanent MAC address, PF assign a random
> mac address to its VFs on each PF initialization.

Yes, this is for VF's startup /shutdown cycles only, but that's the original concern from customer.
> 
> > Now this is not
> > necessary, since we already provide an API to set VF mac address from
> > PF side (rte_pmd_i40e_set_vf_addr).
> 
> rte_pmd_i40e_set_vf_mac_addr?
Exactly!
> 
> > Application
> > can use this API to lock down VF's mac address.
> 
> As far as I can see how it was (before this patch):
> 
> 1- During PF init, assign random MAC to VFs.
> 2- Application can overwrite VF MAC address with above API.
> 3- During VF init, if there is a valid MAC assigned by PF use it.
> 4- During VF init, if PF not assigned MAC, assign a random one.
> 
> So this patch removes above step 1.

Yes, we remove step 1 intentionally to align with kernel PF driver's behavior, 
because for kernel VF, during initialization, once it found PF already assigned a valid MAC, it cannot be changed from VF anymore
but if it is not a valid address, VF is given the permission to change it.

> 
> Overall even if application will explicitly assign a MAC to VF or not, VF will have
> a valid MAC address. So patch looks good, just please reword commit log to
> describe above.


Sure, I will update the commit log
> 
> > This change also sync DPDK PF's behavior with kernel driver.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> >  drivers/net/i40e/i40e_pf.c | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
> > index ddfc140..4e05dca 100644
> > --- a/drivers/net/i40e/i40e_pf.c
> > +++ b/drivers/net/i40e/i40e_pf.c
> > @@ -1053,7 +1053,6 @@ i40e_pf_host_init(struct rte_eth_dev *dev)
> >  		ret = i40e_pf_host_vf_reset(&pf->vfs[i], 0);
> >  		if (ret != I40E_SUCCESS)
> >  			goto fail;
> > -		eth_random_addr(pf->vfs[i].mac_addr.addr_bytes);
> >  	}
> >
> >  	/* restore irq0 */
> >

Regards
Qi

Re: [PATCH v2] net/i40e: no more initial VF MAC address

[Ferruh Yigit]

On 2/21/2017 10:45 PM, Qi Zhang wrote:
> During PF initialization, PF will generate an initial mac address
> for VFs, the purpose is to help VF keep a constant mac address between
> its startup/shutdown cycles. Now this is not necessary, since we already
> provide an API to set VF's MAC address from PF side
> (rte_pmd_i40e_set_vf_mac_addr).
> Application can use this API to lock down VF's mac address.(of course this
> should happen before VF init)
> While without this patch, we still can use rte_pmd_i40e_set_vf_mac_addr
> to overwrite the random one, but this patch align DPDK's default behavior
> with Kernel PF driver's, and this help to give an identical experience
> when work with kernel VF driver.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net/master, thanks.