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?