On Tue, Oct 01, 2024 at 11:12:54AM +0200, David Marchand wrote:
> Following two upstream Linux kernel changes (see links), the mac address
> of a iavf port, serviced by a i40e PF driver, is lost when the DPDK iavf
> driver probes the port again (which may be triggered at any point of a
> DPDK application life, like when a reset event is triggered by the PF).
> 
> A first change results in the mac address of the VF port being reset to 0
> during the VIRTCHNL_OP_GET_VF_RESOURCES query.
> The i40e PF driver change is pretty obscure but the iavf Linux driver does
> set VIRTCHNL_VF_OFFLOAD_USO.
> Announcing such a capability in the DPDK driver does not seem to be an
> issue, so do the same in DPDK to keep the legacy behavior of a fixed mac.
> 
> Then a second change in the kernel results in the VF mac address being
> cleared when the VF driver remove its default mac address.
> Removing (unicast or multicast) mac addresses is not done by the kernel VF
> driver in general.
> The reason why the DPDK driver behaves like this is undocumented
> (and lost because the authors are not active anymore).
> Aligning DPDK behavior to the upstream kernel driver is safer in any
> case.
> 

One question inline below.

/Bruce

> Cc: sta...@dpdk.org
> 
> Link: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fed0d9f13266
> Link: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ceb29474bbbc
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
>  drivers/net/iavf/iavf_ethdev.c | 22 +++++-----------------
>  drivers/net/iavf/iavf_vchnl.c  |  1 +
>  2 files changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index 11036bc179..97cdb1cbe0 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -1044,7 +1044,7 @@ iavf_dev_start(struct rte_eth_dev *dev)
>               if (iavf_configure_queues(adapter,
>                               IAVF_CFG_Q_NUM_PER_BUF, index) != 0) {
>                       PMD_DRV_LOG(ERR, "configure queues failed");
> -                     goto err_queue;
> +                     goto error;
>               }
>               num_queue_pairs -= IAVF_CFG_Q_NUM_PER_BUF;
>               index += IAVF_CFG_Q_NUM_PER_BUF;
> @@ -1052,12 +1052,12 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>       if (iavf_configure_queues(adapter, num_queue_pairs, index) != 0) {
>               PMD_DRV_LOG(ERR, "configure queues failed");
> -             goto err_queue;
> +             goto error;
>       }
>  
>       if (iavf_config_rx_queues_irqs(dev, intr_handle) != 0) {
>               PMD_DRV_LOG(ERR, "configure irq failed");
> -             goto err_queue;
> +             goto error;
>       }
>       /* re-enable intr again, because efd assign may change */
>       if (dev->data->dev_conf.intr_conf.rxq != 0) {
> @@ -1077,14 +1077,12 @@ iavf_dev_start(struct rte_eth_dev *dev)
>  
>       if (iavf_start_queues(dev) != 0) {
>               PMD_DRV_LOG(ERR, "enable queues failed");
> -             goto err_mac;
> +             goto error;
>       }
>  
>       return 0;
>  
> -err_mac:
> -     iavf_add_del_all_mac_addr(adapter, false);
> -err_queue:
> +error:
>       return -1;
>  }
>  
> @@ -1113,16 +1111,6 @@ iavf_dev_stop(struct rte_eth_dev *dev)
>       /* Rx interrupt vector mapping free */
>       rte_intr_vec_list_free(intr_handle);
>  
> -     /* adminq will be disabled when vf is resetting. */
> -     if (!vf->in_reset_recovery) {
> -             /* remove all mac addrs */
> -             iavf_add_del_all_mac_addr(adapter, false);
> -
> -             /* remove all multicast addresses */
> -             iavf_add_del_mc_addr_list(adapter, vf->mc_addrs, 
> vf->mc_addrs_num,
> -                               false);
> -     }
> -

Question on this: while I understand we don't want to remove the default
mac address, should all other non-default macs not still be removed?

Reply via email to